On Wednesday 23 April 2025 07:54:40 NeilBrown wrote: > On Wed, 23 Apr 2025, Pali Rohár wrote: > > On Sunday 20 April 2025 12:12:22 Mike Snitzer wrote: > > > On Sat, Apr 19, 2025 at 01:52:31PM -0400, Chuck Lever wrote: > > > > On 4/18/25 5:34 PM, Mike Snitzer wrote: > > > > > On Wed, Apr 16, 2025 at 09:31:55AM -0400, Chuck Lever wrote: > > > > >> On 4/15/25 10:41 PM, Vincent Mailhol wrote: > > > > >>> +CC: Neil Brown > > > > >>> +CC: Olga Kornievskaia > > > > >>> +CC: Dai Ngo > > > > >>> +CC: Tom Talpey > > > > >>> +CC: Trond Myklebust > > > > >>> +CC: Anna Schumaker > > > > >>> > > > > >>> (just to make sure that anyone listed in > > > > >>> > > > > >>> ./scripts/get_maintainer.pl fs/nfs_common/nfslocalio.c > > > > >>> > > > > >>> get copied). > > > > >>> > > > > >>> Here is the link to the full thread: > > > > >>> > > > > >>> https://lore.kernel.org/all/Z_coQbSdvMWD92IA@xxxxxxxxxx/ > > > > >>> > > > > >>> > > > > >>> On 10/04/2025 at 11:09, Mike Snitzer: > > > > >>>> Add dummy definition for nfsd_file in both nfslocalio.c and localio.c > > > > >>>> so various compilers (e.g. gcc 8.5.0 and 9.5.0) can be used. Otherwise > > > > >>>> RCU code (rcu_dereference and rcu_access_pointer) will dereference > > > > >>>> what should just be an opaque pointer (by using typeof(*ptr)). > > > > >>>> > > > > >>>> Fixes: 86e00412254a ("nfs: cache all open LOCALIO nfsd_file(s) in client") > > > > >>>> Cc: stable@xxxxxxxxxxxxxxx > > > > >>>> Tested-by: Jeff Johnson <jeff.johnson@xxxxxxxxxxxxxxxx> > > > > >>>> Tested-by: Pali Rohár <pali@xxxxxxxxxx> > > > > >>>> Tested-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> > > > > >>>> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> > > > > >>> > > > > >>> Hi everyone, > > > > >>> > > > > >>> The build has been broken for several weeks already. Does anyone have > > > > >>> intention to pick-up this patch? > > > > >>> > > > > >>> (please ignore if someone already picked it up and if it is already on > > > > >>> its way to Linus's tree). > > > > >> > > > > >> I assumed that, like all LOCALIO-related changes, this fix would go > > > > >> in through the NFS client tree. Let me know if it needs to go via NFSD. > > > > > > > > > > Since we haven't heard from Trond or Anna about it, I think you'd be > > > > > perfectly fine to pick it up. It is a compiler fixup associated with > > > > > nfd_file being kept opaque to the client -- but given it is "struct > > > > > nfsd_file" that gives you full license to grab it (IMO). > > > > > > > > > > I'm also unaware of any conflicting changes in the NFS client tree. > > > > > > > > Hi Mike - > > > > > > > > I just looked at this one again. The patch's diffstat is: > > > > > > > > fs/nfs/localio.c | 8 ++++++++ > > > > fs/nfs_common/nfslocalio.c | 8 ++++++++ > > > > > > > > Although fs/nfs_common/ is part of both trees, fs/nfs/localio.c is > > > > definitely a client file. I'm still happy to pick it up, but technically > > > > I would need an Acked-by: from one of the NFS client maintainers. > > > > > > > > My impression is that Trond is managing the NFS client pulls for v6.15. > > > > > > Sure, that's my understanding too. Feel free to offer your Acked-by > > > (for fs/nfs_common/) and hopefully it'll get picked up. I can > > > followup with Trond later this coming week if/as needed. > > > > > > Thanks, > > > Mike > > > > Can we move forward here? The compilation of kernel is broken for at > > least 2 months. Also I have tried to contact Trond for more months but > > has not responded to my emails. > > > > Could be this change picked with a slightly higher priority than just > > waiting another two months? Note that nobody objected this particular > > fix and there are 3+ Tested-by lines. And it is not a good image if some > > kernel component does not compile... > > > > Actually I do object to this fix (though I've been busy and hadn't had > much change to look at it properly). > The fix is ugly. At the very least it should be wrapping in an > #if GCC_VERSION < whatever The problem is that this compile issue happens also with some builds of gcc 13.3.0 as was discussed in the email thread. So is not clear what is that "whatever". For me it looks like that it would be more than the version, probably also some build characteristics of gcc. But I have not been investigating it deeper. > to make the purpose more clear. But I'd rather a deeper fix. > > GCC is complaining that rcu_dereference is being called on a point to a > structure that it doesn't know the content of. > So the code is says "I'm going to dereference this pointer even that > that is actually impossible as I don't know what any of the fields are". > > I'd rather it didn't do that. I've been playing with the code and I > think it can be made quite a bit cleaner by moving the rcu_dereference() > call into the nfsd side of the code. Hopefully I'll have a patch in a > day or so to demonstrate what I mean. > > I understand your desire for some action - but the reality is that you > have the full source code of the kernel and you can do whatever you want > to the kernel that you are working on. You don't need us. > Getting code upstream is certainly good and we should continue to work > on doing that, but if you *need* something to be upstream then you might > want to consider whether your processes are really serving you. > > Trond does often seem slow to take patches, and often they simply appear > in git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git without any > reply to the emails, but he or Anna does usually get to stuff > eventually. > > NeilBrown
