Re: nfs: add dummy definition for nfsd_file

Mike Snitzer
2025-03-27T20:17:16Z

On Thu, Mar 27, 2025 at 09:28:33AM +0100, Pali Rohár wrote:
> On Wednesday 26 March 2025 22:00:02 Mike Snitzer wrote:
> > On Wed, Mar 26, 2025 at 09:59:19PM +0100, Pali Rohár wrote:
> > > On Wednesday 26 March 2025 08:33:32 Jeff Johnson wrote:
> > > > On 3/26/2025 8:09 AM, Mike Snitzer wrote:
> > > > > Add dummy definition for nfsd_file in both nfslocalio.c and localio.c
> > > > > so older gcc (e.g. EL8's 8.5.0) can be used. Older gcc causes RCU
> > > > > code (rcu_dereference and rcu_access_pointer) to dereference what
> > > > > should just be an opaque pointer with its use of typeof.
> > > > >
> > > > > So without the dummy definition compiling with older gcc fails.
> > > > >
> > > > > Link: https://lore.kernel.org/all/Zsyhco1OrOI_uSbd@kernel.org/
> > > > > Fixes: 55a9742d02eff ("nfs: cache all open LOCALIO nfsd_file(s) in client")
> > >
> > > As this change is fixing compile error, should not be there also cc: stable line?
> >
> > Any commit with a Fixes: tag will automatically be picked up by
> > stable@ once it is merged. An explicit cc: sttable@ would be
> > redundant.
>
> From my experience, when the backporting of commit with both Fixes:+Cc:
> fails then the committer of the patch and also other is informed by
> robot email. If there is only Fixes: without Cc: then people are not
> informed.
>
> So I used the Fixes:+Cc: for such build issues or important issues to
> ensure that the change is really backported.

If a Fixes commit isn't able to be easily backported then mail is sent
giving notice to the author. That has always been sufficient for
anything I have authored.

> > > >
> > > > I saw this issue using crosstools/gcc-13.3.0-nolibc and this patch fixes it.
> > >
> > > So maybe the commit message can be adjusted, so it does not say only
> > > "older gcc"?
> >
> > I don't see the need to list all compilers, I documented the compiler
> > that motivated my fix. Fact that it applicable for
> > crosstools/gcc-13.3.0-nolibc (which I don't have context for what it
> > is.. but if this commit is needed for it then it is a suspect "new"
> > compiler).
>
> My impression was that the commit is fixing just the compilation with
> old gcc versions. But it seems that also new are affected. That is why I
> suggested to adjust it, so it would be clear that it applies for new gcc
> versions too.

Really not sure why you're being so pedantic.

I submitted what I developed as a fix for my needs with an old gcc
(from EL8) and so I submitted what I had. We don't need to impose
every compiler be covered in a commit header (especially when the new
case occurred 4 months later).

crosstools/gcc-13.3.0-nolibc is not something I'm going to hunt down
and pick apart to prove that it is using old compiler technology, but
I'm inclined to think it is.

Mike