| Message ID | 20190923235224.GW2229799@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag") | expand |
On Mon, Sep 23, 2019 at 04:52:24PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Revert this commit, as it caused periodic regressions in xfs/173 w/ > 1k blocks[1]. > > [1] https://lore.kernel.org/lkml/20190919014602.GN15734@shao2-debian/ > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_alloc.h | 7 ++++--- > fs/xfs/libxfs/xfs_bmap.c | 8 ++++++-- > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h > index 58fa85cec325..d6ed5d2c07c2 100644 > --- a/fs/xfs/libxfs/xfs_alloc.h > +++ b/fs/xfs/libxfs/xfs_alloc.h > @@ -81,9 +81,10 @@ typedef struct xfs_alloc_arg { > /* > * Defines for datatype > */ > -#define XFS_ALLOC_INITIAL_USER_DATA (1 << 0)/* special case start of file */ > -#define XFS_ALLOC_USERDATA_ZERO (1 << 1)/* zero extent on allocation */ > -#define XFS_ALLOC_NOBUSY (1 << 2)/* Busy extents not allowed */ > +#define XFS_ALLOC_USERDATA (1 << 0)/* allocation is for user data*/ > +#define XFS_ALLOC_INITIAL_USER_DATA (1 << 1)/* special case start of file */ > +#define XFS_ALLOC_USERDATA_ZERO (1 << 2)/* zero extent on allocation */ > +#define XFS_ALLOC_NOBUSY (1 << 3)/* Busy extents not allowed */ > > static inline bool > xfs_alloc_is_userdata(int datatype) > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index eaf2d4250a26..4edc25a2ba80 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -4042,8 +4042,12 @@ xfs_bmapi_allocate( > */ > if (!(bma->flags & XFS_BMAPI_METADATA)) { > bma->datatype = XFS_ALLOC_NOBUSY; > - if (whichfork == XFS_DATA_FORK && bma->offset == 0) > - bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA; > + if (whichfork == XFS_DATA_FORK) { > + if (bma->offset == 0) > + bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA; > + else > + bma->datatype |= XFS_ALLOC_USERDATA; > + } > if (bma->flags & XFS_BMAPI_ZERO) > bma->datatype |= XFS_ALLOC_USERDATA_ZERO; > }
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 9/23/19 6:52 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Revert this commit, as it caused periodic regressions in xfs/173 w/ > 1k blocks[1]. > > [1] https://lore.kernel.org/lkml/20190919014602.GN15734@shao2-debian/ > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Came across this randomly ... since there's no explanation in the changelog as to /why/ this broke things, I think it's due to: static inline bool xfs_alloc_is_userdata(int datatype) { return (datatype & ~XFS_ALLOC_NOBUSY) != 0; } which would previously have returned true if (datatype & XFS_ALLOC_USERDATA), but without the any flag set returned false. So failing to set XFS_ALLOC_USERDATA indirectly changed the result of this test. So, you're welcome, future code archaeologists. :) -Eric
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h index 58fa85cec325..d6ed5d2c07c2 100644 --- a/fs/xfs/libxfs/xfs_alloc.h +++ b/fs/xfs/libxfs/xfs_alloc.h @@ -81,9 +81,10 @@ typedef struct xfs_alloc_arg { /* * Defines for datatype */ -#define XFS_ALLOC_INITIAL_USER_DATA (1 << 0)/* special case start of file */ -#define XFS_ALLOC_USERDATA_ZERO (1 << 1)/* zero extent on allocation */ -#define XFS_ALLOC_NOBUSY (1 << 2)/* Busy extents not allowed */ +#define XFS_ALLOC_USERDATA (1 << 0)/* allocation is for user data*/ +#define XFS_ALLOC_INITIAL_USER_DATA (1 << 1)/* special case start of file */ +#define XFS_ALLOC_USERDATA_ZERO (1 << 2)/* zero extent on allocation */ +#define XFS_ALLOC_NOBUSY (1 << 3)/* Busy extents not allowed */ static inline bool xfs_alloc_is_userdata(int datatype) diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index eaf2d4250a26..4edc25a2ba80 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -4042,8 +4042,12 @@ xfs_bmapi_allocate( */ if (!(bma->flags & XFS_BMAPI_METADATA)) { bma->datatype = XFS_ALLOC_NOBUSY; - if (whichfork == XFS_DATA_FORK && bma->offset == 0) - bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA; + if (whichfork == XFS_DATA_FORK) { + if (bma->offset == 0) + bma->datatype |= XFS_ALLOC_INITIAL_USER_DATA; + else + bma->datatype |= XFS_ALLOC_USERDATA; + } if (bma->flags & XFS_BMAPI_ZERO) bma->datatype |= XFS_ALLOC_USERDATA_ZERO; }