[PATCH 1/1] container_of: Document container_of() is not to be used in new code

[PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Sakari Ailus]

There is a warning in the kerneldoc documentation of container_of() that
constness of its ptr argument is lost. While this is a faible suggestion
container_of_const() should be used instead, the vast majority of new code
still uses container_of():

$ git diff v6.13 v6.14|grep container_of\(|wc -l
646
$ git diff v6.13 v6.14|grep container_of_const|wc -l
9

Make an explicit recommendation to use container_of_const().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hi Greg, Andy,

I guess we generally agree the additional constness check in
container_of_const() is useful, but adding the same check to
container_of() generates warnings -- there are some errors, too -- such as
this one currently:

In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
                 from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
/home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
/home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
      |              ^

As noted above, 646 new missing constness checks were introduced through
container_of() macro use during the 6.14 cycle alone. Most of these are
likely harmless, but with so many new users some are bound to be ignoring
constness.

Once the warnings from bad container_of() use are worked out in a way or
another, the constness check could be added to the container_of() macro
and the current container_of_const() be dropped altogether.

If this patch is accepted, I'll see how to add a warning on container_of()
to checkpatch.pl.

- Sakari

 include/linux/container_of.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 713890c867be..40139b52036a 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -13,7 +13,8 @@
  * @type:	the type of the container struct this is embedded in.
  * @member:	the name of the member within the struct.
  *
- * WARNING: any const qualifier of @ptr is lost.
+ * WARNING: any const qualifier of @ptr is lost. DO NOT USE container_of() IN
+ * NEW CODE.
  */
 #define container_of(ptr, type, member) ({				\
 	void *__mptr = (void *)(ptr);					\
@@ -28,6 +29,8 @@
  * @ptr:		the pointer to the member
  * @type:		the type of the container struct this is embedded in.
  * @member:		the name of the member within the struct.
+ *
+ * Always prefer container_of_const() over container_of() in new code.
  */
 #define container_of_const(ptr, type, member)				\
 	_Generic(ptr,							\
-- 
2.39.5

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Andy Shevchenko]

On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> There is a warning in the kerneldoc documentation of container_of() that
> constness of its ptr argument is lost. While this is a faible suggestion
> container_of_const() should be used instead, the vast majority of new code
> still uses container_of():
> 
> $ git diff v6.13 v6.14|grep container_of\(|wc -l
> 646
> $ git diff v6.13 v6.14|grep container_of_const|wc -l
> 9
> 
> Make an explicit recommendation to use container_of_const().
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Hi Greg, Andy,
> 
> I guess we generally agree the additional constness check in
> container_of_const() is useful, but adding the same check to
> container_of() generates warnings -- there are some errors, too -- such as
> this one currently:
> 
> In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
>                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
>       |              ^
> 
> As noted above, 646 new missing constness checks were introduced through
> container_of() macro use during the 6.14 cycle alone. Most of these are
> likely harmless, but with so many new users some are bound to be ignoring
> constness.
> 
> Once the warnings from bad container_of() use are worked out in a way or
> another, the constness check could be added to the container_of() macro
> and the current container_of_const() be dropped altogether.
> 
> If this patch is accepted, I'll see how to add a warning on container_of()
> to checkpatch.pl.

Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
to the container_of() instead of doing these comments?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Tue, May 20, 2025 at 05:16:45PM +0300, Andy Shevchenko wrote:
> On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> > There is a warning in the kerneldoc documentation of container_of() that
> > constness of its ptr argument is lost. While this is a faible suggestion
> > container_of_const() should be used instead, the vast majority of new code
> > still uses container_of():
> > 
> > $ git diff v6.13 v6.14|grep container_of\(|wc -l
> > 646
> > $ git diff v6.13 v6.14|grep container_of_const|wc -l
> > 9
> > 
> > Make an explicit recommendation to use container_of_const().
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> > Hi Greg, Andy,
> > 
> > I guess we generally agree the additional constness check in
> > container_of_const() is useful, but adding the same check to
> > container_of() generates warnings -- there are some errors, too -- such as
> > this one currently:
> > 
> > In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
> >                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> > /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> > /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> >   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
> >       |              ^
> > 
> > As noted above, 646 new missing constness checks were introduced through
> > container_of() macro use during the 6.14 cycle alone. Most of these are
> > likely harmless, but with so many new users some are bound to be ignoring
> > constness.
> > 
> > Once the warnings from bad container_of() use are worked out in a way or
> > another, the constness check could be added to the container_of() macro
> > and the current container_of_const() be dropped altogether.
> > 
> > If this patch is accepted, I'll see how to add a warning on container_of()
> > to checkpatch.pl.
> 
> Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
> to the container_of() instead of doing these comments?

Yes, fixing up the existing places where it is broken would be best, how
many of them are there now?

thanks,

greg k-h

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Sakari Ailus]

Hi Greg,

On Tue, May 20, 2025 at 04:30:19PM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 20, 2025 at 05:16:45PM +0300, Andy Shevchenko wrote:
> > On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> > > There is a warning in the kerneldoc documentation of container_of() that
> > > constness of its ptr argument is lost. While this is a faible suggestion
> > > container_of_const() should be used instead, the vast majority of new code
> > > still uses container_of():
> > > 
> > > $ git diff v6.13 v6.14|grep container_of\(|wc -l
> > > 646
> > > $ git diff v6.13 v6.14|grep container_of_const|wc -l
> > > 9
> > > 
> > > Make an explicit recommendation to use container_of_const().
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > > Hi Greg, Andy,
> > > 
> > > I guess we generally agree the additional constness check in
> > > container_of_const() is useful, but adding the same check to
> > > container_of() generates warnings -- there are some errors, too -- such as
> > > this one currently:
> > > 
> > > In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
> > >                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> > > /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> > > /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> > >   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
> > >       |              ^
> > > 
> > > As noted above, 646 new missing constness checks were introduced through
> > > container_of() macro use during the 6.14 cycle alone. Most of these are
> > > likely harmless, but with so many new users some are bound to be ignoring
> > > constness.
> > > 
> > > Once the warnings from bad container_of() use are worked out in a way or
> > > another, the constness check could be added to the container_of() macro
> > > and the current container_of_const() be dropped altogether.
> > > 
> > > If this patch is accepted, I'll see how to add a warning on container_of()
> > > to checkpatch.pl.
> > 
> > Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
> > to the container_of() instead of doing these comments?
> 
> Yes, fixing up the existing places where it is broken would be best, how
> many of them are there now?

Adding constness check for container_of(), with my partial build on x86-64
I'm getting 893 such warnings. A fair number are probably duplicates or
repeat of the same pattern, but also the compilation didn't succeed --
there were multiple compilation failures.

-- 
Regards,

Sakari Ailus

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Tue, May 20, 2025 at 10:09:18PM +0000, Sakari Ailus wrote:
> Hi Greg,
> 
> On Tue, May 20, 2025 at 04:30:19PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, May 20, 2025 at 05:16:45PM +0300, Andy Shevchenko wrote:
> > > On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> > > > There is a warning in the kerneldoc documentation of container_of() that
> > > > constness of its ptr argument is lost. While this is a faible suggestion
> > > > container_of_const() should be used instead, the vast majority of new code
> > > > still uses container_of():
> > > > 
> > > > $ git diff v6.13 v6.14|grep container_of\(|wc -l
> > > > 646
> > > > $ git diff v6.13 v6.14|grep container_of_const|wc -l
> > > > 9
> > > > 
> > > > Make an explicit recommendation to use container_of_const().
> > > > 
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > ---
> > > > Hi Greg, Andy,
> > > > 
> > > > I guess we generally agree the additional constness check in
> > > > container_of_const() is useful, but adding the same check to
> > > > container_of() generates warnings -- there are some errors, too -- such as
> > > > this one currently:
> > > > 
> > > > In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
> > > >                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> > > > /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> > > > /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> > > >   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
> > > >       |              ^
> > > > 
> > > > As noted above, 646 new missing constness checks were introduced through
> > > > container_of() macro use during the 6.14 cycle alone. Most of these are
> > > > likely harmless, but with so many new users some are bound to be ignoring
> > > > constness.
> > > > 
> > > > Once the warnings from bad container_of() use are worked out in a way or
> > > > another, the constness check could be added to the container_of() macro
> > > > and the current container_of_const() be dropped altogether.
> > > > 
> > > > If this patch is accepted, I'll see how to add a warning on container_of()
> > > > to checkpatch.pl.
> > > 
> > > Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
> > > to the container_of() instead of doing these comments?
> > 
> > Yes, fixing up the existing places where it is broken would be best, how
> > many of them are there now?
> 
> Adding constness check for container_of(), with my partial build on x86-64
> I'm getting 893 such warnings. A fair number are probably duplicates or
> repeat of the same pattern, but also the compilation didn't succeed --
> there were multiple compilation failures.

So who is going to do that work?  I just did it for drivers/usb and it
was pretty trivial, just declaring "do not use this!" feels like the
easy way out, absolving yourself from any responsibility here :)

thanks,

greg k-h

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 20, 2025 at 10:09:18PM +0000, Sakari Ailus wrote:
> > Hi Greg,
> > 
> > On Tue, May 20, 2025 at 04:30:19PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, May 20, 2025 at 05:16:45PM +0300, Andy Shevchenko wrote:
> > > > On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> > > > > There is a warning in the kerneldoc documentation of container_of() that
> > > > > constness of its ptr argument is lost. While this is a faible suggestion
> > > > > container_of_const() should be used instead, the vast majority of new code
> > > > > still uses container_of():
> > > > > 
> > > > > $ git diff v6.13 v6.14|grep container_of\(|wc -l
> > > > > 646
> > > > > $ git diff v6.13 v6.14|grep container_of_const|wc -l
> > > > > 9
> > > > > 
> > > > > Make an explicit recommendation to use container_of_const().
> > > > > 
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > ---
> > > > > Hi Greg, Andy,
> > > > > 
> > > > > I guess we generally agree the additional constness check in
> > > > > container_of_const() is useful, but adding the same check to
> > > > > container_of() generates warnings -- there are some errors, too -- such as
> > > > > this one currently:
> > > > > 
> > > > > In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
> > > > >                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> > > > > /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> > > > > /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> > > > >   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
> > > > >       |              ^
> > > > > 
> > > > > As noted above, 646 new missing constness checks were introduced through
> > > > > container_of() macro use during the 6.14 cycle alone. Most of these are
> > > > > likely harmless, but with so many new users some are bound to be ignoring
> > > > > constness.
> > > > > 
> > > > > Once the warnings from bad container_of() use are worked out in a way or
> > > > > another, the constness check could be added to the container_of() macro
> > > > > and the current container_of_const() be dropped altogether.
> > > > > 
> > > > > If this patch is accepted, I'll see how to add a warning on container_of()
> > > > > to checkpatch.pl.
> > > > 
> > > > Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
> > > > to the container_of() instead of doing these comments?
> > > 
> > > Yes, fixing up the existing places where it is broken would be best, how
> > > many of them are there now?
> > 
> > Adding constness check for container_of(), with my partial build on x86-64
> > I'm getting 893 such warnings. A fair number are probably duplicates or
> > repeat of the same pattern, but also the compilation didn't succeed --
> > there were multiple compilation failures.
> 
> So who is going to do that work?  I just did it for drivers/usb and it
> was pretty trivial, just declaring "do not use this!" feels like the
> easy way out, absolving yourself from any responsibility here :)

I tried it for the whole tree, and ugh, there are some real "errors" in
there.  The nfs inode handling logic is crazy, passing in a const
pointer and then setting fields in it.  So this will be some real work
to unwind and fix in some places.

thanks,

greg k-h

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Wed, May 21, 2025 at 03:31:36PM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, May 20, 2025 at 10:09:18PM +0000, Sakari Ailus wrote:
> > > Hi Greg,
> > > 
> > > On Tue, May 20, 2025 at 04:30:19PM +0200, Greg Kroah-Hartman wrote:
> > > > On Tue, May 20, 2025 at 05:16:45PM +0300, Andy Shevchenko wrote:
> > > > > On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> > > > > > There is a warning in the kerneldoc documentation of container_of() that
> > > > > > constness of its ptr argument is lost. While this is a faible suggestion
> > > > > > container_of_const() should be used instead, the vast majority of new code
> > > > > > still uses container_of():
> > > > > > 
> > > > > > $ git diff v6.13 v6.14|grep container_of\(|wc -l
> > > > > > 646
> > > > > > $ git diff v6.13 v6.14|grep container_of_const|wc -l
> > > > > > 9
> > > > > > 
> > > > > > Make an explicit recommendation to use container_of_const().
> > > > > > 
> > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > > ---
> > > > > > Hi Greg, Andy,
> > > > > > 
> > > > > > I guess we generally agree the additional constness check in
> > > > > > container_of_const() is useful, but adding the same check to
> > > > > > container_of() generates warnings -- there are some errors, too -- such as
> > > > > > this one currently:
> > > > > > 
> > > > > > In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
> > > > > >                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> > > > > > /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> > > > > > /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> > > > > >   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
> > > > > >       |              ^
> > > > > > 
> > > > > > As noted above, 646 new missing constness checks were introduced through
> > > > > > container_of() macro use during the 6.14 cycle alone. Most of these are
> > > > > > likely harmless, but with so many new users some are bound to be ignoring
> > > > > > constness.
> > > > > > 
> > > > > > Once the warnings from bad container_of() use are worked out in a way or
> > > > > > another, the constness check could be added to the container_of() macro
> > > > > > and the current container_of_const() be dropped altogether.
> > > > > > 
> > > > > > If this patch is accepted, I'll see how to add a warning on container_of()
> > > > > > to checkpatch.pl.
> > > > > 
> > > > > Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
> > > > > to the container_of() instead of doing these comments?
> > > > 
> > > > Yes, fixing up the existing places where it is broken would be best, how
> > > > many of them are there now?
> > > 
> > > Adding constness check for container_of(), with my partial build on x86-64
> > > I'm getting 893 such warnings. A fair number are probably duplicates or
> > > repeat of the same pattern, but also the compilation didn't succeed --
> > > there were multiple compilation failures.
> > 
> > So who is going to do that work?  I just did it for drivers/usb and it
> > was pretty trivial, just declaring "do not use this!" feels like the
> > easy way out, absolving yourself from any responsibility here :)
> 
> I tried it for the whole tree, and ugh, there are some real "errors" in
> there.  The nfs inode handling logic is crazy, passing in a const
> pointer and then setting fields in it.  So this will be some real work
> to unwind and fix in some places.

Oh, and here's the patch I used to test this out if anyone else wants to
attempt it.  I've sent out fixes for drivers/usb/ already, let me see
about a few other small subsystems...


diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 713890c867be..2f62729e49d8 100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -15,7 +15,7 @@
  *
  * WARNING: any const qualifier of @ptr is lost.
  */
-#define container_of(ptr, type, member) ({				\
+#define __container_of(ptr, type, member) ({				\
 	void *__mptr = (void *)(ptr);					\
 	static_assert(__same_type(*(ptr), ((type *)0)->member) ||	\
 		      __same_type(*(ptr), void),			\
@@ -31,8 +31,11 @@
  */
 #define container_of_const(ptr, type, member)				\
 	_Generic(ptr,							\
-		const typeof(*(ptr)) *: ((const type *)container_of(ptr, type, member)),\
-		default: ((type *)container_of(ptr, type, member))	\
+		const typeof(*(ptr)) *: ((const type *)__container_of(ptr, type, member)),\
+		default: ((type *)__container_of(ptr, type, member))	\
 	)
 
+#define container_of(ptr, type, member) container_of_const(ptr, type, member)
+
+
 #endif	/* _LINUX_CONTAINER_OF_H */

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Wed, May 21, 2025 at 03:31:36PM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, May 20, 2025 at 10:09:18PM +0000, Sakari Ailus wrote:
> > > Hi Greg,
> > > 
> > > On Tue, May 20, 2025 at 04:30:19PM +0200, Greg Kroah-Hartman wrote:
> > > > On Tue, May 20, 2025 at 05:16:45PM +0300, Andy Shevchenko wrote:
> > > > > On Tue, May 20, 2025 at 01:34:37PM +0300, Sakari Ailus wrote:
> > > > > > There is a warning in the kerneldoc documentation of container_of() that
> > > > > > constness of its ptr argument is lost. While this is a faible suggestion
> > > > > > container_of_const() should be used instead, the vast majority of new code
> > > > > > still uses container_of():
> > > > > > 
> > > > > > $ git diff v6.13 v6.14|grep container_of\(|wc -l
> > > > > > 646
> > > > > > $ git diff v6.13 v6.14|grep container_of_const|wc -l
> > > > > > 9
> > > > > > 
> > > > > > Make an explicit recommendation to use container_of_const().
> > > > > > 
> > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > > ---
> > > > > > Hi Greg, Andy,
> > > > > > 
> > > > > > I guess we generally agree the additional constness check in
> > > > > > container_of_const() is useful, but adding the same check to
> > > > > > container_of() generates warnings -- there are some errors, too -- such as
> > > > > > this one currently:
> > > > > > 
> > > > > > In file included from /home/sailus/src/linux/include/linux/bcma/bcma.h:14,
> > > > > >                  from /home/sailus/src/linux/arch/x86/kernel/early-quirks.c:17:
> > > > > > /home/sailus/src/linux/include/linux/ssb/ssb.h: In function ‘dev_to_ssb_dev’:
> > > > > > /home/sailus/src/linux/include/linux/ssb/ssb.h:291:14: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> > > > > >   291 |         wrap = container_of(dev, struct __ssb_dev_wrapper, dev);
> > > > > >       |              ^
> > > > > > 
> > > > > > As noted above, 646 new missing constness checks were introduced through
> > > > > > container_of() macro use during the 6.14 cycle alone. Most of these are
> > > > > > likely harmless, but with so many new users some are bound to be ignoring
> > > > > > constness.
> > > > > > 
> > > > > > Once the warnings from bad container_of() use are worked out in a way or
> > > > > > another, the constness check could be added to the container_of() macro
> > > > > > and the current container_of_const() be dropped altogether.
> > > > > > 
> > > > > > If this patch is accepted, I'll see how to add a warning on container_of()
> > > > > > to checkpatch.pl.
> > > > > 
> > > > > Hmm... Wouldn't be better to fix non-const cases and add the const check, etc
> > > > > to the container_of() instead of doing these comments?
> > > > 
> > > > Yes, fixing up the existing places where it is broken would be best, how
> > > > many of them are there now?
> > > 
> > > Adding constness check for container_of(), with my partial build on x86-64
> > > I'm getting 893 such warnings. A fair number are probably duplicates or
> > > repeat of the same pattern, but also the compilation didn't succeed --
> > > there were multiple compilation failures.
> > 
> > So who is going to do that work?  I just did it for drivers/usb and it
> > was pretty trivial, just declaring "do not use this!" feels like the
> > easy way out, absolving yourself from any responsibility here :)
> 
> I tried it for the whole tree, and ugh, there are some real "errors" in
> there.  The nfs inode handling logic is crazy, passing in a const
> pointer and then setting fields in it.  So this will be some real work
> to unwind and fix in some places.

Turns out that many of these errors are my fault, so I'll send some
patches out for them as I dig into this more...

thanks,

greg k-h

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[David Laight]

On Wed, 21 May 2025 15:31:36 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:
...
> I tried it for the whole tree, and ugh, there are some real "errors" in
> there.  The nfs inode handling logic is crazy, passing in a const
> pointer and then setting fields in it.  So this will be some real work
> to unwind and fix in some places.

Perhaps change the really dodgy ones to container_of_deconst().
And fix the easy ones so they compile with the 'const' check.

Then most code will just contain container_of() and be fine and
not need churning.

So you'd have container_of_deconst() which is the old container_of() code
and contain_of_const() that preserves constness.
Then container_of() which is compile time selectable (for now) between them.
Get one of the build bots to enable it and things will slowly get fixed.

	David

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Thu, May 22, 2025 at 10:01:42PM +0100, David Laight wrote:
> On Wed, 21 May 2025 15:31:36 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:
> ...
> > I tried it for the whole tree, and ugh, there are some real "errors" in
> > there.  The nfs inode handling logic is crazy, passing in a const
> > pointer and then setting fields in it.  So this will be some real work
> > to unwind and fix in some places.
> 
> Perhaps change the really dodgy ones to container_of_deconst().
> And fix the easy ones so they compile with the 'const' check.

Ick, no, let me fix these up properly.  I'm picking them off, and have
found some real issues here.  It will give me something to build patches
for over time while doing stable kernel test builds :)

thanks,

greg k-h

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[David Laight]

On Fri, 23 May 2025 10:36:45 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Thu, May 22, 2025 at 10:01:42PM +0100, David Laight wrote:
> > On Wed, 21 May 2025 15:31:36 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >   
> > > On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:  
> > ...  
> > > I tried it for the whole tree, and ugh, there are some real "errors" in
> > > there.  The nfs inode handling logic is crazy, passing in a const
> > > pointer and then setting fields in it.  So this will be some real work
> > > to unwind and fix in some places.  
> > 
> > Perhaps change the really dodgy ones to container_of_deconst().
> > And fix the easy ones so they compile with the 'const' check.  
> 
> Ick, no, let me fix these up properly.  I'm picking them off, and have
> found some real issues here.  It will give me something to build patches
> for over time while doing stable kernel test builds :)

I was mostly thinking of it as temporary measure help find the easy cases.

But having container_of_const() that preserves 'const-ness' and
container_of() that always removes it seems wrong.
Wouldn't preserving const-ness for a W=1 build would be a more normal way
to do it?

	David

Re: [PATCH 1/1] container_of: Document container_of() is not to be used in new code

[Greg Kroah-Hartman]

On Sat, May 24, 2025 at 01:45:26PM +0100, David Laight wrote:
> On Fri, 23 May 2025 10:36:45 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Thu, May 22, 2025 at 10:01:42PM +0100, David Laight wrote:
> > > On Wed, 21 May 2025 15:31:36 +0200
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > >   
> > > > On Wed, May 21, 2025 at 03:27:19PM +0200, Greg Kroah-Hartman wrote:  
> > > ...  
> > > > I tried it for the whole tree, and ugh, there are some real "errors" in
> > > > there.  The nfs inode handling logic is crazy, passing in a const
> > > > pointer and then setting fields in it.  So this will be some real work
> > > > to unwind and fix in some places.  
> > > 
> > > Perhaps change the really dodgy ones to container_of_deconst().
> > > And fix the easy ones so they compile with the 'const' check.  
> > 
> > Ick, no, let me fix these up properly.  I'm picking them off, and have
> > found some real issues here.  It will give me something to build patches
> > for over time while doing stable kernel test builds :)
> 
> I was mostly thinking of it as temporary measure help find the easy cases.
> 
> But having container_of_const() that preserves 'const-ness' and
> container_of() that always removes it seems wrong.
> Wouldn't preserving const-ness for a W=1 build would be a more normal way
> to do it?

Maybe, yes, but as container_of() has ALWAYS removed the const-ness,
that's what the codebase is used to, so let's just give me a release
cycle or two to clean up the tree and then I'll just move
container_of() to preserve it and all will be good.

thanks,

greg k-h