Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

nfs: block notification on fs with its own ->lock

NFSv4.1 supports an optional lock notification feature which notifies
the client when a lock comes available. (Normally NFSv4 clients just
poll for locks if necessary.) To make that work, we need to request a
blocking lock from the filesystem.

We turned that off for NFS in commit f657f8eef3ff ("nfs: don't atempt
blocking locks on nfs reexports") [sic] because it actually blocks the
nfsd thread while waiting for the lock.

Thanks to Vasily Averin for pointing out that NFS isn't the only
filesystem with that problem.

Any filesystem that leaves ->lock NULL will use posix_lock_file(), which
does the right thing. Simplest is just to assume that any filesystem
that defines its own ->lock is not safe to request a blocking lock from.

So, this patch mostly reverts commit f657f8eef3ff ("nfs: don't atempt
blocking locks on nfs reexports") [sic] and commit b840be2f00c0 ("lockd:
don't attempt blocking locks on nfs reexports"), and instead uses a
check of ->lock (Vasily's suggestion) to decide whether to support
blocking lock notifications on a given filesystem. Also add a little
documentation.

Perhaps someday we could add back an export flag later to allow
filesystems with "good" ->lock methods to support blocking lock
notifications.

Reported-by: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
[ cel: Description rewritten to address checkpatch nits ]
[ cel: Fixed warning when SUNRPC debugging is disabled ]
[ cel: Fixed NULL check ]
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Vasily Averin <vvs@virtuozzo.com>

authored by

J. Bruce Fields and committed by
Chuck Lever
40595cdc cd2e999c

+24 -13
+4 -2
fs/lockd/svclock.c
··· 470 470 struct nlm_host *host, struct nlm_lock *lock, int wait, 471 471 struct nlm_cookie *cookie, int reclaim) 472 472 { 473 - struct nlm_block *block = NULL; 473 + #if IS_ENABLED(CONFIG_SUNRPC_DEBUG) 474 474 struct inode *inode = nlmsvc_file_inode(file); 475 + #endif 476 + struct nlm_block *block = NULL; 475 477 int error; 476 478 int mode; 477 479 int async_block = 0; ··· 486 484 (long long)lock->fl.fl_end, 487 485 wait); 488 486 489 - if (inode->i_sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS) { 487 + if (nlmsvc_file_file(file)->f_op->lock) { 490 488 async_block = wait; 491 489 wait = 0; 492 490 }
+1 -1
fs/nfs/export.c
··· 158 158 .fetch_iversion = nfs_fetch_iversion, 159 159 .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK| 160 160 EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS| 161 - EXPORT_OP_NOATOMIC_ATTR|EXPORT_OP_SYNC_LOCKS, 161 + EXPORT_OP_NOATOMIC_ATTR, 162 162 };
+12 -6
fs/nfsd/nfs4state.c
··· 6842 6842 struct nfsd4_blocked_lock *nbl = NULL; 6843 6843 struct file_lock *file_lock = NULL; 6844 6844 struct file_lock *conflock = NULL; 6845 - struct super_block *sb; 6846 6845 __be32 status = 0; 6847 6846 int lkflg; 6848 6847 int err; ··· 6863 6864 dprintk("NFSD: nfsd4_lock: permission denied!\n"); 6864 6865 return status; 6865 6866 } 6866 - sb = cstate->current_fh.fh_dentry->d_sb; 6867 6867 6868 6868 if (lock->lk_is_new) { 6869 6869 if (nfsd4_has_session(cstate)) ··· 6914 6916 fp = lock_stp->st_stid.sc_file; 6915 6917 switch (lock->lk_type) { 6916 6918 case NFS4_READW_LT: 6917 - if (nfsd4_has_session(cstate) && 6918 - !(sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS)) 6919 + if (nfsd4_has_session(cstate)) 6919 6920 fl_flags |= FL_SLEEP; 6920 6921 fallthrough; 6921 6922 case NFS4_READ_LT: ··· 6926 6929 fl_type = F_RDLCK; 6927 6930 break; 6928 6931 case NFS4_WRITEW_LT: 6929 - if (nfsd4_has_session(cstate) && 6930 - !(sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS)) 6932 + if (nfsd4_has_session(cstate)) 6931 6933 fl_flags |= FL_SLEEP; 6932 6934 fallthrough; 6933 6935 case NFS4_WRITE_LT: ··· 6946 6950 status = nfserr_openmode; 6947 6951 goto out; 6948 6952 } 6953 + 6954 + /* 6955 + * Most filesystems with their own ->lock operations will block 6956 + * the nfsd thread waiting to acquire the lock. That leads to 6957 + * deadlocks (we don't want every nfsd thread tied up waiting 6958 + * for file locks), so don't attempt blocking lock notifications 6959 + * on those filesystems: 6960 + */ 6961 + if (nf->nf_file->f_op->lock) 6962 + fl_flags &= ~FL_SLEEP; 6949 6963 6950 6964 nbl = find_or_allocate_block(lock_sop, &fp->fi_fhandle, nn); 6951 6965 if (!nbl) {
-2
include/linux/exportfs.h
··· 221 221 #define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply 222 222 atomic attribute updates 223 223 */ 224 - #define EXPORT_OP_SYNC_LOCKS (0x20) /* Filesystem can't do 225 - asychronous blocking locks */ 226 224 unsigned long flags; 227 225 }; 228 226
+7 -2
include/linux/lockd/lockd.h
··· 303 303 int nlmsvc_unlock_all_by_sb(struct super_block *sb); 304 304 int nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr); 305 305 306 + static inline struct file *nlmsvc_file_file(struct nlm_file *file) 307 + { 308 + return file->f_file[O_RDONLY] ? 309 + file->f_file[O_RDONLY] : file->f_file[O_WRONLY]; 310 + } 311 + 306 312 static inline struct inode *nlmsvc_file_inode(struct nlm_file *file) 307 313 { 308 - return locks_inode(file->f_file[O_RDONLY] ? 309 - file->f_file[O_RDONLY] : file->f_file[O_WRONLY]); 314 + return locks_inode(nlmsvc_file_file(file)); 310 315 } 311 316 312 317 static inline int __nlm_privileged_request4(const struct sockaddr *sap)