David Chinner <dgc@xxxxxxx> writes:
> On Wed, Apr 02, 2008 at 01:58:09PM +1100, Niv Sardi wrote:
>> David Chinner <dgc@xxxxxxx> writes:
>> > On Tue, Mar 11, 2008 at 12:04:21PM +1100, David Chinner wrote:
>> >> A recent paper at the FAST08 conference highlighted a large number
>> >> of unchecked error paths in Linux filesystems and I/O layers. As a
>> >> subsystem, XFS had the highest aggregate numbers of bad error
>> >> propagation. A tarball which contains a quilt patch series of 32
>> >> patches aimed at improving this situation can be found here:
>> >>
>> >> http://oss.sgi.com/~dgc/xfs/error-check/xfs-error-checking.tar.gz
>>
>> All looks good except some minor typo-editing,
>>
>> and
>>
>> NOK xfs-mustcheck-quotamount.patch # need to check if can happen when
>> forcing quotas
>>
>> I'm not sure what happens if we really DO want quotas (specified on
>> mount line and such).
>
> The behaviour will be exactly the same as previously, because the
> error returned by xfs_qm_mount_quotas() is ignored. i.e. if we try
> to mount with quotas and the quota mount fails, we continue (after
> issuing a warning to syslog) that quotas were not turned on.
>
> This is especially important for root filesystems with quota
> enabled....
OK, I wasn't sure.
All the rest are minor aesthetics/typos my messed up notes, and can be
ignored…
>> OK xfs-mustcheck-reset-dqcounts.patch
>> OK xfs-mustcheck-dqflushall.patch
>> OK xfs-mustcheck-acl-setmode.patch
>> OK xfs-mustcheck-search-busy.patch
>> EDITED xfs-mustcheck-compute-diff.patch # xfs_fs_cmn_err alignment
>
> That patch doesn't have any calls to xfs_fs_cmn_err() in it. Can you
> clarify, please?
Oops, the edit was for:
-+STATIC void /* success (>= minlen) */
++STATIC void
as it didn't really make sense anymore.
>> OK xfs-mustcheck-bmap-adjacent.patch
>> OK xfs-mustcheck-iflush-fork.patch # less error handeling !!
>
> You can't have less error handling than intentionally ignoring
> the return from a function that can't return an error. You can
> have simpler code, though, by declaring the function void....
hum, I can't remember why I wrote that anymore, oh well… looks good now.
>> OK xfs-mustcheck-bulkstat-dinode.patch
>> OK xfs-mustcheck-quiesce-fs.patch
>> OK xfs-mustcheck-bdstrat.patch
>> OK xfs-fix-error-prototypes.patch # not error handeling related
>> OK xfs-mustcheck-acl-vremove.patch
>> OK xfs-mustcheck-icsb-disable.patch
>> OK xfs-mustcheck-ioend-unwritten.patch
>> OK xfs-mustcheck-buf-associate.patch
>> OK xfs-mustcheck-reserve-blocks.patch
>> EDITED xfs-mustcheck-bawrite.patch # xfs_fs_cmn_err alignment
>
> Which means?
That's purely aesthetic, sometimes we split the string and keep it
aligned, and sometimes we pad it left so that it fits, I prefer
splitting.
>> OK xfs-mustcheck-bdwrite.patch
>> OK xfs-mustcheck-truncate-page.patch # might be incomplete
Note to self: re-read one's notes before sending them out, I wanted to
look at why we couldn't propagate error better, but now it's all
understood =)
>> EDITED xfs-mustcheck-dqflush.patch # slight style change/typo
> Details?
-hence we nevre know if we've failed to flush a dquot to disk.
+hence we never know if we've failed to flush a dquot to disk.
and xfs_fs_cmn_err stuff.
>> OK xfs-mustcheck-reset-sbqflags.patch
>> OK xfs-mustcheck-quotaoff.patch
>> EDITED xfs-mustcheck-inactive.patch # slight style change/typo
>
> Details?
-correctly. if we fail to write the final quota off trnasaction,
+correctly. if we fail to write the final quota off transaction,
and xfs_fs_cmn_err stuff.
Cheers,
--
Niv Sardi
|