From: "J. Bruce Fields" <[email protected]>
This is my second attempt to fix the NFS server so we don't
unnecessarily recall delegations when the operation breaking the
delegation comes from the same client that holds the delegation.
To do that we need some way to pass the identity of the breaker down
through the VFS. In my first attempt I tried passing that explicitly,
but that touches a lot of code. So instead I'm experimenting with
adding a field to the cred struct.
Testing has confirmed that this works. (And that the pynfs tests are
broken--they *require* the server to break delegations on operations
coming from the client, even though that's the less desireable behavior.
I'm fixing that...).
J. Bruce Fields (10):
vfs: remove unnecessary fl_owner_t typedef
nfsd: simplify put of fi_deleg_file
nfsd: simplify nfs4_put_deleg_lease calls
nfsd4: set fl_owner to delegation, not file pointer
nfsd4: dp->dl_stid.sc_file doesn't need locking
nfsd: make nfs4_get_existing_delegation less confusing
nfsd: factor out common delegation-destruction code
nfsd: move sc_file assignment into alloc_init_deleg
nfsd: create a separate lease for each delegation
nfsd: clients don't need to break their own delegations
Documentation/filesystems/Locking | 2 +
Documentation/filesystems/vfs.txt | 2 +-
arch/ia64/kernel/perfmon.c | 2 +-
arch/powerpc/platforms/cell/spufs/file.c | 2 +-
arch/tile/kernel/hardwall.c | 2 +-
drivers/android/binder.c | 2 +-
drivers/char/ps3flash.c | 2 +-
drivers/char/xillybus/xillybus_core.c | 2 +-
drivers/firmware/efi/capsule-loader.c | 2 +-
drivers/input/evdev.c | 2 +-
drivers/misc/mic/scif/scif_fd.c | 2 +-
drivers/scsi/osst.c | 2 +-
drivers/scsi/st.c | 2 +-
drivers/staging/lustre/lustre/llite/file.c | 2 +-
drivers/usb/class/cdc-wdm.c | 2 +-
drivers/usb/usb-skeleton.c | 2 +-
fs/afs/internal.h | 2 +-
fs/afs/write.c | 2 +-
fs/cifs/cifsfs.h | 2 +-
fs/cifs/file.c | 4 +-
fs/ecryptfs/file.c | 2 +-
fs/exofs/file.c | 2 +-
fs/f2fs/file.c | 2 +-
fs/fuse/file.c | 13 +-
fs/fuse/fuse_i.h | 2 +-
fs/lockd/clntproc.c | 4 +-
fs/lockd/svc4proc.c | 2 +-
fs/lockd/svcproc.c | 2 +-
fs/locks.c | 6 +-
fs/nfs/file.c | 2 +-
fs/nfs/inode.c | 2 +-
fs/nfs/nfs4_fs.h | 2 +-
fs/nfs/nfs4file.c | 2 +-
fs/nfs/nfs4state.c | 8 +-
fs/nfsd/auth.c | 2 +
fs/nfsd/nfs4proc.c | 1 +
fs/nfsd/nfs4state.c | 267 ++++++++++++-----------------
fs/nfsd/nfssvc.c | 1 +
fs/notify/dnotify/dnotify.c | 6 +-
fs/open.c | 2 +-
include/linux/cred.h | 3 +
include/linux/dnotify.h | 6 +-
include/linux/fs.h | 18 +-
include/linux/lockd/lockd.h | 4 +-
include/linux/nfs_fs.h | 4 +-
include/linux/sunrpc/svc.h | 1 +
include/trace/events/filelock.h | 6 +-
ipc/mqueue.c | 2 +-
48 files changed, 193 insertions(+), 223 deletions(-)
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
The convention is to avoid this kind of typedef. It doesn't seem
useful, and it requires a lot of casts.
Reviewed-by: NeilBrown <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
Documentation/filesystems/vfs.txt | 2 +-
arch/ia64/kernel/perfmon.c | 2 +-
arch/powerpc/platforms/cell/spufs/file.c | 2 +-
arch/tile/kernel/hardwall.c | 2 +-
drivers/android/binder.c | 2 +-
drivers/char/ps3flash.c | 2 +-
drivers/char/xillybus/xillybus_core.c | 2 +-
drivers/firmware/efi/capsule-loader.c | 2 +-
drivers/input/evdev.c | 2 +-
drivers/misc/mic/scif/scif_fd.c | 2 +-
drivers/scsi/osst.c | 2 +-
drivers/scsi/st.c | 2 +-
drivers/staging/lustre/lustre/llite/file.c | 2 +-
drivers/usb/class/cdc-wdm.c | 2 +-
drivers/usb/usb-skeleton.c | 2 +-
fs/afs/internal.h | 2 +-
fs/afs/write.c | 2 +-
fs/cifs/cifsfs.h | 2 +-
fs/cifs/file.c | 4 ++--
fs/ecryptfs/file.c | 2 +-
fs/exofs/file.c | 2 +-
fs/f2fs/file.c | 2 +-
fs/fuse/file.c | 13 ++++++-------
fs/fuse/fuse_i.h | 2 +-
fs/lockd/clntproc.c | 4 ++--
fs/lockd/svc4proc.c | 2 +-
fs/lockd/svcproc.c | 2 +-
fs/locks.c | 2 +-
fs/nfs/file.c | 2 +-
fs/nfs/inode.c | 2 +-
fs/nfs/nfs4_fs.h | 2 +-
fs/nfs/nfs4file.c | 2 +-
fs/nfs/nfs4state.c | 8 ++++----
fs/nfsd/nfs4state.c | 18 +++++++++---------
fs/notify/dnotify/dnotify.c | 6 +++---
fs/open.c | 2 +-
include/linux/dnotify.h | 6 +++---
include/linux/fs.h | 17 +++++++----------
include/linux/lockd/lockd.h | 4 ++--
include/linux/nfs_fs.h | 4 ++--
include/trace/events/filelock.h | 6 +++---
ipc/mqueue.c | 2 +-
42 files changed, 74 insertions(+), 78 deletions(-)
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 5fd325df59e2..7e22079c7396 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -862,7 +862,7 @@ struct file_operations {
int (*mmap) (struct file *, struct vm_area_struct *);
int (*mremap)(struct file *, struct vm_area_struct *);
int (*open) (struct inode *, struct file *);
- int (*flush) (struct file *, fl_owner_t id);
+ int (*flush) (struct file *, void *id);
int (*release) (struct inode *, struct file *);
int (*fsync) (struct file *, loff_t, loff_t, int datasync);
int (*fasync) (int, struct file *, int);
diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index 8fb280e33114..c78e62a76d20 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -1809,7 +1809,7 @@ pfm_syswide_cleanup_other_cpu(pfm_context_t *ctx)
* When caller is self-monitoring, the context is unloaded.
*/
static int
-pfm_flush(struct file *filp, fl_owner_t id)
+pfm_flush(struct file *filp, void *id)
{
pfm_context_t *ctx;
struct task_struct *task;
diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index 469bdd0b748f..e5a204f8169b 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -1720,7 +1720,7 @@ static __poll_t spufs_mfc_poll(struct file *file,poll_table *wait)
return mask;
}
-static int spufs_mfc_flush(struct file *file, fl_owner_t id)
+static int spufs_mfc_flush(struct file *file, void *id)
{
struct spu_context *ctx = file->private_data;
int ret;
diff --git a/arch/tile/kernel/hardwall.c b/arch/tile/kernel/hardwall.c
index 2fd1694ac1d0..b2cf21d1edb0 100644
--- a/arch/tile/kernel/hardwall.c
+++ b/arch/tile/kernel/hardwall.c
@@ -1030,7 +1030,7 @@ static long hardwall_compat_ioctl(struct file *file,
#endif
/* The user process closed the file; revoke access to user networks. */
-static int hardwall_flush(struct file *file, fl_owner_t owner)
+static int hardwall_flush(struct file *file, void *owner)
{
struct hardwall_info *info = file->private_data;
struct task_struct *task, *tmp;
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 15e3d3c2260d..f7706142974c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4776,7 +4776,7 @@ static int binder_open(struct inode *nodp, struct file *filp)
return 0;
}
-static int binder_flush(struct file *filp, fl_owner_t id)
+static int binder_flush(struct file *filp, void *id)
{
struct binder_proc *proc = filp->private_data;
diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
index b526dc15c271..5a03dd0eb2f1 100644
--- a/drivers/char/ps3flash.c
+++ b/drivers/char/ps3flash.c
@@ -281,7 +281,7 @@ static ssize_t ps3flash_kernel_write(const void *buf, size_t count,
return res;
}
-static int ps3flash_flush(struct file *file, fl_owner_t id)
+static int ps3flash_flush(struct file *file, void *id)
{
return ps3flash_writeback(ps3flash_dev);
}
diff --git a/drivers/char/xillybus/xillybus_core.c b/drivers/char/xillybus/xillybus_core.c
index a11af94e2e65..b76074c976b9 100644
--- a/drivers/char/xillybus/xillybus_core.c
+++ b/drivers/char/xillybus/xillybus_core.c
@@ -1156,7 +1156,7 @@ static int xillybus_myflush(struct xilly_channel *channel, long timeout)
return rc;
}
-static int xillybus_flush(struct file *filp, fl_owner_t id)
+static int xillybus_flush(struct file *filp, void *id)
{
if (!(filp->f_mode & FMODE_WRITE))
return 0;
diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index e456f4602df1..ba444f726465 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -246,7 +246,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
* will be treated as upload termination and will free those completed
* buffer pages and -ECANCELED will be returned.
**/
-static int efi_capsule_flush(struct file *file, fl_owner_t id)
+static int efi_capsule_flush(struct file *file, void *id)
{
int ret = 0;
struct capsule_info *cap_info = file->private_data;
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index c81c79d01d93..fd9e02e133b1 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -348,7 +348,7 @@ static int evdev_fasync(int fd, struct file *file, int on)
return fasync_helper(fd, file, on, &client->fasync);
}
-static int evdev_flush(struct file *file, fl_owner_t id)
+static int evdev_flush(struct file *file, void *id)
{
struct evdev_client *client = file->private_data;
struct evdev *evdev = client->evdev;
diff --git a/drivers/misc/mic/scif/scif_fd.c b/drivers/misc/mic/scif/scif_fd.c
index 5c2a57ae4f85..e8baa7b93c05 100644
--- a/drivers/misc/mic/scif/scif_fd.c
+++ b/drivers/misc/mic/scif/scif_fd.c
@@ -48,7 +48,7 @@ static __poll_t scif_fdpoll(struct file *f, poll_table *wait)
return __scif_pollfd(f, wait, priv);
}
-static int scif_fdflush(struct file *f, fl_owner_t id)
+static int scif_fdflush(struct file *f, void *id)
{
struct scif_endpt *ep = f->private_data;
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 20ec1c01dbd5..fdb213261249 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -4822,7 +4822,7 @@ static int os_scsi_tape_open(struct inode * inode, struct file * filp)
/* Flush the tape buffer before close */
-static int os_scsi_tape_flush(struct file * filp, fl_owner_t id)
+static int os_scsi_tape_flush(struct file * filp, void *id)
{
int result = 0, result2;
struct osst_tape * STp = filp->private_data;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 6c399480783d..ed01299f6f51 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -1338,7 +1338,7 @@ static int st_open(struct inode *inode, struct file *filp)
/* Flush the tape buffer before close */
-static int st_flush(struct file *filp, fl_owner_t id)
+static int st_flush(struct file *filp, void *id)
{
int result = 0, result2;
unsigned char cmd[MAX_COMMAND_SIZE];
diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 938b859b6650..a8fe2072ea7c 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2281,7 +2281,7 @@ static loff_t ll_file_seek(struct file *file, loff_t offset, int origin)
ll_file_maxbytes(inode), eof);
}
-static int ll_flush(struct file *file, fl_owner_t id)
+static int ll_flush(struct file *file, void *id)
{
struct inode *inode = file_inode(file);
struct ll_inode_info *lli = ll_i2info(inode);
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index a0d284ef3f40..bda5b9812ccb 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -581,7 +581,7 @@ static ssize_t wdm_read
return rv;
}
-static int wdm_flush(struct file *file, fl_owner_t id)
+static int wdm_flush(struct file *file, void *id)
{
struct wdm_device *desc = file->private_data;
diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c
index 26ca0ec01fd5..d20a23d77fbe 100644
--- a/drivers/usb/usb-skeleton.c
+++ b/drivers/usb/usb-skeleton.c
@@ -132,7 +132,7 @@ static int skel_release(struct inode *inode, struct file *file)
return 0;
}
-static int skel_flush(struct file *file, fl_owner_t id)
+static int skel_flush(struct file *file, void *id)
{
struct usb_skel *dev;
int res;
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index f38d6a561a84..18b1e2bc2a14 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -947,7 +947,7 @@ extern int afs_writepage(struct page *, struct writeback_control *);
extern int afs_writepages(struct address_space *, struct writeback_control *);
extern void afs_pages_written_back(struct afs_vnode *, struct afs_call *);
extern ssize_t afs_file_write(struct kiocb *, struct iov_iter *);
-extern int afs_flush(struct file *, fl_owner_t);
+extern int afs_flush(struct file *, void *);
extern int afs_fsync(struct file *, loff_t, loff_t, int);
extern int afs_page_mkwrite(struct vm_fault *);
extern void afs_prune_wb_keys(struct afs_vnode *);
diff --git a/fs/afs/write.c b/fs/afs/write.c
index 9370e2feb999..b4ecbe4e46a4 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -737,7 +737,7 @@ int afs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
* Flush out all outstanding writes on a file opened for writing when it is
* closed.
*/
-int afs_flush(struct file *file, fl_owner_t id)
+int afs_flush(struct file *file, void *id)
{
_enter("");
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 013ba2aed8d9..09de0b6bd6a7 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -108,7 +108,7 @@ extern ssize_t cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from);
extern int cifs_lock(struct file *, int, struct file_lock *);
extern int cifs_fsync(struct file *, loff_t, loff_t, int);
extern int cifs_strict_fsync(struct file *, loff_t, loff_t, int);
-extern int cifs_flush(struct file *, fl_owner_t id);
+extern int cifs_flush(struct file *, void *id);
extern int cifs_file_mmap(struct file * , struct vm_area_struct *);
extern int cifs_file_strict_mmap(struct file * , struct vm_area_struct *);
extern const struct file_operations cifs_dir_ops;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 7cee97b93a61..835c155e64ed 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1174,7 +1174,7 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile)
}
static __u32
-hash_lockowner(fl_owner_t owner)
+hash_lockowner(void *owner)
{
return cifs_lock_secret ^ hash32_ptr((const void *)owner);
}
@@ -2393,7 +2393,7 @@ int cifs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
* As file closes, flush all cached write data for this inode checking
* for write behind errors.
*/
-int cifs_flush(struct file *file, fl_owner_t id)
+int cifs_flush(struct file *file, void *id)
{
struct inode *inode = file_inode(file);
int rc = 0;
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index c74ed3ca3372..78fb58667791 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -290,7 +290,7 @@ static int ecryptfs_dir_open(struct inode *inode, struct file *file)
return 0;
}
-static int ecryptfs_flush(struct file *file, fl_owner_t td)
+static int ecryptfs_flush(struct file *file, void *td)
{
struct file *lower_file = ecryptfs_file_to_lower(file);
diff --git a/fs/exofs/file.c b/fs/exofs/file.c
index a94594ea2aa3..86d27d835a11 100644
--- a/fs/exofs/file.c
+++ b/fs/exofs/file.c
@@ -58,7 +58,7 @@ static int exofs_file_fsync(struct file *filp, loff_t start, loff_t end,
return ret;
}
-static int exofs_flush(struct file *file, fl_owner_t id)
+static int exofs_flush(struct file *file, void *id)
{
int ret = vfs_fsync(file, 0);
/* TODO: Flush the OSD target */
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 672a542e5464..57076d46fd08 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1557,7 +1557,7 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
return 0;
}
-static int f2fs_file_flush(struct file *file, fl_owner_t id)
+static int f2fs_file_flush(struct file *file, void *id)
{
struct inode *inode = file_inode(file);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a201fb0ac64f..746624182249 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -254,8 +254,7 @@ void fuse_release_common(struct file *file, int opcode)
if (ff->flock) {
struct fuse_release_in *inarg = &req->misc.release.in;
inarg->release_flags |= FUSE_RELEASE_FLOCK_UNLOCK;
- inarg->lock_owner = fuse_lock_owner_id(ff->fc,
- (fl_owner_t) file);
+ inarg->lock_owner = fuse_lock_owner_id(ff->fc, file);
}
/* Hold inode until release is finished */
req->misc.release.inode = igrab(file_inode(file));
@@ -307,7 +306,7 @@ EXPORT_SYMBOL_GPL(fuse_sync_release);
* Scramble the ID space with XTEA, so that the value of the files_struct
* pointer is not exposed to userspace.
*/
-u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id)
+u64 fuse_lock_owner_id(struct fuse_conn *fc, void *id)
{
u32 *k = fc->scramble_key;
u64 v = (unsigned long) id;
@@ -390,7 +389,7 @@ static void fuse_sync_writes(struct inode *inode)
fuse_release_nowrite(inode);
}
-static int fuse_flush(struct file *file, fl_owner_t id)
+static int fuse_flush(struct file *file, void *id)
{
struct inode *inode = file_inode(file);
struct fuse_conn *fc = get_fuse_conn(inode);
@@ -643,7 +642,7 @@ static size_t fuse_async_req_send(struct fuse_conn *fc, struct fuse_req *req,
}
static size_t fuse_send_read(struct fuse_req *req, struct fuse_io_priv *io,
- loff_t pos, size_t count, fl_owner_t owner)
+ loff_t pos, size_t count, void *owner)
{
struct file *file = io->iocb->ki_filp;
struct fuse_file *ff = file->private_data;
@@ -958,7 +957,7 @@ static void fuse_write_fill(struct fuse_req *req, struct fuse_file *ff,
}
static size_t fuse_send_write(struct fuse_req *req, struct fuse_io_priv *io,
- loff_t pos, size_t count, fl_owner_t owner)
+ loff_t pos, size_t count, void *owner)
{
struct kiocb *iocb = io->iocb;
struct file *file = iocb->ki_filp;
@@ -1358,7 +1357,7 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter,
io->should_dirty = !write && iter_is_iovec(iter);
while (count) {
size_t nres;
- fl_owner_t owner = current->files;
+ void *owner = current->files;
size_t nbytes = min(count, nmax);
err = fuse_get_user_pages(req, iter, &nbytes, write);
if (err && !nbytes)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index c4c093bbf456..e4c2da57c16c 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -900,7 +900,7 @@ int fuse_valid_type(int m);
*/
int fuse_allow_current_process(struct fuse_conn *fc);
-u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id);
+u64 fuse_lock_owner_id(struct fuse_conn *fc, void *id);
void fuse_update_ctime(struct inode *inode);
diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
index a2c0dfc6fdc0..a42dff055260 100644
--- a/fs/lockd/clntproc.c
+++ b/fs/lockd/clntproc.c
@@ -81,7 +81,7 @@ static inline uint32_t __nlm_alloc_pid(struct nlm_host *host)
return res;
}
-static struct nlm_lockowner *__nlm_find_lockowner(struct nlm_host *host, fl_owner_t owner)
+static struct nlm_lockowner *__nlm_find_lockowner(struct nlm_host *host, void *owner)
{
struct nlm_lockowner *lockowner;
list_for_each_entry(lockowner, &host->h_lockowners, list) {
@@ -92,7 +92,7 @@ static struct nlm_lockowner *__nlm_find_lockowner(struct nlm_host *host, fl_owne
return NULL;
}
-static struct nlm_lockowner *nlm_find_lockowner(struct nlm_host *host, fl_owner_t owner)
+static struct nlm_lockowner *nlm_find_lockowner(struct nlm_host *host, void *owner)
{
struct nlm_lockowner *res, *new = NULL;
diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 1bddf70d9656..004777845b61 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -46,7 +46,7 @@ nlm4svc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
/* Set up the missing parts of the file_lock structure */
lock->fl.fl_file = file->f_file;
- lock->fl.fl_owner = (fl_owner_t) host;
+ lock->fl.fl_owner = host;
lock->fl.fl_lmops = &nlmsvc_lock_operations;
}
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index ea77c66d3cc3..bbf2329821db 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -76,7 +76,7 @@ nlmsvc_retrieve_args(struct svc_rqst *rqstp, struct nlm_args *argp,
/* Set up the missing parts of the file_lock structure */
lock->fl.fl_file = file->f_file;
- lock->fl.fl_owner = (fl_owner_t) host;
+ lock->fl.fl_owner = host;
lock->fl.fl_lmops = &nlmsvc_lock_operations;
}
diff --git a/fs/locks.c b/fs/locks.c
index d6ff4beb70ce..63aa52bcdf5a 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2462,7 +2462,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd,
* from the task's fd array. POSIX locks belonging to this task
* are deleted at this time.
*/
-void locks_remove_posix(struct file *filp, fl_owner_t owner)
+void locks_remove_posix(struct file *filp, void *owner)
{
int error;
struct inode *inode = locks_inode(filp);
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 81cca49a8375..922ea233c391 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -136,7 +136,7 @@ EXPORT_SYMBOL_GPL(nfs_file_llseek);
* Flush all dirty pages, and check for write errors.
*/
static int
-nfs_file_flush(struct file *file, fl_owner_t id)
+nfs_file_flush(struct file *file, void *id)
{
struct inode *inode = file_inode(file);
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 7d893543cf3b..0b2d6f839bc2 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -927,7 +927,7 @@ struct nfs_open_context *alloc_nfs_open_context(struct dentry *dentry,
ctx->mode = f_mode;
ctx->flags = 0;
ctx->error = 0;
- ctx->flock_owner = (fl_owner_t)filp;
+ ctx->flock_owner = filp;
nfs_init_lock_context(&ctx->lock_context);
ctx->lock_context.open_context = ctx;
INIT_LIST_HEAD(&ctx->list);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b374f680830c..5079334cdf37 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -146,7 +146,7 @@ struct nfs4_lock_state {
struct nfs_seqid_counter ls_seqid;
nfs4_stateid ls_stateid;
refcount_t ls_count;
- fl_owner_t ls_owner;
+ void * ls_owner;
};
/* bits for nfs4_state->flags */
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index 6b3b372b59b9..7045ff1fb46d 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -107,7 +107,7 @@ nfs4_file_open(struct inode *inode, struct file *filp)
* Flush all dirty pages, and check for write errors.
*/
static int
-nfs4_file_flush(struct file *file, fl_owner_t id)
+nfs4_file_flush(struct file *file, void *id)
{
struct inode *inode = file_inode(file);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 91a4d4eeb235..7938ef7b8b27 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -822,7 +822,7 @@ void nfs4_close_sync(struct nfs4_state *state, fmode_t fmode)
*/
static struct nfs4_lock_state *
__nfs4_find_lock_state(struct nfs4_state *state,
- fl_owner_t fl_owner, fl_owner_t fl_owner2)
+ void *fl_owner, void *fl_owner2)
{
struct nfs4_lock_state *pos, *ret = NULL;
list_for_each_entry(pos, &state->lock_states, ls_locks) {
@@ -843,7 +843,7 @@ __nfs4_find_lock_state(struct nfs4_state *state,
* exists, return an uninitialized one.
*
*/
-static struct nfs4_lock_state *nfs4_alloc_lock_state(struct nfs4_state *state, fl_owner_t fl_owner)
+static struct nfs4_lock_state *nfs4_alloc_lock_state(struct nfs4_state *state, void *fl_owner)
{
struct nfs4_lock_state *lsp;
struct nfs_server *server = state->owner->so_server;
@@ -877,7 +877,7 @@ void nfs4_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp
* exists, return an uninitialized one.
*
*/
-static struct nfs4_lock_state *nfs4_get_lock_state(struct nfs4_state *state, fl_owner_t owner)
+static struct nfs4_lock_state *nfs4_get_lock_state(struct nfs4_state *state, void *owner)
{
struct nfs4_lock_state *lsp, *new = NULL;
@@ -968,7 +968,7 @@ static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
const struct nfs_lock_context *l_ctx)
{
struct nfs4_lock_state *lsp;
- fl_owner_t fl_owner, fl_flock_owner;
+ void *fl_owner, *fl_flock_owner;
int ret = -ENOENT;
if (l_ctx == NULL)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 150521c9671b..1bdfefe2e6ec 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1204,7 +1204,7 @@ static void nfs4_free_lock_stateid(struct nfs4_stid *stid)
file = find_any_file(stp->st_stid.sc_file);
if (file)
- filp_close(file, (fl_owner_t)lo);
+ filp_close(file, lo);
nfs4_free_ol_stateid(stid);
}
@@ -4268,7 +4268,7 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
fl->fl_flags = FL_DELEG;
fl->fl_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
fl->fl_end = OFFSET_MAX;
- fl->fl_owner = (fl_owner_t)fp;
+ fl->fl_owner = fp;
fl->fl_pid = current->tgid;
return fl;
}
@@ -5563,8 +5563,8 @@ nfs4_transform_lock_offset(struct file_lock *lock)
lock->fl_end = OFFSET_MAX;
}
-static fl_owner_t
-nfsd4_fl_get_owner(fl_owner_t owner)
+static void *
+nfsd4_fl_get_owner(void *owner)
{
struct nfs4_lockowner *lo = (struct nfs4_lockowner *)owner;
@@ -5573,7 +5573,7 @@ nfsd4_fl_get_owner(fl_owner_t owner)
}
static void
-nfsd4_fl_put_owner(fl_owner_t owner)
+nfsd4_fl_put_owner(void *owner)
{
struct nfs4_lockowner *lo = (struct nfs4_lockowner *)owner;
@@ -6008,7 +6008,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
file_lock = &nbl->nbl_lock;
file_lock->fl_type = fl_type;
- file_lock->fl_owner = (fl_owner_t)lockowner(nfs4_get_stateowner(&lock_sop->lo_owner));
+ file_lock->fl_owner = lockowner(nfs4_get_stateowner(&lock_sop->lo_owner));
file_lock->fl_pid = current->tgid;
file_lock->fl_file = filp;
file_lock->fl_flags = fl_flags;
@@ -6162,7 +6162,7 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
lo = find_lockowner_str(cstate->clp, &lockt->lt_owner);
if (lo)
- file_lock->fl_owner = (fl_owner_t)lo;
+ file_lock->fl_owner = lo;
file_lock->fl_pid = current->tgid;
file_lock->fl_flags = FL_POSIX;
@@ -6224,7 +6224,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
}
file_lock->fl_type = F_UNLCK;
- file_lock->fl_owner = (fl_owner_t)lockowner(nfs4_get_stateowner(stp->st_stateowner));
+ file_lock->fl_owner = lockowner(nfs4_get_stateowner(stp->st_stateowner));
file_lock->fl_pid = current->tgid;
file_lock->fl_file = filp;
file_lock->fl_flags = FL_POSIX;
@@ -6283,7 +6283,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
if (flctx && !list_empty_careful(&flctx->flc_posix)) {
spin_lock(&flctx->flc_lock);
list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
- if (fl->fl_owner == (fl_owner_t)lowner) {
+ if (fl->fl_owner == lowner) {
status = true;
break;
}
diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 63a1ca4b9dee..6b309bd1552d 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -145,7 +145,7 @@ static const struct fsnotify_ops dnotify_fsnotify_ops = {
* dnotify_struct. If that was the last dnotify_struct also remove the
* fsnotify_mark.
*/
-void dnotify_flush(struct file *filp, fl_owner_t id)
+void dnotify_flush(struct file *filp, void *id)
{
struct fsnotify_mark *fsn_mark;
struct dnotify_mark *dn_mark;
@@ -223,7 +223,7 @@ static __u32 convert_arg(unsigned long arg)
* that list, or it |= the mask onto an existing dnofiy_struct.
*/
static int attach_dn(struct dnotify_struct *dn, struct dnotify_mark *dn_mark,
- fl_owner_t id, int fd, struct file *filp, __u32 mask)
+ void *id, int fd, struct file *filp, __u32 mask)
{
struct dnotify_struct *odn;
@@ -259,7 +259,7 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
struct fsnotify_mark *new_fsn_mark, *fsn_mark;
struct dnotify_struct *dn;
struct inode *inode;
- fl_owner_t id = current->files;
+ void *id = current->files;
struct file *f;
int destroy = 0, error = 0;
__u32 mask;
diff --git a/fs/open.c b/fs/open.c
index 7ea118471dce..b04595bca741 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1123,7 +1123,7 @@ SYSCALL_DEFINE2(creat, const char __user *, pathname, umode_t, mode)
* "id" is the POSIX thread ID. We use the
* files pointer for this..
*/
-int filp_close(struct file *filp, fl_owner_t id)
+int filp_close(struct file *filp, void *id)
{
int retval = 0;
diff --git a/include/linux/dnotify.h b/include/linux/dnotify.h
index 0aad774beaec..4be77f0e8c58 100644
--- a/include/linux/dnotify.h
+++ b/include/linux/dnotify.h
@@ -14,7 +14,7 @@ struct dnotify_struct {
__u32 dn_mask;
int dn_fd;
struct file * dn_filp;
- fl_owner_t dn_owner;
+ void * dn_owner;
};
#ifdef __KERNEL__
@@ -30,12 +30,12 @@ struct dnotify_struct {
FS_MOVED_FROM | FS_MOVED_TO)
extern int dir_notify_enable;
-extern void dnotify_flush(struct file *, fl_owner_t);
+extern void dnotify_flush(struct file *, void *);
extern int fcntl_dirnotify(int, struct file *, unsigned long);
#else
-static inline void dnotify_flush(struct file *filp, fl_owner_t id)
+static inline void dnotify_flush(struct file *filp, void *id)
{
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2a815560fda0..ef9269bf7e69 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -942,9 +942,6 @@ static inline struct file *get_file(struct file *f)
*/
#define FILE_LOCK_DEFERRED 1
-/* legacy typedef, should eventually be removed */
-typedef void *fl_owner_t;
-
struct file_lock;
struct file_lock_operations {
@@ -955,8 +952,8 @@ struct file_lock_operations {
struct lock_manager_operations {
int (*lm_compare_owner)(struct file_lock *, struct file_lock *);
unsigned long (*lm_owner_key)(struct file_lock *);
- fl_owner_t (*lm_get_owner)(fl_owner_t);
- void (*lm_put_owner)(fl_owner_t);
+ void *(*lm_get_owner)(void *);
+ void (*lm_put_owner)(void *);
void (*lm_notify)(struct file_lock *); /* unblock callback */
int (*lm_grant)(struct file_lock *, int);
bool (*lm_break)(struct file_lock *);
@@ -1004,7 +1001,7 @@ struct file_lock {
struct list_head fl_list; /* link into file_lock_context */
struct hlist_node fl_link; /* node in global lists */
struct list_head fl_block; /* circular list of blocked processes */
- fl_owner_t fl_owner;
+ void *fl_owner;
unsigned int fl_flags;
unsigned char fl_type;
unsigned int fl_pid;
@@ -1080,7 +1077,7 @@ extern void locks_init_lock(struct file_lock *);
extern struct file_lock * locks_alloc_lock(void);
extern void locks_copy_lock(struct file_lock *, struct file_lock *);
extern void locks_copy_conflock(struct file_lock *, struct file_lock *);
-extern void locks_remove_posix(struct file *, fl_owner_t);
+extern void locks_remove_posix(struct file *, void *);
extern void locks_remove_file(struct file *);
extern void locks_release_private(struct file_lock *);
extern void posix_test_lock(struct file *, struct file_lock *);
@@ -1154,7 +1151,7 @@ static inline void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
return;
}
-static inline void locks_remove_posix(struct file *filp, fl_owner_t owner)
+static inline void locks_remove_posix(struct file *filp, void *owner)
{
return;
}
@@ -1713,7 +1710,7 @@ struct file_operations {
int (*mmap) (struct file *, struct vm_area_struct *);
unsigned long mmap_supported_flags;
int (*open) (struct inode *, struct file *);
- int (*flush) (struct file *, fl_owner_t id);
+ int (*flush) (struct file *, void *id);
int (*release) (struct inode *, struct file *);
int (*fsync) (struct file *, loff_t, loff_t, int datasync);
int (*fasync) (int, struct file *, int);
@@ -2397,7 +2394,7 @@ extern struct file *filp_open(const char *, int, umode_t);
extern struct file *file_open_root(struct dentry *, struct vfsmount *,
const char *, int, umode_t);
extern struct file * dentry_open(const struct path *, int, const struct cred *);
-extern int filp_close(struct file *, fl_owner_t id);
+extern int filp_close(struct file *, void *id);
extern struct filename *getname_flags(const char __user *, int, int *);
extern struct filename *getname(const char __user *);
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 4fd95dbeb52f..484156a8f11a 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -119,14 +119,14 @@ static inline struct sockaddr *nlm_srcaddr(const struct nlm_host *host)
}
/*
- * Map an fl_owner_t into a unique 32-bit "pid"
+ * Map a lock owner into a unique 32-bit "pid"
*/
struct nlm_lockowner {
struct list_head list;
refcount_t count;
struct nlm_host *host;
- fl_owner_t owner;
+ void *owner;
uint32_t pid;
};
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 38187c68063d..889d3494dbbb 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -60,14 +60,14 @@ struct nfs_lock_context {
refcount_t count;
struct list_head list;
struct nfs_open_context *open_context;
- fl_owner_t lockowner;
+ void *lockowner;
atomic_t io_count;
};
struct nfs4_state;
struct nfs_open_context {
struct nfs_lock_context lock_context;
- fl_owner_t flock_owner;
+ void *flock_owner;
struct dentry *dentry;
struct rpc_cred *cred;
struct nfs4_state *state;
diff --git a/include/trace/events/filelock.h b/include/trace/events/filelock.h
index d1faf3597b9d..345fdb312cab 100644
--- a/include/trace/events/filelock.h
+++ b/include/trace/events/filelock.h
@@ -69,7 +69,7 @@ DECLARE_EVENT_CLASS(filelock_lock,
__field(unsigned long, i_ino)
__field(dev_t, s_dev)
__field(struct file_lock *, fl_next)
- __field(fl_owner_t, fl_owner)
+ __field(void *, fl_owner)
__field(unsigned int, fl_pid)
__field(unsigned int, fl_flags)
__field(unsigned char, fl_type)
@@ -123,7 +123,7 @@ DECLARE_EVENT_CLASS(filelock_lease,
__field(unsigned long, i_ino)
__field(dev_t, s_dev)
__field(struct file_lock *, fl_next)
- __field(fl_owner_t, fl_owner)
+ __field(void *, fl_owner)
__field(unsigned int, fl_flags)
__field(unsigned char, fl_type)
__field(unsigned long, fl_break_time)
@@ -176,7 +176,7 @@ TRACE_EVENT(generic_add_lease,
__field(int, dcount)
__field(int, icount)
__field(dev_t, s_dev)
- __field(fl_owner_t, fl_owner)
+ __field(void *, fl_owner)
__field(unsigned int, fl_flags)
__field(unsigned char, fl_type)
),
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index d7f309f74dec..197cb72af8de 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -557,7 +557,7 @@ static ssize_t mqueue_read_file(struct file *filp, char __user *u_data,
return ret;
}
-static int mqueue_flush_file(struct file *filp, fl_owner_t id)
+static int mqueue_flush_file(struct file *filp, void *id)
{
struct mqueue_inode_info *info = MQUEUE_I(file_inode(filp));
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
For now this makes no difference, as for files having delegations,
there's a one-to-one relationship between an nfs4_file and its
nfs4_delegation.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 51f633c85776..0466312a2a4e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -859,7 +859,7 @@ static void nfs4_put_deleg_lease(struct nfs4_delegation *dp)
spin_unlock(&fp->fi_lock);
if (filp) {
- vfs_setlease(filp, F_UNLCK, NULL, (void **)&fp);
+ vfs_setlease(filp, F_UNLCK, NULL, (void **)&dp);
fput(filp);
}
}
@@ -3909,13 +3909,9 @@ static bool
nfsd_break_deleg_cb(struct file_lock *fl)
{
bool ret = false;
- struct nfs4_file *fp = (struct nfs4_file *)fl->fl_owner;
- struct nfs4_delegation *dp;
+ struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
+ struct nfs4_file *fp = dp->dl_stid.sc_file;
- if (!fp) {
- WARN(1, "(%p)->fl_owner NULL\n", fl);
- return ret;
- }
if (fp->fi_had_conflict) {
WARN(1, "duplicate break on %p\n", fp);
return ret;
@@ -4261,7 +4257,8 @@ static bool nfsd4_cb_channel_good(struct nfs4_client *clp)
return clp->cl_minorversion && clp->cl_cb_state == NFSD4_CB_UNKNOWN;
}
-static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
+static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
+ int flag)
{
struct file_lock *fl;
@@ -4272,7 +4269,7 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_file *fp, int flag)
fl->fl_flags = FL_DELEG;
fl->fl_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
fl->fl_end = OFFSET_MAX;
- fl->fl_owner = fp;
+ fl->fl_owner = dp;
fl->fl_pid = current->tgid;
return fl;
}
@@ -4296,7 +4293,7 @@ static int nfs4_setlease(struct nfs4_delegation *dp)
struct file *filp;
int status = 0;
- fl = nfs4_alloc_init_lease(fp, NFS4_OPEN_DELEGATE_READ);
+ fl = nfs4_alloc_init_lease(dp, NFS4_OPEN_DELEGATE_READ);
if (!fl)
return -ENOMEM;
filp = find_readable_file(fp);
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
Every single caller gets the file out of the delegation, so let's do
that once in nfs4_put_deleg_lease.
Plus we'll need it there for other reasons.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1a0b38fddde4..51f633c85776 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -845,9 +845,10 @@ nfs4_inc_and_copy_stateid(stateid_t *dst, struct nfs4_stid *stid)
spin_unlock(&stid->sc_lock);
}
-static void nfs4_put_deleg_lease(struct nfs4_file *fp)
+static void nfs4_put_deleg_lease(struct nfs4_delegation *dp)
{
struct file *filp = NULL;
+ struct nfs4_file *fp = dp->dl_stid.sc_file;
WARN_ON_ONCE(!fp->fi_delegees);
@@ -962,7 +963,7 @@ static void destroy_delegation(struct nfs4_delegation *dp)
spin_unlock(&state_lock);
if (unhashed) {
put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp->dl_stid.sc_file);
+ nfs4_put_deleg_lease(dp);
nfs4_put_stid(&dp->dl_stid);
}
}
@@ -974,7 +975,7 @@ static void revoke_delegation(struct nfs4_delegation *dp)
WARN_ON(!list_empty(&dp->dl_recall_lru));
put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp->dl_stid.sc_file);
+ nfs4_put_deleg_lease(dp);
if (clp->cl_minorversion == 0)
nfs4_put_stid(&dp->dl_stid);
@@ -1885,7 +1886,7 @@ __destroy_client(struct nfs4_client *clp)
dp = list_entry(reaplist.next, struct nfs4_delegation, dl_recall_lru);
list_del_init(&dp->dl_recall_lru);
put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp->dl_stid.sc_file);
+ nfs4_put_deleg_lease(dp);
nfs4_put_stid(&dp->dl_stid);
}
while (!list_empty(&clp->cl_revoked)) {
@@ -7226,7 +7227,7 @@ nfs4_state_shutdown_net(struct net *net)
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
list_del_init(&dp->dl_recall_lru);
put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp->dl_stid.sc_file);
+ nfs4_put_deleg_lease(dp);
nfs4_put_stid(&dp->dl_stid);
}
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
fi_delegees is basically just a reference count on users of
fi_deleg_file, which is cleared when fi_delegees goes to zero. The
fi_deleg_file check here is redundant. Also add an assertion to make
sure we don't have unbalanced puts.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1bdfefe2e6ec..1a0b38fddde4 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -849,9 +849,12 @@ static void nfs4_put_deleg_lease(struct nfs4_file *fp)
{
struct file *filp = NULL;
+ WARN_ON_ONCE(!fp->fi_delegees);
+
spin_lock(&fp->fi_lock);
- if (fp->fi_deleg_file && --fp->fi_delegees == 0)
+ if (--fp->fi_delegees == 0) {
swap(filp, fp->fi_deleg_file);
+ }
spin_unlock(&fp->fi_lock);
if (filp) {
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
Currently we only take one vfs-level delegation (lease) for each file,
no matter how many clients hold delegations on that file.
Let's instead keep a one-to-one mapping between NFSv4 delegations and
VFS delegations. This turns out to be simpler.
There is still a many-to-one mapping of NFS opens to NFS files, and the
delegations on one file are all associated with one struct file. The
VFS can still distinguish between these delegations since we're setting
fl_owner to the struct nfs4_delegation now, not to the shared file.
I'm replacing at least one complicated function wholesale, which I don't
like to do, but I haven't figured out how to do this more incrementally.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 173 ++++++++++++++++++++--------------------------------
1 file changed, 65 insertions(+), 108 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 26880d0122c5..ca8a5644d9ff 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -848,29 +848,34 @@ nfs4_inc_and_copy_stateid(stateid_t *dst, struct nfs4_stid *stid)
spin_unlock(&stid->sc_lock);
}
-static void nfs4_put_deleg_lease(struct nfs4_delegation *dp)
+static void put_deleg_file(struct nfs4_file *fp)
{
struct file *filp = NULL;
- struct nfs4_file *fp = dp->dl_stid.sc_file;
-
- WARN_ON_ONCE(!fp->fi_delegees);
spin_lock(&fp->fi_lock);
- if (--fp->fi_delegees == 0) {
+ if (--fp->fi_delegees == 0)
swap(filp, fp->fi_deleg_file);
- }
spin_unlock(&fp->fi_lock);
- if (filp) {
- vfs_setlease(filp, F_UNLCK, NULL, (void **)&dp);
+ if (filp)
fput(filp);
- }
+}
+
+static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp)
+{
+ struct nfs4_file *fp = dp->dl_stid.sc_file;
+ struct file *filp = fp->fi_deleg_file;
+
+ WARN_ON_ONCE(!fp->fi_delegees);
+
+ vfs_setlease(filp, F_UNLCK, NULL, (void **)&dp);
+ put_deleg_file(fp);
}
static void destroy_unhashed_deleg(struct nfs4_delegation *dp)
{
put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp);
+ nfs4_unlock_deleg_lease(dp);
nfs4_put_stid(&dp->dl_stid);
}
@@ -929,7 +934,6 @@ hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
if (nfs4_delegation_exists(clp, fp))
return -EAGAIN;
- ++fp->fi_delegees;
refcount_inc(&dp->dl_stid.sc_count);
dp->dl_stid.sc_type = NFS4_DELEG_STID;
list_add(&dp->dl_perfile, &fp->fi_delegations);
@@ -3908,10 +3912,6 @@ nfsd_break_deleg_cb(struct file_lock *fl)
struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
struct nfs4_file *fp = dp->dl_stid.sc_file;
- if (fp->fi_had_conflict) {
- WARN(1, "duplicate break on %p\n", fp);
- return ret;
- }
/*
* We don't want the locks code to timeout the lease for us;
* we'll remove it ourself if a delegation isn't returned
@@ -3921,15 +3921,7 @@ nfsd_break_deleg_cb(struct file_lock *fl)
spin_lock(&fp->fi_lock);
fp->fi_had_conflict = true;
- /*
- * If there are no delegations on the list, then return true
- * so that the lease code will go ahead and delete it.
- */
- if (list_empty(&fp->fi_delegations))
- ret = true;
- else
- list_for_each_entry(dp, &fp->fi_delegations, dl_perfile)
- nfsd_break_one_deleg(dp);
+ nfsd_break_one_deleg(dp);
spin_unlock(&fp->fi_lock);
return ret;
}
@@ -4267,121 +4259,86 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
fl->fl_end = OFFSET_MAX;
fl->fl_owner = dp;
fl->fl_pid = current->tgid;
+ fl->fl_file = dp->dl_stid.sc_file->fi_deleg_file;
return fl;
}
-/**
- * nfs4_setlease - Obtain a delegation by requesting lease from vfs layer
- * @dp: a pointer to the nfs4_delegation we're adding.
- *
- * Return:
- * On success: Return code will be 0 on success.
- *
- * On error: -EAGAIN if there was an existing delegation.
- * nonzero if there is an error in other cases.
- *
- */
-
-static int nfs4_setlease(struct nfs4_delegation *dp)
-{
- struct nfs4_file *fp = dp->dl_stid.sc_file;
- struct file_lock *fl;
- struct file *filp;
- int status = 0;
-
- fl = nfs4_alloc_init_lease(dp, NFS4_OPEN_DELEGATE_READ);
- if (!fl)
- return -ENOMEM;
- filp = find_readable_file(fp);
- if (!filp) {
- /* We should always have a readable file here */
- WARN_ON_ONCE(1);
- locks_free_lock(fl);
- return -EBADF;
- }
- fl->fl_file = filp;
- status = vfs_setlease(filp, fl->fl_type, &fl, NULL);
- if (fl)
- locks_free_lock(fl);
- if (status)
- goto out_fput;
- spin_lock(&state_lock);
- spin_lock(&fp->fi_lock);
- /* Did the lease get broken before we took the lock? */
- status = -EAGAIN;
- if (fp->fi_had_conflict)
- goto out_unlock;
- /* Race breaker */
- if (fp->fi_deleg_file) {
- status = hash_delegation_locked(dp, fp);
- goto out_unlock;
- }
- fp->fi_deleg_file = filp;
- fp->fi_delegees = 0;
- status = hash_delegation_locked(dp, fp);
- spin_unlock(&fp->fi_lock);
- spin_unlock(&state_lock);
- if (status) {
- /* Should never happen, this is a new fi_deleg_file */
- WARN_ON_ONCE(1);
- goto out_fput;
- }
- return 0;
-out_unlock:
- spin_unlock(&fp->fi_lock);
- spin_unlock(&state_lock);
-out_fput:
- fput(filp);
- return status;
-}
-
static struct nfs4_delegation *
nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
struct nfs4_file *fp, struct nfs4_clnt_odstate *odstate)
{
int status = 0;
struct nfs4_delegation *dp;
+ struct file *filp;
+ struct file_lock *fl;
+ /*
+ * The fi_had_conflict and nfs_get_existing_delegation checks
+ * here are just optimizations; we'll need to recheck them at
+ * the end:
+ */
if (fp->fi_had_conflict)
return ERR_PTR(-EAGAIN);
+ filp = find_readable_file(fp);
+ if (!filp) {
+ /* We should always have a readable file here */
+ WARN_ON_ONCE(1);
+ return ERR_PTR(-EBADF);
+ }
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
if (nfs4_delegation_exists(clp, fp))
status = -EAGAIN;
+ else if (!fp->fi_deleg_file) {
+ fp->fi_deleg_file = filp;
+ /* increment early to prevent fi_deleg_file from being
+ * cleared */
+ fp->fi_delegees = 1;
+ filp = NULL;
+ } else
+ fp->fi_delegees++;
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
-
+ if (filp)
+ fput(filp);
if (status)
return ERR_PTR(status);
+ status = -ENOMEM;
dp = alloc_init_deleg(clp, fp, fh, odstate);
if (!dp)
- return ERR_PTR(-ENOMEM);
+ goto out_delegees;
+
+ fl = nfs4_alloc_init_lease(dp, NFS4_OPEN_DELEGATE_READ);
+ if (!fl)
+ goto out_stid;
+
+ status = vfs_setlease(fp->fi_deleg_file, fl->fl_type, &fl, NULL);
+ if (fl)
+ locks_free_lock(fl);
+ if (status)
+ goto out_clnt_odstate;
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
- if (!fp->fi_deleg_file) {
- spin_unlock(&fp->fi_lock);
- spin_unlock(&state_lock);
- status = nfs4_setlease(dp);
- goto out;
- }
- if (fp->fi_had_conflict) {
+ if (fp->fi_had_conflict)
status = -EAGAIN;
- goto out_unlock;
- }
- status = hash_delegation_locked(dp, fp);
-out_unlock:
+ else
+ status = hash_delegation_locked(dp, fp);
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
-out:
- if (status) {
- put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_stid(&dp->dl_stid);
- return ERR_PTR(status);
- }
+
+ if (status)
+ destroy_unhashed_deleg(dp);
return dp;
+out_clnt_odstate:
+ put_clnt_odstate(dp->dl_clnt_odstate);
+out_stid:
+ nfs4_put_stid(&dp->dl_stid);
+out_delegees:
+ put_deleg_file(fp);
+ return ERR_PTR(status);
}
static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
This doesn't "get" anything.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3fc0682ca799..b49000797c6f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -870,20 +870,16 @@ void nfs4_unhash_stid(struct nfs4_stid *s)
}
/**
- * nfs4_get_existing_delegation - Discover if this delegation already exists
+ * nfs4_delegation_exists - Discover if this delegation already exists
* @clp: a pointer to the nfs4_client we're granting a delegation to
* @fp: a pointer to the nfs4_file we're granting a delegation on
*
* Return:
- * On success: NULL if an existing delegation was not found.
- *
- * On error: -EAGAIN if one was previously granted to this nfs4_client
- * for this nfs4_file.
- *
+ * On success: true iff an existing delegation is found
*/
-static int
-nfs4_get_existing_delegation(struct nfs4_client *clp, struct nfs4_file *fp)
+static bool
+nfs4_delegation_exists(struct nfs4_client *clp, struct nfs4_file *fp)
{
struct nfs4_delegation *searchdp = NULL;
struct nfs4_client *searchclp = NULL;
@@ -916,15 +912,13 @@ nfs4_get_existing_delegation(struct nfs4_client *clp, struct nfs4_file *fp)
static int
hash_delegation_locked(struct nfs4_delegation *dp, struct nfs4_file *fp)
{
- int status;
struct nfs4_client *clp = dp->dl_stid.sc_client;
lockdep_assert_held(&state_lock);
lockdep_assert_held(&fp->fi_lock);
- status = nfs4_get_existing_delegation(clp, fp);
- if (status)
- return status;
+ if (nfs4_delegation_exists(clp, fp))
+ return -EAGAIN;
++fp->fi_delegees;
refcount_inc(&dp->dl_stid.sc_count);
dp->dl_stid.sc_type = NFS4_DELEG_STID;
@@ -4343,7 +4337,7 @@ static struct nfs4_delegation *
nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
struct nfs4_file *fp, struct nfs4_clnt_odstate *odstate)
{
- int status;
+ int status = 0;
struct nfs4_delegation *dp;
if (fp->fi_had_conflict)
@@ -4351,7 +4345,8 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
- status = nfs4_get_existing_delegation(clp, fp);
+ if (nfs4_delegation_exists(clp, fp))
+ status = -EAGAIN;
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
We currently revoke read delegations on any write open or any operation
that modifies file data or metadata (including rename, link, and
unlink). But if the delegation in question is the only read delegation
and is held by the client performing the operation, that's not really
necessary.
It's not always possible to prevent this in the NFSv4.0 case, because
there's not always a way to determine which client an NFSv4.0 delegation
came from. (In theory we could try to guess this from the transport
layer, e.g., by assuming all traffic on a given TCP connection comes
from the same client. But that's not really correct.)
In the NFSv4.1 case the session layer always tells us the client.
This patch should remove such self-conflicts in all cases where we can
reliably determine the client from the compound.
To do that we need to track "who" is performing a given (possibly
lease-breaking) file operation. There are a lot of those. So it seems
simpler to pass this in struct task; this patch uses a new field in
struct cred.
Signed-off-by: J. Bruce Fields <[email protected]>
---
Documentation/filesystems/Locking | 2 ++
fs/locks.c | 4 ++++
fs/nfsd/auth.c | 2 ++
fs/nfsd/nfs4proc.c | 1 +
fs/nfsd/nfs4state.c | 8 ++++++++
fs/nfsd/nfssvc.c | 1 +
include/linux/cred.h | 3 +++
include/linux/fs.h | 1 +
include/linux/sunrpc/svc.h | 1 +
9 files changed, 23 insertions(+)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 75d2d57e2c44..739c55825330 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -366,6 +366,7 @@ prototypes:
int (*lm_grant)(struct file_lock *, struct file_lock *, int);
void (*lm_break)(struct file_lock *); /* break_lease callback */
int (*lm_change)(struct file_lock **, int);
+ bool (*lm_breaker_owns_lease)(void *, struct file_lock *);
locking rules:
@@ -376,6 +377,7 @@ lm_notify: yes yes no
lm_grant: no no no
lm_break: yes no no
lm_change yes no no
+lm_breaker_owns_lease: no no no
[1]: ->lm_compare_owner and ->lm_owner_key are generally called with
*an* inode->i_lock held. It may not be the i_lock of the inode
diff --git a/fs/locks.c b/fs/locks.c
index 63aa52bcdf5a..22ed02b20559 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1414,6 +1414,9 @@ static void time_out_leases(struct inode *inode, struct list_head *dispose)
static bool leases_conflict(struct file_lock *lease, struct file_lock *breaker)
{
+ if (lease->fl_lmops->lm_breaker_owns_lease && breaker->fl_owner &&
+ lease->fl_lmops->lm_breaker_owns_lease(breaker->fl_owner, lease))
+ return false;
if ((breaker->fl_flags & FL_LAYOUT) != (lease->fl_flags & FL_LAYOUT))
return false;
if ((breaker->fl_flags & FL_DELEG) && (lease->fl_flags & FL_LEASE))
@@ -1462,6 +1465,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
if (IS_ERR(new_fl))
return PTR_ERR(new_fl);
new_fl->fl_flags = type;
+ new_fl->fl_owner = current_cred()->lease_breaker;
/* typically we will check that ctx is non-NULL before calling */
ctx = smp_load_acquire(&inode->i_flctx);
diff --git a/fs/nfsd/auth.c b/fs/nfsd/auth.c
index fdf2aad73470..d2562ae21b8e 100644
--- a/fs/nfsd/auth.c
+++ b/fs/nfsd/auth.c
@@ -81,6 +81,8 @@ int nfsd_setuser(struct svc_rqst *rqstp, struct svc_export *exp)
else
new->cap_effective = cap_raise_nfsd_set(new->cap_effective,
new->cap_permitted);
+ if (rqstp->rq_lease_breaker)
+ new->lease_breaker = *rqstp->rq_lease_breaker;
validate_process_creds();
put_cred(override_creds(new));
put_cred(new);
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a0bed2b2004d..e1341dbaf657 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1713,6 +1713,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
op->status = status;
goto encode_op;
}
+ rqstp->rq_lease_breaker = (void **)&cstate->clp;
while (!status && resp->opcnt < args->opcnt) {
op = &args->ops[resp->opcnt++];
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ca8a5644d9ff..4036544b5b66 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3926,6 +3926,13 @@ nfsd_break_deleg_cb(struct file_lock *fl)
return ret;
}
+static bool nfsd_breaker_owns_lease(void *breaker, struct file_lock *fl)
+{
+ struct nfs4_delegation *dl = fl->fl_owner;
+
+ return dl->dl_stid.sc_client == breaker;
+}
+
static int
nfsd_change_deleg_cb(struct file_lock *onlist, int arg,
struct list_head *dispose)
@@ -3937,6 +3944,7 @@ nfsd_change_deleg_cb(struct file_lock *onlist, int arg,
}
static const struct lock_manager_operations nfsd_lease_mng_ops = {
+ .lm_breaker_owns_lease = nfsd_breaker_owns_lease,
.lm_break = nfsd_break_deleg_cb,
.lm_change = nfsd_change_deleg_cb,
};
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 89cb484f1cfb..c4e86a0a8dd3 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -803,6 +803,7 @@ nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp)
*statp = rpc_garbage_args;
return 1;
}
+ rqstp->rq_lease_breaker = NULL;
/*
* Give the xdr decoder a chance to change this if it wants
* (necessary in the NFSv4.0 compound case)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 631286535d0f..d567c27eebae 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -139,6 +139,9 @@ struct cred {
struct key *thread_keyring; /* keyring private to this thread */
struct key *request_key_auth; /* assumed request_key authority */
#endif
+#ifdef CONFIG_FILE_LOCKING
+ void *lease_breaker; /* identify NFS client breaking a delegation */
+#endif
#ifdef CONFIG_SECURITY
void *security; /* subjective LSM security */
#endif
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ef9269bf7e69..43e1d2f47cb3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -959,6 +959,7 @@ struct lock_manager_operations {
bool (*lm_break)(struct file_lock *);
int (*lm_change)(struct file_lock *, int, struct list_head *);
void (*lm_setup)(struct file_lock *, void **);
+ bool (*lm_breaker_owns_lease)(void *, struct file_lock *);
};
struct lock_manager {
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 786ae2255f05..f2bbfee1662a 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -293,6 +293,7 @@ struct svc_rqst {
struct svc_cacherep * rq_cacherep; /* cache info */
struct task_struct *rq_task; /* service thread */
spinlock_t rq_lock; /* per-request lock */
+ void ** rq_lease_breaker; /* The v4 client breaking a lease */
};
#define SVC_NET(svc_rqst) (svc_rqst->rq_xprt->xpt_net)
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
The delegation isn't visible to anyone yet.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0466312a2a4e..3fc0682ca799 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4363,9 +4363,10 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
return ERR_PTR(-ENOMEM);
get_nfs4_file(fp);
+ dp->dl_stid.sc_file = fp;
+
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
- dp->dl_stid.sc_file = fp;
if (!fp->fi_deleg_file) {
spin_unlock(&fp->fi_lock);
spin_unlock(&state_lock);
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
Take an easy chance to simplify the caller a little.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 268d095477e8..26880d0122c5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -777,7 +777,8 @@ static void block_delegations(struct knfsd_fh *fh)
}
static struct nfs4_delegation *
-alloc_init_deleg(struct nfs4_client *clp, struct svc_fh *current_fh,
+alloc_init_deleg(struct nfs4_client *clp, struct nfs4_file *fp,
+ struct svc_fh *current_fh,
struct nfs4_clnt_odstate *odstate)
{
struct nfs4_delegation *dp;
@@ -808,6 +809,8 @@ alloc_init_deleg(struct nfs4_client *clp, struct svc_fh *current_fh,
dp->dl_retries = 1;
nfsd4_init_cb(&dp->dl_recall, dp->dl_stid.sc_client,
&nfsd4_cb_recall_ops, NFSPROC4_CLNT_CB_RECALL);
+ get_nfs4_file(fp);
+ dp->dl_stid.sc_file = fp;
return dp;
out_dec:
atomic_long_dec(&num_delegations);
@@ -4352,13 +4355,10 @@ nfs4_set_delegation(struct nfs4_client *clp, struct svc_fh *fh,
if (status)
return ERR_PTR(status);
- dp = alloc_init_deleg(clp, fh, odstate);
+ dp = alloc_init_deleg(clp, fp, fh, odstate);
if (!dp)
return ERR_PTR(-ENOMEM);
- get_nfs4_file(fp);
- dp->dl_stid.sc_file = fp;
-
spin_lock(&state_lock);
spin_lock(&fp->fi_lock);
if (!fp->fi_deleg_file) {
--
2.14.3
From: "J. Bruce Fields" <[email protected]>
Pull some duplicated code into a common helper.
This changes the order in destroy_delegation a little, but it looks to
me like that shouldn't matter.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b49000797c6f..268d095477e8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -864,6 +864,13 @@ static void nfs4_put_deleg_lease(struct nfs4_delegation *dp)
}
}
+static void destroy_unhashed_deleg(struct nfs4_delegation *dp)
+{
+ put_clnt_odstate(dp->dl_clnt_odstate);
+ nfs4_put_deleg_lease(dp);
+ nfs4_put_stid(&dp->dl_stid);
+}
+
void nfs4_unhash_stid(struct nfs4_stid *s)
{
s->sc_type = 0;
@@ -955,11 +962,8 @@ static void destroy_delegation(struct nfs4_delegation *dp)
spin_lock(&state_lock);
unhashed = unhash_delegation_locked(dp);
spin_unlock(&state_lock);
- if (unhashed) {
- put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp);
- nfs4_put_stid(&dp->dl_stid);
- }
+ if (unhashed)
+ destroy_unhashed_deleg(dp);
}
static void revoke_delegation(struct nfs4_delegation *dp)
@@ -968,17 +972,14 @@ static void revoke_delegation(struct nfs4_delegation *dp)
WARN_ON(!list_empty(&dp->dl_recall_lru));
- put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp);
-
- if (clp->cl_minorversion == 0)
- nfs4_put_stid(&dp->dl_stid);
- else {
+ if (clp->cl_minorversion) {
dp->dl_stid.sc_type = NFS4_REVOKED_DELEG_STID;
+ refcount_inc(&dp->dl_stid.sc_count);
spin_lock(&clp->cl_lock);
list_add(&dp->dl_recall_lru, &clp->cl_revoked);
spin_unlock(&clp->cl_lock);
}
+ destroy_unhashed_deleg(dp);
}
/*
@@ -1879,9 +1880,7 @@ __destroy_client(struct nfs4_client *clp)
while (!list_empty(&reaplist)) {
dp = list_entry(reaplist.next, struct nfs4_delegation, dl_recall_lru);
list_del_init(&dp->dl_recall_lru);
- put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp);
- nfs4_put_stid(&dp->dl_stid);
+ destroy_unhashed_deleg(dp);
}
while (!list_empty(&clp->cl_revoked)) {
dp = list_entry(clp->cl_revoked.next, struct nfs4_delegation, dl_recall_lru);
@@ -7219,9 +7218,7 @@ nfs4_state_shutdown_net(struct net *net)
list_for_each_safe(pos, next, &reaplist) {
dp = list_entry (pos, struct nfs4_delegation, dl_recall_lru);
list_del_init(&dp->dl_recall_lru);
- put_clnt_odstate(dp->dl_clnt_odstate);
- nfs4_put_deleg_lease(dp);
- nfs4_put_stid(&dp->dl_stid);
+ destroy_unhashed_deleg(dp);
}
BUG_ON(!list_empty(&reaplist));
--
2.14.3
On Mon, 2018-03-19 at 10:36 -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> This is my second attempt to fix the NFS server so we don't
> unnecessarily recall delegations when the operation breaking the
> delegation comes from the same client that holds the delegation.
>
> To do that we need some way to pass the identity of the breaker down
> through the VFS. In my first attempt I tried passing that explicitly,
> but that touches a lot of code. So instead I'm experimenting with
> adding a field to the cred struct.
>
> Testing has confirmed that this works. (And that the pynfs tests are
> broken--they *require* the server to break delegations on operations
> coming from the client, even though that's the less desireable behavior.
> I'm fixing that...).
>
> J. Bruce Fields (10):
> vfs: remove unnecessary fl_owner_t typedef
> nfsd: simplify put of fi_deleg_file
> nfsd: simplify nfs4_put_deleg_lease calls
> nfsd4: set fl_owner to delegation, not file pointer
> nfsd4: dp->dl_stid.sc_file doesn't need locking
> nfsd: make nfs4_get_existing_delegation less confusing
> nfsd: factor out common delegation-destruction code
> nfsd: move sc_file assignment into alloc_init_deleg
> nfsd: create a separate lease for each delegation
> nfsd: clients don't need to break their own delegations
>
> Documentation/filesystems/Locking | 2 +
> Documentation/filesystems/vfs.txt | 2 +-
> arch/ia64/kernel/perfmon.c | 2 +-
> arch/powerpc/platforms/cell/spufs/file.c | 2 +-
> arch/tile/kernel/hardwall.c | 2 +-
> drivers/android/binder.c | 2 +-
> drivers/char/ps3flash.c | 2 +-
> drivers/char/xillybus/xillybus_core.c | 2 +-
> drivers/firmware/efi/capsule-loader.c | 2 +-
> drivers/input/evdev.c | 2 +-
> drivers/misc/mic/scif/scif_fd.c | 2 +-
> drivers/scsi/osst.c | 2 +-
> drivers/scsi/st.c | 2 +-
> drivers/staging/lustre/lustre/llite/file.c | 2 +-
> drivers/usb/class/cdc-wdm.c | 2 +-
> drivers/usb/usb-skeleton.c | 2 +-
> fs/afs/internal.h | 2 +-
> fs/afs/write.c | 2 +-
> fs/cifs/cifsfs.h | 2 +-
> fs/cifs/file.c | 4 +-
> fs/ecryptfs/file.c | 2 +-
> fs/exofs/file.c | 2 +-
> fs/f2fs/file.c | 2 +-
> fs/fuse/file.c | 13 +-
> fs/fuse/fuse_i.h | 2 +-
> fs/lockd/clntproc.c | 4 +-
> fs/lockd/svc4proc.c | 2 +-
> fs/lockd/svcproc.c | 2 +-
> fs/locks.c | 6 +-
> fs/nfs/file.c | 2 +-
> fs/nfs/inode.c | 2 +-
> fs/nfs/nfs4_fs.h | 2 +-
> fs/nfs/nfs4file.c | 2 +-
> fs/nfs/nfs4state.c | 8 +-
> fs/nfsd/auth.c | 2 +
> fs/nfsd/nfs4proc.c | 1 +
> fs/nfsd/nfs4state.c | 267 ++++++++++++-----------------
> fs/nfsd/nfssvc.c | 1 +
> fs/notify/dnotify/dnotify.c | 6 +-
> fs/open.c | 2 +-
> include/linux/cred.h | 3 +
> include/linux/dnotify.h | 6 +-
> include/linux/fs.h | 18 +-
> include/linux/lockd/lockd.h | 4 +-
> include/linux/nfs_fs.h | 4 +-
> include/linux/sunrpc/svc.h | 1 +
> include/trace/events/filelock.h | 6 +-
> ipc/mqueue.c | 2 +-
> 48 files changed, 193 insertions(+), 223 deletions(-)
>
Nice work.
The whole first part of the series looks like a nice set of cleanups
(sans the first patch, as there seems to be support to keep fl_owner_t
typedef around). Giving each delegation its own lease makes a lot more
sense, IMO.
On patches 2-9, you can add:
Reviewed-by: Jeff Layton <[email protected]>
...and maybe get those into -next soon? The delegation code has been a
source of subtle bugs in the past, so we really want this to get a lot
of testing.
With patch 10, I have some concern about growing a common structure like
struct cred with such a special-purpose field.
An alternative might be to utilize the keyrings there -- maybe stash a
special sort of key on one of the keyrings with this pointer? We'd need
to take care to prevent userland from doing this, but that seems like a
solvable problem.
Cheers,
--
Jeff Layton <[email protected]>
J. Bruce Fields <[email protected]> wrote:
> @@ -139,6 +139,9 @@ struct cred {
> struct key *thread_keyring; /* keyring private to this thread */
> struct key *request_key_auth; /* assumed request_key authority */
> #endif
> +#ifdef CONFIG_FILE_LOCKING
> + void *lease_breaker; /* identify NFS client breaking a delegation */
> +#endif
> #ifdef CONFIG_SECURITY
> void *security; /* subjective LSM security */
> #endif
Sorry, but ewww.
Two reasons for that comment:
(1) The cred struct may get retained long past where you expect if it gets
attached to another process or a file descriptor.
(2) The ->lease_breaker pointer needs lifetime management in cred.c. It will
potentially get copied around and may need cleaning up.
Can you stick your breaker identity in a key struct as Jeff suggested?
David
T24gVHVlLCAyMDE4LTAzLTIwIGF0IDEzOjM1ICswMDAwLCBEYXZpZCBIb3dlbGxzIHdyb3RlOg0K
PiBKLiBCcnVjZSBGaWVsZHMgPGJmaWVsZHNAcmVkaGF0LmNvbT4gd3JvdGU6DQo+IA0KPiA+IEBA
IC0xMzksNiArMTM5LDkgQEAgc3RydWN0IGNyZWQgew0KPiA+ICAJc3RydWN0IGtleQkqdGhyZWFk
X2tleXJpbmc7IC8qIGtleXJpbmcgcHJpdmF0ZSB0bw0KPiA+IHRoaXMgdGhyZWFkICovDQo+ID4g
IAlzdHJ1Y3Qga2V5CSpyZXF1ZXN0X2tleV9hdXRoOyAvKiBhc3N1bWVkDQo+ID4gcmVxdWVzdF9r
ZXkgYXV0aG9yaXR5ICovDQo+ID4gICNlbmRpZg0KPiA+ICsjaWZkZWYgQ09ORklHX0ZJTEVfTE9D
S0lORw0KPiA+ICsJdm9pZAkJKmxlYXNlX2JyZWFrZXI7IC8qIGlkZW50aWZ5IE5GUyBjbGllbnQN
Cj4gPiBicmVha2luZyBhIGRlbGVnYXRpb24gKi8NCj4gPiArI2VuZGlmDQo+ID4gICNpZmRlZiBD
T05GSUdfU0VDVVJJVFkNCj4gPiAgCXZvaWQJCSpzZWN1cml0eTsJLyogc3ViamVjdGl2ZSBMU00N
Cj4gPiBzZWN1cml0eSAqLw0KPiA+ICAjZW5kaWYNCj4gDQo+IFNvcnJ5LCBidXQgZXd3dy4NCj4g
DQo+IFR3byByZWFzb25zIGZvciB0aGF0IGNvbW1lbnQ6DQo+IA0KPiAgKDEpIFRoZSBjcmVkIHN0
cnVjdCBtYXkgZ2V0IHJldGFpbmVkIGxvbmcgcGFzdCB3aGVyZSB5b3UgZXhwZWN0IGlmDQo+IGl0
IGdldHMNCj4gICAgICBhdHRhY2hlZCB0byBhbm90aGVyIHByb2Nlc3Mgb3IgYSBmaWxlIGRlc2Ny
aXB0b3IuDQo+IA0KPiAgKDIpIFRoZSAtPmxlYXNlX2JyZWFrZXIgcG9pbnRlciBuZWVkcyBsaWZl
dGltZSBtYW5hZ2VtZW50IGluDQo+IGNyZWQuYy4gIEl0IHdpbGwNCj4gICAgICBwb3RlbnRpYWxs
eSBnZXQgY29waWVkIGFyb3VuZCBhbmQgbWF5IG5lZWQgY2xlYW5pbmcgdXAuDQo+IA0KPiBDYW4g
eW91IHN0aWNrIHlvdXIgYnJlYWtlciBpZGVudGl0eSBpbiBhIGtleSBzdHJ1Y3QgYXMgSmVmZg0K
PiBzdWdnZXN0ZWQ/DQo+IA0KDQpCcnVjZSwNCg0KRG8geW91IHJlYWxseSBuZWVkIHRvIGRvIG1v
cmUgdGhhbiBqdXN0IGlkZW50aWZ5IHRoYXQgdGhpcyBpcyBhIGtuZnNkDQp0aHJlYWQgdnMgbm90
IGEga25mc2QgdGhyZWFkPyBJJ20gYXNzdW1pbmcgdGhhdCBhIGtuZnNkIHRocmVhZCB3aWxsDQp1
c3VhbGx5IGJlIGluIGEgcG9zaXRpb24gdG8gcmVjYWxsIGRlbGVnYXRpb25zIGJlZm9yZSBpdCBl
dmVuIGluaXRpYXRlcw0KYW4gb3BlcmF0aW9uIG9uIHRoZSBpbm9kZSBpbiBxdWVzdGlvbiwgd29u
J3QgaXQ/DQoNCklPVzogd2hhdCBpZiB5b3Ugd2VyZSB0byBtb2RpZnkgdGhlIGxlYXNlIGNvZGUg
dG8gYWxsb3cga25mc2QgdGhyZWFkcw0KdG8gcmV0dXJuIGEgInBsZWFzZSBpZ25vcmUgbWUsIGFu
ZCBwcm9jZWVkIHdpdGggdGhlIG9wZXJhdGlvbiB0aGF0DQp0cmlnZ2VyZWQgdGhlIGxlYXNlIGJy
ZWFrIiByZXBseSwgYW5kIHRoZW4gaGFuZGxlIGNvbmZsaWN0cyBiZXR3ZWVuIE5GUw0KY2xpZW50
cyBvdXRzaWRlIHRoZSBsZWFzZSBjYWxsYmFjayBjb2RlIGFsdG9nZXRoZXI/DQoNCkNoZWVycw0K
ICBUcm9uZA0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5l
ciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg==
On Tue, Mar 20, 2018 at 01:46:20PM +0000, Trond Myklebust wrote:
> On Tue, 2018-03-20 at 13:35 +0000, David Howells wrote:
> > J. Bruce Fields <[email protected]> wrote:
> >
> > > @@ -139,6 +139,9 @@ struct cred {
> > > struct key *thread_keyring; /* keyring private to
> > > this thread */
> > > struct key *request_key_auth; /* assumed
> > > request_key authority */
> > > #endif
> > > +#ifdef CONFIG_FILE_LOCKING
> > > + void *lease_breaker; /* identify NFS client
> > > breaking a delegation */
> > > +#endif
> > > #ifdef CONFIG_SECURITY
> > > void *security; /* subjective LSM
> > > security */
> > > #endif
> >
> > Sorry, but ewww.
> >
> > Two reasons for that comment:
> >
> > (1) The cred struct may get retained long past where you expect if
> > it gets
> > attached to another process or a file descriptor.
> >
> > (2) The ->lease_breaker pointer needs lifetime management in
> > cred.c. It will
> > potentially get copied around and may need cleaning up.
> >
> > Can you stick your breaker identity in a key struct as Jeff
> > suggested?
> >
>
> Bruce,
>
> Do you really need to do more than just identify that this is a knfsd
> thread vs not a knfsd thread? I'm assuming that a knfsd thread will
> usually be in a position to recall delegations before it even initiates
> an operation on the inode in question, won't it?
I think it could. I'm reluctant:
- Once we support write delegations, I think we end up having to
do that before basically every operation on a inode.
- I'd like this to make it easy for someone to extend delegation
support to userspace eventually too. I'm not sure exactly how
we'd identify self-conflicts in that case (struct files?), but
anyway I'd rather this wasn't too nfsd-specific.
That said, I'm still curious:
> IOW: what if you were to modify the lease code to allow knfsd threads
> to return a "please ignore me, and proceed with the operation that
> triggered the lease break" reply, and then handle conflicts between NFS
> clients outside the lease callback code altogether?
So if you're a random bit of code, how would you recommend testing
whether you're running in a knfsd thread?
--b.
On Tue, Mar 20, 2018 at 01:35:51PM +0000, David Howells wrote:
> J. Bruce Fields <[email protected]> wrote:
>
> > @@ -139,6 +139,9 @@ struct cred {
> > struct key *thread_keyring; /* keyring private to this thread */
> > struct key *request_key_auth; /* assumed request_key authority */
> > #endif
> > +#ifdef CONFIG_FILE_LOCKING
> > + void *lease_breaker; /* identify NFS client breaking a delegation */
> > +#endif
> > #ifdef CONFIG_SECURITY
> > void *security; /* subjective LSM security */
> > #endif
>
> Sorry, but ewww.
I'm sure you're right, but, to make sure I understand:
> Two reasons for that comment:
>
> (1) The cred struct may get retained long past where you expect if it gets
> attached to another process or a file descriptor.
How would that happen in the case of a knfsd thread?
> (2) The ->lease_breaker pointer needs lifetime management in cred.c. It will
> potentially get copied around and may need cleaning up.
Hm, OK.
> Can you stick your breaker identity in a key struct as Jeff suggested?
Probably so. I'm unfamiliar with the keyring code. How would you
recommend doing that?
--b.
T24gVHVlLCAyMDE4LTAzLTIwIGF0IDEwOjQ5IC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6
DQo+IE9uIFR1ZSwgTWFyIDIwLCAyMDE4IGF0IDAxOjQ2OjIwUE0gKzAwMDAsIFRyb25kIE15a2xl
YnVzdCB3cm90ZToNCj4gPiBPbiBUdWUsIDIwMTgtMDMtMjAgYXQgMTM6MzUgKzAwMDAsIERhdmlk
IEhvd2VsbHMgd3JvdGU6DQo+ID4gPiBKLiBCcnVjZSBGaWVsZHMgPGJmaWVsZHNAcmVkaGF0LmNv
bT4gd3JvdGU6DQo+ID4gPiANCj4gPiA+ID4gQEAgLTEzOSw2ICsxMzksOSBAQCBzdHJ1Y3QgY3Jl
ZCB7DQo+ID4gPiA+ICAJc3RydWN0IGtleQkqdGhyZWFkX2tleXJpbmc7IC8qIGtleXJpbmcgcHJp
dmF0ZQ0KPiA+ID4gPiB0bw0KPiA+ID4gPiB0aGlzIHRocmVhZCAqLw0KPiA+ID4gPiAgCXN0cnVj
dCBrZXkJKnJlcXVlc3Rfa2V5X2F1dGg7IC8qIGFzc3VtZWQNCj4gPiA+ID4gcmVxdWVzdF9rZXkg
YXV0aG9yaXR5ICovDQo+ID4gPiA+ICAjZW5kaWYNCj4gPiA+ID4gKyNpZmRlZiBDT05GSUdfRklM
RV9MT0NLSU5HDQo+ID4gPiA+ICsJdm9pZAkJKmxlYXNlX2JyZWFrZXI7IC8qIGlkZW50aWZ5IE5G
Uw0KPiA+ID4gPiBjbGllbnQNCj4gPiA+ID4gYnJlYWtpbmcgYSBkZWxlZ2F0aW9uICovDQo+ID4g
PiA+ICsjZW5kaWYNCj4gPiA+ID4gICNpZmRlZiBDT05GSUdfU0VDVVJJVFkNCj4gPiA+ID4gIAl2
b2lkCQkqc2VjdXJpdHk7CS8qIHN1YmplY3RpdmUNCj4gPiA+ID4gTFNNDQo+ID4gPiA+IHNlY3Vy
aXR5ICovDQo+ID4gPiA+ICAjZW5kaWYNCj4gPiA+IA0KPiA+ID4gU29ycnksIGJ1dCBld3d3Lg0K
PiA+ID4gDQo+ID4gPiBUd28gcmVhc29ucyBmb3IgdGhhdCBjb21tZW50Og0KPiA+ID4gDQo+ID4g
PiAgKDEpIFRoZSBjcmVkIHN0cnVjdCBtYXkgZ2V0IHJldGFpbmVkIGxvbmcgcGFzdCB3aGVyZSB5
b3UgZXhwZWN0DQo+ID4gPiBpZg0KPiA+ID4gaXQgZ2V0cw0KPiA+ID4gICAgICBhdHRhY2hlZCB0
byBhbm90aGVyIHByb2Nlc3Mgb3IgYSBmaWxlIGRlc2NyaXB0b3IuDQo+ID4gPiANCj4gPiA+ICAo
MikgVGhlIC0+bGVhc2VfYnJlYWtlciBwb2ludGVyIG5lZWRzIGxpZmV0aW1lIG1hbmFnZW1lbnQg
aW4NCj4gPiA+IGNyZWQuYy4gIEl0IHdpbGwNCj4gPiA+ICAgICAgcG90ZW50aWFsbHkgZ2V0IGNv
cGllZCBhcm91bmQgYW5kIG1heSBuZWVkIGNsZWFuaW5nIHVwLg0KPiA+ID4gDQo+ID4gPiBDYW4g
eW91IHN0aWNrIHlvdXIgYnJlYWtlciBpZGVudGl0eSBpbiBhIGtleSBzdHJ1Y3QgYXMgSmVmZg0K
PiA+ID4gc3VnZ2VzdGVkPw0KPiA+ID4gDQo+ID4gDQo+ID4gQnJ1Y2UsDQo+ID4gDQo+ID4gRG8g
eW91IHJlYWxseSBuZWVkIHRvIGRvIG1vcmUgdGhhbiBqdXN0IGlkZW50aWZ5IHRoYXQgdGhpcyBp
cyBhDQo+ID4ga25mc2QNCj4gPiB0aHJlYWQgdnMgbm90IGEga25mc2QgdGhyZWFkPyBJJ20gYXNz
dW1pbmcgdGhhdCBhIGtuZnNkIHRocmVhZCB3aWxsDQo+ID4gdXN1YWxseSBiZSBpbiBhIHBvc2l0
aW9uIHRvIHJlY2FsbCBkZWxlZ2F0aW9ucyBiZWZvcmUgaXQgZXZlbg0KPiA+IGluaXRpYXRlcw0K
PiA+IGFuIG9wZXJhdGlvbiBvbiB0aGUgaW5vZGUgaW4gcXVlc3Rpb24sIHdvbid0IGl0Pw0KPiAN
Cj4gSSB0aGluayBpdCBjb3VsZC4gIEknbSByZWx1Y3RhbnQ6DQo+IA0KPiAJLSBPbmNlIHdlIHN1
cHBvcnQgd3JpdGUgZGVsZWdhdGlvbnMsIEkgdGhpbmsgd2UgZW5kIHVwIGhhdmluZw0KPiB0bw0K
PiAJICBkbyB0aGF0IGJlZm9yZSBiYXNpY2FsbHkgZXZlcnkgb3BlcmF0aW9uIG9uIGEgaW5vZGUu
DQo+IAktIEknZCBsaWtlIHRoaXMgdG8gbWFrZSBpdCBlYXN5IGZvciBzb21lb25lIHRvIGV4dGVu
ZA0KPiBkZWxlZ2F0aW9uDQo+IAkgIHN1cHBvcnQgdG8gdXNlcnNwYWNlIGV2ZW50dWFsbHkgdG9v
LiAgSSdtIG5vdCBzdXJlIGV4YWN0bHkNCj4gaG93DQo+IAkgIHdlJ2QgaWRlbnRpZnkgc2VsZi1j
b25mbGljdHMgaW4gdGhhdCBjYXNlIChzdHJ1Y3QgZmlsZXM/KSwNCj4gYnV0DQo+IAkgIGFueXdh
eSBJJ2QgcmF0aGVyIHRoaXMgd2Fzbid0IHRvbyBuZnNkLXNwZWNpZmljLg0KDQpUaGF0J3MgbXkg
cG9pbnQuIEEgdXNlcnNwYWNlIGFwcGxpY2F0aW9uIGlzIGdvaW5nIHRvIGhhdmUgdG8gZG8NCnNv
bWV0aGluZyBsaWtlIHRoaXMgYW55d2F5LiBJdCBjYW5ub3QgaW5zdGFsbCBob29rcyBpbiB0aGUg
a2VybmVsIHRvDQpwZXJmb3JtIGVsYWJvcmF0ZSB0ZXN0cywgc28gaXQgaXMgZ29pbmcgdG8gaGF2
ZSB0byByZWx5IG9uIHNvbWV0aGluZw0KbGlrZSB0aGUgc3RydWN0IGZpbGVfbG9jayAnZmxfbnNw
aWQnIGZpZWxkIGluIG9yZGVyIHRvIGRldGVybWluZQ0Kd2hldGhlciBvciBub3QgdG8gYXBwbHkg
YSBsZWFzZSBicmVhay4NCg0KaS5lLjogdGhlIHVzZXJzcGFjZSBydWxlIHNob3VsZCBiZSB0byBi
cmVhayB0aGUgbGVhc2UgaWYgYW5kIG9ubHkgaWYgaXQNCmlzIG5vdCBvd25lZCBieSBteSBwcm9j
ZXNzLg0KDQo+IFRoYXQgc2FpZCwgSSdtIHN0aWxsIGN1cmlvdXM6DQo+IA0KPiA+IElPVzogd2hh
dCBpZiB5b3Ugd2VyZSB0byBtb2RpZnkgdGhlIGxlYXNlIGNvZGUgdG8gYWxsb3cga25mc2QNCj4g
PiB0aHJlYWRzDQo+ID4gdG8gcmV0dXJuIGEgInBsZWFzZSBpZ25vcmUgbWUsIGFuZCBwcm9jZWVk
IHdpdGggdGhlIG9wZXJhdGlvbiB0aGF0DQo+ID4gdHJpZ2dlcmVkIHRoZSBsZWFzZSBicmVhayIg
cmVwbHksIGFuZCB0aGVuIGhhbmRsZSBjb25mbGljdHMgYmV0d2Vlbg0KPiA+IE5GUw0KPiA+IGNs
aWVudHMgb3V0c2lkZSB0aGUgbGVhc2UgY2FsbGJhY2sgY29kZSBhbHRvZ2V0aGVyPw0KPiANCj4g
U28gaWYgeW91J3JlIGEgcmFuZG9tIGJpdCBvZiBjb2RlLCBob3cgd291bGQgeW91IHJlY29tbWVu
ZCB0ZXN0aW5nDQo+IHdoZXRoZXIgeW91J3JlIHJ1bm5pbmcgaW4gYSBrbmZzZCB0aHJlYWQ/DQoN
ClJpZ2h0IG5vdywgdGhlIGtuZnNkIHRocmVhZHMgYXJlIHJlZ3VsYXIga2VybmVsIHRocmVhZHMs
IHdpdGggZWFjaA0KdGhyZWFkIGFwcGVhcmluZyB0byB1c2Vyc3BhY2UgdG8gYmUgYSBwcm9jZXNz
IGluIGl0cyBvd24gcmlnaHQuDQpJcyB0aGVyZSBhbnkgcmVhc29uIHdoeSB3ZSBjb3VsZCBub3Qg
YXNzaWduIHRoZW0gYSBncm91cCBwaWQgdGhhdCB3b3VsZA0KYWxsb3cgdGhlIGZsX25zcGlkIG1l
Y2hhbmlzbSB0byB3b3JrIChpLmUuIHNldCB0YXNrLT5ncm91cF9sZWFkZXIpPw0KDQoNCi0tIA0K
VHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRh
DQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=
On Tue, Mar 20, 2018 at 03:13:47PM +0000, Trond Myklebust wrote:
> On Tue, 2018-03-20 at 10:49 -0400, J. Bruce Fields wrote:
> > On Tue, Mar 20, 2018 at 01:46:20PM +0000, Trond Myklebust wrote:
> > > On Tue, 2018-03-20 at 13:35 +0000, David Howells wrote:
> > > > J. Bruce Fields <[email protected]> wrote:
> > > >
> > > > > @@ -139,6 +139,9 @@ struct cred {
> > > > > struct key *thread_keyring; /* keyring private
> > > > > to
> > > > > this thread */
> > > > > struct key *request_key_auth; /* assumed
> > > > > request_key authority */
> > > > > #endif
> > > > > +#ifdef CONFIG_FILE_LOCKING
> > > > > + void *lease_breaker; /* identify NFS
> > > > > client
> > > > > breaking a delegation */
> > > > > +#endif
> > > > > #ifdef CONFIG_SECURITY
> > > > > void *security; /* subjective
> > > > > LSM
> > > > > security */
> > > > > #endif
> > > >
> > > > Sorry, but ewww.
> > > >
> > > > Two reasons for that comment:
> > > >
> > > > (1) The cred struct may get retained long past where you expect
> > > > if
> > > > it gets
> > > > attached to another process or a file descriptor.
> > > >
> > > > (2) The ->lease_breaker pointer needs lifetime management in
> > > > cred.c. It will
> > > > potentially get copied around and may need cleaning up.
> > > >
> > > > Can you stick your breaker identity in a key struct as Jeff
> > > > suggested?
> > > >
> > >
> > > Bruce,
> > >
> > > Do you really need to do more than just identify that this is a
> > > knfsd
> > > thread vs not a knfsd thread? I'm assuming that a knfsd thread will
> > > usually be in a position to recall delegations before it even
> > > initiates
> > > an operation on the inode in question, won't it?
> >
> > I think it could. I'm reluctant:
> >
> > - Once we support write delegations, I think we end up having
> > to
> > do that before basically every operation on a inode.
> > - I'd like this to make it easy for someone to extend
> > delegation
> > support to userspace eventually too. I'm not sure exactly
> > how
> > we'd identify self-conflicts in that case (struct files?),
> > but
> > anyway I'd rather this wasn't too nfsd-specific.
>
> That's my point. A userspace application is going to have to do
> something like this anyway. It cannot install hooks in the kernel to
> perform elaborate tests, so it is going to have to rely on something
> like the struct file_lock 'fl_nspid' field in order to determine
> whether or not to apply a lease break.
>
> i.e.: the userspace rule should be to break the lease if and only if it
> is not owned by my process.
I was thinking of using struct file (whoops, not "struct files", sorry
for the typo above) to avoid assuming too much about any userspace
server's threading model.
> > That said, I'm still curious:
> >
> > > IOW: what if you were to modify the lease code to allow knfsd
> > > threads
> > > to return a "please ignore me, and proceed with the operation that
> > > triggered the lease break" reply, and then handle conflicts between
> > > NFS
> > > clients outside the lease callback code altogether?
> >
> > So if you're a random bit of code, how would you recommend testing
> > whether you're running in a knfsd thread?
>
> Right now, the knfsd threads are regular kernel threads, with each
> thread appearing to userspace to be a process in its own right.
> Is there any reason why we could not assign them a group pid that would
> allow the fl_nspid mechanism to work (i.e. set task->group_leader)?
Huh, OK, I hadn't thought about that.
--b.
On Tue, Mar 20, 2018 at 12:02:57PM -0400, [email protected] wrote:
> On Tue, Mar 20, 2018 at 03:13:47PM +0000, Trond Myklebust wrote:
> > Right now, the knfsd threads are regular kernel threads, with each
> > thread appearing to userspace to be a process in its own right.
> > Is there any reason why we could not assign them a group pid that would
> > allow the fl_nspid mechanism to work (i.e. set task->group_leader)?
>
> Huh, OK, I hadn't thought about that.
Thinking about it: as far as I can tell that's not meant to be modified
on a running thread, so we need to create the thread the right way--all
the nfsd threads need to be children of the same thread.
Currently we're using kthead_create() which makes them all children of
kthreadd.
I spent a little time looking into forking threads from existing nfsd
threads and started to feel like I was duplicating a lot of kthread
code.
So now I wonder if it's possible to generalize kthreadd somehow.
--b.