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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2023-08-30 at 08:38 -0400, Alexander Aring wrote:
> Hi,
> 
> On Fri, Aug 25, 2023 at 2:18 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > 
> > 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 was
> > > using IS_SETLKW() was being used which is not usable for lock requests
> > > coming from lockd when EXPORT_OP_SAFE_ASYNC_LOCK inside the export flags
> > > is set.
> > > 
> > > Signed-off-by: Alexander Aring <aahringo@xxxxxxxxxx>
> > > ---
> > >  fs/dlm/plock.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > 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, u64 number, struct file *file,
> > >       op->info.optype         = DLM_PLOCK_OP_LOCK;
> > >       op->info.pid            = fl->fl_pid;
> > >       op->info.ex             = (fl->fl_type == F_WRLCK);
> > > -     op->info.wait           = IS_SETLKW(cmd);
> > > +     op->info.wait           = !!(fl->fl_flags & FL_SLEEP);
> > >       op->info.fsid           = ls->ls_global_id;
> > >       op->info.number         = number;
> > >       op->info.start          = fl->fl_start;
> > 
> > Not sure you really need the !!, but ok...
> > 
> 
> 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
> == 1) but it should be set to 1 or 0.
> 

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) == FL_SLEEP). I am
> 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.
-- 
Jeff Layton <jlayton@xxxxxxxxxx>




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux