xfs
[Top] [All Lists]

Re: [PATCH 06/13] xfs: xfs_sync_data is redundant.

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 06/13] xfs: xfs_sync_data is redundant.
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 3 Oct 2012 06:24:56 +1000
Cc: Brian Foster <bfoster@xxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20121002132205.GA14572@xxxxxxxxxxxxx>
References: <1348807485-20165-1-git-send-email-david@xxxxxxxxxxxxx> <1348807485-20165-7-git-send-email-david@xxxxxxxxxxxxx> <5069F9B0.50804@xxxxxxxxxx> <20121002001021.GJ23520@dastard> <20121002132205.GA14572@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Oct 02, 2012 at 09:22:05AM -0400, Christoph Hellwig wrote:
> On Tue, Oct 02, 2012 at 10:10:22AM +1000, Dave Chinner wrote:
> > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > index da69c18..0ec7a46 100644
> > --- a/fs/xfs/xfs_inode.h
> > +++ b/fs/xfs/xfs_inode.h
> > @@ -294,7 +294,7 @@ xfs_new_eof(struct xfs_inode *ip, xfs_fsize_t new_size)
> >  static inline void
> >  xfs_flush_inodes(struct xfs_inode *ip)
> >  {
> > -   writeback_inodes_sb_if_idle(VFS_I(ip)->i_sb, WB_REASON_FS_FREE_SPACE);
> > +   sync_inodes_sb(VFS_I(ip)->i_sb);
> 
> sync_inodes_sb needs s_umount held and asserts that, while our callers
> usually won't have it.

Ah, bugger. I didn't notice that, and hadn't checked dmesg after I
ran 273 a few times with this patch. I just figured that because it
waited for completion, it didn't need it.

Adding s_umount is fine for the wirte path, but the create path
holds a directory i_mutex so I'd guess that means it has to be a
trylock to avoid lock inversion warnings from lockdep....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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