On 5/9/25 11:01 PM, NeilBrown wrote: > On Sat, 10 May 2025, Chuck Lever wrote: >> [ adding Paul McK ] >> >> On 5/8/25 8:46 PM, NeilBrown wrote: >>> This is a revised version a the earlier series. I've actually tested >>> this time and fixed a few issues including the one that Mike found. >> >> As Mike mentioned in a previous thread, at this point, any fix for this >> issue will need to be applied to recent stable kernels as well. This >> series looks a bit too complicated for that. >> >> I expect that other subsystems will encounter this issue eventually, >> so it would be beneficial to address the root cause. For that purpose, I >> think I like Vincent's proposal the best: >> >> https://lore.kernel.org/linux-nfs/8c67a295-8caa-4e53-a764-f691657bbe62@xxxxxxxxxx/raw >> >> None of this is to say that Neil's patches shouldn't be applied. But >> perhaps these are not a broad solution to the RCU compilation issue. > > Do we need a "broad solution to the RCU compilation issue"? Fair question. If the current localio code is simply incorrect as it stands, then I suppose the answer is no. Because gcc is happy to compile it in most cases, I thought the problem was with older versions of gcc, not with localio (even though, I agree, the use of an incomplete structure definition is somewhat brittle when used with RCU). > Does it ever make sense to "dereference" a pointer to a structure that is > not fully specified? What does that even mean? > > I find it harder to argue against use of rcu_access_pointer() in that > context, at least for test-against-NULL, but sparse doesn't complain > about a bare test of an __rcu pointer against NULL, so maybe there is no > need for rcu_access_pointer() for simple tests - in which case the > documentation should be updated. For backporting purposes, inventing our own local RCU helper to handle the situation might be best. Then going forward, apply your patches to rectify the use of the incomplete structure definition, and the local helper can eventually be removed. My interest is getting to a narrow set of changes that can be applied now and backported as needed. The broader clean-ups can then be applied to future kernels (or as subsequent patches in the same merge window). My 2 cents, worth every penny. > (of course rcu_dereference() doesn't actually dereference the pointer, > despite its name. It just declared that there is an imminent intention > to dereference the pointer.....) > > NeilBrown -- Chuck Lever
