On Mon, 2022-10-31 at 13:14 +0000, Chuck Lever III wrote: > > > On Oct 31, 2022, at 6:08 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > On Mon, 2022-10-31 at 02:51 +0000, Chuck Lever III wrote: > > > > > > > On Oct 30, 2022, at 5:45 PM, NeilBrown <neilb@xxxxxxx> wrote: > > > > > > > > On Sat, 29 Oct 2022, Chuck Lever III wrote: > > > > > > > > > > > On Oct 28, 2022, at 2:57 PM, Jeff Layton <jlayton@xxxxxxxxxx> 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 <jlayton@xxxxxxxxxx>
