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 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:

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