Re: [PATCH] ksmbd: use F_SETLK when unlocking a file

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2022-11-15 at 01:01 -0800, Christoph Hellwig wrote:
> On Fri, Nov 11, 2022 at 08:11:53AM -0500, Jeff Layton wrote:
> > ksmbd seems to be trying to use a cmd value of 0 when unlocking a file.
> > That activity requires a type of F_UNLCK with a cmd of F_SETLK. For
> > local POSIX locking, it doesn't matter much since vfs_lock_file ignores
> > @cmd, but filesystems that define their own ->lock operation expect to
> > see it set sanely.
> 
> Btw, I really wonder if we should split vfs_lock_file into separate
> calls for locking vs unlocking.  The current interface seems very
> confusing.

Maybe, though the current scheme basically of mirrors the userland API,
as do the ->lock and ->flock file_operations.

FWIW, the filelocking API is pretty rife with warts. Several other
things that I wouldn't mind doing, just off the top of my head:

- move the file locking API into a separate header. No need for it to be
in fs.h, which is already too bloated.

- define a new struct for leases, and drop lease-specific fields from
file_lock

- remove more separate filp and inode arguments

- maybe rename locks.c to filelock.c? "locks.c" is too ambiguous

Any others?
-- 
Jeff Layton <jlayton@kernel.org>