On Mon, 2022-10-31 at 13:14 +0000, Chuck Lever III wrote:

quoted

On Oct 31, 2022, at 6:08 AM, Jeff Layton [off-list ref] wrote:

On Mon, 2022-10-31 at 02:51 +0000, Chuck Lever III wrote:

quoted

quoted

On Oct 30, 2022, at 5:45 PM, NeilBrown [off-list ref] wrote:

On Sat, 29 Oct 2022, Chuck Lever III wrote:

quoted

quoted

On Oct 28, 2022, at 2:57 PM, Jeff Layton [off-list ref] wrote:

The list_lru_add and list_lru_del functions use list_empty checks to see
whether the object is already on the LRU. That's fine in most cases, but
we occasionally repurpose nf_lru after unhashing. It's possible for an
LRU removal to remove it from a different list altogether if we lose a
race.

Can that issue be resolved by simply adding a "struct list_head nf_dispose"
field? That might be more straightforward than adding conditional logic.

Yes, though that would take more memory.

Not really. pahole says struct nfsd_file is currently 40 bytes short
of two cache lines. So adding a list_head field should not push the
size of nfsd_file to the point where kmalloc would have to allocate
more memory per object.

I'm wondering if a separate list_head field would help simplify
nfsd_file_put() ?

Probably not. It wouldn't need the flag anymore, but the logic would
still be roughly the same. We still have to check for the race between
unhashing and adding to the LRU either way.
 
-- 
Jeff Layton [off-list ref]