| Message ID | 1360354046-32392-3-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 02/08/2013 09:07 PM, Joe Hershberger wrote: > The prints are out of control. SILENCE! > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > --- > common/cmd_ubi.c | 3 +++ > drivers/mtd/mtdpart.c | 14 ++++++++------ > drivers/mtd/ubi/ubi.h | 3 ++- > fs/ubifs/ubifs.h | 2 +- > 4 files changed, 14 insertions(+), 8 deletions(-) > > diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c > index 01335dd..02b6b81 100644 > --- a/common/cmd_ubi.c > +++ b/common/cmd_ubi.c > @@ -23,6 +23,9 @@ > #include <asm/errno.h> > #include <jffs2/load_kernel.h> > > +#undef ubi_msg > +#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) > + > #define DEV_TYPE_NONE 0 > #define DEV_TYPE_NAND 1 > #define DEV_TYPE_ONENAND 2 > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index 96dcda2..2c60293 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master, > if (mtd_mod_by_eb(cur_offset, master) != 0) { > /* Round up to next erasesize */ > slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize; > - printk(KERN_NOTICE "Moving partition %d: " > - "0x%012llx -> 0x%012llx\n", partno, > - (unsigned long long)cur_offset, (unsigned long long)slave->offset); > + debug("Moving partition %d: 0x%012llx -> 0x%012llx\n", > + partno, (unsigned long long)cur_offset, > + (unsigned long long)slave->offset); > } > } > if (slave->mtd.size == MTDPART_SIZ_FULL) > slave->mtd.size = master->size - slave->offset; > > - printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset, > - (unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name); > + debug("0x%012llx-0x%012llx : \"%s\"\n", > + (unsigned long long)slave->offset, > + (unsigned long long)(slave->offset + slave->mtd.size), > + slave->mtd.name); > > /* let's do some sanity checks */ > if (slave->offset >= master->size) { > @@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master, > if (mtd_partitions.next == NULL) > INIT_LIST_HEAD(&mtd_partitions); > > - printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); > + debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); Not so sure about this one. Other users might find this message quite useful. Does this output really interfere with your UBI env handling? > for (i = 0; i < nbparts; i++) { > slave = add_one_partition(master, parts + i, i, cur_offset); > diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h > index 14c3a5f..f4ed7d8 100644 > --- a/drivers/mtd/ubi/ubi.h > +++ b/drivers/mtd/ubi/ubi.h > @@ -59,7 +59,8 @@ > #define UBI_NAME_STR "ubi" > > /* Normal UBI messages */ > -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) > +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \ > + ##__VA_ARGS__)*/ Hmmm.... > /* UBI warning messages */ > #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \ > __func__, ##__VA_ARGS__) > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h > index 0af471a..4ab1cbd 100644 > --- a/fs/ubifs/ubifs.h > +++ b/fs/ubifs/ubifs.h > @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry) > > /* Normal UBIFS messages */ > #define ubifs_msg(fmt, ...) \ > - printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__) > + /*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/ ... Not sure again about this silencing. And these "removed" printk's are really ugly. Could you please give an example of UBI usage (message logs from "ubi create ..., ubi part ..., ubi read ...") without this patch (or complete patchset) and with it? So that we see the real difference? Thanks, Stefan
Hi Stefan, Sorry for the delay. On Mon, Feb 11, 2013 at 4:52 AM, Stefan Roese <sr@denx.de> wrote: > On 02/08/2013 09:07 PM, Joe Hershberger wrote: >> The prints are out of control. SILENCE! >> >> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >> --- >> common/cmd_ubi.c | 3 +++ >> drivers/mtd/mtdpart.c | 14 ++++++++------ >> drivers/mtd/ubi/ubi.h | 3 ++- >> fs/ubifs/ubifs.h | 2 +- >> 4 files changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c >> index 01335dd..02b6b81 100644 >> --- a/common/cmd_ubi.c >> +++ b/common/cmd_ubi.c >> @@ -23,6 +23,9 @@ >> #include <asm/errno.h> >> #include <jffs2/load_kernel.h> >> >> +#undef ubi_msg >> +#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) >> + >> #define DEV_TYPE_NONE 0 >> #define DEV_TYPE_NAND 1 >> #define DEV_TYPE_ONENAND 2 >> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c >> index 96dcda2..2c60293 100644 >> --- a/drivers/mtd/mtdpart.c >> +++ b/drivers/mtd/mtdpart.c >> @@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master, >> if (mtd_mod_by_eb(cur_offset, master) != 0) { >> /* Round up to next erasesize */ >> slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize; >> - printk(KERN_NOTICE "Moving partition %d: " >> - "0x%012llx -> 0x%012llx\n", partno, >> - (unsigned long long)cur_offset, (unsigned long long)slave->offset); >> + debug("Moving partition %d: 0x%012llx -> 0x%012llx\n", >> + partno, (unsigned long long)cur_offset, >> + (unsigned long long)slave->offset); >> } >> } >> if (slave->mtd.size == MTDPART_SIZ_FULL) >> slave->mtd.size = master->size - slave->offset; >> >> - printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset, >> - (unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name); >> + debug("0x%012llx-0x%012llx : \"%s\"\n", >> + (unsigned long long)slave->offset, >> + (unsigned long long)(slave->offset + slave->mtd.size), >> + slave->mtd.name); >> >> /* let's do some sanity checks */ >> if (slave->offset >= master->size) { >> @@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master, >> if (mtd_partitions.next == NULL) >> INIT_LIST_HEAD(&mtd_partitions); >> >> - printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); >> + debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); > > Not so sure about this one. Other users might find this message quite > useful. Does this output really interfere with your UBI env handling? It makes it quite chatty. I'll show an example below. >> for (i = 0; i < nbparts; i++) { >> slave = add_one_partition(master, parts + i, i, cur_offset); >> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h >> index 14c3a5f..f4ed7d8 100644 >> --- a/drivers/mtd/ubi/ubi.h >> +++ b/drivers/mtd/ubi/ubi.h >> @@ -59,7 +59,8 @@ >> #define UBI_NAME_STR "ubi" >> >> /* Normal UBI messages */ >> -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) >> +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \ >> + ##__VA_ARGS__)*/ > > Hmmm.... Yes... this is ugly... I'll clean it up. >> /* UBI warning messages */ >> #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \ >> __func__, ##__VA_ARGS__) >> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h >> index 0af471a..4ab1cbd 100644 >> --- a/fs/ubifs/ubifs.h >> +++ b/fs/ubifs/ubifs.h >> @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry) >> >> /* Normal UBIFS messages */ >> #define ubifs_msg(fmt, ...) \ >> - printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__) >> + /*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/ > > ... Not sure again about this silencing. And these "removed" printk's > are really ugly. Agreed... I'll clean it up. > Could you please give an example of UBI usage (message logs from "ubi > create ..., ubi part ..., ubi read ...") without this patch (or complete > patchset) and with it? So that we see the real difference? Here is boot: <snip> NAND: 1024 MiB MMC: SDHCI: 0 UBI: attaching mtd1 to ubi0 UBI: physical eraseblock size: 131072 bytes (128 KiB) UBI: logical eraseblock size: 129024 bytes UBI: smallest flash I/O unit: 2048 UBI: sub-page size: 512 UBI: VID header offset: 512 (aligned 512) UBI: data offset: 2048 UBI: attached mtd1 to ubi0 UBI: MTD device name: "mtd=2" UBI: MTD device size: 70 MiB UBI: number of good PEBs: 560 UBI: number of bad PEBs: 0 UBI: max. allowed volumes: 128 UBI: wear-leveling threshold: 4096 UBI: number of internal volumes: 1 UBI: number of user volumes: 4 UBI: available PEBs: 0 UBI: total number of reserved PEBs: 560 UBI: number of PEBs reserved for bad PEB handling: 80 UBI: max/mean erase counter: 7/1 In: serial Out: serial Err: serial <snip> And here is a "env save" Saving Environment to UBI... UBI: mtd1 is detached from ubi0 UBI: attaching mtd1 to ubi0 UBI: physical eraseblock size: 131072 bytes (128 KiB) UBI: logical eraseblock size: 129024 bytes UBI: smallest flash I/O unit: 2048 UBI: sub-page size: 512 UBI: VID header offset: 512 (aligned 512) UBI: data offset: 2048 UBI: attached mtd1 to ubi0 UBI: MTD device name: "mtd=2" UBI: MTD device size: 70 MiB UBI: number of good PEBs: 560 UBI: number of bad PEBs: 0 UBI: max. allowed volumes: 128 UBI: wear-leveling threshold: 4096 UBI: number of internal volumes: 1 UBI: number of user volumes: 4 UBI: available PEBs: 0 UBI: total number of reserved PEBs: 560 UBI: number of PEBs reserved for bad PEB handling: 80 UBI: max/mean erase counter: 5/1 Writing to UBI... done It's pretty out of control. With this patch everything that starts with "UBI:" is gone. Only errors remain (if any). Cheers, -Joe
Hi Joe, On 20.03.2013 11:07, Joe Hershberger wrote: <snip> >>> /* Normal UBI messages */ >>> -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) >>> +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \ >>> + ##__VA_ARGS__)*/ >> >> Hmmm.... > > Yes... this is ugly... I'll clean it up. Thanks. >>> /* UBI warning messages */ >>> #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \ >>> __func__, ##__VA_ARGS__) >>> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h >>> index 0af471a..4ab1cbd 100644 >>> --- a/fs/ubifs/ubifs.h >>> +++ b/fs/ubifs/ubifs.h >>> @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry) >>> >>> /* Normal UBIFS messages */ >>> #define ubifs_msg(fmt, ...) \ >>> - printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__) >>> + /*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/ >> >> ... Not sure again about this silencing. And these "removed" printk's >> are really ugly. > > Agreed... I'll clean it up. > >> Could you please give an example of UBI usage (message logs from "ubi >> create ..., ubi part ..., ubi read ...") without this patch (or complete >> patchset) and with it? So that we see the real difference? > > Here is boot: > > <snip> > NAND: 1024 MiB > MMC: SDHCI: 0 > UBI: attaching mtd1 to ubi0 > UBI: physical eraseblock size: 131072 bytes (128 KiB) > UBI: logical eraseblock size: 129024 bytes > UBI: smallest flash I/O unit: 2048 > UBI: sub-page size: 512 > UBI: VID header offset: 512 (aligned 512) > UBI: data offset: 2048 > UBI: attached mtd1 to ubi0 > UBI: MTD device name: "mtd=2" > UBI: MTD device size: 70 MiB > UBI: number of good PEBs: 560 > UBI: number of bad PEBs: 0 > UBI: max. allowed volumes: 128 > UBI: wear-leveling threshold: 4096 > UBI: number of internal volumes: 1 > UBI: number of user volumes: 4 > UBI: available PEBs: 0 > UBI: total number of reserved PEBs: 560 > UBI: number of PEBs reserved for bad PEB handling: 80 > UBI: max/mean erase counter: 7/1 > In: serial > Out: serial > Err: serial > <snip> > > > And here is a "env save" > > > Saving Environment to UBI... > UBI: mtd1 is detached from ubi0 > UBI: attaching mtd1 to ubi0 > UBI: physical eraseblock size: 131072 bytes (128 KiB) > UBI: logical eraseblock size: 129024 bytes > UBI: smallest flash I/O unit: 2048 > UBI: sub-page size: 512 > UBI: VID header offset: 512 (aligned 512) > UBI: data offset: 2048 > UBI: attached mtd1 to ubi0 > UBI: MTD device name: "mtd=2" > UBI: MTD device size: 70 MiB > UBI: number of good PEBs: 560 > UBI: number of bad PEBs: 0 > UBI: max. allowed volumes: 128 > UBI: wear-leveling threshold: 4096 > UBI: number of internal volumes: 1 > UBI: number of user volumes: 4 > UBI: available PEBs: 0 > UBI: total number of reserved PEBs: 560 > UBI: number of PEBs reserved for bad PEB handling: 80 > UBI: max/mean erase counter: 5/1 > Writing to UBI... done > > > It's pretty out of control. > > With this patch everything that starts with "UBI:" is gone. Only > errors remain (if any). I see. This is definitely helpful for your use-case, env in UBI. But I would like to keep the UBI printf's for all other use cases. Or at least make it configurable. How about adding a switch/define, to optionally disable the UBI output? This seems to be the most flexible option to me. Thanks, Stefan
Hi Stefan, On Wed, Mar 20, 2013 at 5:14 AM, Stefan Roese <sr@denx.de> wrote: <snip> > I see. This is definitely helpful for your use-case, env in UBI. But I > would like to keep the UBI printf's for all other use cases. Or at least > make it configurable. That makes sense. > How about adding a switch/define, to optionally disable the UBI output? > This seems to be the most flexible option to me. Will do! -Joe
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/20/2013 06:19 AM, Joe Hershberger wrote: > Hi Stefan, > > On Wed, Mar 20, 2013 at 5:14 AM, Stefan Roese <sr@denx.de> wrote: > <snip> >> I see. This is definitely helpful for your use-case, env in UBI. >> But I would like to keep the UBI printf's for all other use >> cases. Or at least make it configurable. > > That makes sense. > >> How about adding a switch/define, to optionally disable the UBI >> output? This seems to be the most flexible option to me. > > > Will do! Or a "verbose" flag for some of the ubi commands? Or ubi info even? Just thinking out loud here (and I need to throw a UBI/UBIFS onto something), but all of those prints are handy for making sure you're trying to mount things right, etc. But yes, lots of prints are bad for time and shouldn't be the default. - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRSbV+AAoJENk4IS6UOR1WCOUQAKN741OE2IV+f8EcKyC9JaP9 CuN7ZovoOFaq1yxjnM4HhWBinpntN+PeEI7HEnEeX/y8elnXlpuSknJqJ6ZvpgHs yRhnv2NGYCjQ9imVQ+99HNBJB14EDO/zKruw+0ztSmSVvIJ9j4aGFWXr28ot28je zQsbIgEK2kyJ1nBl70L8iSQOw45pTt9Ho4IxP9pofMzyZsf7hxLN1iYvmJ1V59A7 11XMgAFKvn8Lh6k2Ao/R/BfsQBhrFO0I1KcRH3ZX8u1ki0B8GL95UAgt/9L38nXl apRMRuwBRUQEb9Qyg3OlktF29D11z9IDhTu4dcTRKdCW4iOATtXWarOmgDuudwaQ yRvZMM6e9OYtbZJbIbSTRpJra7vh1gL45baGHSx3o1rUdYmOpOPTZg7dw0O9kepf oPzT5iDFY1rHOK6pv/ETPTjA6aeVPXoOaMFkDRq1Kifr4KmXry2Lrs2espbxvZ/W 4e3KoyBdPMqJnb4QbAKrkOekbr1iQjGBzThX08z0YJHW5KbxCwLed5dczKckHZx9 I+R7aCtx9bSTtCMBf7cL40owMdN7BOm3mWzFQqOPieFxxh0+N3af8UJyFTFZY9Bf zS/c/IFgCllqLPlznWUzc0RMLuoie+QU7FGskwYO6ayvjJO1zwRMrQ1zVflFBg9x Quq0moE72+fvfNi+7WZ6 =n3wP -----END PGP SIGNATURE-----
diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c index 01335dd..02b6b81 100644 --- a/common/cmd_ubi.c +++ b/common/cmd_ubi.c @@ -23,6 +23,9 @@ #include <asm/errno.h> #include <jffs2/load_kernel.h> +#undef ubi_msg +#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) + #define DEV_TYPE_NONE 0 #define DEV_TYPE_NAND 1 #define DEV_TYPE_ONENAND 2 diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index 96dcda2..2c60293 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -347,16 +347,18 @@ static struct mtd_part *add_one_partition(struct mtd_info *master, if (mtd_mod_by_eb(cur_offset, master) != 0) { /* Round up to next erasesize */ slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize; - printk(KERN_NOTICE "Moving partition %d: " - "0x%012llx -> 0x%012llx\n", partno, - (unsigned long long)cur_offset, (unsigned long long)slave->offset); + debug("Moving partition %d: 0x%012llx -> 0x%012llx\n", + partno, (unsigned long long)cur_offset, + (unsigned long long)slave->offset); } } if (slave->mtd.size == MTDPART_SIZ_FULL) slave->mtd.size = master->size - slave->offset; - printk(KERN_NOTICE "0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset, - (unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name); + debug("0x%012llx-0x%012llx : \"%s\"\n", + (unsigned long long)slave->offset, + (unsigned long long)(slave->offset + slave->mtd.size), + slave->mtd.name); /* let's do some sanity checks */ if (slave->offset >= master->size) { @@ -463,7 +465,7 @@ int add_mtd_partitions(struct mtd_info *master, if (mtd_partitions.next == NULL) INIT_LIST_HEAD(&mtd_partitions); - printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); + debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); for (i = 0; i < nbparts; i++) { slave = add_one_partition(master, parts + i, i, cur_offset); diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 14c3a5f..f4ed7d8 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -59,7 +59,8 @@ #define UBI_NAME_STR "ubi" /* Normal UBI messages */ -#define ubi_msg(fmt, ...) printk(KERN_NOTICE "UBI: " fmt "\n", ##__VA_ARGS__) +#define ubi_msg(fmt, ...) /*printk(KERN_NOTICE "UBI: " fmt "\n", \ + ##__VA_ARGS__)*/ /* UBI warning messages */ #define ubi_warn(fmt, ...) printk(KERN_WARNING "UBI warning: %s: " fmt "\n", \ __func__, ##__VA_ARGS__) diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 0af471a..4ab1cbd 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -464,7 +464,7 @@ static inline ino_t parent_ino(struct dentry *dentry) /* Normal UBIFS messages */ #define ubifs_msg(fmt, ...) \ - printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__) + /*printk(KERN_NOTICE "UBIFS: " fmt "\n", ##__VA_ARGS__)*/ /* UBIFS error messages */ #define ubifs_err(fmt, ...) \ printk(KERN_ERR "UBIFS error (pid %d): %s: " fmt "\n", 0, \
The prints are out of control. SILENCE! Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> --- common/cmd_ubi.c | 3 +++ drivers/mtd/mtdpart.c | 14 ++++++++------ drivers/mtd/ubi/ubi.h | 3 ++- fs/ubifs/ubifs.h | 2 +- 4 files changed, 14 insertions(+), 8 deletions(-)