Re: [PATCH] mm: forward declare struct rcuwait together with rcuwait_wake_up()

[Suren Baghdasaryan <surenb@google.com>]

On Tue, Sep 16, 2025 at 11:12 PM Thomas Weißschuh
<thomas.weissschuh@linutronix.de> wrote:
>
> On Tue, Sep 16, 2025 at 03:04:21PM -0700, Suren Baghdasaryan wrote:
> > On Tue, Sep 16, 2025 at 6:59 AM Thomas Weißschuh
> > <thomas.weissschuh@linutronix.de> wrote:
> > >
> > > At the point of the forward declaration of rcuwait_wake_up()
> > > in mmap_lock.h 'struct rcuwait' may have not yet been declared,
> > > leading to compiler errors.
> > >
> > > Add an explicit forward declaration for the struct.
> > >
> > > Fixes: 75404e07663b ("mm: move mmap/vma locking logic into specific files")
> > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > > ---
> > > This doesn't seem to break any in-tree code right now.
> > > I stumbled upon it while building a series for the next cycle.
> > > Instead of putting this fix into my series and spamming all the mm
> > > maintainers with it, maybe this could be part of the last mm bugfix pull
> > > for this cycle.
> >
> > `struct rcuwait` is defined inside include/linux/types.h and
> > mmap_lock.h includes that file.
>
> Yes, linux/types.h is included, but only after the usage of 'struct rcuwait'.
> We could also order around the '#include <linux/types.h>' before the
> declaration of rcuwait_wake_up(), but to me my current proposal looks cleaner.
>
> > Could you please explain in more
> > detail what exactly failed when you were building it?
>
> With the following change for test purposes:
>
> diff --git a/init/main.c b/init/main.c
> index 0d4510a7a5c2..7523786e6ad1 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -10,6 +10,8 @@
>   *  Simplified starting of init:  Michael A. Griffith <grif@acm.org>
>   */
>
> +#include <linux/mmap_lock.h>
> +
>  #define DEBUG          /* Enable initcall_debug */
>
>  #include <linux/types.h>
>
>
> This is the error:
>
> In file included from .../init/main.c:13:
> .../include/linux/mmap_lock.h:6:35: error: 'struct rcuwait' declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
>     6 | extern int rcuwait_wake_up(struct rcuwait *w);
>       |                                   ^~~~~~~
> In file included from .../include/linux/percpu-rwsem.h:7,
>                  from .../include/linux/fs.h:34,
>                  from .../include/linux/proc_fs.h:10,
>                  from .../init/main.c:21:
> .../include/linux/rcuwait.h:26:12: error: conflicting types for 'rcuwait_wake_up'; have 'int(struct rcuwait *)'
>    26 | extern int rcuwait_wake_up(struct rcuwait *w);
>       |            ^~~~~~~~~~~~~~~
> .../include/linux/mmap_lock.h:6:12: note: previous declaration of 'rcuwait_wake_up' with type 'int(struct rcuwait *)'
>     6 | extern int rcuwait_wake_up(struct rcuwait *w);
>       |            ^~~~~~~~~~~~~~~
>

I see, thanks!
I would prefer simply moving rcuwait_wake_up() forward declaration
after the includes, like this:


 #ifndef _LINUX_MMAP_LOCK_H
 #define _LINUX_MMAP_LOCK_H

-/* Avoid a dependency loop by declaring here. */
-extern int rcuwait_wake_up(struct rcuwait *w);
-
 #include <linux/lockdep.h>
 #include <linux/mm_types.h>
 #include <linux/mmdebug.h>
@@ -14,6 +11,9 @@ extern int rcuwait_wake_up(struct rcuwait *w);
 #include <linux/cleanup.h>
 #include <linux/sched/mm.h>

+extern int rcuwait_wake_up(struct rcuwait *w);
+
 #define MMAP_LOCK_INITIALIZER(name) \


This would avoid extra forward declarations. Not sure why it was
placed before includes in the first place. I tried moving it and
things build just fine. Lorenzo, do you recall specific reasons the
forward declaration should have been placed before the includes?

> > > ---
> > >  include/linux/mmap_lock.h | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> > > index 11a078de9150df1beff4f0bfb16e199333767614..9792dd4fff0ff73829833aae8ea3229a31757d61 100644
> > > --- a/include/linux/mmap_lock.h
> > > +++ b/include/linux/mmap_lock.h
> > > @@ -3,6 +3,7 @@
> > >  #define _LINUX_MMAP_LOCK_H
> > >
> > >  /* Avoid a dependency loop by declaring here. */
> > > +struct rcuwait;
> > >  extern int rcuwait_wake_up(struct rcuwait *w);
> > >
> > >  #include <linux/lockdep.h>
> > >
> > > ---
> > > base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> > > change-id: 20250916-mm-rcuwait-03c5fe95f36d
> > >
> > > Best regards,
> > > --
> > > Thomas Weißschuh <thomas.weissschuh@linutronix.de>