linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
To: Suren Baghdasaryan <surenb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 "Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	 Vlastimil Babka <vbabka@suse.cz>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: forward declare struct rcuwait together with rcuwait_wake_up()
Date: Wed, 17 Sep 2025 08:12:53 +0200	[thread overview]
Message-ID: <20250917080725-14901c66-b02d-4d81-8a42-1283333d3966@linutronix.de> (raw)
In-Reply-To: <CAJuCfpE-DPBp-0b0908FHvguSo_PqxE2LdUXvBiMLu1UCd5icQ@mail.gmail.com>

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);
      |            ^~~~~~~~~~~~~~~

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


  reply	other threads:[~2025-09-17  6:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-16 13:59 Thomas Weißschuh
2025-09-16 22:04 ` Suren Baghdasaryan
2025-09-17  6:12   ` Thomas Weißschuh [this message]
2025-09-17 16:32     ` Suren Baghdasaryan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250917080725-14901c66-b02d-4d81-8a42-1283333d3966@linutronix.de \
    --to=thomas.weissschuh@linutronix.de \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=shakeel.butt@linux.dev \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox