diff mbox series

nfs4: handle async processing of F_SETLK with FL_SLEEP

Message ID 3a2c6cb9-abe7-ab32-b11c-d78621361555@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series nfs4: handle async processing of F_SETLK with FL_SLEEP | expand

Commit Message

Vasily Averin Dec. 27, 2021, 10:45 a.m. UTC 
nfsd and lockd use F_SETLK cmd with the FL_SLEEP flag set to request
asynchronous processing of blocking locks.

Currently nfs4 use locks_lock_inode_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 <vvs@virtuozzo.com>
---
VvS: I'm not sure that request->fl_file points to the same state->inode
used in locks_lock_inode_wait(). If it is not, posix_lock_inode() can be
used here, but this function is static currently and should be exported first.
---
 fs/nfs/nfs4proc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

kernel test robot Dec. 27, 2021, 5:58 p.m. UTC | #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/nfs4-handle-async-processing-of-F_SETLK-with-FL_SLEEP/20211227-184632
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: i386-randconfig-r005-20211227 (https://download.01.org/0day-ci/archive/20211228/202112280146.yo82FThq-lkp@intel.com/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/7ae55d384b2f337e6630509e451c35dda45ae185
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vasily-Averin/nfs4-handle-async-processing-of-F_SETLK-with-FL_SLEEP/20211227-184632
        git checkout 7ae55d384b2f337e6630509e451c35dda45ae185
        # 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 <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/nfs/nfs4proc.c:7202:25: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
           if ((request->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
                                  ^  ~~~~~~~~
   fs/nfs/nfs4proc.c:7202:25: note: use '&' for a bitwise operation
           if ((request->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
                                  ^~
                                  &
   fs/nfs/nfs4proc.c:7202:25: note: remove constant to silence this warning
           if ((request->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
                                 ~^~~~~~~~~~~
   1 warning generated.


vim +7202 fs/nfs/nfs4proc.c

  7193	
  7194	static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
  7195	{
  7196		struct nfs_inode *nfsi = NFS_I(state->inode);
  7197		struct nfs4_state_owner *sp = state->owner;
  7198		unsigned char fl_flags = request->fl_flags;
  7199		int status;
  7200	
  7201		request->fl_flags |= FL_ACCESS;
> 7202		if ((request->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
  7203			status = posix_lock_file(request->fl_file, request, NULL);
  7204		else
  7205			status = locks_lock_inode_wait(state->inode, request);
  7206		if (status)
  7207			goto out;
  7208		mutex_lock(&sp->so_delegreturn_mutex);
  7209		down_read(&nfsi->rwsem);
  7210		if (test_bit(NFS_DELEGATED_STATE, &state->flags)) {
  7211			/* Yes: cache locks! */
  7212			/* ...but avoid races with delegation recall... */
  7213			request->fl_flags = fl_flags & ~FL_SLEEP;
  7214			status = locks_lock_inode_wait(state->inode, request);
  7215			up_read(&nfsi->rwsem);
  7216			mutex_unlock(&sp->so_delegreturn_mutex);
  7217			goto out;
  7218		}
  7219		up_read(&nfsi->rwsem);
  7220		mutex_unlock(&sp->so_delegreturn_mutex);
  7221		status = _nfs4_do_setlk(state, cmd, request, NFS_LOCK_NEW);
  7222	out:
  7223		request->fl_flags = fl_flags;
  7224		return status;
  7225	}
  7226	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Dan Carpenter Jan. 6, 2022, 10:37 a.m. UTC | #2 
Hi Vasily,

url:    https://github.com/0day-ci/linux/commits/Vasily-Averin/nfs4-handle-async-processing-of-F_SETLK-with-FL_SLEEP/20211227-184632
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/202112281638.RNX7e40X-lkp@intel.com/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 <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/nfs/nfs4proc.c:7202 _nfs4_proc_setlk() warn: should this be a bitwise op?

vim +7202 fs/nfs/nfs4proc.c

^1da177e4c3f41 Linus Torvalds  2005-04-16  7194  static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock *request)
^1da177e4c3f41 Linus Torvalds  2005-04-16  7195  {
19e03c570e6099 Trond Myklebust 2008-12-23  7196  	struct nfs_inode *nfsi = NFS_I(state->inode);
11476e9dec39d9 Chuck Lever     2016-04-11  7197  	struct nfs4_state_owner *sp = state->owner;
01c3b861cd77b2 Trond Myklebust 2006-06-29  7198  	unsigned char fl_flags = request->fl_flags;
1ea67dbd982827 Jeff Layton     2016-09-17  7199  	int status;
^1da177e4c3f41 Linus Torvalds  2005-04-16  7200  
01c3b861cd77b2 Trond Myklebust 2006-06-29  7201  	request->fl_flags |= FL_ACCESS;
7ae55d384b2f33 Vasily Averin   2021-12-27 @7202  	if ((request->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
                                                                               ^^
Same thing but for nfsv4.

7ae55d384b2f33 Vasily Averin   2021-12-27  7203  		status = posix_lock_file(request->fl_file, request, NULL);
7ae55d384b2f33 Vasily Averin   2021-12-27  7204  	else
75575ddf29cbbf Jeff Layton     2016-09-17  7205  		status = locks_lock_inode_wait(state->inode, request);
7ae55d384b2f33 Vasily Averin   2021-12-27  7206  	if (status)
01c3b861cd77b2 Trond Myklebust 2006-06-29  7207  		goto out;
11476e9dec39d9 Chuck Lever     2016-04-11  7208  	mutex_lock(&sp->so_delegreturn_mutex);
19e03c570e6099 Trond Myklebust 2008-12-23  7209  	down_read(&nfsi->rwsem);
01c3b861cd77b2 Trond Myklebust 2006-06-29  7210  	if (test_bit(NFS_DELEGATED_STATE, &state->flags)) {
01c3b861cd77b2 Trond Myklebust 2006-06-29  7211  		/* Yes: cache locks! */
01c3b861cd77b2 Trond Myklebust 2006-06-29  7212  		/* ...but avoid races with delegation recall... */
01c3b861cd77b2 Trond Myklebust 2006-06-29  7213  		request->fl_flags = fl_flags & ~FL_SLEEP;
75575ddf29cbbf Jeff Layton     2016-09-17  7214  		status = locks_lock_inode_wait(state->inode, request);
9a99af494bd714 Trond Myklebust 2013-02-04  7215  		up_read(&nfsi->rwsem);
11476e9dec39d9 Chuck Lever     2016-04-11  7216  		mutex_unlock(&sp->so_delegreturn_mutex);
9a99af494bd714 Trond Myklebust 2013-02-04  7217  		goto out;
9a99af494bd714 Trond Myklebust 2013-02-04  7218  	}
19e03c570e6099 Trond Myklebust 2008-12-23  7219  	up_read(&nfsi->rwsem);
11476e9dec39d9 Chuck Lever     2016-04-11  7220  	mutex_unlock(&sp->so_delegreturn_mutex);
c69899a17ca483 Trond Myklebust 2015-01-24  7221  	status = _nfs4_do_setlk(state, cmd, request, NFS_LOCK_NEW);
01c3b861cd77b2 Trond Myklebust 2006-06-29  7222  out:
01c3b861cd77b2 Trond Myklebust 2006-06-29  7223  	request->fl_flags = fl_flags;
^1da177e4c3f41 Linus Torvalds  2005-04-16  7224  	return status;
^1da177e4c3f41 Linus Torvalds  2005-04-16  7225  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ee3bc79f6ca3..54431b296c85 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7200,8 +7200,11 @@  static int _nfs4_proc_setlk(struct nfs4_state *state, int cmd, struct file_lock
 	int status;
 
 	request->fl_flags |= FL_ACCESS;
-	status = locks_lock_inode_wait(state->inode, request);
-	if (status < 0)
+	if ((request->fl_flags && FL_SLEEP) && IS_SETLK(cmd))
+		status = posix_lock_file(request->fl_file, request, NULL);
+	else
+		status = locks_lock_inode_wait(state->inode, request);
+	if (status)
 		goto out;
 	mutex_lock(&sp->so_delegreturn_mutex);
 	down_read(&nfsi->rwsem);