Re: [PATCH 0/6 v2] nfs_localio: fixes for races and errors from older compilers

Chuck Lever
2025-05-10T16:02:49Z

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@wanadoo.fr/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