Re: [PATCH RFC v15 01/11] fs/lock: add helper locks_any_blockers to check for blockers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/9/22 12:56 PM, Chuck Lever III wrote:

On Mar 4, 2022, at 7:37 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote:

Add helper locks_any_blockers to check if there is any blockers
for a file_lock.

Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
include/linux/fs.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 831b20430d6e..7f5756bfcc13 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1200,6 +1200,11 @@ extern void lease_unregister_notifier(struct notifier_block *);
struct files_struct;
extern void show_fd_locks(struct seq_file *f,
			 struct file *filp, struct files_struct *files);
+
+static inline bool locks_has_blockers_locked(struct file_lock *lck)
+{
+	return !list_empty(&lck->fl_blocked_requests);
+}
#else /* !CONFIG_FILE_LOCKING */
static inline int fcntl_getlk(struct file *file, unsigned int cmd,
			      struct flock __user *user)
@@ -1335,6 +1340,11 @@ static inline int lease_modify(struct file_lock *fl, int arg,
struct files_struct;
static inline void show_fd_locks(struct seq_file *f,
			struct file *filp, struct files_struct *files) {}
+
+static inline bool locks_has_blockers_locked(struct file_lock *lck)
+{
+	return false;
+}
#endif /* !CONFIG_FILE_LOCKING */

static inline struct inode *file_inode(const struct file *f)
Hm. This is not exactly what I had in mind.

In order to be more kABI friendly, fl_blocked_requests should be
dereferenced only in fs/locks.c. IMO you want to take the inner
loop in nfs4_lockowner_has_blockers() and make that a function
that lives in fs/locks.c. Something akin to:

fs/locks.c:

/**
  * locks_owner_has_blockers - Check for blocking lock requests
  * @flctx: file lock context
  * @owner: lock owner
  *
  * Return values:
  *   %true: @ctx has at least one blocker
  *   %false: @ctx has no blockers
  */
bool locks_owner_has_blockers(struct file_lock_context *flctx,
			      fl_owner_t owner)
{
	struct file_lock *fl;

	spin_lock(&flctx->flc_lock);
	list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
		if (fl->fl_owner != owner)
			continue;
		if (!list_empty(&fl->fl_blocked_requests)) {
			spin_unlock(&flctx->flc_lock);
			return true;
		}
	}
	spin_unlock(&flctx->flc_lock);
	return false;
}
EXPORT_SYMBOL(locks_owner_has_blockers);

As a subsequent clean up (which anyone can do at a later point),
a similar change could be done to check_for_locks(). This bit of
code seems to appear in several other filesystems, for example:

I used check_for_locks as reference for locks_owner_has_blockers
so both should be updated to be more kABI friendly as you suggested.
Can I update both in a subsequent cleanup patch? I plan to have
a number of small cleanup up patches for these and also some nits.

Thanks,
-Dai



7643         inode = locks_inode(nf->nf_file);
7644         flctx = inode->i_flctx;
7645
7646         if (flctx && !list_empty_careful(&flctx->flc_posix)) {
7647                 spin_lock(&flctx->flc_lock);
7648                 list_for_each_entry(fl, &flctx->flc_posix, fl_list) {
7649                         if (fl->fl_owner == (fl_owner_t)lowner) {
7650                                 status = true;
7651                                 break;
7652                         }
7653                 }
7654                 spin_unlock(&flctx->flc_lock);
7655         }


--
Chuck Lever






[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux