xfs
[Top] [All Lists]

Re: [PATCH 4/4] xfs: remove rbpp check from xfs_rtmodify_summary_int

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 4/4] xfs: remove rbpp check from xfs_rtmodify_summary_int
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Sat, 23 Aug 2014 18:50:14 -0500
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140822132004.GD3915@xxxxxxxxxxxxxx>
References: <53F6942B.80808@xxxxxxxxxx> <53F696D7.2030906@xxxxxxxxxxx> <20140822132004.GD3915@xxxxxxxxxxxxxx>
On 8/22/14, 8:20 AM, Brian Foster wrote:
> On Thu, Aug 21, 2014 at 08:03:19PM -0500, Eric Sandeen wrote:
>> rbpp is always passed into xfs_rtmodify_summary
>> and xfs_rtget_summary, so there is no need to
>> test for it in xfs_rtmodify_summary_int.
>>
> 
> Looks fine, but this is also called through a variety of twisty paths.
> Could we add a top-level error check or assert?

We could, though it'll oops pretty fast if we don't send rbpp anyway.
An ASSERT might be decent for documentation, though...

-Eric

> Brian
> 
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> ---
>>
>> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
>> index 50e3b93..7c818f1 100644
>> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
>> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
>> @@ -460,7 +460,7 @@ xfs_rtmodify_summary_int(
>>      /*
>>       * If we have an old buffer, and the block number matches, use that.
>>       */
>> -    if (rbpp && *rbpp && *rsb == sb)
>> +    if (*rbpp && *rsb == sb)
>>              bp = *rbpp;
>>      /*
>>       * Otherwise we have to get the buffer.
>> @@ -469,7 +469,7 @@ xfs_rtmodify_summary_int(
>>              /*
>>               * If there was an old one, get rid of it first.
>>               */
>> -            if (rbpp && *rbpp)
>> +            if (*rbpp)
>>                      xfs_trans_brelse(tp, *rbpp);
>>              error = xfs_rtbuf_get(mp, tp, sb, 1, &bp);
>>              if (error) {
>> @@ -478,10 +478,8 @@ xfs_rtmodify_summary_int(
>>              /*
>>               * Remember this buffer and block for the next call.
>>               */
>> -            if (rbpp) {
>> -                    *rbpp = bp;
>> -                    *rsb = sb;
>> -            }
>> +            *rbpp = bp;
>> +            *rsb = sb;
>>      }
>>      /*
>>       * Point to the summary information, modify/log it, and/or copy it out.
>> @@ -493,14 +491,8 @@ xfs_rtmodify_summary_int(
>>              *sp += delta;
>>              xfs_trans_log_buf(tp, bp, first, first + sizeof(*sp) - 1);
>>      }
>> -    if (sum) {
>> -            /*
>> -             * Drop the buffer if we're not asked to remember it.
>> -             */
>> -            if (!rbpp)
>> -                    xfs_trans_brelse(tp, bp);
>> +    if (sum)
>>              *sum = *sp;
>> -    }
>>      return 0;
>>  }
>>  
>>
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@xxxxxxxxxxx
>> http://oss.sgi.com/mailman/listinfo/xfs
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

<Prev in Thread] Current Thread [Next in Thread>