On Tue, Oct 29, 2019 at 08:58:43AM +0100, Christoph Hellwig wrote:
> 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@xxxxxx>
> > > ---
> > > 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.
<shrug> I could just fix it, unless you're about to resend the whole series?
--D
