Re: nfs: infinite loop in fcntl(F_SETLKW)

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

 



> > OK. So the correct fix here should really be applied to fcntl_setlk().
> > There is absolutely no reason why we should be looping at all if the
> > filesystem has a ->lock() method.
> > 
> > In fact, this looping behaviour was introduced recently in commit
> > 7723ec9777d9832849b76475b1a21a2872a40d20.
> 
> Apologies, that was indeed a behavioral change introduced in a commit
> that claimed just to be shuffling code around.

Yeah, that patch looks totally wrong.  It's not generally a good idea
to do a loop where the exit condition depends on something you don't
control.  And error values from filesystem methods are typically like
that.  For example with fuse, the error code could come from an
unprivileged userspace process.

I didn't realize this aspect of the bug previously, because I
concentrated on the lockd inconsistency.

Btw, why hasn't this work been posted on -fsdevel prior to merging
into mainline?

> 
> > Marc, Bruce, why was this
> > done, and how are filesystems now supposed to behave?
> > 
> 
> The assumption must have been that -EAGAIN could only mean that we
> needed to keep blocking, and hence was a nonsensical return from a
> filesystem lock method that waited itself for the lock to become
> available--such a method would return 0, -EINTR (or some more exotic
> error), or continue waiting.

EAGAIN for a blocking lock is nonsensical, so my original patch could
still make sense.  But that's no longer a regression, and not all that
important.

Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux