[PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Joe Perches]

The existing logging macros are fairly large and converting the
macros to functions make the object code smaller.

Use %pV and __builtin_return_address(0) as appropriate.

Fix typo of sytem to system.

$ size fs/ubifs/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 575831	 309688	 161312	1046831	  ff92f	fs/ubifs/built-in.o.allyesconfig.new
 622457	 312872	 161120	1096449	 10bb01	fs/ubifs/built-in.o.allyesconfig.old
 223785	    640	    644	 225069	  36f2d	fs/ubifs/built-in.o.defconfig.new
 251873	    640	    644	 253157	  3dce5	fs/ubifs/built-in.o.defconfig.old

Signed-off-by: Joe Perches <joe@perches.com>
---

V2:	It seems I should use checkpatch after all.
	o Remove unnecessary line continuations leftover from macros
	o Fix sytem typo

 fs/ubifs/Makefile |  1 +
 fs/ubifs/misc.c   | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ubifs/ubifs.h  | 41 +++++++++++++++++----------------------
 3 files changed, 75 insertions(+), 24 deletions(-)
 create mode 100644 fs/ubifs/misc.c

diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile
index 2c6f0cb..c54a243 100644
--- a/fs/ubifs/Makefile
+++ b/fs/ubifs/Makefile
@@ -4,3 +4,4 @@ ubifs-y += shrinker.o journal.o file.o dir.o super.o sb.o io.o
 ubifs-y += tnc.o master.o scan.o replay.o log.o commit.o gc.o orphan.o
 ubifs-y += budget.o find.o tnc_commit.o compress.o lpt.o lprops.o
 ubifs-y += recovery.o ioctl.o lpt_commit.o tnc_misc.o xattr.o debug.o
+ubifs-y += misc.o
diff --git a/fs/ubifs/misc.c b/fs/ubifs/misc.c
new file mode 100644
index 0000000..621e77c
--- /dev/null
+++ b/fs/ubifs/misc.c
@@ -0,0 +1,57 @@
+#include <linux/kernel.h>
+#include "ubifs.h"
+
+/* Normal UBIFS messages */
+void ubifs_msg(const struct ubifs_info *c, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	pr_notice("UBIFS (ubi%d:%d): %pV\n",
+		  c->vi.ubi_num, c->vi.vol_id, &vaf);
+
+	va_end(args);
+}
+
+/* UBIFS error messages */
+void ubifs_err(const struct ubifs_info *c, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	pr_err("UBIFS error (ubi%d:%d pid %d): %ps: %pV\n",
+	       c->vi.ubi_num, c->vi.vol_id, current->pid,
+	       __builtin_return_address(0),
+	       &vaf);
+
+	va_end(args);
+}
+
+/* UBIFS warning messages */
+void ubifs_warn(const struct ubifs_info *c, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	pr_warn("UBIFS warning (ubi%d:%d pid %d): %ps: %pV\n",
+		c->vi.ubi_num, c->vi.vol_id, current->pid,
+		__builtin_return_address(0),
+		&vaf);
+
+	va_end(args);
+}
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index a5697de..0e946c8 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -42,30 +42,6 @@
 /* Version of this UBIFS implementation */
 #define UBIFS_VERSION 1
 
-/* Normal UBIFS messages */
-#define ubifs_msg(c, fmt, ...)                                      \
-	pr_notice("UBIFS (ubi%d:%d): " fmt "\n",                    \
-		  (c)->vi.ubi_num, (c)->vi.vol_id, ##__VA_ARGS__)
-/* UBIFS error messages */
-#define ubifs_err(c, fmt, ...)                                      \
-	pr_err("UBIFS error (ubi%d:%d pid %d): %s: " fmt "\n",      \
-	       (c)->vi.ubi_num, (c)->vi.vol_id, current->pid,       \
-	       __func__, ##__VA_ARGS__)
-/* UBIFS warning messages */
-#define ubifs_warn(c, fmt, ...)                                     \
-	pr_warn("UBIFS warning (ubi%d:%d pid %d): %s: " fmt "\n",   \
-		(c)->vi.ubi_num, (c)->vi.vol_id, current->pid,      \
-		__func__, ##__VA_ARGS__)
-/*
- * A variant of 'ubifs_err()' which takes the UBIFS file-sytem description
- * object as an argument.
- */
-#define ubifs_errc(c, fmt, ...)                                     \
-	do {                                                        \
-		if (!(c)->probing)                                  \
-			ubifs_err(c, fmt, ##__VA_ARGS__);           \
-	} while (0)
-
 /* UBIFS file system VFS magic number */
 #define UBIFS_SUPER_MAGIC 0x24051905
 
@@ -1802,4 +1778,21 @@ int ubifs_decompress(const struct ubifs_info *c, const void *buf, int len,
 #include "misc.h"
 #include "key.h"
 
+/* Normal UBIFS messages */
+__printf(2, 3)
+void ubifs_msg(const struct ubifs_info *c, const char *fmt, ...);
+__printf(2, 3)
+void ubifs_err(const struct ubifs_info *c, const char *fmt, ...);
+__printf(2, 3)
+void ubifs_warn(const struct ubifs_info *c, const char *fmt, ...);
+/*
+ * A variant of 'ubifs_err()' which takes the UBIFS file-system description
+ * object as an argument.
+ */
+#define ubifs_errc(c, fmt, ...)						\
+do {									\
+	if (!(c)->probing)						\
+		ubifs_err(c, fmt, ##__VA_ARGS__);			\
+} while (0)
+
 #endif /* !__UBIFS_H__ */
-- 
2.6.3.368.gf34be46

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Joe Perches]

On Tue, 2016-02-23 at 12:21 -0800, Joe Perches wrote:
> The existing logging macros are fairly large and converting the
> macros to functions make the object code smaller.

Artem and Adrian are the nominal maintainers for ubifs.

Artem last had a sign-off on a ubifs patch 6 months ago and
Adrian's
last one was over 3 years ago.

Is there an expected ack/nack/applied handling time for a
ubifs patch like this?

Should this patch go to someone else?

Should the MAINTAINERS entry for ubifs be changed to orphan?

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Joe Perches]

On Wed, 2016-03-02 at 19:46 +0100, Richard Weinberger wrote:
> Am 02.03.2016 7:19 nachm. schrieb Joe Perches <joe@perches.com>:
> > On Tue, 2016-02-23 at 12:21 -0800, Joe Perches wrote:
> > > The existing logging macros are fairly large and converting the 
> > > macros to functions make the object code smaller. 
> > Artem and Adrian are the nominal maintainers for ubifs. 
> > 
> > Artem last had a sign-off on a ubifs patch 6 months ago and 
> > Adrian's last one was over 3 years ago. 
> > 
> > Is there an expected ack/nack/applied handling time for a 
> > ubifs patch like this? 
> > 
> > Should this patch go to someone else? 
> > 
> > Should the MAINTAINERS entry for ubifs be changed to orphan? 
> No need to panic. I take care of the patch. :)

No worries, I'm calm.  But if you're taking care of the patch
and not the nominal maintainers, likely the MAINTAINERS entry
should be updated.

Presumably the equivalent ubi patch will be handled in the
same fashion,

For drivers/mtd/ubi, Artem is also a nominal maintainer, but
last signed a patch in 2014.

Likely the MAINTAINERS entry for UBI should be updated too.

Maybe something like:
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 81302e4..927557c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11266,8 +11266,7 @@ S:	Maintained
 F:	drivers/scsi/u14-34f.c
 
 UBI FILE SYSTEM (UBIFS)
-M:	Artem Bityutskiy <dedekind1@gmail.com>
-M:	Adrian Hunter <adrian.hunter@intel.com>
+M:	Richard Weinberger <richard@nod.at>
 L:	linux-mtd@lists.infradead.org
 T:	git git://git.infradead.org/ubifs-2.6.git
 W:	http://www.linux-mtd.infradead.org/doc/ubifs.html
@@ -11350,7 +11349,6 @@ F:	Documentation/scsi/ufs.txt
 F:	drivers/scsi/ufs/
 
 UNSORTED BLOCK IMAGES (UBI)
-M:	Artem Bityutskiy <dedekind1@gmail.com>
 M:	Richard Weinberger <richard@nod.at>
 W:	http://www.linux-mtd.infradead.org/
 L:	linux-mtd@lists.infradead.org

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Richard Weinberger]

Joe,

Am 02.03.2016 um 19:58 schrieb Joe Perches:
> On Wed, 2016-03-02 at 19:46 +0100, Richard Weinberger wrote:
>> Am 02.03.2016 7:19 nachm. schrieb Joe Perches <joe@perches.com>:
>>> On Tue, 2016-02-23 at 12:21 -0800, Joe Perches wrote:
>>>> The existing logging macros are fairly large and converting the 
>>>> macros to functions make the object code smaller. 
>>> Artem and Adrian are the nominal maintainers for ubifs. 
>>>
>>> Artem last had a sign-off on a ubifs patch 6 months ago and 
>>> Adrian's last one was over 3 years ago. 
>>>
>>> Is there an expected ack/nack/applied handling time for a 
>>> ubifs patch like this? 
>>>
>>> Should this patch go to someone else? 
>>>
>>> Should the MAINTAINERS entry for ubifs be changed to orphan? 
>> No need to panic. I take care of the patch. :)
> 
> No worries, I'm calm.  But if you're taking care of the patch
> and not the nominal maintainers, likely the MAINTAINERS entry
> should be updated.
> 
> Presumably the equivalent ubi patch will be handled in the
> same fashion,
> 
> For drivers/mtd/ubi, Artem is also a nominal maintainer, but
> last signed a patch in 2014.
> 
> Likely the MAINTAINERS entry for UBI should be updated too.
> 
> Maybe something like:
> ---
>  MAINTAINERS | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 81302e4..927557c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11266,8 +11266,7 @@ S:	Maintained
>  F:	drivers/scsi/u14-34f.c
>  
>  UBI FILE SYSTEM (UBIFS)
> -M:	Artem Bityutskiy <dedekind1@gmail.com>
> -M:	Adrian Hunter <adrian.hunter@intel.com>
> +M:	Richard Weinberger <richard@nod.at>

I can be the official patch monkey for UBIFS.
Especial as the git tree is the same as for UBI.

But I definitely want to keep Artem.
He gives very valuable input and knows UBIFS much better than I do.

>  L:	linux-mtd@lists.infradead.org
>  T:	git git://git.infradead.org/ubifs-2.6.git
>  W:	http://www.linux-mtd.infradead.org/doc/ubifs.html
> @@ -11350,7 +11349,6 @@ F:	Documentation/scsi/ufs.txt
>  F:	drivers/scsi/ufs/
>  
>  UNSORTED BLOCK IMAGES (UBI)
> -M:	Artem Bityutskiy <dedekind1@gmail.com>
>  M:	Richard Weinberger <richard@nod.at>
>  W:	http://www.linux-mtd.infradead.org/
>  L:	linux-mtd@lists.infradead.org

Same here. Artem has to stay.

Thanks,
//richard

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Joe Perches]

On Wed, 2016-03-02 at 22:24 +0100, Richard Weinberger wrote:
> Am 02.03.2016 um 19:58 schrieb Joe Perches:
[]
> > For drivers/mtd/ubi, Artem is also a nominal maintainer, but
> > last signed a patch in 2014.
> > 
> > Likely the MAINTAINERS entry for UBI should be updated too.
> > 
> > Maybe something like:
[]
> > diff --git a/MAINTAINERS b/MAINTAINERS
[]
> > @@ -11266,8 +11266,7 @@ S:	Maintained
> >  F:	drivers/scsi/u14-34f.c
> >  
> >  UBI FILE SYSTEM (UBIFS)
> > -M:	Artem Bityutskiy <dedekind1@gmail.com>
> > -M:	Adrian Hunter <adrian.hunter@intel.com>
> > +M:	Richard Weinberger <richard@nod.at>

> I can be the official patch monkey for UBIFS.
> Especial as the git tree is the same as for UBI.
> 
> But I definitely want to keep Artem.
> He gives very valuable input and knows UBIFS much better than I do.

That's fine and it's your (and his) choice what do to.

> >  L:	linux-mtd@lists.infradead.org
> >  T:	git git://git.infradead.org/ubifs-2.6.git
> >  W:	http://www.linux-mtd.infradead.org/doc/ubifs.html
> > @@ -11350,7 +11349,6 @@ F:	Documentation/scsi/ufs.txt
> >  F:	drivers/scsi/ufs/
> >  
> >  UNSORTED BLOCK IMAGES (UBI)
> > -M:	Artem Bityutskiy <dedekind1@gmail.com>
> >  M:	Richard Weinberger <richard@nod.at>
> >  W:	http://www.linux-mtd.infradead.org/
> >  L:	linux-mtd@lists.infradead.org
> Same here. Artem has to stay.

Perhaps an "R:" (reviewer) entry is appropriate for Artem.

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Artem Bityutskiy]

On Wed, 2016-03-02 at 10:58 -0800, Joe Perches wrote:
> 
>  UBI FILE SYSTEM (UBIFS)
> -M:	Artem Bityutskiy <dedekind1@gmail.com>
> -M:	Adrian Hunter <adrian.hunter@intel.com>
> +M:	Richard Weinberger <richard@nod.at>
>  L:	linux-mtd@lists.infradead.org
>  T:	git git://git.infradead.org/ubifs-2.6.git
>  W:	http://www.linux-mtd.infradead.org/doc/ubifs.html

Hi Joe,

could please, re-send this patch with the following modifications:

1. Put Richard's name first.
2. Do not remove mine and Adrian's name. We are not very active, but
still useful.

Thanks!

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Joe Perches]

On Thu, 2016-03-03 at 10:39 +0200, Artem Bityutskiy wrote:
> On Wed, 2016-03-02 at 10:58 -0800, Joe Perches wrote:
> > 
> >  
> >  UBI FILE SYSTEM (UBIFS)
> > -M:	Artem Bityutskiy <dedekind1@gmail.com>
> > -M:	Adrian Hunter <adrian.hunter@intel.com>
> > +M:	Richard Weinberger <richard@nod.at>
> >  L:	linux-mtd@lists.infradead.org
> >  T:	git git://git.infradead.org/ubifs-2.6.git
> >  W:	http://www.linux-mtd.infradead.org/doc/ubifs.html
> Hi Joe,
> 
> could please, re-send this patch with the following modifications:
> 
> 1. Put Richard's name first.
> 2. Do not remove mine and Adrian's name. We are not very active, but
> still useful.
> 
> Thanks!

Hello Artem.

It's not really for me to change Richard's maintainership.
It should be his sign-off, not mine.

My suggestion would be to add him and designate you and Adrian
as R: reviewers instead of M: maintainers like below.
---
 MAINTAINERS | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 81302e4..61e17a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11266,8 +11266,9 @@ S:	Maintained
 F:	drivers/scsi/u14-34f.c
 
 UBI FILE SYSTEM (UBIFS)
-M:	Artem Bityutskiy <dedekind1@gmail.com>
-M:	Adrian Hunter <adrian.hunter@intel.com>
+M:	Richard Weinberger <richard@nod.at>
+R:	Artem Bityutskiy <dedekind1@gmail.com>
+R:	Adrian Hunter <adrian.hunter@intel.com>
 L:	linux-mtd@lists.infradead.org
 T:	git git://git.infradead.org/ubifs-2.6.git
 W:	http://www.linux-mtd.infradead.org/doc/ubifs.html
@@ -11350,8 +11351,8 @@ F:	Documentation/scsi/ufs.txt
 F:	drivers/scsi/ufs/
 
 UNSORTED BLOCK IMAGES (UBI)
-M:	Artem Bityutskiy <dedekind1@gmail.com>
 M:	Richard Weinberger <richard@nod.at>
+R:	Artem Bityutskiy <dedekind1@gmail.com>
 W:	http://www.linux-mtd.infradead.org/
 L:	linux-mtd@lists.infradead.org
 T:	git git://git.infradead.org/ubifs-2.6.git

Re: [PATCH V2] ubifs: Add logging functions for ubifs_msg, ubifs_err and ubifs_warn

[Artem Bityutskiy]

On Thu, 2016-03-03 at 04:25 -0800, Joe Perches wrote:
> My suggestion would be to add him and designate you and Adrian
> as R: reviewers instead of M: maintainers like below.

I think it is a little bit too early.