lkml.org 
[lkml]   [2022]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 7/7] configfs: Fix LOCKDEP nesting issues with fragment semaphores
Can you send me the whole series?  Reviewing a patch 7 out of 7 without
context is basically impossible.

On Mon, Jul 04, 2022 at 04:42:49PM +0100, Mike Leach wrote:
> CoreSight uses configfs to represent the user interface to programmed
> configurations, which can be loaded and unloaded dynamically via configfs.
>
> These add and remove configurations using register/unregister group
> calls.
>
> It has been found that if CONFIG_LOCKDEP is enabled, then it appears to
> be confused by the nesting inherent in the fragment semaphores used
> by groups and the underlying subsystem.
>
> This patch sets up a mechanism to use separate classes for the fragment
> semaphores, in a similar way to that already in place to fix nesting
> issues with the i_mutexes.
>
> Cc: Joel Becker <jlbec@evilplan.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
> fs/configfs/configfs_internal.h | 3 +++
> fs/configfs/dir.c | 45 +++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/fs/configfs/configfs_internal.h b/fs/configfs/configfs_internal.h
> index c0395363eab9..736c74ec4b7a 100644
> --- a/fs/configfs/configfs_internal.h
> +++ b/fs/configfs/configfs_internal.h
> @@ -22,6 +22,9 @@ struct configfs_fragment {
> atomic_t frag_count;
> struct rw_semaphore frag_sem;
> bool frag_dead;
> +#ifdef CONFIG_LOCKDEP
> + int frag_depth;
> +#endif
> };
>
> void put_fragment(struct configfs_fragment *);
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index d1f9d2632202..6ecd8961afc3 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -133,6 +133,41 @@ configfs_adjust_dir_dirent_depth_after_populate(struct configfs_dirent *sd)
> sd->s_depth = -1;
> }
>
> +/* fragment semaphore needs some lockdep handling */
> +static struct lock_class_key default_frag_class[MAX_LOCK_DEPTH];
> +
> +/*
> + * Set the lockdep depth for a new fragment based on the parent frag depth.
> + * Called from register_subsystem() with NULL parent group to set root subsystem
> + * depth which defaults to 0 in a new fragment, and from register_group() with the
> + * parent group to set a new group fragment based on the parent fragment depth.
> + *
> + * Prevents lockdep getting upset on the unregister_group() call if it cannot
> + * understand the hierarchy of fragments.
> + */
> +static void configfs_adjust_frag_depth(struct configfs_fragment *frag,
> + struct config_group *parent_group)
> +{
> + struct configfs_dirent *parent_dirent;
> +
> + if (parent_group) {
> + // find parent frag
> + parent_dirent = parent_group->cg_item.ci_dentry->d_fsdata;
> + frag->frag_depth = parent_dirent->s_frag->frag_depth + 1;
> + }
> +
> + if (frag->frag_depth < ARRAY_SIZE(default_frag_class)) {
> + lockdep_set_class(&frag->frag_sem,
> + &default_frag_class[frag->frag_depth]);
> + } else {
> + /*
> + * In practice the maximum level of locking depth is
> + * already reached. Just inform about possible reasons.
> + */
> + pr_info("Too many levels of fragments for the locking correctness validator.\n");
> + }
> +}
> +
> #else /* CONFIG_LOCKDEP */
>
> static void configfs_init_dirent_depth(struct configfs_dirent *sd)
> @@ -154,6 +189,11 @@ configfs_adjust_dir_dirent_depth_after_populate(struct configfs_dirent *sd)
> {
> }
>
> +static void configfs_adjust_frag_depth(struct configfs_fragment *frag,
> + struct config_group *parent_group)
> +{
> +}
> +
> #endif /* CONFIG_LOCKDEP */
>
> static struct configfs_fragment *new_fragment(void)
> @@ -165,6 +205,9 @@ static struct configfs_fragment *new_fragment(void)
> atomic_set(&p->frag_count, 1);
> init_rwsem(&p->frag_sem);
> p->frag_dead = false;
> +#ifdef CONFIG_LOCKDEP
> + p->frag_depth = 0;
> +#endif
> }
> return p;
> }
> @@ -1742,6 +1785,7 @@ int configfs_register_group(struct config_group *parent_group,
> parent = parent_group->cg_item.ci_dentry;
>
> inode_lock_nested(d_inode(parent), I_MUTEX_PARENT);
> + configfs_adjust_frag_depth(frag, parent_group);
> ret = create_default_group(parent_group, group, frag);
> if (ret)
> goto err_out;
> @@ -1872,6 +1916,7 @@ int configfs_register_subsystem(struct configfs_subsystem *subsys)
> mutex_unlock(&configfs_subsystem_mutex);
>
> inode_lock_nested(d_inode(root), I_MUTEX_PARENT);
> + configfs_adjust_frag_depth(frag, NULL);
>
> err = -ENOMEM;
> dentry = d_alloc_name(root, group->cg_item.ci_name);
> --
> 2.17.1
---end quoted text---

\
 
 \ /
  Last update: 2022-07-06 09:20    [W:2.693 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site