| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <14d66c0d-b436-7ac8-9e4b-21cfdf9631e6@ya.ru>
Date: Thu, 9 Mar 2023 22:49:31 +0300
From: Kirill Tkhai <tkhai@...ru>
To: Qi Zheng <zhengqi.arch@...edance.com>, akpm@...ux-foundation.org,
hannes@...xchg.org, shakeelb@...gle.com, mhocko@...nel.org,
roman.gushchin@...ux.dev, muchun.song@...ux.dev, david@...hat.com,
shy828301@...il.com, rppt@...nel.org
Cc: sultan@...neltoast.com, dave@...olabs.net,
penguin-kernel@...ove.SAKURA.ne.jp, paulmck@...nel.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 8/8] mm: shrinkers: convert shrinker_rwsem to mutex
On 07.03.2023 09:56, Qi Zheng wrote:
> Now there are no readers of shrinker_rwsem, so we
> can simply replace it with mutex lock.
>
> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
Acked-by: Kirill Tkhai <tkhai@...ru>
> ---
> drivers/md/dm-cache-metadata.c | 2 +-
> drivers/md/dm-thin-metadata.c | 2 +-
> fs/super.c | 2 +-
> mm/shrinker_debug.c | 14 +++++++-------
> mm/vmscan.c | 34 +++++++++++++++++-----------------
> 5 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
> index acffed750e3e..9e0c69958587 100644
> --- a/drivers/md/dm-cache-metadata.c
> +++ b/drivers/md/dm-cache-metadata.c
> @@ -1828,7 +1828,7 @@ int dm_cache_metadata_abort(struct dm_cache_metadata *cmd)
> * Replacement block manager (new_bm) is created and old_bm destroyed outside of
> * cmd root_lock to avoid ABBA deadlock that would result (due to life-cycle of
> * shrinker associated with the block manager's bufio client vs cmd root_lock).
> - * - must take shrinker_rwsem without holding cmd->root_lock
> + * - must take shrinker_mutex without holding cmd->root_lock
> */
> new_bm = dm_block_manager_create(cmd->bdev, DM_CACHE_METADATA_BLOCK_SIZE << SECTOR_SHIFT,
> CACHE_MAX_CONCURRENT_LOCKS);
> diff --git a/drivers/md/dm-thin-metadata.c b/drivers/md/dm-thin-metadata.c
> index fd464fb024c3..9f5cb52c5763 100644
> --- a/drivers/md/dm-thin-metadata.c
> +++ b/drivers/md/dm-thin-metadata.c
> @@ -1887,7 +1887,7 @@ int dm_pool_abort_metadata(struct dm_pool_metadata *pmd)
> * Replacement block manager (new_bm) is created and old_bm destroyed outside of
> * pmd root_lock to avoid ABBA deadlock that would result (due to life-cycle of
> * shrinker associated with the block manager's bufio client vs pmd root_lock).
> - * - must take shrinker_rwsem without holding pmd->root_lock
> + * - must take shrinker_mutex without holding pmd->root_lock
> */
> new_bm = dm_block_manager_create(pmd->bdev, THIN_METADATA_BLOCK_SIZE << SECTOR_SHIFT,
> THIN_MAX_CONCURRENT_LOCKS);
> diff --git a/fs/super.c b/fs/super.c
> index 84332d5cb817..91a4037b1d95 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -54,7 +54,7 @@ static char *sb_writers_name[SB_FREEZE_LEVELS] = {
> * One thing we have to be careful of with a per-sb shrinker is that we don't
> * drop the last active reference to the superblock from within the shrinker.
> * If that happens we could trigger unregistering the shrinker from within the
> - * shrinker path and that leads to deadlock on the shrinker_rwsem. Hence we
> + * shrinker path and that leads to deadlock on the shrinker_mutex. Hence we
> * take a passive reference to the superblock to avoid this from occurring.
> */
> static unsigned long super_cache_scan(struct shrinker *shrink,
> diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c
> index 6aa7a7ec69da..b0f6aff372df 100644
> --- a/mm/shrinker_debug.c
> +++ b/mm/shrinker_debug.c
> @@ -7,7 +7,7 @@
> #include <linux/memcontrol.h>
>
> /* defined in vmscan.c */
> -extern struct rw_semaphore shrinker_rwsem;
> +extern struct mutex shrinker_mutex;
> extern struct list_head shrinker_list;
> extern struct srcu_struct shrinker_srcu;
>
> @@ -167,7 +167,7 @@ int shrinker_debugfs_add(struct shrinker *shrinker)
> char buf[128];
> int id;
>
> - lockdep_assert_held(&shrinker_rwsem);
> + lockdep_assert_held(&shrinker_mutex);
>
> /* debugfs isn't initialized yet, add debugfs entries later. */
> if (!shrinker_debugfs_root)
> @@ -210,7 +210,7 @@ int shrinker_debugfs_rename(struct shrinker *shrinker, const char *fmt, ...)
> if (!new)
> return -ENOMEM;
>
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
>
> old = shrinker->name;
> shrinker->name = new;
> @@ -228,7 +228,7 @@ int shrinker_debugfs_rename(struct shrinker *shrinker, const char *fmt, ...)
> shrinker->debugfs_entry = entry;
> }
>
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
>
> kfree_const(old);
>
> @@ -240,7 +240,7 @@ struct dentry *shrinker_debugfs_remove(struct shrinker *shrinker)
> {
> struct dentry *entry = shrinker->debugfs_entry;
>
> - lockdep_assert_held(&shrinker_rwsem);
> + lockdep_assert_held(&shrinker_mutex);
>
> kfree_const(shrinker->name);
> shrinker->name = NULL;
> @@ -265,14 +265,14 @@ static int __init shrinker_debugfs_init(void)
> shrinker_debugfs_root = dentry;
>
> /* Create debugfs entries for shrinkers registered at boot */
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> list_for_each_entry(shrinker, &shrinker_list, list)
> if (!shrinker->debugfs_entry) {
> ret = shrinker_debugfs_add(shrinker);
> if (ret)
> break;
> }
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
>
> return ret;
> }
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ac7ab4aa344f..c00302fabc3d 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -35,7 +35,7 @@
> #include <linux/cpuset.h>
> #include <linux/compaction.h>
> #include <linux/notifier.h>
> -#include <linux/rwsem.h>
> +#include <linux/mutex.h>
> #include <linux/delay.h>
> #include <linux/kthread.h>
> #include <linux/freezer.h>
> @@ -202,7 +202,7 @@ static void set_task_reclaim_state(struct task_struct *task,
> }
>
> LIST_HEAD(shrinker_list);
> -DECLARE_RWSEM(shrinker_rwsem);
> +DEFINE_MUTEX(shrinker_mutex);
> DEFINE_SRCU(shrinker_srcu);
> static atomic_t shrinker_srcu_generation = ATOMIC_INIT(0);
>
> @@ -225,7 +225,7 @@ static struct shrinker_info *shrinker_info_protected(struct mem_cgroup *memcg,
> {
> return srcu_dereference_check(memcg->nodeinfo[nid]->shrinker_info,
> &shrinker_srcu,
> - lockdep_is_held(&shrinker_rwsem));
> + lockdep_is_held(&shrinker_mutex));
> }
>
> static struct shrinker_info *shrinker_info_srcu(struct mem_cgroup *memcg,
> @@ -310,7 +310,7 @@ int alloc_shrinker_info(struct mem_cgroup *memcg)
> int nid, size, ret = 0;
> int map_size, defer_size = 0;
>
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> map_size = shrinker_map_size(shrinker_nr_max);
> defer_size = shrinker_defer_size(shrinker_nr_max);
> size = map_size + defer_size;
> @@ -326,7 +326,7 @@ int alloc_shrinker_info(struct mem_cgroup *memcg)
> info->map_nr_max = shrinker_nr_max;
> rcu_assign_pointer(memcg->nodeinfo[nid]->shrinker_info, info);
> }
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
>
> return ret;
> }
> @@ -342,7 +342,7 @@ static int expand_shrinker_info(int new_id)
> if (!root_mem_cgroup)
> goto out;
>
> - lockdep_assert_held(&shrinker_rwsem);
> + lockdep_assert_held(&shrinker_mutex);
>
> map_size = shrinker_map_size(new_nr_max);
> defer_size = shrinker_defer_size(new_nr_max);
> @@ -392,7 +392,7 @@ static int prealloc_memcg_shrinker(struct shrinker *shrinker)
> if (mem_cgroup_disabled())
> return -ENOSYS;
>
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> id = idr_alloc(&shrinker_idr, shrinker, 0, 0, GFP_KERNEL);
> if (id < 0)
> goto unlock;
> @@ -406,7 +406,7 @@ static int prealloc_memcg_shrinker(struct shrinker *shrinker)
> shrinker->id = id;
> ret = 0;
> unlock:
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
> return ret;
> }
>
> @@ -416,7 +416,7 @@ static void unregister_memcg_shrinker(struct shrinker *shrinker)
>
> BUG_ON(id < 0);
>
> - lockdep_assert_held(&shrinker_rwsem);
> + lockdep_assert_held(&shrinker_mutex);
>
> idr_remove(&shrinker_idr, id);
> }
> @@ -451,7 +451,7 @@ void reparent_shrinker_deferred(struct mem_cgroup *memcg)
> parent = root_mem_cgroup;
>
> /* Prevent from concurrent shrinker_info expand */
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> for_each_node(nid) {
> child_info = shrinker_info_protected(memcg, nid);
> parent_info = shrinker_info_protected(parent, nid);
> @@ -460,7 +460,7 @@ void reparent_shrinker_deferred(struct mem_cgroup *memcg)
> atomic_long_add(nr, &parent_info->nr_deferred[i]);
> }
> }
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
> }
>
> static bool cgroup_reclaim(struct scan_control *sc)
> @@ -709,9 +709,9 @@ void free_prealloced_shrinker(struct shrinker *shrinker)
> shrinker->name = NULL;
> #endif
> if (shrinker->flags & SHRINKER_MEMCG_AWARE) {
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> unregister_memcg_shrinker(shrinker);
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
> return;
> }
>
> @@ -721,11 +721,11 @@ void free_prealloced_shrinker(struct shrinker *shrinker)
>
> void register_shrinker_prepared(struct shrinker *shrinker)
> {
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> list_add_tail_rcu(&shrinker->list, &shrinker_list);
> shrinker->flags |= SHRINKER_REGISTERED;
> shrinker_debugfs_add(shrinker);
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
> }
>
> static int __register_shrinker(struct shrinker *shrinker)
> @@ -775,13 +775,13 @@ void unregister_shrinker(struct shrinker *shrinker)
> if (!(shrinker->flags & SHRINKER_REGISTERED))
> return;
>
> - down_write(&shrinker_rwsem);
> + mutex_lock(&shrinker_mutex);
> list_del_rcu(&shrinker->list);
> shrinker->flags &= ~SHRINKER_REGISTERED;
> if (shrinker->flags & SHRINKER_MEMCG_AWARE)
> unregister_memcg_shrinker(shrinker);
> debugfs_entry = shrinker_debugfs_remove(shrinker);
> - up_write(&shrinker_rwsem);
> + mutex_unlock(&shrinker_mutex);
>
> atomic_inc(&shrinker_srcu_generation);
> synchronize_srcu(&shrinker_srcu);
Powered by blists - more mailing lists