Re: [PATCH 3/4] xfs: combine xfs_rtmodify_summary and xfs_rtget_summary

Eric Sandeen
2014-08-22T15:01:38Z

On 8/22/14, 8:19 AM, Brian Foster wrote:
> On Thu, Aug 21, 2014 at 08:00:45PM -0500, Eric Sandeen wrote:

...

>> @@ -480,15 +484,40 @@ xfs_rtmodify_summary(
>> }
>> }
>> /*
>> - * Point to the summary information, modify and log it.
>> + * Point to the summary information, modify/log it, and/or copy it out.
>> */
>> sp = XFS_SUMPTR(mp, bp, so);
>> - *sp += delta;
>> - xfs_trans_log_buf(tp, bp, (uint)((char *)sp - (char *)bp->b_addr),
>> - (uint)((char *)sp - (char *)bp->b_addr + sizeof(*sp) - 1));
>> + if (delta) {
>> + uint first = (uint)((char *)sp - (char *)bp->b_addr);
>> +
>> + *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);
>
> This introduces some potentially weird circumstances (e.g., acquire,
> log, release of a buffer), but I think it's resolved by the next patch.
>
> Reviewed-by: Brian Foster <bfoster@redhat.com>

does it introduce it, or just highlight it? I thought it was weird too,
but I think it existed before; that's what prompted me to go looking at
callers and drop the rbpp checks, FWIW.

-Eric