Re: [PATCH 4/8] xfs: don't log the inode in xfs_fs_map_blocks if it wasn't modified

[Christoph Hellwig <hch@lst.de>]

On Mon, Oct 28, 2019 at 09:12:45AM -0700, Darrick J. Wong wrote:
> On Fri, Oct 25, 2019 at 05:03:32PM +0200, Christoph Hellwig wrote:
> > Even if we are asked for a write layout there is no point in logging
> > the inode unless we actually modified it in some way.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  fs/xfs/xfs_pnfs.c | 43 +++++++++++++++++++------------------------
> >  1 file changed, 19 insertions(+), 24 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> > index 9c96493be9e0..fa90c6334c7c 100644
> > --- a/fs/xfs/xfs_pnfs.c
> > +++ b/fs/xfs/xfs_pnfs.c
> > @@ -147,32 +147,27 @@ xfs_fs_map_blocks(
> >  	if (error)
> >  		goto out_unlock;
> >  
> > -	if (write) {
> > -		enum xfs_prealloc_flags	flags = 0;
> > -
> > +	if (write &&
> > +	    (!nimaps || imap.br_startblock == HOLESTARTBLOCK)) {
> >  		ASSERT(imap.br_startblock != DELAYSTARTBLOCK);
> 
> The change in code flow makes this assert rather useless, I think, since
> we only end up in this branch if we have a write and a hole.  If the
> condition that it checks is important (and it seems to be?) then it
> ought to be hoisted up a level and turned into:
> 
> ASSERT(!write || !nimaps || imap.br_startblock != DELAYSTARTBLOCK);
> 
> Right?

Actually even for !write we should not see delalloc blocks here.
So I'll fix up the assert in a separate prep patch.