public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Brian Foster <bfoster@redhat.com>
Cc: Eric Sandeen <sandeen@redhat.com>, xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH 3/4] xfs: combine xfs_rtmodify_summary and xfs_rtget_summary
Date: Fri, 22 Aug 2014 10:01:40 -0500	[thread overview]
Message-ID: <53F75B54.8050802@sandeen.net> (raw)
In-Reply-To: <20140822131950.GC3915@laptop.bfoster>

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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-08-22 15:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-22  0:51 [PATCH 0/4] xfs: more code refactoring Eric Sandeen
2014-08-22  0:55 ` [PATCH 1/4] xfs: check resblks before calling xfs_dir_canenter Eric Sandeen
2014-08-22 13:19   ` Brian Foster
2014-08-29  0:59   ` Christoph Hellwig
2014-08-22  0:58 ` [PATCH 2/4] xfs: combine xfs_dir_canenter into xfs_dir_createname Eric Sandeen
2014-08-22 13:19   ` Brian Foster
2014-08-29  1:00   ` Christoph Hellwig
2014-08-29  2:14   ` [PATCH 2/4 V2] " Eric Sandeen
2014-08-22  1:00 ` [PATCH 3/4] xfs: combine xfs_rtmodify_summary and xfs_rtget_summary Eric Sandeen
2014-08-22 13:19   ` Brian Foster
2014-08-22 15:01     ` Eric Sandeen [this message]
2014-08-22 15:23       ` Eric Sandeen
2014-08-22  1:03 ` [PATCH 4/4] xfs: remove rbpp check from xfs_rtmodify_summary_int Eric Sandeen
2014-08-22 13:20   ` Brian Foster
2014-08-23 23:50     ` Eric Sandeen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53F75B54.8050802@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=bfoster@redhat.com \
    --cc=sandeen@redhat.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox