linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag")
@ 2019-09-23 23:52 Darrick J. Wong
  2019-09-24  9:39 ` Brian Foster
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Darrick J. Wong @ 2019-09-23 23:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Brian Foster, linux-xfs

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>
---
 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;
 	}

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag")
  2019-09-23 23:52 [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag") Darrick J. Wong
@ 2019-09-24  9:39 ` Brian Foster
  2019-09-24 21:30 ` Christoph Hellwig
  2019-12-04 16:59 ` Eric Sandeen
  2 siblings, 0 replies; 4+ messages in thread
From: Brian Foster @ 2019-09-24  9:39 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs

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;
>  	}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag")
  2019-09-23 23:52 [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag") Darrick J. Wong
  2019-09-24  9:39 ` Brian Foster
@ 2019-09-24 21:30 ` Christoph Hellwig
  2019-12-04 16:59 ` Eric Sandeen
  2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2019-09-24 21:30 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, Brian Foster, linux-xfs

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag")
  2019-09-23 23:52 [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag") Darrick J. Wong
  2019-09-24  9:39 ` Brian Foster
  2019-09-24 21:30 ` Christoph Hellwig
@ 2019-12-04 16:59 ` Eric Sandeen
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2019-12-04 16:59 UTC (permalink / raw)
  To: Darrick J. Wong, Christoph Hellwig; +Cc: Brian Foster, linux-xfs

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-12-04 16:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-23 23:52 [PATCH] xfs: revert 1baa2800e62d ("xfs: remove the unused XFS_ALLOC_USERDATA flag") Darrick J. Wong
2019-09-24  9:39 ` Brian Foster
2019-09-24 21:30 ` Christoph Hellwig
2019-12-04 16:59 ` Eric Sandeen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).