2022-11-16 15:20:00

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 0/7] fs: fix inode->i_flctx accesses

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(-)

--
2.38.1



2022-11-16 15:20:00

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 1/7] filelock: add a new locks_inode_context accessor function

There are a number of places in the kernel that are accessing the
inode->i_flctx field without smp_load_acquire. This is required to
ensure that the caller doesn't see a partially-initialized structure.

Add a new accessor function for it to make this clear.

Signed-off-by: Jeff Layton <[email protected]>
---
fs/locks.c | 20 ++++++++++----------
include/linux/fs.h | 14 ++++++++++++++
2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 9ccf89b6c95d..07436328dd0a 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -175,7 +175,7 @@ locks_get_lock_context(struct inode *inode, int type)
struct file_lock_context *ctx;

/* paired with cmpxchg() below */
- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (likely(ctx) || type == F_UNLCK)
goto out;

@@ -194,7 +194,7 @@ locks_get_lock_context(struct inode *inode, int type)
*/
if (cmpxchg(&inode->i_flctx, NULL, ctx)) {
kmem_cache_free(flctx_cache, ctx);
- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
}
out:
trace_locks_get_lock_context(inode, type, ctx);
@@ -891,7 +891,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
void *owner;
void (*func)(void);

- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (!ctx || list_empty_careful(&ctx->flc_posix)) {
fl->fl_type = F_UNLCK;
return;
@@ -1483,7 +1483,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
new_fl->fl_flags = type;

/* typically we will check that ctx is non-NULL before calling */
- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (!ctx) {
WARN_ON_ONCE(1);
goto free_lock;
@@ -1588,7 +1588,7 @@ void lease_get_mtime(struct inode *inode, struct timespec64 *time)
struct file_lock_context *ctx;
struct file_lock *fl;

- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (ctx && !list_empty_careful(&ctx->flc_lease)) {
spin_lock(&ctx->flc_lock);
fl = list_first_entry_or_null(&ctx->flc_lease,
@@ -1634,7 +1634,7 @@ int fcntl_getlease(struct file *filp)
int type = F_UNLCK;
LIST_HEAD(dispose);

- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (ctx && !list_empty_careful(&ctx->flc_lease)) {
percpu_down_read(&file_rwsem);
spin_lock(&ctx->flc_lock);
@@ -1823,7 +1823,7 @@ static int generic_delete_lease(struct file *filp, void *owner)
struct file_lock_context *ctx;
LIST_HEAD(dispose);

- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (!ctx) {
trace_generic_delete_lease(inode, NULL);
return error;
@@ -2563,7 +2563,7 @@ void locks_remove_posix(struct file *filp, fl_owner_t owner)
* posix_lock_file(). Another process could be setting a lock on this
* file at the same time, but we wouldn't remove that lock anyway.
*/
- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (!ctx || list_empty(&ctx->flc_posix))
return;

@@ -2684,7 +2684,7 @@ bool vfs_inode_has_locks(struct inode *inode)
struct file_lock_context *ctx;
bool ret;

- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (!ctx)
return false;

@@ -2865,7 +2865,7 @@ void show_fd_locks(struct seq_file *f,
struct file_lock_context *ctx;
int id = 0;

- ctx = smp_load_acquire(&inode->i_flctx);
+ ctx = locks_inode_context(inode);
if (!ctx)
return;

diff --git a/include/linux/fs.h b/include/linux/fs.h
index d6cb42b7e91c..092673178e13 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1187,6 +1187,13 @@ extern void show_fd_locks(struct seq_file *f,
struct file *filp, struct files_struct *files);
extern bool locks_owner_has_blockers(struct file_lock_context *flctx,
fl_owner_t owner);
+
+static inline struct file_lock_context *
+locks_inode_context(const struct inode *inode)
+{
+ return smp_load_acquire(&inode->i_flctx);
+}
+
#else /* !CONFIG_FILE_LOCKING */
static inline int fcntl_getlk(struct file *file, unsigned int cmd,
struct flock __user *user)
@@ -1327,6 +1334,13 @@ static inline bool locks_owner_has_blockers(struct file_lock_context *flctx,
{
return false;
}
+
+static inline struct file_lock_context *
+locks_inode_context(const struct inode *inode)
+{
+ return NULL;
+}
+
#endif /* !CONFIG_FILE_LOCKING */

static inline struct inode *file_inode(const struct file *f)
--
2.38.1


2022-11-16 15:20:19

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 5/7] lockd: use locks_inode_context helper

lockd currently doesn't access i_flctx safely. This requires a
smp_load_acquire, as the pointer is set via cmpxchg (a release
operation).

Cc: Trond Myklebust <[email protected]>
Cc: Anna Schumaker <[email protected]>
Cc: Chuck Lever <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/lockd/svcsubs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index e1c4617de771..720684345817 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -207,7 +207,7 @@ nlm_traverse_locks(struct nlm_host *host, struct nlm_file *file,
{
struct inode *inode = nlmsvc_file_inode(file);
struct file_lock *fl;
- struct file_lock_context *flctx = inode->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(inode);
struct nlm_host *lockhost;

if (!flctx || list_empty_careful(&flctx->flc_posix))
@@ -262,7 +262,7 @@ nlm_file_inuse(struct nlm_file *file)
{
struct inode *inode = nlmsvc_file_inode(file);
struct file_lock *fl;
- struct file_lock_context *flctx = inode->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(inode);

if (file->f_count || !list_empty(&file->f_blocks) || file->f_shares)
return 1;
--
2.38.1


2022-11-16 15:20:23

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 2/7] ceph: use locks_inode_context helper

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 <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
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;
--
2.38.1


2022-11-16 15:20:33

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 4/7] ksmbd: use locks_inode_context helper

ksmbd currently doesn't access i_flctx safely. This requires a
smp_load_acquire, as the pointer is set via cmpxchg (a release
operation).

Cc: Namjae Jeon <[email protected]>
Cc: Steve French <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/ksmbd/vfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
index 8de970d6146f..f9e85d6a160e 100644
--- a/fs/ksmbd/vfs.c
+++ b/fs/ksmbd/vfs.c
@@ -321,7 +321,7 @@ static int check_lock_range(struct file *filp, loff_t start, loff_t end,
unsigned char type)
{
struct file_lock *flock;
- struct file_lock_context *ctx = file_inode(filp)->i_flctx;
+ struct file_lock_context *ctx = locks_inode_context(file_inode(filp));
int error = 0;

if (!ctx || list_empty_careful(&ctx->flc_posix))
--
2.38.1


2022-11-16 15:20:34

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 3/7] cifs: use locks_inode_context helper

cifs currently doesn't access i_flctx safely. This requires a
smp_load_acquire, as the pointer is set via cmpxchg (a release
operation).

Cc: Steve French <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/cifs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index cd9698209930..6c1431979495 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1413,7 +1413,7 @@ cifs_push_posix_locks(struct cifsFileInfo *cfile)
struct inode *inode = d_inode(cfile->dentry);
struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
struct file_lock *flock;
- struct file_lock_context *flctx = inode->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(inode);
unsigned int count = 0, i;
int rc = 0, xid, type;
struct list_head locks_to_send, *el;
--
2.38.1


2022-11-16 15:20:49

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 6/7] nfs: use locks_inode_context helper

nfs currently doesn't access i_flctx safely. This requires a
smp_load_acquire, as the pointer is set via cmpxchg (a release
operation).

Cc: Trond Myklebust <[email protected]>
Cc: Anna Schumaker <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/delegation.c | 2 +-
fs/nfs/nfs4state.c | 2 +-
fs/nfs/pagelist.c | 2 +-
fs/nfs/write.c | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index ead8a0e06abf..cf7365581031 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -146,7 +146,7 @@ static int nfs_delegation_claim_locks(struct nfs4_state *state, const nfs4_state
{
struct inode *inode = state->inode;
struct file_lock *fl;
- struct file_lock_context *flctx = inode->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(inode);
struct list_head *list;
int status = 0;

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index a2d2d5d1b088..dd18344648f3 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1501,7 +1501,7 @@ static int nfs4_reclaim_locks(struct nfs4_state *state, const struct nfs4_state_
struct file_lock *fl;
struct nfs4_lock_state *lsp;
int status = 0;
- struct file_lock_context *flctx = inode->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(inode);
struct list_head *list;

if (flctx == NULL)
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 317cedfa52bf..16be6dae524f 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -1055,7 +1055,7 @@ static unsigned int nfs_coalesce_size(struct nfs_page *prev,
if (prev) {
if (!nfs_match_open_context(nfs_req_openctx(req), nfs_req_openctx(prev)))
return 0;
- flctx = d_inode(nfs_req_openctx(req)->dentry)->i_flctx;
+ flctx = locks_inode_context(d_inode(nfs_req_openctx(req)->dentry));
if (flctx != NULL &&
!(list_empty_careful(&flctx->flc_posix) &&
list_empty_careful(&flctx->flc_flock)) &&
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index f41d24b54fd1..80c240e50952 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1185,7 +1185,7 @@ int nfs_flush_incompatible(struct file *file, struct page *page)
{
struct nfs_open_context *ctx = nfs_file_open_context(file);
struct nfs_lock_context *l_ctx;
- struct file_lock_context *flctx = file_inode(file)->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(file_inode(file));
struct nfs_page *req;
int do_flush, status;
/*
@@ -1321,7 +1321,7 @@ static int nfs_can_extend_write(struct file *file, struct page *page,
struct inode *inode, unsigned int pagelen)
{
int ret;
- struct file_lock_context *flctx = inode->i_flctx;
+ struct file_lock_context *flctx = locks_inode_context(inode);
struct file_lock *fl;

if (file->f_flags & O_DSYNC)
--
2.38.1


2022-11-16 15:21:09

by Jeffrey Layton

[permalink] [raw]
Subject: [PATCH 7/7] nfsd: use locks_inode_context helper

nfsd currently doesn't access i_flctx safely everywhere. This requires a
smp_load_acquire, as the pointer is set via cmpxchg (a release
operation).

Cc: Chuck Lever <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 836bd825ca4a..da8d0ea66229 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4758,7 +4758,7 @@ nfs4_share_conflict(struct svc_fh *current_fh, unsigned int deny_type)

static bool nfsd4_deleg_present(const struct inode *inode)
{
- struct file_lock_context *ctx = smp_load_acquire(&inode->i_flctx);
+ struct file_lock_context *ctx = locks_inode_context(inode);

return ctx && !list_empty_careful(&ctx->flc_lease);
}
@@ -5897,7 +5897,7 @@ nfs4_lockowner_has_blockers(struct nfs4_lockowner *lo)

list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
nf = stp->st_stid.sc_file;
- ctx = nf->fi_inode->i_flctx;
+ ctx = locks_inode_context(nf->fi_inode);
if (!ctx)
continue;
if (locks_owner_has_blockers(ctx, lo))
@@ -7713,7 +7713,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
}

inode = locks_inode(nf->nf_file);
- flctx = inode->i_flctx;
+ flctx = locks_inode_context(inode);

if (flctx && !list_empty_careful(&flctx->flc_posix)) {
spin_lock(&flctx->flc_lock);
--
2.38.1


2022-11-16 15:23:30

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 7/7] nfsd: use locks_inode_context helper



> On Nov 16, 2022, at 10:17 AM, Jeff Layton <[email protected]> wrote:
>
> nfsd currently doesn't access i_flctx safely everywhere. This requires a
> smp_load_acquire, as the pointer is set via cmpxchg (a release
> operation).
>
> Cc: Chuck Lever <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>

Acked-by: Chuck Lever <[email protected]>


> ---
> fs/nfsd/nfs4state.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 836bd825ca4a..da8d0ea66229 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4758,7 +4758,7 @@ nfs4_share_conflict(struct svc_fh *current_fh, unsigned int deny_type)
>
> static bool nfsd4_deleg_present(const struct inode *inode)
> {
> - struct file_lock_context *ctx = smp_load_acquire(&inode->i_flctx);
> + struct file_lock_context *ctx = locks_inode_context(inode);
>
> return ctx && !list_empty_careful(&ctx->flc_lease);
> }
> @@ -5897,7 +5897,7 @@ nfs4_lockowner_has_blockers(struct nfs4_lockowner *lo)
>
> list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
> nf = stp->st_stid.sc_file;
> - ctx = nf->fi_inode->i_flctx;
> + ctx = locks_inode_context(nf->fi_inode);
> if (!ctx)
> continue;
> if (locks_owner_has_blockers(ctx, lo))
> @@ -7713,7 +7713,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
> }
>
> inode = locks_inode(nf->nf_file);
> - flctx = inode->i_flctx;
> + flctx = locks_inode_context(inode);
>
> if (flctx && !list_empty_careful(&flctx->flc_posix)) {
> spin_lock(&flctx->flc_lock);
> --
> 2.38.1
>

--
Chuck Lever




2022-11-16 15:36:52

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH 7/7] nfsd: use locks_inode_context helper

On Wed, 2022-11-16 at 15:21 +0000, Chuck Lever III wrote:
>
> > On Nov 16, 2022, at 10:17 AM, Jeff Layton <[email protected]> wrote:
> >
> > nfsd currently doesn't access i_flctx safely everywhere. This requires a
> > smp_load_acquire, as the pointer is set via cmpxchg (a release
> > operation).
> >
> > Cc: Chuck Lever <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
>
> Acked-by: Chuck Lever <[email protected]>
>
>
> > ---
> > fs/nfsd/nfs4state.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 836bd825ca4a..da8d0ea66229 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -4758,7 +4758,7 @@ nfs4_share_conflict(struct svc_fh *current_fh, unsigned int deny_type)
> >
> > static bool nfsd4_deleg_present(const struct inode *inode)
> > {
> > - struct file_lock_context *ctx = smp_load_acquire(&inode->i_flctx);
> > + struct file_lock_context *ctx = locks_inode_context(inode);
> >
> > return ctx && !list_empty_careful(&ctx->flc_lease);
> > }
> > @@ -5897,7 +5897,7 @@ nfs4_lockowner_has_blockers(struct nfs4_lockowner *lo)
> >
> > list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) {
> > nf = stp->st_stid.sc_file;
> > - ctx = nf->fi_inode->i_flctx;
> > + ctx = locks_inode_context(nf->fi_inode);

Thanks Chuck.

To be clear: I think the above access is probably OK. We wouldn't have a
lock stateid unless we had a valid lock context in the inode. That said,
doing it this way keeps everything consistent, so I'm inclined to leave
the patch as-is.

check_for_locks definitely needs this though.

> > if (!ctx)
> > continue;
> > if (locks_owner_has_blockers(ctx, lo))
> > @@ -7713,7 +7713,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
> > }
> >
> > inode = locks_inode(nf->nf_file);
> > - flctx = inode->i_flctx;
> > + flctx = locks_inode_context(inode);
> >
> > if (flctx && !list_empty_careful(&flctx->flc_posix)) {
> > spin_lock(&flctx->flc_lock);
> > --
> > 2.38.1
> >
>
> --
> Chuck Lever
>
>
>

--
Jeff Layton <[email protected]>

2022-11-16 16:10:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/7] filelock: add a new locks_inode_context accessor function

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 <[email protected]>

2022-11-16 16:10:42

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/7] cifs: use locks_inode_context helper

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2022-11-16 16:10:42

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 4/7] ksmbd: use locks_inode_context helper

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2022-11-16 16:10:43

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/7] ceph: use locks_inode_context helper

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2022-11-16 16:12:53

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 5/7] lockd: use locks_inode_context helper

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2022-11-16 16:13:22

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 6/7] nfs: use locks_inode_context helper

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 <[email protected]>

2022-11-16 16:13:23

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 7/7] nfsd: use locks_inode_context helper

On Wed, Nov 16, 2022 at 10:17:26AM -0500, Jeff Layton wrote:
> nfsd currently doesn't access i_flctx safely everywhere. This requires a
> smp_load_acquire, as the pointer is set via cmpxchg (a release
> operation).

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2022-11-16 17:48:02

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH 1/7] filelock: add a new locks_inode_context accessor function

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 <[email protected]>

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!
--
Jeff Layton <[email protected]>

2022-11-16 23:47:14

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH 4/7] ksmbd: use locks_inode_context helper

2022-11-17 0:17 GMT+09:00, Jeff Layton <[email protected]>:
> ksmbd currently doesn't access i_flctx safely. This requires a
> smp_load_acquire, as the pointer is set via cmpxchg (a release
> operation).
>
> Cc: Namjae Jeon <[email protected]>
> Cc: Steve French <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
Acked-by: Namjae Jeon <[email protected]>

Thanks for your patch!

2022-11-17 05:02:05

by Xiubo Li

[permalink] [raw]
Subject: Re: [PATCH 2/7] ceph: use locks_inode_context helper


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 <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> 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 <[email protected]>