| Message ID | 20230620095507.2677463-2-stsp2@yandex.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: F_OFD_GETLK should provide more info | expand |
On Tue, 2023-06-20 at 14:55 +0500, Stas Sergeev wrote: > Currently F_UNLCK with F_OFD_GETLK returns -EINVAL. > The proposed extension allows to use it for getting the lock > information from the particular fd. > > Signed-off-by: Stas Sergeev <stsp2@yandex.ru> > > CC: Jeff Layton <jlayton@kernel.org> > CC: Chuck Lever <chuck.lever@oracle.com> > CC: Alexander Viro <viro@zeniv.linux.org.uk> > CC: Christian Brauner <brauner@kernel.org> > CC: linux-fsdevel@vger.kernel.org > CC: linux-kernel@vger.kernel.org > > --- > fs/locks.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index df8b26a42524..210766007e63 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -868,6 +868,21 @@ static bool posix_locks_conflict(struct file_lock *caller_fl, > return locks_conflict(caller_fl, sys_fl); > } > > +/* Determine if lock sys_fl blocks lock caller_fl. Used on xx_GETLK > + * path so checks for additional GETLK-specific things like F_UNLCK. > + */ > +static bool posix_test_locks_conflict(struct file_lock *caller_fl, > + struct file_lock *sys_fl) > +{ > + /* F_UNLCK checks any locks on the same fd. */ > + if (caller_fl->fl_type == F_UNLCK) { > + if (!posix_same_owner(caller_fl, sys_fl)) > + return false; > + return locks_overlap(caller_fl, sys_fl); > + } > + return posix_locks_conflict(caller_fl, sys_fl); > +} > + > /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific > * checking before calling the locks_conflict(). > */ > @@ -901,7 +916,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl) > retry: > spin_lock(&ctx->flc_lock); > list_for_each_entry(cfl, &ctx->flc_posix, fl_list) { > - if (!posix_locks_conflict(fl, cfl)) > + if (!posix_test_locks_conflict(fl, cfl)) > continue; > if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_expirable > && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { > @@ -2207,7 +2222,8 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) > if (fl == NULL) > return -ENOMEM; > error = -EINVAL; > - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) > + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK > + && flock->l_type != F_WRLCK) > goto out; > > error = flock_to_posix_lock(filp, fl, flock); > @@ -2414,7 +2430,8 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock) > return -ENOMEM; > > error = -EINVAL; > - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) > + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK > + && flock->l_type != F_WRLCK) > goto out; > > error = flock64_to_posix_lock(filp, fl, flock); This seems like a reasonable sort of interface to add, particularly for the CRIU case. Using F_UNLCK for this is a bit kludgey, but adding a new constant is probably worse. I'm willing to take this in with an eye toward v6.6. Are you also willing to draft up some manpage patches that detail this new interface?
Hello, 20.06.2023 15:46, Jeff Layton пишет: > On Tue, 2023-06-20 at 14:55 +0500, Stas Sergeev wrote: >> Currently F_UNLCK with F_OFD_GETLK returns -EINVAL. >> The proposed extension allows to use it for getting the lock >> information from the particular fd. >> >> Signed-off-by: Stas Sergeev <stsp2@yandex.ru> >> >> CC: Jeff Layton <jlayton@kernel.org> >> CC: Chuck Lever <chuck.lever@oracle.com> >> CC: Alexander Viro <viro@zeniv.linux.org.uk> >> CC: Christian Brauner <brauner@kernel.org> >> CC: linux-fsdevel@vger.kernel.org >> CC: linux-kernel@vger.kernel.org >> >> --- >> fs/locks.c | 23 ++++++++++++++++++++--- >> 1 file changed, 20 insertions(+), 3 deletions(-) >> >> diff --git a/fs/locks.c b/fs/locks.c >> index df8b26a42524..210766007e63 100644 >> --- a/fs/locks.c >> +++ b/fs/locks.c >> @@ -868,6 +868,21 @@ static bool posix_locks_conflict(struct file_lock *caller_fl, >> return locks_conflict(caller_fl, sys_fl); >> } >> >> +/* Determine if lock sys_fl blocks lock caller_fl. Used on xx_GETLK >> + * path so checks for additional GETLK-specific things like F_UNLCK. >> + */ >> +static bool posix_test_locks_conflict(struct file_lock *caller_fl, >> + struct file_lock *sys_fl) >> +{ >> + /* F_UNLCK checks any locks on the same fd. */ >> + if (caller_fl->fl_type == F_UNLCK) { >> + if (!posix_same_owner(caller_fl, sys_fl)) >> + return false; >> + return locks_overlap(caller_fl, sys_fl); >> + } >> + return posix_locks_conflict(caller_fl, sys_fl); >> +} >> + >> /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific >> * checking before calling the locks_conflict(). >> */ >> @@ -901,7 +916,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl) >> retry: >> spin_lock(&ctx->flc_lock); >> list_for_each_entry(cfl, &ctx->flc_posix, fl_list) { >> - if (!posix_locks_conflict(fl, cfl)) >> + if (!posix_test_locks_conflict(fl, cfl)) >> continue; >> if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_expirable >> && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { >> @@ -2207,7 +2222,8 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) >> if (fl == NULL) >> return -ENOMEM; >> error = -EINVAL; >> - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) >> + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK >> + && flock->l_type != F_WRLCK) >> goto out; >> >> error = flock_to_posix_lock(filp, fl, flock); >> @@ -2414,7 +2430,8 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock) >> return -ENOMEM; >> >> error = -EINVAL; >> - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) >> + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK >> + && flock->l_type != F_WRLCK) >> goto out; >> >> error = flock64_to_posix_lock(filp, fl, flock); > This seems like a reasonable sort of interface to add, particularly for > the CRIU case. Just for the record: my own cases are the remaining 2. CRIU case is not mine and I haven't talked to CRIU people about that. > Using F_UNLCK for this is a bit kludgey, but adding a new > constant is probably worse. > > I'm willing to take this in with an eye toward v6.6. Are you also > willing to draft up some manpage patches that detail this new interface? Sure thing. As soon as its applied, I'll prepare a man patch, or should it be done before that point?
On Tue, 2023-06-20 at 16:00 +0500, stsp wrote: > Hello, > > 20.06.2023 15:46, Jeff Layton пишет: > > On Tue, 2023-06-20 at 14:55 +0500, Stas Sergeev wrote: > > > Currently F_UNLCK with F_OFD_GETLK returns -EINVAL. > > > The proposed extension allows to use it for getting the lock > > > information from the particular fd. > > > > > > Signed-off-by: Stas Sergeev <stsp2@yandex.ru> > > > > > > CC: Jeff Layton <jlayton@kernel.org> > > > CC: Chuck Lever <chuck.lever@oracle.com> > > > CC: Alexander Viro <viro@zeniv.linux.org.uk> > > > CC: Christian Brauner <brauner@kernel.org> > > > CC: linux-fsdevel@vger.kernel.org > > > CC: linux-kernel@vger.kernel.org > > > > > > --- > > > fs/locks.c | 23 ++++++++++++++++++++--- > > > 1 file changed, 20 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/locks.c b/fs/locks.c > > > index df8b26a42524..210766007e63 100644 > > > --- a/fs/locks.c > > > +++ b/fs/locks.c > > > @@ -868,6 +868,21 @@ static bool posix_locks_conflict(struct file_lock *caller_fl, > > > return locks_conflict(caller_fl, sys_fl); > > > } > > > > > > +/* Determine if lock sys_fl blocks lock caller_fl. Used on xx_GETLK > > > + * path so checks for additional GETLK-specific things like F_UNLCK. > > > + */ > > > +static bool posix_test_locks_conflict(struct file_lock *caller_fl, > > > + struct file_lock *sys_fl) > > > +{ > > > + /* F_UNLCK checks any locks on the same fd. */ > > > + if (caller_fl->fl_type == F_UNLCK) { > > > + if (!posix_same_owner(caller_fl, sys_fl)) > > > + return false; > > > + return locks_overlap(caller_fl, sys_fl); > > > + } > > > + return posix_locks_conflict(caller_fl, sys_fl); > > > +} > > > + > > > /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific > > > * checking before calling the locks_conflict(). > > > */ > > > @@ -901,7 +916,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl) > > > retry: > > > spin_lock(&ctx->flc_lock); > > > list_for_each_entry(cfl, &ctx->flc_posix, fl_list) { > > > - if (!posix_locks_conflict(fl, cfl)) > > > + if (!posix_test_locks_conflict(fl, cfl)) > > > continue; > > > if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_expirable > > > && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { > > > @@ -2207,7 +2222,8 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) > > > if (fl == NULL) > > > return -ENOMEM; > > > error = -EINVAL; > > > - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) > > > + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK > > > + && flock->l_type != F_WRLCK) > > > goto out; > > > > > > error = flock_to_posix_lock(filp, fl, flock); > > > @@ -2414,7 +2430,8 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock) > > > return -ENOMEM; > > > > > > error = -EINVAL; > > > - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) > > > + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK > > > + && flock->l_type != F_WRLCK) > > > goto out; > > > > > > error = flock64_to_posix_lock(filp, fl, flock); > > This seems like a reasonable sort of interface to add, particularly for > > the CRIU case. > > Just for the record: my own cases are > the remaining 2. CRIU case is not mine > and I haven't talked to CRIU people > about that. > > > > Using F_UNLCK for this is a bit kludgey, but adding a new > > constant is probably worse. > > > > I'm willing to take this in with an eye toward v6.6. Are you also > > willing to draft up some manpage patches that detail this new interface? > Sure thing. > As soon as its applied, I'll prepare a man > patch, or should it be done before that point? These days, it's a good idea to go ahead and draft that up early. You'll be surprised what sort of details you notice once you have to start writing documentation. ;) You can post it as part of this set on the next posting and just mention that it's a draft manpage patch. You should also include the linux-api mailing list on the next posting so we get some feedback on the interface itself.
20.06.2023 16:15, Jeff Layton пишет: > These days, it's a good idea to go ahead and draft that up early. You'll > be surprised what sort of details you notice once you have to start > writing documentation. ;) > > You can post it as part of this set on the next posting and just mention > that it's a draft manpage patch. You should also include the linux-api > mailing list on the next posting so we get some feedback on the > interface itself. v2 is sent with the proposed man page update and a drop of an l_pid. Thanks.
diff --git a/fs/locks.c b/fs/locks.c index df8b26a42524..210766007e63 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -868,6 +868,21 @@ static bool posix_locks_conflict(struct file_lock *caller_fl, return locks_conflict(caller_fl, sys_fl); } +/* Determine if lock sys_fl blocks lock caller_fl. Used on xx_GETLK + * path so checks for additional GETLK-specific things like F_UNLCK. + */ +static bool posix_test_locks_conflict(struct file_lock *caller_fl, + struct file_lock *sys_fl) +{ + /* F_UNLCK checks any locks on the same fd. */ + if (caller_fl->fl_type == F_UNLCK) { + if (!posix_same_owner(caller_fl, sys_fl)) + return false; + return locks_overlap(caller_fl, sys_fl); + } + return posix_locks_conflict(caller_fl, sys_fl); +} + /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific * checking before calling the locks_conflict(). */ @@ -901,7 +916,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl) retry: spin_lock(&ctx->flc_lock); list_for_each_entry(cfl, &ctx->flc_posix, fl_list) { - if (!posix_locks_conflict(fl, cfl)) + if (!posix_test_locks_conflict(fl, cfl)) continue; if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_expirable && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { @@ -2207,7 +2222,8 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock) if (fl == NULL) return -ENOMEM; error = -EINVAL; - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK + && flock->l_type != F_WRLCK) goto out; error = flock_to_posix_lock(filp, fl, flock); @@ -2414,7 +2430,8 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock) return -ENOMEM; error = -EINVAL; - if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK) + if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK + && flock->l_type != F_WRLCK) goto out; error = flock64_to_posix_lock(filp, fl, flock);
Currently F_UNLCK with F_OFD_GETLK returns -EINVAL. The proposed extension allows to use it for getting the lock information from the particular fd. Signed-off-by: Stas Sergeev <stsp2@yandex.ru> CC: Jeff Layton <jlayton@kernel.org> CC: Chuck Lever <chuck.lever@oracle.com> CC: Alexander Viro <viro@zeniv.linux.org.uk> CC: Christian Brauner <brauner@kernel.org> CC: linux-fsdevel@vger.kernel.org CC: linux-kernel@vger.kernel.org --- fs/locks.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)