Re: [Cluster-devel] [PATCH 6/7] dlm: use FL_SLEEP to determine blocking vs non-blocking

On Wed, 2023-08-30 at 08:38 -0400, Alexander Aring wrote:
> Hi,
>=20
> On Fri, Aug 25, 2023 at 2:18=E2=80=AFPM Jeff Layton <[email protected]> =
wrote:
> >=20
> > On Wed, 2023-08-23 at 17:33 -0400, Alexander Aring wrote:
> > > This patch uses the FL_SLEEP flag in struct file_lock to determine if
> > > the lock request is a blocking or non-blocking request. Before dlm wa=
s
> > > using IS_SETLKW() was being used which is not usable for lock request=
s
> > > coming from lockd when EXPORT_OP_SAFE_ASYNC_LOCK inside the export fl=
ags
> > > is set.
> > >=20
> > > Signed-off-by: Alexander Aring <[email protected]>
> > > ---
> > >  fs/dlm/plock.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >=20
> > > diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
> > > index 0094fa4004cc..0c6ed5eeb840 100644
> > > --- a/fs/dlm/plock.c
> > > +++ b/fs/dlm/plock.c
> > > @@ -140,7 +140,7 @@ int dlm_posix_lock(dlm_lockspace_t *lockspace, u6=
4 number, struct file *file,
> > >       op->info.optype         =3D DLM_PLOCK_OP_LOCK;
> > >       op->info.pid            =3D fl->fl_pid;
> > >       op->info.ex             =3D (fl->fl_type =3D=3D F_WRLCK);
> > > -     op->info.wait           =3D IS_SETLKW(cmd);
> > > +     op->info.wait           =3D !!(fl->fl_flags & FL_SLEEP);
> > >       op->info.fsid           =3D ls->ls_global_id;
> > >       op->info.number         =3D number;
> > >       op->info.start          =3D fl->fl_start;
> >=20
> > Not sure you really need the !!, but ok...
> >=20
>=20
> The wait is a byte value and FL_SLEEP doesn't fit into it, I already
> run into problems with it. I don't think somebody does a if (foo->wait
> =3D=3D 1) but it should be set to 1 or 0.
>=20

AIUI, any halfway decent compiler should take the result of the &, and
implicitly cast that properly to bool. Basically, any value other than 0
should be true.

If the compiler just blindly casts the lowest byte though, then you do
need the double-negative.

> An alternative would be: ((fl->fl_flags & FL_SLEEP) =3D=3D FL_SLEEP). I a=
m
> not sure what the coding style says here. I think it's more important
> what the C standard says about !!(condition), but there are other
> users of this in the Linux kernel. :-/

I don't care too much either way, but my understanding was that you
don't need to do the !! trick in most cases with modern compilers.
--=20
Jeff Layton <[email protected]>