nfsd and lockd use F_SETLK cmd with the FL_SLEEP flag set to request
asynchronous processing of blocking locks.
Currently nfs mounted with 'local_lock' option use locks_lock_file_wait()
function blocked on such requests.
To handle such requests properly, non-blocking posix_file_lock()
function should be used instead.
https://bugzilla.kernel.org/show_bug.cgi?id=215383
Signed-off-by: Vasily Averin <[email protected]>
---
fs/nfs/file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 24e7dccce355..c9c974ee2f43 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -769,9 +769,11 @@ do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
*/
if (!is_local)
status = NFS_PROTO(inode)->lock(filp, cmd, fl);
+ else if ((fl->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
+ status = posix_lock_file(filp, fl, NULL);
else
status = locks_lock_file_wait(filp, fl);
- if (status < 0)
+ if (status)
goto out;
/*
--
2.25.1
Hi Vasily,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on trondmy-nfs/linux-next]
[also build test WARNING on v5.16-rc7 next-20211224]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Vasily-Averin/nfs-local_lock-handle-async-processing-of-F_SETLK-with-FL_SLEEP/20211227-184705
base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: i386-randconfig-r034-20211227 (https://download.01.org/0day-ci/archive/20211227/[email protected]/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 511726c64d3b6cca66f7c54d457d586aa3129f67)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/00e7edbaaa914f1b049ab8a652c5a91483f4141d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vasily-Averin/nfs-local_lock-handle-async-processing-of-F_SETLK-with-FL_SLEEP/20211227-184705
git checkout 00e7edbaaa914f1b049ab8a652c5a91483f4141d
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/nfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> fs/nfs/file.c:772:25: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
else if ((fl->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
^ ~~~~~~~~
fs/nfs/file.c:772:25: note: use '&' for a bitwise operation
else if ((fl->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
^~
&
fs/nfs/file.c:772:25: note: remove constant to silence this warning
else if ((fl->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
~^~~~~~~~~~~
1 warning generated.
vim +772 fs/nfs/file.c
751
752 static int
753 do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
754 {
755 struct inode *inode = filp->f_mapping->host;
756 int status;
757
758 /*
759 * Flush all pending writes before doing anything
760 * with locks..
761 */
762 status = nfs_sync_mapping(filp->f_mapping);
763 if (status != 0)
764 goto out;
765
766 /*
767 * Use local locking if mounted with "-onolock" or with appropriate
768 * "-olocal_lock="
769 */
770 if (!is_local)
771 status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> 772 else if ((fl->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
773 status = posix_lock_file(filp, fl, NULL);
774 else
775 status = locks_lock_file_wait(filp, fl);
776 if (status)
777 goto out;
778
779 /*
780 * Invalidate cache to prevent missing any changes. If
781 * the file is mapped, clear the page cache as well so
782 * those mappings will be loaded.
783 *
784 * This makes locking act as a cache coherency point.
785 */
786 nfs_sync_mapping(filp->f_mapping);
787 if (!NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) {
788 nfs_zap_caches(inode);
789 if (mapping_mapped(filp->f_mapping))
790 nfs_revalidate_mapping(inode, filp->f_mapping);
791 }
792 out:
793 return status;
794 }
795
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
nfsd and lockd use F_SETLK cmd with the FL_SLEEP flag set to request
asynchronous processing of blocking locks.
Currently nfs mounted with 'local_lock' option use locks_lock_file_wait()
function blocked on such requests.
To handle such requests properly, non-blocking posix_file_lock()
function should be used instead.
https://bugzilla.kernel.org/show_bug.cgi?id=215383
Signed-off-by: Vasily Averin <[email protected]>
---
v2: fixed 'fl_flags && FL_SLEEP' => 'fl_flags & FL_SLEEP'
---
fs/nfs/file.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 24e7dccce355..38e1821cff5d 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -769,9 +769,11 @@ do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
*/
if (!is_local)
status = NFS_PROTO(inode)->lock(filp, cmd, fl);
+ else if ((fl->fl_flags & FL_SLEEP) && IS_SETLK(cmd))
+ status = posix_lock_file(filp, fl, NULL);
else
status = locks_lock_file_wait(filp, fl);
- if (status < 0)
+ if (status)
goto out;
/*
--
2.25.1
Patch is wrong,
I missed that FL_FLOCK are handled here too.
On 27.12.2021 18:50, Vasily Averin wrote:
> nfsd and lockd use F_SETLK cmd with the FL_SLEEP flag set to request
> asynchronous processing of blocking locks.
>
> Currently nfs mounted with 'local_lock' option use locks_lock_file_wait()
> function blocked on such requests.
>
> To handle such requests properly, non-blocking posix_file_lock()
> function should be used instead.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215383
> Signed-off-by: Vasily Averin <[email protected]>
> ---
> v2: fixed 'fl_flags && FL_SLEEP' => 'fl_flags & FL_SLEEP'
> ---
> fs/nfs/file.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 24e7dccce355..38e1821cff5d 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -769,9 +769,11 @@ do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
> */
> if (!is_local)
> status = NFS_PROTO(inode)->lock(filp, cmd, fl);
> + else if ((fl->fl_flags & FL_SLEEP) && IS_SETLK(cmd))
> + status = posix_lock_file(filp, fl, NULL);
> else
> status = locks_lock_file_wait(filp, fl);
> - if (status < 0)
> + if (status)
> goto out;
>
> /*
>
Hi Vasily,
url: https://github.com/0day-ci/linux/commits/Vasily-Averin/nfs-local_lock-handle-async-processing-of-F_SETLK-with-FL_SLEEP/20211227-184705
base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: i386-randconfig-m021-20211227 (https://download.01.org/0day-ci/archive/20211228/[email protected]/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
smatch warnings:
fs/nfs/file.c:772 do_setlk() warn: should this be a bitwise op?
vim +772 fs/nfs/file.c
5eebde23223aeb Suresh Jayaraman 2010-09-23 752 static int
5eebde23223aeb Suresh Jayaraman 2010-09-23 753 do_setlk(struct file *filp, int cmd, struct file_lock *fl, int is_local)
^1da177e4c3f41 Linus Torvalds 2005-04-16 754 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 755 struct inode *inode = filp->f_mapping->host;
^1da177e4c3f41 Linus Torvalds 2005-04-16 756 int status;
^1da177e4c3f41 Linus Torvalds 2005-04-16 757
^1da177e4c3f41 Linus Torvalds 2005-04-16 758 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 759 * Flush all pending writes before doing anything
^1da177e4c3f41 Linus Torvalds 2005-04-16 760 * with locks..
^1da177e4c3f41 Linus Torvalds 2005-04-16 761 */
29884df0d89c1d Trond Myklebust 2005-12-13 762 status = nfs_sync_mapping(filp->f_mapping);
29884df0d89c1d Trond Myklebust 2005-12-13 763 if (status != 0)
^1da177e4c3f41 Linus Torvalds 2005-04-16 764 goto out;
^1da177e4c3f41 Linus Torvalds 2005-04-16 765
5eebde23223aeb Suresh Jayaraman 2010-09-23 766 /*
5eebde23223aeb Suresh Jayaraman 2010-09-23 767 * Use local locking if mounted with "-onolock" or with appropriate
5eebde23223aeb Suresh Jayaraman 2010-09-23 768 * "-olocal_lock="
5eebde23223aeb Suresh Jayaraman 2010-09-23 769 */
5eebde23223aeb Suresh Jayaraman 2010-09-23 770 if (!is_local)
^1da177e4c3f41 Linus Torvalds 2005-04-16 771 status = NFS_PROTO(inode)->lock(filp, cmd, fl);
00e7edbaaa914f Vasily Averin 2021-12-27 @772 else if ((fl->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
^
&& vs &
00e7edbaaa914f Vasily Averin 2021-12-27 773 status = posix_lock_file(filp, fl, NULL);
c4d7c402b788b7 Trond Myklebust 2008-04-01 774 else
75575ddf29cbbf Jeff Layton 2016-09-17 775 status = locks_lock_file_wait(filp, fl);
00e7edbaaa914f Vasily Averin 2021-12-27 776 if (status)
^1da177e4c3f41 Linus Torvalds 2005-04-16 777 goto out;
6b96724e507fec Ricardo Labiaga 2010-10-12 778
^1da177e4c3f41 Linus Torvalds 2005-04-16 779 /*
779eafab06036f NeilBrown 2017-08-18 780 * Invalidate cache to prevent missing any changes. If
779eafab06036f NeilBrown 2017-08-18 781 * the file is mapped, clear the page cache as well so
779eafab06036f NeilBrown 2017-08-18 782 * those mappings will be loaded.
6b96724e507fec Ricardo Labiaga 2010-10-12 783 *
^1da177e4c3f41 Linus Torvalds 2005-04-16 784 * This makes locking act as a cache coherency point.
^1da177e4c3f41 Linus Torvalds 2005-04-16 785 */
29884df0d89c1d Trond Myklebust 2005-12-13 786 nfs_sync_mapping(filp->f_mapping);
779eafab06036f NeilBrown 2017-08-18 787 if (!NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) {
442ce0499c0535 NeilBrown 2017-07-24 788 nfs_zap_caches(inode);
779eafab06036f NeilBrown 2017-08-18 789 if (mapping_mapped(filp->f_mapping))
779eafab06036f NeilBrown 2017-08-18 790 nfs_revalidate_mapping(inode, filp->f_mapping);
779eafab06036f NeilBrown 2017-08-18 791 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 792 out:
^1da177e4c3f41 Linus Torvalds 2005-04-16 793 return status;
^1da177e4c3f41 Linus Torvalds 2005-04-16 794 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]