xfs
[Top] [All Lists]

Re: [PATCH 08/25] xfs: introduce xfs_bmapi_delay()

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH 08/25] xfs: introduce xfs_bmapi_delay()
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 6 Sep 2011 11:27:06 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <1315002238.2069.88.camel@doink>
References: <20110824060428.789245205@xxxxxxxxxxxxxxxxxxxxxx> <20110824060641.979845427@xxxxxxxxxxxxxxxxxxxxxx> <1315002238.2069.88.camel@doink>
User-agent: Mutt/1.5.21 (2010-09-15)
> > +   xfs_extnum_t            lastx;  /* last useful extent number */
> > +   int                     eof;    /* we've hit the end of extents */
> > +   int                     n = 0;  /* current extent index */
> > +   int                     error = 0;
> > +
> > +   ASSERT(*nmap >= 1);
> > +   ASSERT(*nmap <= XFS_BMAP_MAX_NMAP);
> > +   ASSERT(!(flags & ~XFS_BMAPI_ENTIRE));
> > +
> 
> Rearrange the following test to use the pattern (assigning error)
> used in xfs_bmapi_read().

Hmm - given that error is used as a boolean there I don't actually like
that pattern very much as error is generally used to hold an errno
value.

> 
> > +   if (unlikely(XFS_TEST_ERROR(
> > +       (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS &&
> > +        XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE),
> 
> Are you certain that XFS_DINODE_FMT_LOCAL is not possible here?
> I tried to trace it back but I'm still not sure.  The transaction
> pointer passed is null, so it would have tripped an assertion
> in the previous code.  (A simple explanation would reassure me.)

We can't hit it because we do not support the local format for regular
files at all, and we do not support delayed allocations for anything
but regular files.

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