On Mon 30-04-12 10:36:41, Christoph Hellwig wrote:
> On Tue, Mar 20, 2012 at 11:56:25PM +0100, Jan Kara wrote:
> > Move clearing of I_SYNC into inode_sync_complete(). It is more logical to have
> > clearing of I_SYNC bit and waking of waiters in one place. Also later we will
> > have two places needing to clear I_SYNC and wake up waiters so this allows them
> > to use the common helper. Moving of I_SYNC clearing to a later stage of
> > writeback_single_inode() is safe since we hold i_lock all the time.
> >
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
>
> The code changes look good, but should we really remove a comment that
> describes memory barrier? IMHO any undocumented barrier is a bug
> waiting to happen.
>
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
The old comment didn't look too useful to me and the necessity of a
barrier is documented in wake_up_bit(). But yes, some comment should be
there. I added there simple:
inode->i_state &= ~I_SYNC;
/* Waiters must see I_SYNC cleared before being woken up */
smp_mb();
wake_up_bit(&inode->i_state, __I_SYNC);
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
