Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <822387b7-4895-4e64-5806-0f56b5d6c447@bytedance.com>
Date:   Tue, 10 Oct 2023 10:26:39 +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] mm: add printf attribute to shrinker_debugfs_name_alloc
Content-Language: en-US
To:     Andrew Morton <akpm@linux-foundation.org>,
        Lucy Mielke <lucymielke@icloud.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org
References: <ZSBue-3kM6gI6jCr@mainframe>
 <20231009175006.326e61e6c0e46e6ce78699ff@linux-foundation.org>
From:   Qi Zheng <zhengqi.arch@bytedance.com>
In-Reply-To: <20231009175006.326e61e6c0e46e6ce78699ff@linux-foundation.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On 2023/10/10 08:50, Andrew Morton wrote:
> On Fri, 6 Oct 2023 22:30:51 +0200 Lucy Mielke <lucymielke@icloud.com> wrote:
> 
>> This fixes a compiler warning when compiling an allyesconfig with W=1:
>>
>> mm/internal.h:1235:9: error: function might be a candidate for ‘gnu_printf’
>> format attribute [-Werror=suggest-attribute=format]
> 
> Thanks.  I added
> 
> Fixes: c42d50aefd17 ("mm: shrinker: add infrastructure for dynamically allocating shrinker")
> 
> to this.

Thanks.

> 
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -1229,8 +1229,8 @@ 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)
>> +static inline __printf(2, 0) int shrinker_debugfs_name_alloc(
>> +			struct shrinker *shrinker, const char *fmt, va_list ap)
>>   {
>>   	shrinker->name = kvasprintf_const(GFP_KERNEL, fmt, ap);

And there is also a warning introduced by c42d50aefd17:

mm/shrinker.c: In function 'shrinker_alloc':
mm/shrinker.c:688:2: error: function 'shrinker_alloc' might be a 
candidate for 'gnu_printf' format attribute 
[-Werror=suggest-attribute=format]
   688 |  err = shrinker_debugfs_name_alloc(shrinker, fmt, ap);
       |  ^~~

This also needs to be fixed:

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index e4f93120e0ab..1a00be90d93a 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -131,6 +131,7 @@ struct shrinker {
   */
  #define SHRINKER_NONSLAB       BIT(4)

+__printf(2, 3)
  struct shrinker *shrinker_alloc(unsigned int flags, const char *fmt, ...);
  void shrinker_register(struct shrinker *shrinker);
  void shrinker_free(struct shrinker *shrinker);

Other than that, LGTM.

Reviewed-by: Qi Zheng <zhengqi.arch@bytedance.com>

Thanks,
Qi

>>   
>> -- 
>> 2.42.0