Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Chuck Lever" <chuck.lever@oracle.com>
Cc:     "Jeff Layton" <jlayton@kernel.org>, linux-nfs@vger.kernel.org,
        "Olga Kornievskaia" <kolga@netapp.com>,
        "Dai Ngo" <Dai.Ngo@oracle.com>, "Tom Talpey" <tom@talpey.com>
Subject: Re: [PATCH 3/9] nfsd: split sc_status out of sc_type
In-reply-to: <ZVfEYfslqZDvwW/X@tissot.1015granger.net>
References: <20231117022121.23310-1-neilb@suse.de>,
 <20231117022121.23310-4-neilb@suse.de>,
 <ZVfEYfslqZDvwW/X@tissot.1015granger.net>
Date:   Mon, 20 Nov 2023 09:35:15 +1100
Message-id: <170043331552.19300.14124734701560396770@noble.neil.brown.name>
Precedence: bulk

On Sat, 18 Nov 2023, Chuck Lever wrote:
> On Fri, Nov 17, 2023 at 01:18:49PM +1100, NeilBrown wrote:
> > sc_type identifies the type of a state - open, lock, deleg, layout - and
> > also the status of a state - closed or revoked.
> >=20
> > This is a bit untidy and could get worse when "admin-revoked" states are
> > added.  So clean it up.
> >=20
> > With this patch, the type is now all that is stored in sc_type.  This is
> > zero when the state is first added to ->cl_stateids (causing it to be
> > ignored), and is then set appropriately once it is fully initialised.
> > It is set under ->cl_lock to ensure atomicity w.r.t lookup.  It is now
> > never cleared.
> >=20
> > sc_type is still a bit-set even though at most one bit is set.  This allo=
ws
> > lookup functions to be given a bitmap of acceptable types.
> >=20
> > cl_type is now an unsigned short rather than char.  There is no value in
>=20
> s/cl_type/sc_type
>=20
>=20
> > restricting to just 8 bits.  When passed to a function or stored in local
> > variable, "unsigned int" is used.
>=20
> This seems confusing, and I'd prefer not to introduce implicit type
> conversions where they aren't truly necessary. Why not use "unsigned
> short" or even "unsigned int" everywhere?
>=20

Not really a type conversion - just a size change.
Maybe I could use
   unsigned int sc_type:16, sc_stats:16;
and use unsigned int everywhere?

Maybe it is confusing...  maybe I'll just make it unsigned short
everywhere.=20

Thanks,
NeilBrown