mbox series

[0/7] fs: fix inode->i_flctx accesses

Message ID 20221116151726.129217-1-jlayton@kernel.org
Headers show
Series fs: fix inode->i_flctx accesses | expand

Message

Jeff Layton Nov. 16, 2022, 3:17 p.m. UTC
The inode->i_flctx field is set via a cmpxchg operation, which is a
release op. This means that most callers need to use an acquire to
access it.

This series adds a new helper function for that, and replaces the
existing accesses of i_flctx with that. The later patches then convert
the various subsystems that access i_flctx directly to use the new
helper.

Assuming there are no objectsions, I'll plan to merge this for v6.2 via
my tree. If any maintainers want to take the subsystem patches in via
their trees, let me know and we'll work it out.

Jeff Layton (7):
  filelock: add a new locks_inode_context accessor function
  ceph: use locks_inode_context helper
  cifs: use locks_inode_context helper
  ksmbd: use locks_inode_context helper
  lockd: use locks_inode_context helper
  nfs: use locks_inode_context helper
  nfsd: use locks_inode_context helper

 fs/ceph/locks.c     |  4 ++--
 fs/cifs/file.c      |  2 +-
 fs/ksmbd/vfs.c      |  2 +-
 fs/lockd/svcsubs.c  |  4 ++--
 fs/locks.c          | 20 ++++++++++----------
 fs/nfs/delegation.c |  2 +-
 fs/nfs/nfs4state.c  |  2 +-
 fs/nfs/pagelist.c   |  2 +-
 fs/nfs/write.c      |  4 ++--
 fs/nfsd/nfs4state.c |  6 +++---
 include/linux/fs.h  | 14 ++++++++++++++
 11 files changed, 38 insertions(+), 24 deletions(-)

Comments

Christoph Hellwig Nov. 16, 2022, 4:08 p.m. UTC | #1
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Christoph Hellwig Nov. 16, 2022, 4:09 p.m. UTC | #2
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Christoph Hellwig Nov. 16, 2022, 4:10 p.m. UTC | #3
On Wed, Nov 16, 2022 at 10:17:25AM -0500, Jeff Layton wrote:
> nfs currently doesn't access i_flctx safely. This requires a
> smp_load_acquire, as the pointer is set via cmpxchg (a release
> operation).

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jeff Layton Nov. 16, 2022, 5:43 p.m. UTC | #4
On Wed, 2022-11-16 at 08:08 -0800, Christoph Hellwig wrote:
> On Wed, Nov 16, 2022 at 10:17:20AM -0500, Jeff Layton wrote:
> > -	ctx =  smp_load_acquire(&inode->i_flctx);
> > +	ctx =  locks_inode_context(inode);
> 
> Nit: this might be a good time to drop the weird double space here.
> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Fixed the nit in my tree.

After sending this, I converted locks_remove_file to use the helper too.
I also think we probably need to use this helper in
locks_free_lock_context.

Folded all 3 changes into this patch, and pushed to my linux-next feeder
branch.

Thanks Christoph!
Xiubo Li Nov. 17, 2022, 4:56 a.m. UTC | #5
On 16/11/2022 23:17, Jeff Layton wrote:
> ceph currently doesn't access i_flctx safely. This requires a
> smp_load_acquire, as the pointer is set via cmpxchg (a release
> operation).
>
> Cc: Xiubo Li <xiubli@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/ceph/locks.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/locks.c b/fs/ceph/locks.c
> index 3e2843e86e27..f3b461c708a8 100644
> --- a/fs/ceph/locks.c
> +++ b/fs/ceph/locks.c
> @@ -364,7 +364,7 @@ void ceph_count_locks(struct inode *inode, int *fcntl_count, int *flock_count)
>   	*fcntl_count = 0;
>   	*flock_count = 0;
>   
> -	ctx = inode->i_flctx;
> +	ctx = locks_inode_context(inode);
>   	if (ctx) {
>   		spin_lock(&ctx->flc_lock);
>   		list_for_each_entry(lock, &ctx->flc_posix, fl_list)
> @@ -418,7 +418,7 @@ int ceph_encode_locks_to_buffer(struct inode *inode,
>   				int num_fcntl_locks, int num_flock_locks)
>   {
>   	struct file_lock *lock;
> -	struct file_lock_context *ctx = inode->i_flctx;
> +	struct file_lock_context *ctx = locks_inode_context(inode);
>   	int err = 0;
>   	int seen_fcntl = 0;
>   	int seen_flock = 0;

Thanks Jeff!

Reviewed-by: Xiubo Li <xiubli@redhat.com>