linux-next: build warning after merge of the vfs-brauner tree

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

Documentation/filesystems/api-summary:14: include/linux/fs.h:1655: WARNING: Definition list ends without a blank line; unexpected unindent.
include/linux/fs.h:1658: warning: Function parameter or member 'level' not described in '__sb_write_started'

Introduced by commit

  2a7b49f698d0 ("fs: create __sb_write_started() helper")

-- 
Cheers,
Stephen Rothwell

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

Re: linux-next: build warning after merge of the vfs-brauner tree

[Amir Goldstein]

On Fri, Nov 24, 2023 at 4:13 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the vfs-brauner tree, today's linux-next build (htmldocs)
> produced this warning:
>
> Documentation/filesystems/api-summary:14: include/linux/fs.h:1655: WARNING: Definition list ends without a blank line; unexpected unindent.
> include/linux/fs.h:1658: warning: Function parameter or member 'level' not described in '__sb_write_started'
>
> Introduced by commit
>
>   2a7b49f698d0 ("fs: create __sb_write_started() helper")

Christian,

Do you mind folding:

@@ -1648,6 +1648,7 @@ static inline bool
__sb_start_write_trylock(struct super_block *sb, int level)
 /**
  * __sb_write_started - check if sb freeze level is held
  * @sb: the super we write to
+ * @level: the freeze level
  *
  * > 0 sb freeze level is held
  *   0 sb freeze level is not held

Thanks,
Amir.

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (arm
multi_v7_defconfig) produced this warning:

fs/coredump.c: In function 'dump_user_range':
fs/coredump.c:923:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
  923 | #define dump_page_copy(src, dst) ((dst), (src))
      |                                        ^
fs/coredump.c:948:58: note: in expansion of macro 'dump_page_copy'
  948 |                         int stop = !dump_emit_page(cprm, dump_page_copy(page, dump_page));
      |                                                          ^~~~~~~~~~~~~~

Introduced by commit

  4630f2caafcd ("coredump: get machine check errors early rather than during iov_iter")

-- 
Cheers,
Stephen Rothwell

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

Re: linux-next: build warning after merge of the vfs-brauner tree

[Linus Torvalds]

On Tue, 5 Mar 2024 at 15:51, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> fs/coredump.c: In function 'dump_user_range':
> fs/coredump.c:923:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
>   923 | #define dump_page_copy(src, dst) ((dst), (src))
>       |                                        ^
> fs/coredump.c:948:58: note: in expansion of macro 'dump_page_copy'
>   948 |                         int stop = !dump_emit_page(cprm, dump_page_copy(page, dump_page));
>       |                                                          ^~~~~~~~~~~~~~
>
> Introduced by commit
>
>   4630f2caafcd ("coredump: get machine check errors early rather than during iov_iter")

Bah. If comes from that

  #define dump_page_copy(src,dst) ((dst),(src))

and I did it that way because I wanted to avoid *another* warning,
namely the "dst not used" thing.

But it would have probably been better to either make it an inline
function, or maybe an explicit cast, eg

  #define dump_page_copy(src,dst) ((void)(dst),(src))

or whatever.

                   Linus

Re: linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

On Tue, 5 Mar 2024 18:48:30 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> On Tue, 5 Mar 2024 at 15:51, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > fs/coredump.c: In function 'dump_user_range':
> > fs/coredump.c:923:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
> >   923 | #define dump_page_copy(src, dst) ((dst), (src))
> >       |                                        ^
> > fs/coredump.c:948:58: note: in expansion of macro 'dump_page_copy'
> >   948 |                         int stop = !dump_emit_page(cprm, dump_page_copy(page, dump_page));
> >       |                                                          ^~~~~~~~~~~~~~
> >
> > Introduced by commit
> >
> >   4630f2caafcd ("coredump: get machine check errors early rather than during iov_iter")  
> 
> Bah. If comes from that
> 
>   #define dump_page_copy(src,dst) ((dst),(src))
> 
> and I did it that way because I wanted to avoid *another* warning,
> namely the "dst not used" thing.
> 
> But it would have probably been better to either make it an inline
> function, or maybe an explicit cast, eg
> 
>   #define dump_page_copy(src,dst) ((void)(dst),(src))
> 
> or whatever.

This became a build failure for my i386 defconfig build, so I did this:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 6 Mar 2024 15:28:12 +1100
Subject: [PATCH] fix up for "coredump: get machine check errors early rather
 than during iov_iter"

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

diff --git a/fs/coredump.c b/fs/coredump.c
index ea155ffee14c..5353b7ac67f2 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -920,7 +920,10 @@ static struct page *dump_page_copy(struct page *src, struct page *dst)
 
 #define dump_page_alloc() ((struct page *)8) // Not NULL
 #define dump_page_free(x) do { } while (0)
-#define dump_page_copy(src, dst) ((dst), (src))
+static struct page *dump_page_copy(struct page *src, struct page *dst)
+{
+        return NULL;
+}
 
 #endif
 
-- 
2.43.0

-- 
Cheers,
Stephen Rothwell

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

Re: linux-next: build warning after merge of the vfs-brauner tree

[Linus Torvalds]

On Tue, 5 Mar 2024 at 20:37, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> +static struct page *dump_page_copy(struct page *src, struct page *dst)
> +{
> +        return NULL;
> +}

No, it needs to be "return src;" not NULL.

That

  #define dump_page_copy(src, dst) ((dst), (src))

was supposed to be a "use 'dst', return 'src'" macro, and is correct
as that. The problem - as you noticed - is that it causes that "left
side of comma expression has no effect" warning.

(Technically it *does* have an effect - exactly the "argument is used"
one - but the compiler warning does make sense).

Actually, the simplest thing to do is probably just

  #define dump_page_free(x) ((void)(x))
  #define dump_page_copy(src, dst) (src)

where the "use" of the 'dump_page' argument is that dump_page_free()
void cast, and dump_page_copy() simply doesn't need to use it at all.

Christian?

            Linus

Re: linux-next: build warning after merge of the vfs-brauner tree

[Christian Brauner]

On Tue, Mar 05, 2024 at 08:47:40PM -0800, Linus Torvalds wrote:
> On Tue, 5 Mar 2024 at 20:37, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > +static struct page *dump_page_copy(struct page *src, struct page *dst)
> > +{
> > +        return NULL;
> > +}
> 
> No, it needs to be "return src;" not NULL.
> 
> That
> 
>   #define dump_page_copy(src, dst) ((dst), (src))
> 
> was supposed to be a "use 'dst', return 'src'" macro, and is correct
> as that. The problem - as you noticed - is that it causes that "left
> side of comma expression has no effect" warning.
> 
> (Technically it *does* have an effect - exactly the "argument is used"
> one - but the compiler warning does make sense).
> 
> Actually, the simplest thing to do is probably just
> 
>   #define dump_page_free(x) ((void)(x))
>   #define dump_page_copy(src, dst) (src)
> 
> where the "use" of the 'dump_page' argument is that dump_page_free()
> void cast, and dump_page_copy() simply doesn't need to use it at all.
> 
> Christian?

I would just do it like Stephen did (but returning src ofc) because it's
symmetric to the #ifdef copy_mc_to_kernel definition of dump_page_copy()
and seems easier to read to me.

But I honestly don't care too much. So I'd pick Stephen's change and if
you prefer to do it differently just change it when I send you the pr.

+/*
+ * If we might get machine checks from kernel accesses during the
+ * core dump, let's get those errors early rather than during the
+ * IO. This is not performance-critical enough to warrant having
+ * all the machine check logic in the iovec paths.
+ */
+#ifdef copy_mc_to_kernel
+
+#define dump_page_alloc() alloc_page(GFP_KERNEL)
+#define dump_page_free(x) __free_page(x)
+static struct page *dump_page_copy(struct page *src, struct page *dst)
+{
+       void *buf = kmap_local_page(src);
+       size_t left = copy_mc_to_kernel(page_address(dst), buf, PAGE_SIZE);
+       kunmap_local(buf);
+       return left ? NULL : dst;
+}
+
+#else
+
+/* We just want to return non-NULL; it's never used. */
+#define dump_page_alloc() ERR_PTR(-EINVAL)
+#define dump_page_free(x) ((void)(x))
+static inline struct page *dump_page_copy(struct page *src, struct page *dst)
+{
+       return src;
+}
+#endif

Re: linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

On Wed, 6 Mar 2024 15:37:03 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Tue, 5 Mar 2024 18:48:30 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> > On Tue, 5 Mar 2024 at 15:51, Stephen Rothwell <sfr@canb.auug.org.au> wrote:  
> > >
> > > fs/coredump.c: In function 'dump_user_range':
> > > fs/coredump.c:923:40: warning: left-hand operand of comma expression has no effect [-Wunused-value]
> > >   923 | #define dump_page_copy(src, dst) ((dst), (src))
> > >       |                                        ^
> > > fs/coredump.c:948:58: note: in expansion of macro 'dump_page_copy'
> > >   948 |                         int stop = !dump_emit_page(cprm, dump_page_copy(page, dump_page));
> > >       |                                                          ^~~~~~~~~~~~~~
> > >
> > > Introduced by commit
> > >
> > >   4630f2caafcd ("coredump: get machine check errors early rather than during iov_iter")    
> > 
> > Bah. If comes from that
> > 
> >   #define dump_page_copy(src,dst) ((dst),(src))
> > 
> > and I did it that way because I wanted to avoid *another* warning,
> > namely the "dst not used" thing.
> > 
> > But it would have probably been better to either make it an inline
> > function, or maybe an explicit cast, eg
> > 
> >   #define dump_page_copy(src,dst) ((void)(dst),(src))
> > 
> > or whatever.  
> 
> This became a build failure for my i386 defconfig build, so I did this:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 6 Mar 2024 15:28:12 +1100
> Subject: [PATCH] fix up for "coredump: get machine check errors early rather
>  than during iov_iter"
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  fs/coredump.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/coredump.c b/fs/coredump.c
> index ea155ffee14c..5353b7ac67f2 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -920,7 +920,10 @@ static struct page *dump_page_copy(struct page *src, struct page *dst)
>  
>  #define dump_page_alloc() ((struct page *)8) // Not NULL
>  #define dump_page_free(x) do { } while (0)
> -#define dump_page_copy(src, dst) ((dst), (src))
> +static struct page *dump_page_copy(struct page *src, struct page *dst)
> +{
> +        return NULL;
> +}
>  
>  #endif

On second thoughts I made it return "src" instead of "NULL";

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

Documentation/filesystems/index.rst:63: WARNING: toctree contains reference to nonexisting document 'filesystems/ntfs'

Introduced by commit

  9c67092ed339 ("fs: Remove NTFS classic")

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (s390
defconfig) produced this warning:

arch/s390/configs/defconfig:626:warning: symbol value 'm' invalid for FSCACHE

Introduced by commit

  9896c4f367fc ("netfs, fscache: Combine fscache with netfs")

-- 
Cheers,
Stephen Rothwell

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

Re: linux-next: build warning after merge of the vfs-brauner tree

[David Howells]

Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> 
> After merging the vfs-brauner tree, today's linux-next build (s390
> defconfig) produced this warning:
> 
> arch/s390/configs/defconfig:626:warning: symbol value 'm' invalid for FSCACHE
> 
> Introduced by commit
> 
>   9896c4f367fc ("netfs, fscache: Combine fscache with netfs")

I've fixed the arches that had FSCACHE=m to have NETFS_SUPPORT=m and
FSCACHE=y.

David

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

include/linux/pipe_fs_i.h:132: warning: Function parameter or member 'pipe' not described in 'pipe_has_watch_queue'

Introduced by commit

  7084dde72592 ("fs/pipe: move check to pipe_has_watch_queue()")

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

fs/inode.c:759: warning: Excess function parameter 'kill_dirty' description in 'invalidate_inodes'

Introduced by commit

  fc3b2b8e7199 ("fs: simplify invalidate_inodes")

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

Documentation/filesystems/api-summary:38: fs/super.c:558: ERROR: Unexpected indentation.
Documentation/filesystems/api-summary:38: fs/super.c:559: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/filesystems/api-summary:38: fs/super.c:561: WARNING: Enumerated list ends without a blank line; unexpected unindent.


Introduced by commit

  c9eca69d372c ("fs: add FSCONFIG_CMD_CREATE_EXCL")

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

fs/stat.c:79: warning: Function parameter or member 'request_mask' not described in 'generic_fillattr'

Introduced by commit

  0a6ab6dc6958 ("fs: pass the request_mask to generic_fillattr")

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build
(htmldocs) produced this warning:

Documentation/filesystems/locking.rst:119: ERROR: Malformed table.
Text in column margin in table line 27.

==============  =============================================
ops             i_rwsem(inode)
==============  =============================================
lookup:         shared
create:         exclusive
link:           exclusive (both)
mknod:          exclusive
symlink:        exclusive
mkdir:          exclusive
unlink:         exclusive (both)
rmdir:          exclusive (both)(see below)
rename:         exclusive (all) (see below)
readlink:       no
get_link:       no
setattr:        exclusive
permission:     no (may not block if called in rcu-walk mode)
get_inode_acl:  no
get_acl:        no
getattr:        no
listxattr:      no
fiemap:         no
update_time:    no
atomic_open:    shared (exclusive if O_CREAT is set in open flags)
tmpfile:        no
fileattr_get:   no or exclusive
fileattr_set:   exclusive
get_offset_ctx: no
==============  =============================================

Introduced by commit

  7a3472ae9614 ("libfs: Add directory operations for stable offsets")

-- 
Cheers,
Stephen Rothwell

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

linux-next: build warning after merge of the vfs-brauner tree

[Stephen Rothwell]

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

Hi all,

After merging the vfs-brauner tree, today's linux-next build (htmldocs)
produced this warning:

fs/stat.c:80: warning: Function parameter or member 'request_mask' not described in 'generic_fillattr'

Introduced by commit

  8fb690f6de31 ("fs: pass the request_mask to generic_fillattr")

-- 
Cheers,
Stephen Rothwell

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

Re: linux-next: build warning after merge of the vfs-brauner tree

[Christian Brauner]

On Wed, May 24, 2023 at 01:46:23PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the vfs-brauner tree, today's linux-next build (htmldocs)
> produced this warning:
> 
> fs/stat.c:80: warning: Function parameter or member 'request_mask' not described in 'generic_fillattr'
> 
> Introduced by commit
> 
>   8fb690f6de31 ("fs: pass the request_mask to generic_fillattr")

Thanks, Jeff will send a new version of the series which should fix
this hopefully.