From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F2D6C46CA1 for ; Mon, 18 Sep 2023 12:07:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AC71A6B0317; Mon, 18 Sep 2023 08:07:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A77556B031C; Mon, 18 Sep 2023 08:07:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 919356B031F; Mon, 18 Sep 2023 08:07:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 8243E6B0317 for ; Mon, 18 Sep 2023 08:07:57 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 62F71B442A for ; Mon, 18 Sep 2023 12:07:57 +0000 (UTC) X-FDA: 81249594594.24.4E65123 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf08.hostedemail.com (Postfix) with ESMTP id A6040160018 for ; Mon, 18 Sep 2023 12:07:54 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=b7yutxra; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf08.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695038875; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6b4Dq3JaEn3NjqvHsSqb1vNTF921wb5a6jhyMAxCtW0=; b=zMWCAJYlvSioFlv/RwydwjmNvl+/T+Hd91k7tz8dCksDHGJ/0//EMT+jpgpTHcziRSpnfG rYXLKcZ1dvc1sSCUJdVRgs3TQLDrs+173BVe7BWnRRKfTYq1oM+6IPmWFiW3DtngyploNR VGjEASWG2IWIr4jte+N1lBy/aaG+HYg= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=b7yutxra; dmarc=pass (policy=quarantine) header.from=bytedance.com; spf=pass (imf08.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695038875; a=rsa-sha256; cv=none; b=46FxYZzpa4XazpP0bcuq9whIlPl7FHz++T/XJPf0vrBJ25NapAQ4n9kNKJ/Ns6tfY+wBqp PDhF7zXWHfibSXi85QhyYe5shmbJgWCwOoXShWIpk4uCDv+FtdUgHnPQwK5qKS3habsbGL /Oa2I+BfIS4Miqb340+aUIhtcdyXSz0= Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-1c470c1acbaso3244245ad.0 for ; Mon, 18 Sep 2023 05:07:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1695038873; x=1695643673; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=6b4Dq3JaEn3NjqvHsSqb1vNTF921wb5a6jhyMAxCtW0=; b=b7yutxra16+eJ6XCCbmBniOk+kks9mcC934eddiUrNSaPY+PCdPQYuRJHVp9Oq1Cwf zryuNcFqoXKy8phXj4YVZf8KAXfyd5eYWwXTSGE21kIz81EOjon+AkEwHKqtQUU99aPf R7wE7Pdjkbe9GNd7bDTG5ty6SJxp8VULDspBY5MuVgGFqFqqegPiuTLeAAGIaqpzd1DN PbVpkAWOmR1AM7Irv4kRnNJt9ikac/fxQjAybAJ+gcqDds5aewIqqB8i6JvZKoHQdjQT UNoE1AIfElLvu7Bzz4zjsY8R07Ooc9w4F6OKrmTg+6x+R9zNF/IYFpZDF82Oljq3rAs+ GF+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695038873; x=1695643673; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6b4Dq3JaEn3NjqvHsSqb1vNTF921wb5a6jhyMAxCtW0=; b=l4o2qcBfP1bOiya4Z/1hJx4ONKxPyG8OhTZHur8WH1b1kcm3shH0nLl3+0GJ/BJXwv UA1MiWVpxnTIVuOvlW9w5PKNeRht7rtWhArQNxxWb7rdkeyv9DhyniYyuRnyEHodbzX1 j6s9HAjkpm2/vQUcwuAWhBRFkGupe4bm1XqeoSacnJn2wqDui76tjdz4NdZkWPzUi1aE 2itO/M6fBwYHXHroi2m6TUnlH0HTCw05ysXzlZEiI7AqMyEvb0D3C1AvP8MI24Ic7TRA 5mxJlpBp8CqdLvgdlzeZfKfg0vXA1sN1HRth/G9CUA/cmTlHwJnHORgSFD4AXTP7fvJf 1wSg== X-Gm-Message-State: AOJu0Yz/rAKb0R5f+d4pinP+VhKKJnTQ6ai5eE8nCLd+JuL3nhqN1Os8 wCGQr938m50h+fmbKbNNcNVAuQ== X-Google-Smtp-Source: AGHT+IGTm05O9v2E1cq+E8lJ8Jpivdic35bUYowvO5HvMrEl/Jo2UXoyEa/sR/oNhyotFRmAjKAFmg== X-Received: by 2002:a17:902:e744:b0:1c0:bf60:ba82 with SMTP id p4-20020a170902e74400b001c0bf60ba82mr10598686plf.5.1695038873293; Mon, 18 Sep 2023 05:07:53 -0700 (PDT) Received: from [10.84.155.178] ([203.208.167.146]) by smtp.gmail.com with ESMTPSA id y7-20020a17090322c700b001c0a414695dsm8161538plg.62.2023.09.18.05.07.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Sep 2023 05:07:52 -0700 (PDT) Message-ID: <529bce6f-e23b-4b84-a7e6-cce3c12645aa@bytedance.com> Date: Mon, 18 Sep 2023 20:06:29 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v6 01/45] mm: shrinker: add infrastructure for dynamically allocating shrinker Content-Language: en-US To: Muchun Song , akpm@linux-foundation.org Cc: Qi Zheng , linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, david@fromorbit.com, tkhai@ya.ru, vbabka@suse.cz, roman.gushchin@linux.dev, djwong@kernel.org, brauner@kernel.org, paulmck@kernel.org, tytso@mit.edu, steven.price@arm.com, cel@kernel.org, senozhatsky@chromium.org, yujie.liu@intel.com, gregkh@linuxfoundation.org References: <20230911094444.68966-1-zhengqi.arch@bytedance.com> <20230911094444.68966-2-zhengqi.arch@bytedance.com> <4aff0e17-b40f-406d-65fd-72a2bacfcc1a@linux.dev> From: Qi Zheng In-Reply-To: <4aff0e17-b40f-406d-65fd-72a2bacfcc1a@linux.dev> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: A6040160018 X-Stat-Signature: k49fb5ykasun1g49eb8wooqfi1prq4gg X-HE-Tag: 1695038874-334794 X-HE-Meta: U2FsdGVkX1/ehC5mTDVSN9/7mXgXzjT01cZvv77yGQu+N41pZfAzrbve/qOAnVJk5ZBJ7ikR0++V5LjLQQ22jjYMRYIrBkjFm1eYOWkxx/gX7hzCRloc/7sQ05IJWvP/Cb7USV6kY9weu0iLMGkb5kX6lOVqgEBEj5JApB233rj+FD8QHvwnIMc3PhsnkRjZq1+QVaGhjI3d6l8PzJfUlqu6BWFxFwvh/aL3g+mOp7/egQVSp2kLrIpwd5HRG+D5RCkIfZlSEvKDsbHX89POewHQ+eymkPwKgS/I23L/tpUP/48m/jkdzhUV0xYWMhBd6TY4Pnsc711H1IzMBzjTk7MBy6+aOVrgBM3yY/EiTVHbRA/AmE+HwMwJiveSBHwZsLIezXzpix2KDBGxh1hXR7Z1Lb4hbNUVEp6JSRFOBn9nSLgb2kdfCgWFoWDAE20FIvmJhpWvOxSH582IzOfVIEc8MEQhz1dfSqn4zc0xTfhNgy8dCZcQOvPsVB4bcEcLv3tp2kOG1J4VctfwEygcTk/1y5K3WUnQOuIgVYPsQk7JTOfce+c6UflRBPcsFSyyKEahOp/jNWt9BC/H1yjnQYudG33KiEJg3YMHVmrqkWLD7Kaf/Dx7cG1ngPeTs39UYFSnfDWX8NQMkf/QkzEDu82awM3OUmDCStnAZGU58DKjq794bMDe4jqQfDS/eU+NcIoVEl7K1NUBt4LU/GD9nLXLE/ReDZ5m/x5fQ5Q//MjcCBdvcz7V50WLMttGQX5/XFviIDknYCtYLg2JqI/ToZqWj0PV99sB6HK5/6AmztEbaZIwDuPNFfnq7cfJH6aP7EzIj3wvnEWH15XeLJsPOAhllmuNNOHY8AFW5k4w270Ns0azGBUHtgVOudUGb7aBHeCAUZY5Khz8bnLF7q1wWSjG7PsZwwCs6mkGXnzs++kwGHK0pdRQ6k09s+BJZiHdv7JYiQh1RpucQ9Nfbq0 LrzrEPxu baQHsJMJx7ul1owIYvyYN/9LZIa8UtNnRRX7b4I2jm9Zo3p7uYsXp18870Yy2I+SV9sBPStrLXiXptOsiCisMuMUF6t/6uDjlPY8KMBTkX2DZ/N8m9WMHlrPa0RGu86Lx6njN8v+ySRuWkJVV+D2wXxqcHMTMQ656CpP/g8EYJMty+xw8VQg/cqHlokF2iGjfAHhdFUdj9SoTMkxB3/SjwVVutSENftFzUqxNJQtttrArO1wuDRDPYfi9ECoVg7Y2oNdmD3fZq7kZBgx1J5M+6NpJd2YM3p21jtSFS6NYR/Rz+3S1eOKVzEn1ZnxXzCg9GKAKH+S1KV/2u7x574C2VRJ2yAyIdn4Wyc6eQnYM5A6CUDdaNs8+XoHbS8xmAZrTahGdiVCXVV8yPNCd84keQgKPhgut2+LLwNbtaDX13p4ScFw= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2023/9/18 17:03, Muchun Song wrote: > > ... >> @@ -95,6 +97,11 @@ struct shrinker { >>    * non-MEMCG_AWARE shrinker should not have this flag set. >>    */ >>   #define SHRINKER_NONSLAB    (1 << 3) >> +#define SHRINKER_ALLOCATED    (1 << 4) > > It is better to add a comment here to tell users > it is only used by internals of shrinker. The users > are not supposed to pass this flag to shrink APIs. OK, will annotate SHRINKER_REGISTERED and SHRINKER_ALLOCATED as flags used internally. > >> + >> +struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, >> ...); >> +void shrinker_register(struct shrinker *shrinker); >> +void shrinker_free(struct shrinker *shrinker); >>   extern int __printf(2, 3) prealloc_shrinker(struct shrinker *shrinker, >>                           const char *fmt, ...); >> diff --git a/mm/internal.h b/mm/internal.h >> index 0471d6326d01..5587cae20ebf 100644 >> --- a/mm/internal.h >> +++ b/mm/internal.h >> @@ -1161,6 +1161,9 @@ unsigned long shrink_slab(gfp_t gfp_mask, int >> nid, struct mem_cgroup *memcg, >>   #ifdef CONFIG_SHRINKER_DEBUG >>   extern int shrinker_debugfs_add(struct shrinker *shrinker); >> +extern int shrinker_debugfs_name_alloc(struct shrinker *shrinker, >> +                       const char *fmt, va_list ap); >> +extern void shrinker_debugfs_name_free(struct shrinker *shrinker); >>   extern struct dentry *shrinker_debugfs_detach(struct shrinker >> *shrinker, >>                             int *debugfs_id); >>   extern void shrinker_debugfs_remove(struct dentry *debugfs_entry, >> @@ -1170,6 +1173,14 @@ static inline int shrinker_debugfs_add(struct >> shrinker *shrinker) >>   { >>       return 0; >>   } >> +static inline int shrinker_debugfs_name_alloc(struct shrinker *shrinker, >> +                          const char *fmt, va_list ap) >> +{ >> +    return 0; >> +} >> +static inline void shrinker_debugfs_name_free(struct shrinker *shrinker) >> +{ >> +} >>   static inline struct dentry *shrinker_debugfs_detach(struct shrinker >> *shrinker, >>                                int *debugfs_id) >>   { >> diff --git a/mm/shrinker.c b/mm/shrinker.c >> index a16cd448b924..201211a67827 100644 >> --- a/mm/shrinker.c >> +++ b/mm/shrinker.c >> @@ -550,6 +550,108 @@ unsigned long shrink_slab(gfp_t gfp_mask, int >> nid, struct mem_cgroup *memcg, >>       return freed; >>   } >> +struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, >> ...) >> +{ >> +    struct shrinker *shrinker; >> +    unsigned int size; >> +    va_list ap; >> +    int err; >> + >> +    shrinker = kzalloc(sizeof(struct shrinker), GFP_KERNEL); >> +    if (!shrinker) >> +        return NULL; >> + >> +    va_start(ap, fmt); >> +    err = shrinker_debugfs_name_alloc(shrinker, fmt, ap); >> +    va_end(ap); >> +    if (err) >> +        goto err_name; >> + >> +    shrinker->flags = flags | SHRINKER_ALLOCATED; >> +    shrinker->seeks = DEFAULT_SEEKS; >> + >> +    if (flags & SHRINKER_MEMCG_AWARE) { >> +        err = prealloc_memcg_shrinker(shrinker); >> +        if (err == -ENOSYS) >> +            shrinker->flags &= ~SHRINKER_MEMCG_AWARE; >> +        else if (err == 0) >> +            goto done; >> +        else >> +            goto err_flags; > > Actually, the code here is a little confusing me when I fist look > at it. I think there could be some improvements here. Something > like: > >         if (flags & SHRINKER_MEMCG_AWARE) { >                 err = prealloc_memcg_shrinker(shrinker); >                 if (err == -ENOSYS) { >                         /* Memcg is not supported and fallback to > non-memcg-aware shrinker. */ >                         shrinker->flags &= ~SHRINKER_MEMCG_AWARE; >                         goto non-memcg; >                 } > >                 if (err) >                     goto err_flags; >                 return shrinker; >         } > > non-memcg: >         [...] >         return shrinker; > > In this case, the code becomes more clear (at least for me). We have > split the > code into two part, one is handling memcg-aware case, another is > non-memcg-aware > case. Any side will have a explicit "return" keyword to return once > succeeds. > It is a little implicit that the previous one uses "goto done". > > And the tag of "non-memcg" is also a good annotation to tell us the > following > code handles non-memcg-aware case. Make sense, will do. > >> +    } >> + >> +    /* >> +     * The nr_deferred is available on per memcg level for memcg aware >> +     * shrinkers, so only allocate nr_deferred in the following cases: >> +     *  - non memcg aware shrinkers >> +     *  - !CONFIG_MEMCG >> +     *  - memcg is disabled by kernel command line >> +     */ >> +    size = sizeof(*shrinker->nr_deferred); >> +    if (flags & SHRINKER_NUMA_AWARE) >> +        size *= nr_node_ids; >> + >> +    shrinker->nr_deferred = kzalloc(size, GFP_KERNEL); >> +    if (!shrinker->nr_deferred) >> +        goto err_flags; >> + >> +done: >> +    return shrinker; >> + >> +err_flags: >> +    shrinker_debugfs_name_free(shrinker); >> +err_name: >> +    kfree(shrinker); >> +    return NULL; >> +} >> +EXPORT_SYMBOL_GPL(shrinker_alloc); >> + >> +void shrinker_register(struct shrinker *shrinker) >> +{ >> +    if (unlikely(!(shrinker->flags & SHRINKER_ALLOCATED))) { >> +        pr_warn("Must use shrinker_alloc() to dynamically allocate >> the shrinker"); >> +        return; >> +    } >> + >> +    down_write(&shrinker_rwsem); >> +    list_add_tail(&shrinker->list, &shrinker_list); >> +    shrinker->flags |= SHRINKER_REGISTERED; >> +    shrinker_debugfs_add(shrinker); >> +    up_write(&shrinker_rwsem); >> +} >> +EXPORT_SYMBOL_GPL(shrinker_register); >> + >> +void shrinker_free(struct shrinker *shrinker) >> +{ >> +    struct dentry *debugfs_entry = NULL; >> +    int debugfs_id; >> + >> +    if (!shrinker) >> +        return; >> + >> +    down_write(&shrinker_rwsem); >> +    if (shrinker->flags & SHRINKER_REGISTERED) { >> +        list_del(&shrinker->list); >> +        debugfs_entry = shrinker_debugfs_detach(shrinker, &debugfs_id); >> +        shrinker->flags &= ~SHRINKER_REGISTERED; >> +    } else { >> +        shrinker_debugfs_name_free(shrinker); > > We could remove shrinker_debugfs_name_free() calling from > shrinker_debugfs_detach(), then we could call > shrinker_debugfs_name_free() anyway, otherwise, it it a little > weird for me. And the srinker name is allocated from shrinker_alloc(), > so I think it it reasonable for shrinker_free() to free the > shrinker name. OK, will do. > > Thanks. > >> +    } >> + >> +    if (shrinker->flags & SHRINKER_MEMCG_AWARE) >> +        unregister_memcg_shrinker(shrinker); >> +    up_write(&shrinker_rwsem); >> + >> +    if (debugfs_entry) >> +        shrinker_debugfs_remove(debugfs_entry, debugfs_id); >> + >> +    kfree(shrinker->nr_deferred); >> +    shrinker->nr_deferred = NULL; >> + >> +    kfree(shrinker); >> +} >> +EXPORT_SYMBOL_GPL(shrinker_free); >> + >>   /* >>    * Add a shrinker callback to be called from the vm. >>    */ >> diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c >> index e4ce509f619e..38452f539f40 100644 >> --- a/mm/shrinker_debug.c >> +++ b/mm/shrinker_debug.c >> @@ -193,6 +193,20 @@ int shrinker_debugfs_add(struct shrinker *shrinker) >>       return 0; >>   } >> +int shrinker_debugfs_name_alloc(struct shrinker *shrinker, const char >> *fmt, >> +                va_list ap) >> +{ >> +    shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap); >> + >> +    return shrinker->name ? 0 : -ENOMEM; >> +} >> + >> +void shrinker_debugfs_name_free(struct shrinker *shrinker) >> +{ >> +    kfree_const(shrinker->name); >> +    shrinker->name = NULL; >> +} > > It it better to move both helpers to internal.h and mark them as inline > since both are very simple enough. OK, will do. Hi Andrew, below is the cleanup patch, which has a small conflict with [PATCH v6 41/45]: From 5bc2b77484f5cd4616e510158f91c8877bd033ad Mon Sep 17 00:00:00 2001 From: Qi Zheng Date: Mon, 18 Sep 2023 10:41:15 +0000 Subject: [PATCH] mm: shrinker: some cleanup Signed-off-by: Qi Zheng --- include/linux/shrinker.h | 14 ++++++++------ mm/internal.h | 17 ++++++++++++++--- mm/shrinker.c | 20 ++++++++++++-------- mm/shrinker_debug.c | 16 ---------------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h index 3f3fd9974ce5..f4a5249f00b2 100644 --- a/include/linux/shrinker.h +++ b/include/linux/shrinker.h @@ -88,16 +88,18 @@ struct shrinker { }; #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */ -/* Flags */ -#define SHRINKER_REGISTERED (1 << 0) -#define SHRINKER_NUMA_AWARE (1 << 1) -#define SHRINKER_MEMCG_AWARE (1 << 2) +/* Internal flags */ +#define SHRINKER_REGISTERED BIT(0) +#define SHRINKER_ALLOCATED BIT(1) + +/* Flags for users to use */ +#define SHRINKER_NUMA_AWARE BIT(2) +#define SHRINKER_MEMCG_AWARE BIT(3) /* * It just makes sense when the shrinker is also MEMCG_AWARE for now, * non-MEMCG_AWARE shrinker should not have this flag set. */ -#define SHRINKER_NONSLAB (1 << 3) -#define SHRINKER_ALLOCATED (1 << 4) +#define SHRINKER_NONSLAB BIT(4) struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...); void shrinker_register(struct shrinker *shrinker); diff --git a/mm/internal.h b/mm/internal.h index b9a116dce28e..0f418a11c7a8 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -1161,10 +1161,21 @@ unsigned long shrink_slab(gfp_t gfp_mask, int nid, struct mem_cgroup *memcg, int priority); #ifdef CONFIG_SHRINKER_DEBUG +static inline int shrinker_debugfs_name_alloc(struct shrinker *shrinker, + const char *fmt, va_list ap) +{ + shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap); + + return shrinker->name ? 0 : -ENOMEM; +} + +static inline void shrinker_debugfs_name_free(struct shrinker *shrinker) +{ + kfree_const(shrinker->name); + shrinker->name = NULL; +} + extern int shrinker_debugfs_add(struct shrinker *shrinker); -extern int shrinker_debugfs_name_alloc(struct shrinker *shrinker, - const char *fmt, va_list ap); -extern void shrinker_debugfs_name_free(struct shrinker *shrinker); extern struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker, int *debugfs_id); extern void shrinker_debugfs_remove(struct dentry *debugfs_entry, diff --git a/mm/shrinker.c b/mm/shrinker.c index 201211a67827..d1032a4d5684 100644 --- a/mm/shrinker.c +++ b/mm/shrinker.c @@ -572,18 +572,23 @@ struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...) if (flags & SHRINKER_MEMCG_AWARE) { err = prealloc_memcg_shrinker(shrinker); - if (err == -ENOSYS) + if (err == -ENOSYS) { + /* Memcg is not supported, fallback to non-memcg-aware shrinker. */ shrinker->flags &= ~SHRINKER_MEMCG_AWARE; - else if (err == 0) - goto done; - else + goto non_memcg; + } + + if (err) goto err_flags; + + return shrinker; } +non_memcg: /* * The nr_deferred is available on per memcg level for memcg aware * shrinkers, so only allocate nr_deferred in the following cases: - * - non memcg aware shrinkers + * - non-memcg-aware shrinkers * - !CONFIG_MEMCG * - memcg is disabled by kernel command line */ @@ -595,7 +600,6 @@ struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...) if (!shrinker->nr_deferred) goto err_flags; -done: return shrinker; err_flags: @@ -634,10 +638,10 @@ void shrinker_free(struct shrinker *shrinker) list_del(&shrinker->list); debugfs_entry = shrinker_debugfs_detach(shrinker, &debugfs_id); shrinker->flags &= ~SHRINKER_REGISTERED; - } else { - shrinker_debugfs_name_free(shrinker); } + shrinker_debugfs_name_free(shrinker); + if (shrinker->flags & SHRINKER_MEMCG_AWARE) unregister_memcg_shrinker(shrinker); up_write(&shrinker_rwsem); diff --git a/mm/shrinker_debug.c b/mm/shrinker_debug.c index 38452f539f40..24aebe7c24cc 100644 --- a/mm/shrinker_debug.c +++ b/mm/shrinker_debug.c @@ -193,20 +193,6 @@ int shrinker_debugfs_add(struct shrinker *shrinker) return 0; } -int shrinker_debugfs_name_alloc(struct shrinker *shrinker, const char *fmt, - va_list ap) -{ - shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap); - - return shrinker->name ? 0 : -ENOMEM; -} - -void shrinker_debugfs_name_free(struct shrinker *shrinker) -{ - kfree_const(shrinker->name); - shrinker->name = NULL; -} - int shrinker_debugfs_rename(struct shrinker *shrinker, const char *fmt, ...) { struct dentry *entry; @@ -255,8 +241,6 @@ struct dentry *shrinker_debugfs_detach(struct shrinker *shrinker, lockdep_assert_held(&shrinker_rwsem); - shrinker_debugfs_name_free(shrinker); - *debugfs_id = entry ? shrinker->debugfs_id : -1; shrinker->debugfs_entry = NULL; -- 2.30.2 > > Thanks. > >> + >>   int shrinker_debugfs_rename(struct shrinker *shrinker, const char >> *fmt, ...) >>   { >>       struct dentry *entry; >> @@ -241,8 +255,7 @@ struct dentry *shrinker_debugfs_detach(struct >> shrinker *shrinker, >>       lockdep_assert_held(&shrinker_rwsem); >> -    kfree_const(shrinker->name); >> -    shrinker->name = NULL; >> +    shrinker_debugfs_name_free(shrinker); >>       *debugfs_id = entry ? shrinker->debugfs_id : -1; >>       shrinker->debugfs_entry = NULL; >