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. > > > > This is a bit untidy and could get worse when "admin-revoked" states are > > added. So clean it up. > > > > 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. > > > > sc_type is still a bit-set even though at most one bit is set. This allows > > lookup functions to be given a bitmap of acceptable types. > > > > cl_type is now an unsigned short rather than char. There is no value in > > s/cl_type/sc_type > > > > restricting to just 8 bits. When passed to a function or stored in local > > variable, "unsigned int" is used. > > 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? > 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. Thanks, NeilBrown