Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <014cde00a44c7240414049ac5d179320e96df6c8.camel@kernel.org>
Subject: Re: [PATCH v3 3/4] nfsd: close race between unhashing and LRU
 addition
From:   Jeff Layton <jlayton@kernel.org>
To:     Chuck Lever III <chuck.lever@oracle.com>
Cc:     Neil Brown <neilb@suse.de>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Date:   Mon, 31 Oct 2022 09:28:25 -0400
In-Reply-To: <202AD086-4F1F-41D6-ABDC-BA6C91DA5BBF@oracle.com>
References: <20221028185712.79863-1-jlayton@kernel.org>
         <20221028185712.79863-4-jlayton@kernel.org>
         <08778EE0-CBDC-467B-ACA6-9D8E6719E1BB@oracle.com>
         <166716630911.13915.14442550645061536898@noble.neil.brown.name>
         <1737B8C1-5B93-4887-A673-F9AFA6ED32C0@oracle.com>
         <fb57d2cb6769dbc123e15e76ec2c23b1fa9f32be.camel@kernel.org>
         <202AD086-4F1F-41D6-ABDC-BA6C91DA5BBF@oracle.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2022-10-31 at 13:14 +0000, Chuck Lever III wrote:
>=20
> > On Oct 31, 2022, at 6:08 AM, Jeff Layton <jlayton@kernel.org> wrote:
> >=20
> > On Mon, 2022-10-31 at 02:51 +0000, Chuck Lever III wrote:
> > >=20
> > > > On Oct 30, 2022, at 5:45 PM, NeilBrown <neilb@suse.de> wrote:
> > > >=20
> > > > On Sat, 29 Oct 2022, Chuck Lever III wrote:
> > > > >=20
> > > > > > On Oct 28, 2022, at 2:57 PM, Jeff Layton <jlayton@kernel.org> w=
rote:
> > > > > >=20
> > > > > > The list_lru_add and list_lru_del functions use list_empty chec=
ks to see
> > > > > > whether the object is already on the LRU. That's fine in most c=
ases, 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.
> > >=20
> > > Can that issue be resolved by simply adding a "struct list_head nf_di=
spose"
> > > field? That might be more straightforward than adding conditional log=
ic.
> > >=20
> >=20
> > Yes, though that would take more memory.
>=20
> 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.
>=20
> I'm wondering if a separate list_head field would help simplify
> nfsd_file_put() ?
>=20

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.
=20
--=20
Jeff Layton <jlayton@kernel.org>