Skip to content

Commit

Permalink
nfs: block notification on fs with its own ->lock
Browse files Browse the repository at this point in the history
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 f657f8e "nfs: don't atempt blocking
locks on nfs reexports" 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 f657f8e and b840be2, 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>
  • Loading branch information
J. Bruce Fields authored and intel-lab-lkp committed Dec 16, 2021
1 parent 064a917 commit 61c7ebf
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 12 deletions.
2 changes: 1 addition & 1 deletion fs/lockd/svclock.c
Expand Up @@ -484,7 +484,7 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file *file,
(long long)lock->fl.fl_end,
wait);

if (inode->i_sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS) {
if (nlmsvc_file_file(file)->f_op->lock) {
async_block = wait;
wait = 0;
}
Expand Down
2 changes: 1 addition & 1 deletion fs/nfs/export.c
Expand Up @@ -158,5 +158,5 @@ const struct export_operations nfs_export_ops = {
.fetch_iversion = nfs_fetch_iversion,
.flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK|
EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS|
EXPORT_OP_NOATOMIC_ATTR|EXPORT_OP_SYNC_LOCKS,
EXPORT_OP_NOATOMIC_ATTR,
};
18 changes: 12 additions & 6 deletions fs/nfsd/nfs4state.c
Expand Up @@ -6831,7 +6831,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_blocked_lock *nbl = NULL;
struct file_lock *file_lock = NULL;
struct file_lock *conflock = NULL;
struct super_block *sb;
__be32 status = 0;
int lkflg;
int err;
Expand All @@ -6853,7 +6852,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
dprintk("NFSD: nfsd4_lock: permission denied!\n");
return status;
}
sb = cstate->current_fh.fh_dentry->d_sb;

if (lock->lk_is_new) {
if (nfsd4_has_session(cstate))
Expand Down Expand Up @@ -6905,8 +6903,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
fp = lock_stp->st_stid.sc_file;
switch (lock->lk_type) {
case NFS4_READW_LT:
if (nfsd4_has_session(cstate) &&
!(sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS))
if (nfsd4_has_session(cstate))
fl_flags |= FL_SLEEP;
fallthrough;
case NFS4_READ_LT:
Expand All @@ -6918,8 +6915,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
fl_type = F_RDLCK;
break;
case NFS4_WRITEW_LT:
if (nfsd4_has_session(cstate) &&
!(sb->s_export_op->flags & EXPORT_OP_SYNC_LOCKS))
if (nfsd4_has_session(cstate))
fl_flags |= FL_SLEEP;
fallthrough;
case NFS4_WRITE_LT:
Expand All @@ -6935,6 +6931,16 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
goto out;
}

/*
* Most filesystems with their own ->lock operations will block
* the nfsd thread waiting to acquire the lock. That leads to
* deadlocks (we don't want every nfsd thread tied up waiting
* for file locks), so don't attempt blocking lock notifications
* on those filesystems:
*/
if (nf->nf_file->f_op->lock)
fl_flags &= ~FL_SLEEP;

if (!nf) {
status = nfserr_openmode;
goto out;
Expand Down
2 changes: 0 additions & 2 deletions include/linux/exportfs.h
Expand Up @@ -221,8 +221,6 @@ struct export_operations {
#define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply
atomic attribute updates
*/
#define EXPORT_OP_SYNC_LOCKS (0x20) /* Filesystem can't do
asychronous blocking locks */
unsigned long flags;
};

Expand Down
9 changes: 7 additions & 2 deletions include/linux/lockd/lockd.h
Expand Up @@ -303,10 +303,15 @@ void nlmsvc_invalidate_all(void);
int nlmsvc_unlock_all_by_sb(struct super_block *sb);
int nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr);

static inline struct file *nlmsvc_file_file(struct nlm_file *file)
{
return file->f_file[O_RDONLY] ?
file->f_file[O_RDONLY] : file->f_file[O_WRONLY];
}

static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
{
return locks_inode(file->f_file[O_RDONLY] ?
file->f_file[O_RDONLY] : file->f_file[O_WRONLY]);
return locks_inode(nlmsvc_file_file(file));
}

static inline int __nlm_privileged_request4(const struct sockaddr *sap)
Expand Down

0 comments on commit 61c7ebf

Please sign in to comment.