Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
In-Reply-To: <871qu8tiys.wl-tiwai@suse.de>
References: <20220722142916.29435-1-tiwai@suse.de> <20220722142916.29435-4-tiwai@suse.de>
 <0350c21bcfdc896f2b912363f221958d41ebf1e1.camel@perches.com>
 <87edyc2r2e.wl-tiwai@suse.de> <CAKYAXd_tohLszyrThNLE5tPHt=2Z8Xtt=hzzEQe3iqf0t549EQ@mail.gmail.com>
 <871qu8tiys.wl-tiwai@suse.de>
From:   Namjae Jeon <linkinjeon@kernel.org>
Date:   Tue, 26 Jul 2022 16:54:59 +0900
Message-ID: <CAKYAXd8OAQ91dude9_-rWJ1tDCb4NM4rgdASKbmz4Tiu46cdzg@mail.gmail.com>
Subject: Re: [PATCH 3/4] exfat: Expand exfat_err() and co directly to pr_*() macro
To:     Takashi Iwai <tiwai@suse.de>
Cc:     Joe Perches <joe@perches.com>, linux-fsdevel@vger.kernel.org,
        Sungjong Seo <sj1557.seo@samsung.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

2022-07-26 16:46 GMT+09:00, Takashi Iwai <tiwai@suse.de>:
> On Tue, 26 Jul 2022 09:02:40 +0200,
> Namjae Jeon wrote:
>>
>> 2022-07-23 17:04 GMT+09:00, Takashi Iwai <tiwai@suse.de>:
>> > On Sat, 23 Jul 2022 09:42:12 +0200,
>> > Joe Perches wrote:
>> >>
>> >> On Fri, 2022-07-22 at 16:29 +0200, Takashi Iwai wrote:
>> >> > Currently the error and info messages handled by exfat_err() and co
>> >> > are tossed to exfat_msg() function that does nothing but passes the
>> >> > strings with printk() invocation.  Not only that this is more
>> >> > overhead
>> >> > by the indirect calls, but also this makes harder to extend for the
>> >> > debug print usage; because of the direct printk() call, you cannot
>> >> > make it for dynamic debug or without debug like the standard helpers
>> >> > such as pr_debug() or dev_dbg().
>> >> >
>> >> > For addressing the problem, this patch replaces exfat_msg() function
>> >> > with a macro to expand to pr_*() directly.  This allows us to create
>> >> > exfat_debug() macro that is expanded to pr_debug() (which output can
>> >> > gracefully suppressed via dyndbg).
>> >> []
>> >> > diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
>> >> []
>> >> > @@ -508,14 +508,19 @@ void __exfat_fs_error(struct super_block *sb,
>> >> > int
>> >> > report, const char *fmt, ...)
>> >> >  #define exfat_fs_error_ratelimit(sb, fmt, args...) \
>> >> >  		__exfat_fs_error(sb, __ratelimit(&EXFAT_SB(sb)->ratelimit), \
>> >> >  		fmt, ## args)
>> >> > -void exfat_msg(struct super_block *sb, const char *lv, const char
>> >> > *fmt,
>> >> > ...)
>> >> > -		__printf(3, 4) __cold;
>> >> > +
>> >> > +/* expand to pr_xxx() with prefix */
>> >> > +#define exfat_msg(sb, lv, fmt, ...) \
>> >> > +	pr_##lv("exFAT-fs (%s): " fmt "\n", (sb)->s_id, ##__VA_ARGS__)
>> >> > +
>> >> >  #define exfat_err(sb, fmt, ...)						\
>> >> > -	exfat_msg(sb, KERN_ERR, fmt, ##__VA_ARGS__)
>> >> > +	exfat_msg(sb, err, fmt, ##__VA_ARGS__)
>> >> >  #define exfat_warn(sb, fmt, ...)					\
>> >> > -	exfat_msg(sb, KERN_WARNING, fmt, ##__VA_ARGS__)
>> >> > +	exfat_msg(sb, warn, fmt, ##__VA_ARGS__)
>> >> >  #define exfat_info(sb, fmt, ...)					\
>> >> > -	exfat_msg(sb, KERN_INFO, fmt, ##__VA_ARGS__)
>> >> > +	exfat_msg(sb, info, fmt, ##__VA_ARGS__)
>> >> > +#define exfat_debug(sb, fmt, ...)					\
>> >> > +	exfat_msg(sb, debug, fmt, ##__VA_ARGS__)
>> >>
>> >> I think this would be clearer using pr_<level> directly instead
>> >> of an indirecting macro that uses concatenation of <level> that
>> >> obscures the actual use of pr_<level>
>> >>
>> >> Either: (and this first option would be my preference)
>> >>
>> >> #define exfat_err(sb, fmt, ...) \
>> >> 	pr_err("exFAT-fs (%s): " fmt "\n", (sb)->s_id, ##__VA_ARGS__)
>> >> #define exfat_warn(sb, fmt, ...) \
>> >> 	pr_warn("exFAT-fs (%s): " fmt "\n", (sb)->s_id, ##__VA_ARGS__)
>> >> etc...
>> >
>> > IMO, it's a matter of taste, and I don't mind either way.
>> > Just let me know.
>> Joe has already said that he prefers the first.
>
> My question was about the preference of the exfat maintainers :)
I also agree with his opinion.
>
>> Will you send v2 patch-set ?
>
> Sure.
Thanks a lot!
>
>
> Takashi
>