On 04/23 2014 07:46 AM, Dave Chinner wrote:
> On Tue, Apr 22, 2014 at 08:00:55PM +0800, Jeff Liu wrote:
>>
>> On 04/22 2014 14:59 PM, Dave Chinner wrote:
>>> From: Dave Chinner <dchinner@xxxxxxxxxx>
>>>
>>> Commit e461fcb ("xfs: remote attribute lookups require the value
>>> length") passes the remote attribute length in the xfs_da_args
>>> structure on lookup so that CRC calculations and validity checking
>>> can be performed correctly by related code. This, unfortunately has
>>> the side effect of changing the args->valuelen parameter in cases
>>> where it shouldn't.
>>>
>>> That is, when we replace a remote attribute, the incoming
>>> replacement stores the value and length in args->value and
>>> args->valuelen, but then the lookup which finds the existing remote
>>> attribute overwrites args->valuelen with the length of the remote
>>> attribute being replaced. Hence when we go to create the new
>>> attribute, we create it of the size of the existing remote
>>> attribute, not the size it is supposed to be. When the new attribute
>>> is much smaller than the old attribute, this results in a
>>> transaction overrun and an ASSERT() failure on a debug kernel:
>>>
>>> XFS: Assertion failed: tp->t_blk_res_used <= tp->t_blk_res, file: fs/xfs/xfs_trans.c, line: 331
>>>
>>> Fix this by keeping the remote attribute value length separate to
>>> the attribute value length in the xfs_da_args structure. The enables
>>> us to pass the length of the remote attribute to be removed without
>>> overwriting the new attribute's length.
>>>
>>> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ....
>>> @@ -348,6 +348,11 @@ xfs_attr_rmtval_get(
>>>
>>> ASSERT(!(args->flags & ATTR_KERNOVAL));
>>>
>>> + /* remote value might be different size to the buffer supplied. */
>>> + if (args->rmtvaluelen = args->valuelen)
>> ^^^
>> Here is a typo...
>
> Oh, well spotted.
>
> But, hold on a minute, shouldn't gcc be catching those sorts of
> typos?
>
> /me groans
>
> $ gcc t.c
> $ gcc -Wall t.c
> t.c: In function ¿main¿:
> t.c:10:2: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
> if (foo = bar)
> ^
> $
>
> Only if -Wparentheses is specified.
>
> Why the hell are we suppressing useful warnings on the kernel build?
>
Well, maybe it's due to the different gcc release? I can hit this warning via
gcc-4.8.1 which is the default version on Ubuntu 13.10 as below:
# make SUBDIRS=fs/xfs -j4
...
fs/xfs/xfs_attr_remote.c: In function ‘xfs_attr_rmtval_get’:
fs/xfs/xfs_attr_remote.c:352:2: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
if (args->rmtvaluelen = args->valuelen)
^
CC [M] fs/xfs/xfs_bmap_btree.o
CC [M] fs/xfs/xfs_btree.o
...
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
