These inlines show up in the fast path (e.g., in do_dentry_open()) and
induce said full barrier regarding i_flctx access when in most cases the
pointer is NULL.
The pointer can be safely checked before issuing the barrier, dodging it
in most cases as a result.
It is plausible the consume fence would be sufficient, but I don't want
to go audit all callers regarding what they before calling here.
Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
---
the header file has locks_inode_context and i even found users like
this (lease_get_mtime):
ctx = locks_inode_context(inode);
if (ctx && !list_empty_careful(&ctx->flc_lease)) {
however, without looking further at the code I'm not confident this
would be sufficient here -- for all I know one consumer needs all stores
to be visible before looking further after derefing the pointer
keeping the full fence in place makes this reasonably easy to reason
about the change i think, but someone(tm) willing to sort this out is
most welcome to do so
include/linux/filelock.h | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/linux/filelock.h b/include/linux/filelock.h
index daee999d05f3..bb44224c6676 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -420,28 +420,38 @@ static inline int locks_lock_file_wait(struct file *filp, struct file_lock *fl)
#ifdef CONFIG_FILE_LOCKING
static inline int break_lease(struct inode *inode, unsigned int mode)
{
+ struct file_lock_context *flctx;
+
/*
* Since this check is lockless, we must ensure that any refcounts
* taken are done before checking i_flctx->flc_lease. Otherwise, we
* could end up racing with tasks trying to set a new lease on this
* file.
*/
+ flctx = READ_ONCE(inode->i_flctx);
+ if (!flctx)
+ return 0;
smp_mb();
- if (inode->i_flctx && !list_empty_careful(&inode->i_flctx->flc_lease))
+ if (!list_empty_careful(&flctx->flc_lease))
return __break_lease(inode, mode, FL_LEASE);
return 0;
}
static inline int break_deleg(struct inode *inode, unsigned int mode)
{
+ struct file_lock_context *flctx;
+
/*
* Since this check is lockless, we must ensure that any refcounts
* taken are done before checking i_flctx->flc_lease. Otherwise, we
* could end up racing with tasks trying to set a new lease on this
* file.
*/
+ flctx = READ_ONCE(inode->i_flctx);
+ if (!flctx)
+ return 0;
smp_mb();
- if (inode->i_flctx && !list_empty_careful(&inode->i_flctx->flc_lease))
+ if (!list_empty_careful(&flctx->flc_lease))
return __break_lease(inode, mode, FL_DELEG);
return 0;
}
--
2.43.0
