| 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
| ||
|
Date: Sat, 23 Apr 2022 09:46:25 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Roman Gushchin <roman.gushchin@...ux.dev>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Cc: Dave Chinner <dchinner@...hat.com>, linux-kernel@...r.kernel.org,
Yang Shi <shy828301@...il.com>,
Kent Overstreet <kent.overstreet@...il.com>,
Hillf Danton <hdanton@...a.com>
Subject: Re: [PATCH v2 5/7] mm: provide shrinkers with names
Hi,
Le 22/04/2022 à 22:26, Roman Gushchin a écrit :
> Currently shrinkers are anonymous objects. For debugging purposes they
> can be identified by count/scan function names, but it's not always
> useful: e.g. for superblock's shrinkers it's nice to have at least
> an idea of to which superblock the shrinker belongs.
>
> This commit adds names to shrinkers. register_shrinker() and
> prealloc_shrinker() functions are extended to take a format and
> arguments to master a name. If CONFIG_SHRINKER_DEBUG is on,
> the name is saved until the corresponding debugfs object is created,
> otherwise it's simple ignored.
>
> After this change the shrinker debugfs directory looks like:
> $ cd /sys/kernel/debug/shrinker/
> $ ls
> dqcache-16 sb-cgroup2-30 sb-hugetlbfs-33 sb-proc-41 sb-selinuxfs-22 sb-tmpfs-40 sb-zsmalloc-19
> kfree_rcu-0 sb-configfs-23 sb-iomem-12 sb-proc-44 sb-sockfs-8 sb-tmpfs-42 shadow-18
> sb-aio-20 sb-dax-11 sb-mqueue-21 sb-proc-45 sb-sysfs-26 sb-tmpfs-43 thp_deferred_split-10
> sb-anon_inodefs-15 sb-debugfs-7 sb-nsfs-4 sb-proc-47 sb-tmpfs-1 sb-tmpfs-46 thp_zero-9
> sb-bdev-3 sb-devpts-28 sb-pipefs-14 sb-pstore-31 sb-tmpfs-27 sb-tmpfs-49 xfs_buf-37
> sb-bpf-32 sb-devtmpfs-5 sb-proc-25 sb-rootfs-2 sb-tmpfs-29 sb-tracefs-13 xfs_inodegc-38
> sb-btrfs-24 sb-hugetlbfs-17 sb-proc-39 sb-securityfs-6 sb-tmpfs-35 sb-xfs-36 zspool-34
>
> Signed-off-by: Roman Gushchin <roman.gushchin@...ux.dev>
> ---
[...]
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 59f91e392a2a..1b326b93155c 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7383,7 +7383,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> conf->shrinker.count_objects = raid5_cache_count;
> conf->shrinker.batch = 128;
> conf->shrinker.flags = 0;
> - if (register_shrinker(&conf->shrinker)) {
> + if (register_shrinker(&conf->shrinker, "md")) {
Based on pr_warn below, does it make sense to have something like:
register_shrinker(&conf->shrinker, "md-%s", mdname(mddev))
> pr_warn("md/raid:%s: couldn't register shrinker.\n",
> mdname(mddev));
> goto abort;
[...]
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 121a54a1602b..6025cfda4932 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -613,7 +613,7 @@ static unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru,
> /*
> * Add a shrinker callback to be called from the vm.
> */
> -int prealloc_shrinker(struct shrinker *shrinker)
> +static int __prealloc_shrinker(struct shrinker *shrinker)
> {
> unsigned int size;
> int err;
> @@ -637,6 +637,34 @@ int prealloc_shrinker(struct shrinker *shrinker)
> return 0;
> }
>
> +#ifdef CONFIG_SHRINKER_DEBUG
> +int prealloc_shrinker(struct shrinker *shrinker, const char *fmt, ...)
> +{
> + int err;
> + char buf[64];
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vscnprintf(buf, sizeof(buf), fmt, ap);
> + va_end(ap);
> +
> + shrinker->name = kstrdup(buf, GFP_KERNEL);
> + if (!shrinker->name)
> + return -ENOMEM;
use kvasprintf_const() (and kfree_const() elsewhere) to simplify code
and take advantage of kstrdup_const() in most cases?
> +
> + err = __prealloc_shrinker(shrinker);
> + if (err)
> + kfree(shrinker->name);
> +
> + return err;
> +}
> +#else
> +int prealloc_shrinker(struct shrinker *shrinker, const char *fmt, ...)
> +{
> + return __prealloc_shrinker(shrinker);
> +}
> +#endif
> +
> void free_prealloced_shrinker(struct shrinker *shrinker)
> {
> if (shrinker->flags & SHRINKER_MEMCG_AWARE) {
> @@ -648,6 +676,9 @@ void free_prealloced_shrinker(struct shrinker *shrinker)
>
> kfree(shrinker->nr_deferred);
> shrinker->nr_deferred = NULL;
> +#ifdef CONFIG_SHRINKER_DEBUG
> + kfree(shrinker->name);
> +#endif
> }
>
> void register_shrinker_prepared(struct shrinker *shrinker)
> @@ -659,15 +690,38 @@ void register_shrinker_prepared(struct shrinker *shrinker)
> up_write(&shrinker_rwsem);
> }
>
> -int register_shrinker(struct shrinker *shrinker)
> +static int __register_shrinker(struct shrinker *shrinker)
> {
> - int err = prealloc_shrinker(shrinker);
> + int err = __prealloc_shrinker(shrinker);
>
> if (err)
> return err;
> register_shrinker_prepared(shrinker);
> return 0;
> }
> +
> +#ifdef CONFIG_SHRINKER_DEBUG
> +int register_shrinker(struct shrinker *shrinker, const char *fmt, ...)
> +{
> + char buf[64];
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vscnprintf(buf, sizeof(buf), fmt, ap);
> + va_end(ap);
> +
> + shrinker->name = kstrdup(buf, GFP_KERNEL);
> + if (!shrinker->name)
> + return -ENOMEM;
> +
same as above.
> + return __register_shrinker(shrinker);
Missing error handling and freeing of shrinker->name as done in
prealloc_shrinker()?
CJ
> +}
> +#else
> +int register_shrinker(struct shrinker *shrinker, const char *fmt, ...)
> +{
> + return __register_shrinker(shrinker);
> +}
> +#endif
> EXPORT_SYMBOL(register_shrinker);
>
> /*
[...]
Powered by blists - more mailing lists