linux-next: build failure after merge of the mm tree

linux-next: build failure after merge of the mm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

Hi all,

After merging the mm tree, today's linux-next build (x86_64 allmodconfig)
failed like this:

fs/xfs/xfs_notify_failure.c: In function 'xfs_dax_notify_ddev_failure':
fs/xfs/xfs_notify_failure.c:126:44: error: passing argument 1 of 'xfs_alloc_read_agf' from incompatible pointer type [-Werror=incompatible-pointer-types]
  126 |                 error = xfs_alloc_read_agf(mp, tp, agno, 0, &agf_bp);
      |                                            ^~
      |                                            |
      |                                            struct xfs_mount *
In file included from fs/xfs/xfs_notify_failure.c:12:
fs/xfs/libxfs/xfs_alloc.h:173:42: note: expected 'struct xfs_perag *' but argument is of type 'struct xfs_mount *'
  173 | int xfs_alloc_read_agf(struct xfs_perag *pag, struct xfs_trans *tp, int flags,
      |                        ~~~~~~~~~~~~~~~~~~^~~
fs/xfs/xfs_notify_failure.c:126:25: error: too many arguments to function 'xfs_alloc_read_agf'
  126 |                 error = xfs_alloc_read_agf(mp, tp, agno, 0, &agf_bp);
      |                         ^~~~~~~~~~~~~~~~~~
In file included from fs/xfs/xfs_notify_failure.c:12:
fs/xfs/libxfs/xfs_alloc.h:173:5: note: declared here
  173 | int xfs_alloc_read_agf(struct xfs_perag *pag, struct xfs_trans *tp, int flags,
      |     ^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Caused by commit

  469a9c74c119 ("xfs: implement ->notify_failure() for XFS")

interacting with commit

  c4829aba9c8d ("xfs: pass perag to xfs_alloc_read_agf()")

from the xfs tree.

I have applied the following merge fix patch.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 8 Jul 2022 19:11:56 +1000
Subject: [PATCH] fix up for "xfs: pass perag to xfs_alloc_read_agf()"

interacting with "xfs: implement ->notify_failure() for XFS"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/xfs/xfs_notify_failure.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index aa8dc27c599c..f3c62c19475e 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -18,6 +18,7 @@
 #include "xfs_rmap_btree.h"
 #include "xfs_rtalloc.h"
 #include "xfs_trans.h"
+#include "xfs_ag.h"
 
 #include <linux/mm.h>
 #include <linux/dax.h>
@@ -122,8 +123,10 @@ xfs_dax_notify_ddev_failure(
 		struct failure_info	notify;
 		struct xfs_agf		*agf;
 		xfs_agblock_t		agend;
+		struct xfs_perag	*pag;
 
-		error = xfs_alloc_read_agf(mp, tp, agno, 0, &agf_bp);
+		pag = xfs_perag_get(mp, agno);
+		error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
 		if (error)
 			break;
 
-- 
2.35.1

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the mm tree

[Darrick J. Wong]

On Fri, Jul 08, 2022 at 07:44:37PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the mm tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
> 
> fs/xfs/xfs_notify_failure.c: In function 'xfs_dax_notify_ddev_failure':
> fs/xfs/xfs_notify_failure.c:126:44: error: passing argument 1 of 'xfs_alloc_read_agf' from incompatible pointer type [-Werror=incompatible-pointer-types]
>   126 |                 error = xfs_alloc_read_agf(mp, tp, agno, 0, &agf_bp);
>       |                                            ^~
>       |                                            |
>       |                                            struct xfs_mount *
> In file included from fs/xfs/xfs_notify_failure.c:12:
> fs/xfs/libxfs/xfs_alloc.h:173:42: note: expected 'struct xfs_perag *' but argument is of type 'struct xfs_mount *'
>   173 | int xfs_alloc_read_agf(struct xfs_perag *pag, struct xfs_trans *tp, int flags,
>       |                        ~~~~~~~~~~~~~~~~~~^~~
> fs/xfs/xfs_notify_failure.c:126:25: error: too many arguments to function 'xfs_alloc_read_agf'
>   126 |                 error = xfs_alloc_read_agf(mp, tp, agno, 0, &agf_bp);
>       |                         ^~~~~~~~~~~~~~~~~~
> In file included from fs/xfs/xfs_notify_failure.c:12:
> fs/xfs/libxfs/xfs_alloc.h:173:5: note: declared here
>   173 | int xfs_alloc_read_agf(struct xfs_perag *pag, struct xfs_trans *tp, int flags,
>       |     ^~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Caused by commit
> 
>   469a9c74c119 ("xfs: implement ->notify_failure() for XFS")
> 
> interacting with commit
> 
>   c4829aba9c8d ("xfs: pass perag to xfs_alloc_read_agf()")
> 
> from the xfs tree.
> 
> I have applied the following merge fix patch.
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 8 Jul 2022 19:11:56 +1000
> Subject: [PATCH] fix up for "xfs: pass perag to xfs_alloc_read_agf()"
> 
> interacting with "xfs: implement ->notify_failure() for XFS"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/xfs/xfs_notify_failure.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> index aa8dc27c599c..f3c62c19475e 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -18,6 +18,7 @@
>  #include "xfs_rmap_btree.h"
>  #include "xfs_rtalloc.h"
>  #include "xfs_trans.h"
> +#include "xfs_ag.h"
>  
>  #include <linux/mm.h>
>  #include <linux/dax.h>
> @@ -122,8 +123,10 @@ xfs_dax_notify_ddev_failure(
>  		struct failure_info	notify;
>  		struct xfs_agf		*agf;
>  		xfs_agblock_t		agend;
> +		struct xfs_perag	*pag;
>  
> -		error = xfs_alloc_read_agf(mp, tp, agno, 0, &agf_bp);
> +		pag = xfs_perag_get(mp, agno);
> +		error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
>  		if (error)
>  			break;

This isn't quite correct -- references to xfs_perag objects must be
released once they are acquired.  The following patch against today's
tree fixes this problem:

diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index f3c62c19475e..69d9c83ea4b2 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -127,10 +127,12 @@ xfs_dax_notify_ddev_failure(
 
 		pag = xfs_perag_get(mp, agno);
 		error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
-		if (error)
+		if (error) {
+			xfs_perag_put(pag);
 			break;
+		}
 
-		cur = xfs_rmapbt_init_cursor(mp, tp, agf_bp, agf_bp->b_pag);
+		cur = xfs_rmapbt_init_cursor(mp, tp, agf_bp, pag);
 
 		/*
 		 * Set the rmap range from ri_low to ri_high, which represents
@@ -151,6 +153,7 @@ xfs_dax_notify_ddev_failure(
 				xfs_dax_failure_fn, &notify);
 		xfs_btree_del_cursor(cur, error);
 		xfs_trans_brelse(tp, agf_bp);
+		xfs_perag_put(pag);
 		if (error)
 			break;
 

--D

>  
> -- 
> 2.35.1
> 
> -- 
> Cheers,
> Stephen Rothwell

Re: linux-next: build failure after merge of the mm tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]

Hi Darrick,

On Fri, 8 Jul 2022 08:17:51 -0700 "Darrick J. Wong" <djwong@kernel.org> wrote:
>
> This isn't quite correct -- references to xfs_perag objects must be
> released once they are acquired.  The following patch against today's
> tree fixes this problem:
> 
> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> index f3c62c19475e..69d9c83ea4b2 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -127,10 +127,12 @@ xfs_dax_notify_ddev_failure(
>  
>  		pag = xfs_perag_get(mp, agno);
>  		error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
> -		if (error)
> +		if (error) {
> +			xfs_perag_put(pag);
>  			break;
> +		}
>  
> -		cur = xfs_rmapbt_init_cursor(mp, tp, agf_bp, agf_bp->b_pag);
> +		cur = xfs_rmapbt_init_cursor(mp, tp, agf_bp, pag);
>  
>  		/*
>  		 * Set the rmap range from ri_low to ri_high, which represents
> @@ -151,6 +153,7 @@ xfs_dax_notify_ddev_failure(
>  				xfs_dax_failure_fn, &notify);
>  		xfs_btree_del_cursor(cur, error);
>  		xfs_trans_brelse(tp, agf_bp);
> +		xfs_perag_put(pag);
>  		if (error)
>  			break;
>  

Thanks, I will add that to the resolution from today.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build failure after merge of the mm tree

[Matthew Wilcox]

On Tue, Aug 22, 2023 at 11:22:17AM +1000, Stephen Rothwell wrote:
> Hi Matthew,
> 
> On Tue, 22 Aug 2023 02:11:44 +0100 Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Tue, Aug 22, 2023 at 09:55:37AM +1000, Stephen Rothwell wrote:
> > > In file included from include/trace/trace_events.h:27,
> > >                  from include/trace/define_trace.h:102,
> > >                  from fs/xfs/xfs_trace.h:4428,
> > >                  from fs/xfs/xfs_trace.c:45:
> > > include/linux/pgtable.h:8:25: error: initializer element is not constant
> > >     8 | #define PMD_ORDER       (PMD_SHIFT - PAGE_SHIFT)  
> > 
> > Ummm.  PowerPC doesn't have a compile-time constant PMD size?
> 
> Yeah, you are not the first (or probably the last) to be caught by that.

I think this will do the trick.  Any comments?

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 1904eaf7a2e9..d5a4e6c2dcd1 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -796,15 +796,6 @@ DEFINE_INODE_EVENT(xfs_inode_reclaiming);
 DEFINE_INODE_EVENT(xfs_inode_set_need_inactive);
 DEFINE_INODE_EVENT(xfs_inode_inactivating);
 
-/*
- * ftrace's __print_symbolic requires that all enum values be wrapped in the
- * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
- * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
- * code.
- */
-TRACE_DEFINE_ENUM(PMD_ORDER);
-TRACE_DEFINE_ENUM(PUD_ORDER);
-
 TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_SHARED);
 TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_COW);
 
@@ -823,13 +814,10 @@ TRACE_EVENT(xfs_filemap_fault,
 		__entry->order = order;
 		__entry->write_fault = write_fault;
 	),
-	TP_printk("dev %d:%d ino 0x%llx %s write_fault %d",
+	TP_printk("dev %d:%d ino 0x%llx order:%u write_fault %d",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
-		  __print_symbolic(__entry->order,
-			{ 0,		"PTE" },
-			{ PMD_ORDER,	"PMD" },
-			{ PUD_ORDER,	"PUD" }),
+		  __entry->order,
 		  __entry->write_fault)
 )
 

Re: linux-next: build failure after merge of the mm tree

[Darrick J. Wong]

On Tue, Aug 22, 2023 at 02:34:06AM +0100, Matthew Wilcox wrote:
> On Tue, Aug 22, 2023 at 11:22:17AM +1000, Stephen Rothwell wrote:
> > Hi Matthew,
> > 
> > On Tue, 22 Aug 2023 02:11:44 +0100 Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 09:55:37AM +1000, Stephen Rothwell wrote:
> > > > In file included from include/trace/trace_events.h:27,
> > > >                  from include/trace/define_trace.h:102,
> > > >                  from fs/xfs/xfs_trace.h:4428,
> > > >                  from fs/xfs/xfs_trace.c:45:
> > > > include/linux/pgtable.h:8:25: error: initializer element is not constant
> > > >     8 | #define PMD_ORDER       (PMD_SHIFT - PAGE_SHIFT)  
> > > 
> > > Ummm.  PowerPC doesn't have a compile-time constant PMD size?
> > 
> > Yeah, you are not the first (or probably the last) to be caught by that.
> 
> I think this will do the trick.  Any comments?
> 
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 1904eaf7a2e9..d5a4e6c2dcd1 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -796,15 +796,6 @@ DEFINE_INODE_EVENT(xfs_inode_reclaiming);
>  DEFINE_INODE_EVENT(xfs_inode_set_need_inactive);
>  DEFINE_INODE_EVENT(xfs_inode_inactivating);
>  
> -/*
> - * ftrace's __print_symbolic requires that all enum values be wrapped in the
> - * TRACE_DEFINE_ENUM macro so that the enum value can be encoded in the ftrace
> - * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
> - * code.
> - */

Please leave this ^^^ comment, because the need for TRACE_DEFINE_ENUM to
make enums work in tracepoints is not at all obvious.

> -TRACE_DEFINE_ENUM(PMD_ORDER);
> -TRACE_DEFINE_ENUM(PUD_ORDER);
> -
>  TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_SHARED);
>  TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_COW);
>  
> @@ -823,13 +814,10 @@ TRACE_EVENT(xfs_filemap_fault,
>  		__entry->order = order;
>  		__entry->write_fault = write_fault;
>  	),
> -	TP_printk("dev %d:%d ino 0x%llx %s write_fault %d",
> +	TP_printk("dev %d:%d ino 0x%llx order:%u write_fault %d",

"order %u" to match the (non dev_t) style of the rest of the xfs
tracepoints.

--D

>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->ino,
> -		  __print_symbolic(__entry->order,
> -			{ 0,		"PTE" },
> -			{ PMD_ORDER,	"PMD" },
> -			{ PUD_ORDER,	"PUD" }),
> +		  __entry->order,
>  		  __entry->write_fault)
>  )
>  
> 
> 

Re: linux-next: build failure after merge of the mm tree

[Matthew Wilcox]

On Mon, Aug 21, 2023 at 09:00:43PM -0700, Darrick J. Wong wrote:
> Please leave this ^^^ comment, because the need for TRACE_DEFINE_ENUM to
> make enums work in tracepoints is not at all obvious.
> 
> "order %u" to match the (non dev_t) style of the rest of the xfs
> tracepoints.

ACK, thanks.

Andrew, please add this -fix patch for "mm: Remove enum
page_entry_size".

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 1904eaf7a2e9..fd789e00dfd6 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -802,9 +802,6 @@ DEFINE_INODE_EVENT(xfs_inode_inactivating);
  * ring buffer.  Somehow this was only worth mentioning in the ftrace sample
  * code.
  */
-TRACE_DEFINE_ENUM(PMD_ORDER);
-TRACE_DEFINE_ENUM(PUD_ORDER);
-
 TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_SHARED);
 TRACE_DEFINE_ENUM(XFS_REFC_DOMAIN_COW);
 
@@ -823,13 +820,10 @@ TRACE_EVENT(xfs_filemap_fault,
 		__entry->order = order;
 		__entry->write_fault = write_fault;
 	),
-	TP_printk("dev %d:%d ino 0x%llx %s write_fault %d",
+	TP_printk("dev %d:%d ino 0x%llx order %u write_fault %d",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
-		  __print_symbolic(__entry->order,
-			{ 0,		"PTE" },
-			{ PMD_ORDER,	"PMD" },
-			{ PUD_ORDER,	"PUD" }),
+		  __entry->order,
 		  __entry->write_fault)
 )