[PATCH] mtd: Replace the expert mode symbols with a single helper

[PATCH] mtd: Replace the expert mode symbols with a single helper

[Miquel Raynal]

Reduce the number of exported symbols by replacing:
- mtd_expert_analysis_warning (the error string)
- mtd_expert_analysis_mode (the boolean)
with a single helper:
- mtd_check_expert_analysis_mode

Calling this helper will both check/return the content of the internal
boolean -which is not exported anymore- and as well WARN_ONCE() the
user.

While on this function, make the error string local to the helper and
set it const.

Update all the consumers.

Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/mtdcore.c            | 21 +++++++++++++--------
 drivers/mtd/nand/core.c          |  2 +-
 drivers/mtd/nand/raw/nand_base.c |  2 +-
 drivers/mtd/nand/raw/nand_bbt.c  |  2 +-
 include/linux/mtd/mtd.h          |  3 +--
 5 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 70f492dce158..06bd05ef20d6 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -358,6 +358,19 @@ static int mtd_partname_debug_show(struct seq_file *s, void *p)
 
 DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug);
 
+static bool mtd_expert_analysis_mode;
+
+bool mtd_check_expert_analysis_mode(void)
+{
+	const char *mtd_expert_analysis_warning =
+		"Bad block checks have been entirely disabled.\n"
+		"This is only reserved for post-mortem forensics and debug purposes.\n"
+		"Never enable this mode if you do not know what you are doing!\n";
+
+	return WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning);
+}
+EXPORT_SYMBOL_GPL(mtd_check_expert_analysis_mode);
+
 static struct dentry *dfs_dir_mtd;
 
 static void mtd_debugfs_populate(struct mtd_info *mtd)
@@ -2370,14 +2383,6 @@ static struct backing_dev_info * __init mtd_bdi_init(const char *name)
 	return ret ? ERR_PTR(ret) : bdi;
 }
 
-char *mtd_expert_analysis_warning =
-	"Bad block checks have been entirely disabled.\n"
-	"This is only reserved for post-mortem forensics and debug purposes.\n"
-	"Never enable this mode if you do not know what you are doing!\n";
-EXPORT_SYMBOL_GPL(mtd_expert_analysis_warning);
-bool mtd_expert_analysis_mode;
-EXPORT_SYMBOL_GPL(mtd_expert_analysis_mode);
-
 static struct proc_dir_entry *proc_mtd;
 
 static int __init init_mtd(void)
diff --git a/drivers/mtd/nand/core.c b/drivers/mtd/nand/core.c
index 416947f28b67..3ff99a42a63f 100644
--- a/drivers/mtd/nand/core.c
+++ b/drivers/mtd/nand/core.c
@@ -21,7 +21,7 @@
  */
 bool nanddev_isbad(struct nand_device *nand, const struct nand_pos *pos)
 {
-	if (WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning))
+	if (mtd_check_expert_analysis_mode())
 		return false;
 
 	if (nanddev_bbt_is_initialized(nand)) {
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index e7b2ba016d8c..068ecf979033 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -321,7 +321,7 @@ static int nand_isbad_bbm(struct nand_chip *chip, loff_t ofs)
 	if (nand_region_is_secured(chip, ofs, mtd->erasesize))
 		return -EIO;
 
-	if (WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning))
+	if (mtd_check_expert_analysis_mode())
 		return 0;
 
 	if (chip->legacy.block_bad)
diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c
index ab630af3a309..a3723da2e0a0 100644
--- a/drivers/mtd/nand/raw/nand_bbt.c
+++ b/drivers/mtd/nand/raw/nand_bbt.c
@@ -1455,7 +1455,7 @@ int nand_isbad_bbt(struct nand_chip *this, loff_t offs, int allowbbt)
 	pr_debug("nand_isbad_bbt(): bbt info for offs 0x%08x: (block %d) 0x%02x\n",
 		 (unsigned int)offs, block, res);
 
-	if (WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning))
+	if (mtd_check_expert_analysis_mode())
 		return 0;
 
 	switch (res) {
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 1ffa933121f6..5af096325b4d 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -711,7 +711,6 @@ static inline int mtd_is_bitflip_or_eccerr(int err) {
 
 unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
 
-extern char *mtd_expert_analysis_warning;
-extern bool mtd_expert_analysis_mode;
+bool mtd_check_expert_analysis_mode(void);
 
 #endif /* __MTD_MTD_H__ */
-- 
2.27.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: Replace the expert mode symbols with a single helper

[Geert Uytterhoeven]

Hi Miquel,

On Thu, Jan 27, 2022 at 12:08 PM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
> Reduce the number of exported symbols by replacing:
> - mtd_expert_analysis_warning (the error string)
> - mtd_expert_analysis_mode (the boolean)
> with a single helper:
> - mtd_check_expert_analysis_mode
>
> Calling this helper will both check/return the content of the internal
> boolean -which is not exported anymore- and as well WARN_ONCE() the
> user.
>
> While on this function, make the error string local to the helper and
> set it const.
>
> Update all the consumers.
>
> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks for your patch!

> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -358,6 +358,19 @@ static int mtd_partname_debug_show(struct seq_file *s, void *p)
>
>  DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug);
>
> +static bool mtd_expert_analysis_mode;
> +
> +bool mtd_check_expert_analysis_mode(void)
> +{
> +       const char *mtd_expert_analysis_warning =
> +               "Bad block checks have been entirely disabled.\n"
> +               "This is only reserved for post-mortem forensics and debug purposes.\n"
> +               "Never enable this mode if you do not know what you are doing!\n";
> +
> +       return WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning);
> +}
> +EXPORT_SYMBOL_GPL(mtd_check_expert_analysis_mode);

Please put this inside #ifndef CONFIG_DEBUG_FS, as it wastes
176 bytes for the message, and probably the same for the WARN_ONCE(),
for production kernels.

> +
>  static struct dentry *dfs_dir_mtd;
>
>  static void mtd_debugfs_populate(struct mtd_info *mtd)

> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -711,7 +711,6 @@ static inline int mtd_is_bitflip_or_eccerr(int err) {
>
>  unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
>
> -extern char *mtd_expert_analysis_warning;
> -extern bool mtd_expert_analysis_mode;
> +bool mtd_check_expert_analysis_mode(void);

#ifdef CONFIG_DEBUG_FS
bool mtd_check_expert_analysis_mode(void);
#else
static inline bool mtd_check_expert_analysis_mode(void) { return false; }
#endif

>
>  #endif /* __MTD_MTD_H__ */

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: Replace the expert mode symbols with a single helper

[Miquel Raynal]

Hi Geert,

geert@linux-m68k.org wrote on Thu, 27 Jan 2022 13:01:18 +0100:

> Hi Miquel,
> 
> On Thu, Jan 27, 2022 at 12:08 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> > Reduce the number of exported symbols by replacing:
> > - mtd_expert_analysis_warning (the error string)
> > - mtd_expert_analysis_mode (the boolean)
> > with a single helper:
> > - mtd_check_expert_analysis_mode
> >
> > Calling this helper will both check/return the content of the internal
> > boolean -which is not exported anymore- and as well WARN_ONCE() the
> > user.
> >
> > While on this function, make the error string local to the helper and
> > set it const.
> >
> > Update all the consumers.
> >
> > Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> 
> Thanks for your patch!
> 
> > --- a/drivers/mtd/mtdcore.c
> > +++ b/drivers/mtd/mtdcore.c
> > @@ -358,6 +358,19 @@ static int mtd_partname_debug_show(struct seq_file *s, void *p)
> >
> >  DEFINE_SHOW_ATTRIBUTE(mtd_partname_debug);
> >
> > +static bool mtd_expert_analysis_mode;
> > +
> > +bool mtd_check_expert_analysis_mode(void)
> > +{
> > +       const char *mtd_expert_analysis_warning =
> > +               "Bad block checks have been entirely disabled.\n"
> > +               "This is only reserved for post-mortem forensics and debug purposes.\n"
> > +               "Never enable this mode if you do not know what you are doing!\n";
> > +
> > +       return WARN_ONCE(mtd_expert_analysis_mode, mtd_expert_analysis_warning);
> > +}
> > +EXPORT_SYMBOL_GPL(mtd_check_expert_analysis_mode);  
> 
> Please put this inside #ifndef CONFIG_DEBUG_FS, as it wastes
> 176 bytes for the message, and probably the same for the WARN_ONCE(),
> for production kernels.

Yes I will do that. I sent this version before seeing your first answer
about the #ifdef request and arguing about it.

> 
> > +
> >  static struct dentry *dfs_dir_mtd;
> >
> >  static void mtd_debugfs_populate(struct mtd_info *mtd)  
> 
> > --- a/include/linux/mtd/mtd.h
> > +++ b/include/linux/mtd/mtd.h
> > @@ -711,7 +711,6 @@ static inline int mtd_is_bitflip_or_eccerr(int err) {
> >
> >  unsigned mtd_mmap_capabilities(struct mtd_info *mtd);
> >
> > -extern char *mtd_expert_analysis_warning;
> > -extern bool mtd_expert_analysis_mode;
> > +bool mtd_check_expert_analysis_mode(void);  
> 
> #ifdef CONFIG_DEBUG_FS
> bool mtd_check_expert_analysis_mode(void);
> #else
> static inline bool mtd_check_expert_analysis_mode(void) { return false; }
> #endif
> 
> >
> >  #endif /* __MTD_MTD_H__ */  
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/