[PATCH next] ocfs2: Reduce object size of mlog uses

[PATCH next] ocfs2: Reduce object size of mlog uses

[Joe Perches]

Using a function for __mlog_printk instead of a macro
reduces the object size of built-in.o more than 120KB, or
~10% overall (x86-64 defconfig with all ocfs2 options)

$ size fs/ocfs2/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 936255	 118071	 134408	1188734	 12237e	fs/ocfs2/built-in.o.new
1064081	 118071	 134408	1316560	 1416d0	fs/ocfs2/built-in.o.old

Miscellanea:

o Neaten macros around the __mlog_printk uses.
o Use __func__ for __PRETTY_FUNCTION__
o Use ##__VA_ARGS__ for args...

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

 fs/ocfs2/cluster/masklog.c | 17 +++++++++++++++++
 fs/ocfs2/cluster/masklog.h | 28 +++++++++++++++++-----------
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
index af7598b..5b7a351 100644
--- a/fs/ocfs2/cluster/masklog.c
+++ b/fs/ocfs2/cluster/masklog.c
@@ -64,6 +64,23 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
 	return count;
 }
 
+void __mlog_printk(const char *level, const char *func, int line,
+		   const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	printk("%s(%s,%u,%lu):%s:%d %pV",
+	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
+	       func, line, &vaf);
+
+	va_end(args);
+}
 struct mlog_attribute {
 	struct attribute attr;
 	u64 mask;
diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
index 7fdc25a..6036e6a 100644
--- a/fs/ocfs2/cluster/masklog.h
+++ b/fs/ocfs2/cluster/masklog.h
@@ -168,7 +168,8 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
  * scream.  just do this instead of trying to guess which we're building
  * against.. *sigh*.
  */
-#define __mlog_cpu_guess ({		\
+#define __mlog_cpu_guess		\
+({					\
 	unsigned long _cpu = get_cpu();	\
 	put_cpu();			\
 	_cpu;				\
@@ -178,21 +179,25 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
  * before ##args is intentional. Otherwise, gcc 2.95 will eat the
  * previous token if args expands to nothing.
  */
-#define __mlog_printk(level, fmt, args...)				\
-	printk(level "(%s,%u,%lu):%s:%d " fmt, current->comm,		\
-	       task_pid_nr(current), __mlog_cpu_guess,			\
-	       __PRETTY_FUNCTION__, __LINE__ , ##args)
+__printf(4, 5)
+void __mlog_printk(const char *level, const char *func, int line,
+		   const char *fmt, ...);
 
-#define mlog(mask, fmt, args...) do {					\
+#define mlog(mask, fmt, ...)						\
+do {									\
 	u64 __m = MLOG_MASK_PREFIX | (mask);				\
 	if ((__m & ML_ALLOWED_BITS) &&					\
 	    __mlog_test_u64(__m, mlog_and_bits) &&			\
 	    !__mlog_test_u64(__m, mlog_not_bits)) {			\
 		if (__m & ML_ERROR)					\
-			__mlog_printk(KERN_ERR, "ERROR: "fmt , ##args);	\
+			__mlog_printk(KERN_ERR, __func__, __LINE__,	\
+				      "ERROR: " fmt, ##__VA_ARGS__);	\
 		else if (__m & ML_NOTICE)				\
-			__mlog_printk(KERN_NOTICE, fmt , ##args);	\
-		else __mlog_printk(KERN_INFO, fmt , ##args);		\
+			__mlog_printk(KERN_NOTICE, __func__, __LINE__,	\
+				      fmt, ##__VA_ARGS__);		\
+		else							\
+			__mlog_printk(KERN_INFO, __func__, __LINE__,	\
+				      fmt, ##__VA_ARGS__);		\
 	}								\
 } while (0)
 
@@ -205,10 +210,11 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
 	_st;								\
 })
 
-#define mlog_bug_on_msg(cond, fmt, args...) do {			\
+#define mlog_bug_on_msg(cond, fmt, ...)					\
+do {									\
 	if (cond) {							\
 		mlog(ML_ERROR, "bug expression: " #cond "\n");		\
-		mlog(ML_ERROR, fmt, ##args);				\
+		mlog(ML_ERROR, fmt, ##__VA_ARGS__);			\
 		BUG();							\
 	}								\
 } while (0)

Re: [PATCH next] ocfs2: Reduce object size of mlog uses

[Andrew Morton]

On Fri, 17 Apr 2015 00:17:50 -0700 Joe Perches <joe@perches.com> wrote:

> Using a function for __mlog_printk instead of a macro
> reduces the object size of built-in.o more than 120KB, or
> ~10% overall (x86-64 defconfig with all ocfs2 options)
> 
> $ size fs/ocfs2/built-in.o*
>    text	   data	    bss	    dec	    hex	filename
>  936255	 118071	 134408	1188734	 12237e	fs/ocfs2/built-in.o.new
> 1064081	 118071	 134408	1316560	 1416d0	fs/ocfs2/built-in.o.old

It's a start.

> --- a/fs/ocfs2/cluster/masklog.c
> +++ b/fs/ocfs2/cluster/masklog.c
> @@ -64,6 +64,23 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
>  	return count;
>  }
>  
> +void __mlog_printk(const char *level, const char *func, int line,
> +		   const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	printk("%s(%s,%u,%lu):%s:%d %pV",
> +	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
> +	       func, line, &vaf);
> +
> +	va_end(args);
> +}

Logging function-name and line-number was a bit weird.  I wonder if
anyone will mind if this is converted to file-n-line, as God intended. 
That will shrink rodata a bit, because number-of-files is a lot less
than number-of-functions.

>  struct mlog_attribute {
>  	struct attribute attr;
>  	u64 mask;
> diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
> index 7fdc25a..6036e6a 100644
> --- a/fs/ocfs2/cluster/masklog.h
> +++ b/fs/ocfs2/cluster/masklog.h
> @@ -168,7 +168,8 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
>   * scream.  just do this instead of trying to guess which we're building
>   * against.. *sigh*.
>   */
> -#define __mlog_cpu_guess ({		\
> +#define __mlog_cpu_guess		\
> +({					\

While we're in there we should turn this into __mlog_cpu_guess().

Or, preferably, just zap the sorry thing and use
raw_smp_processor_id().

>  	unsigned long _cpu = get_cpu();	\
>  	put_cpu();			\
>  	_cpu;				\
> @@ -178,21 +179,25 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
>   * before ##args is intentional. Otherwise, gcc 2.95 will eat the
>   * previous token if args expands to nothing.
>   */
> -#define __mlog_printk(level, fmt, args...)				\
> -	printk(level "(%s,%u,%lu):%s:%d " fmt, current->comm,		\
> -	       task_pid_nr(current), __mlog_cpu_guess,			\
> -	       __PRETTY_FUNCTION__, __LINE__ , ##args)
> +__printf(4, 5)
> +void __mlog_printk(const char *level, const char *func, int line,
> +		   const char *fmt, ...);
>  
> -#define mlog(mask, fmt, args...) do {					\
> +#define mlog(mask, fmt, ...)						\
> +do {									\
>  	u64 __m = MLOG_MASK_PREFIX | (mask);				\
>  	if ((__m & ML_ALLOWED_BITS) &&					\
>  	    __mlog_test_u64(__m, mlog_and_bits) &&			\
>  	    !__mlog_test_u64(__m, mlog_not_bits)) {			\
>  		if (__m & ML_ERROR)					\

All this goop can also be uninlined?

> -			__mlog_printk(KERN_ERR, "ERROR: "fmt , ##args);	\
> +			__mlog_printk(KERN_ERR, __func__, __LINE__,	\
> +				      "ERROR: " fmt, ##__VA_ARGS__);	\
>  		else if (__m & ML_NOTICE)				\
> -			__mlog_printk(KERN_NOTICE, fmt , ##args);	\
> -		else __mlog_printk(KERN_INFO, fmt , ##args);		\
> +			__mlog_printk(KERN_NOTICE, __func__, __LINE__,	\
> +				      fmt, ##__VA_ARGS__);		\
> +		else							\
> +			__mlog_printk(KERN_INFO, __func__, __LINE__,	\
> +				      fmt, ##__VA_ARGS__);		\
>  	}								\
>  } while (0)
>  

I guess this patch is a step on the way - a 10% shrink is decent.  But
I believe that with full uninlining of the ocfs2 logging code we can
shrink the filesystem's footprint by 50%.

This code needs some pretty serious rework and rethink, perhaps
involving a change to the emitted info.  I was hoping one of the ocfs2
developers would take the bait, but they're all in hiding.

If you feel like undertaking such a rotorooting then go wild - that should
wake 'em up ;)

Re: [PATCH next] ocfs2: Reduce object size of mlog uses

[Joe Perches]

On Wed, 2015-04-22 at 15:46 -0700, Andrew Morton wrote:
> On Fri, 17 Apr 2015 00:17:50 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > Using a function for __mlog_printk instead of a macro
> > reduces the object size of built-in.o more than 120KB, or
> > ~10% overall (x86-64 defconfig with all ocfs2 options)
> > 
> > $ size fs/ocfs2/built-in.o*
> >    text	   data	    bss	    dec	    hex	filename
> >  936255	 118071	 134408	1188734	 12237e	fs/ocfs2/built-in.o.new
> > 1064081	 118071	 134408	1316560	 1416d0	fs/ocfs2/built-in.o.old
> 
> It's a start.
> 
> > --- a/fs/ocfs2/cluster/masklog.c
> > +++ b/fs/ocfs2/cluster/masklog.c
> > @@ -64,6 +64,23 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
> >  	return count;
> >  }
> >  
> > +void __mlog_printk(const char *level, const char *func, int line,
> > +		   const char *fmt, ...)
> > +{
> > +	struct va_format vaf;
> > +	va_list args;
> > +
> > +	va_start(args, fmt);
> > +
> > +	vaf.fmt = fmt;
> > +	vaf.va = &args;
> > +
> > +	printk("%s(%s,%u,%lu):%s:%d %pV",
> > +	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
> > +	       func, line, &vaf);
> > +
> > +	va_end(args);
> > +}
> 
> Logging function-name and line-number was a bit weird.  I wonder if
> anyone will mind if this is converted to file-n-line, as God intended. 
> That will shrink rodata a bit, because number-of-files is a lot less
> than number-of-functions.

I don't care one way or another.

Using __FILE__ vs __func__ reduces built-in.o
by about 25K.

I didn't bother to determine the actual total
reduction in a vmlinux.o

> > -#define mlog(mask, fmt, args...) do {					\
> > +#define mlog(mask, fmt, ...)						\
> > +do {									\
> >  	u64 __m = MLOG_MASK_PREFIX | (mask);				\
> >  	if ((__m & ML_ALLOWED_BITS) &&					\
> >  	    __mlog_test_u64(__m, mlog_and_bits) &&			\
> >  	    !__mlog_test_u64(__m, mlog_not_bits)) {			\
> >  		if (__m & ML_ERROR)					\

> All this goop can also be uninlined?

You have to convert the level pointer to a u64 pointer
passing &__m, but yeah, it's becomes about 65K smaller.

The macro becomes simpler too as the (__m & ML_<LEVEL>)
tests go into the function.

So that's another 7 or 8 % or more total shrinking.

> I guess this patch is a step on the way - a 10% shrink is decent.  But
> I believe that with full uninlining of the ocfs2 logging code we can
> shrink the filesystem's footprint by 50%.

Nope.

Even if CONFIG_PRINTK is not set and the mlog #define
is a no-op, it's not quite that big a reduction.

You have to turn the function tracing code off too for
that 50%.

> If you feel like undertaking such a rotorooting then go wild - that should
> wake 'em up ;)

One step at a time...

[PATCH V2 -next] ocfs2: Reduce object size of mlog uses

[Joe Perches]

Using a function for __mlog_printk instead of a macro
reduces the object size of built-in.o by about 190KB, or
~18% overall (x86-64 defconfig with all ocfs2 options)

$ size fs/ocfs2/built-in.o*
   text    data     bss     dec     hex filename
 870954	 118471	 134408	1123833	 1125f9	fs/ocfs2/built-in.o,new
1064081  118071  134408 1316560  1416d0 fs/ocfs2/built-in.o.old

Miscellanea:

o Move the used-once __mlog_cpu_guess statement expression macro
  to the masklog.c file above the use in __mlog_printk function
o Simplify the mlog macro moving the and/or logic and level code
  into __mlog_printk

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

V2: Moving and/or logic to the __mlog_printk function reduces
    the object size an additional ~65K

 fs/ocfs2/cluster/masklog.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/ocfs2/cluster/masklog.h | 42 ++++++++++++------------------------------
 2 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
index af7598b..fc5e522 100644
--- a/fs/ocfs2/cluster/masklog.c
+++ b/fs/ocfs2/cluster/masklog.c
@@ -64,6 +64,52 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
 	return count;
 }
 
+/*
+ * smp_processor_id() "helpfully" screams when called outside preemptible
+ * regions in current kernels.  sles doesn't have the variants that don't
+ * scream.  just do this instead of trying to guess which we're building
+ * against.. *sigh*.
+ */
+#define __mlog_cpu_guess						\
+({									\
+	unsigned long _cpu = get_cpu();					\
+	put_cpu();							\
+	_cpu;								\
+})
+
+void __mlog_printk(const u64 *mask, const char *func, int line,
+		   const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	const char *level;
+	const char *prefix = "";
+
+	if (!__mlog_test_u64(*mask, mlog_and_bits) ||
+	    __mlog_test_u64(*mask, mlog_not_bits))
+		return;
+
+	if (*mask & ML_ERROR) {
+		level = KERN_ERR;
+		prefix = "ERROR: ";
+	} else if (*mask & ML_NOTICE) {
+		level = KERN_NOTICE;
+	} else {
+		level = KERN_INFO;
+	}
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	printk("%s(%s,%u,%lu):%s:%d %s%pV",
+	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
+	       func, line, prefix, &vaf);
+
+	va_end(args);
+}
+
 struct mlog_attribute {
 	struct attribute attr;
 	u64 mask;
diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
index 7fdc25a..308ea0e 100644
--- a/fs/ocfs2/cluster/masklog.h
+++ b/fs/ocfs2/cluster/masklog.h
@@ -162,38 +162,20 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
 
 #endif
 
-/*
- * smp_processor_id() "helpfully" screams when called outside preemptible
- * regions in current kernels.  sles doesn't have the variants that don't
- * scream.  just do this instead of trying to guess which we're building
- * against.. *sigh*.
- */
-#define __mlog_cpu_guess ({		\
-	unsigned long _cpu = get_cpu();	\
-	put_cpu();			\
-	_cpu;				\
-})
+__printf(4, 5)
+void __mlog_printk(const u64 *m, const char *func, int line,
+		   const char *fmt, ...);
 
-/* In the following two macros, the whitespace after the ',' just
- * before ##args is intentional. Otherwise, gcc 2.95 will eat the
- * previous token if args expands to nothing.
+/*
+ * Testing before the __mlog_printk call lets the compiler eliminate the
+ * call completely when (m & ML_ALLOWED_BITS) is 0.
  */
-#define __mlog_printk(level, fmt, args...)				\
-	printk(level "(%s,%u,%lu):%s:%d " fmt, current->comm,		\
-	       task_pid_nr(current), __mlog_cpu_guess,			\
-	       __PRETTY_FUNCTION__, __LINE__ , ##args)
-
-#define mlog(mask, fmt, args...) do {					\
-	u64 __m = MLOG_MASK_PREFIX | (mask);				\
-	if ((__m & ML_ALLOWED_BITS) &&					\
-	    __mlog_test_u64(__m, mlog_and_bits) &&			\
-	    !__mlog_test_u64(__m, mlog_not_bits)) {			\
-		if (__m & ML_ERROR)					\
-			__mlog_printk(KERN_ERR, "ERROR: "fmt , ##args);	\
-		else if (__m & ML_NOTICE)				\
-			__mlog_printk(KERN_NOTICE, fmt , ##args);	\
-		else __mlog_printk(KERN_INFO, fmt , ##args);		\
-	}								\
+#define mlog(mask, fmt, ...)						\
+do {									\
+	u64 _m = MLOG_MASK_PREFIX | (mask);				\
+	if (_m & ML_ALLOWED_BITS)					\
+		__mlog_printk(&_m, __func__, __LINE__, fmt,		\
+			      ##__VA_ARGS__);				\
 } while (0)
 
 #define mlog_errno(st) ({						\

Re: [PATCH next] ocfs2: Reduce object size of mlog uses

[Mark Fasheh]

On Wed, Apr 22, 2015 at 03:46:04PM -0700, Andrew Morton wrote:
> On Fri, 17 Apr 2015 00:17:50 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > Using a function for __mlog_printk instead of a macro
> > reduces the object size of built-in.o more than 120KB, or
> > ~10% overall (x86-64 defconfig with all ocfs2 options)
> > 
> > $ size fs/ocfs2/built-in.o*
> >    text	   data	    bss	    dec	    hex	filename
> >  936255	 118071	 134408	1188734	 12237e	fs/ocfs2/built-in.o.new
> > 1064081	 118071	 134408	1316560	 1416d0	fs/ocfs2/built-in.o.old
> 
> It's a start.
> 
> > --- a/fs/ocfs2/cluster/masklog.c
> > +++ b/fs/ocfs2/cluster/masklog.c
> > @@ -64,6 +64,23 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
> >  	return count;
> >  }
> >  
> > +void __mlog_printk(const char *level, const char *func, int line,
> > +		   const char *fmt, ...)
> > +{
> > +	struct va_format vaf;
> > +	va_list args;
> > +
> > +	va_start(args, fmt);
> > +
> > +	vaf.fmt = fmt;
> > +	vaf.va = &args;
> > +
> > +	printk("%s(%s,%u,%lu):%s:%d %pV",
> > +	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
> > +	       func, line, &vaf);
> > +
> > +	va_end(args);
> > +}
> 
> Logging function-name and line-number was a bit weird.  I wonder if
> anyone will mind if this is converted to file-n-line, as God intended. 
> That will shrink rodata a bit, because number-of-files is a lot less
> than number-of-functions.

We can live with file-n-line.


> > -			__mlog_printk(KERN_ERR, "ERROR: "fmt , ##args);	\
> > +			__mlog_printk(KERN_ERR, __func__, __LINE__,	\
> > +				      "ERROR: " fmt, ##__VA_ARGS__);	\
> >  		else if (__m & ML_NOTICE)				\
> > -			__mlog_printk(KERN_NOTICE, fmt , ##args);	\
> > -		else __mlog_printk(KERN_INFO, fmt , ##args);		\
> > +			__mlog_printk(KERN_NOTICE, __func__, __LINE__,	\
> > +				      fmt, ##__VA_ARGS__);		\
> > +		else							\
> > +			__mlog_printk(KERN_INFO, __func__, __LINE__,	\
> > +				      fmt, ##__VA_ARGS__);		\
> >  	}								\
> >  } while (0)
> >  
> 
> I guess this patch is a step on the way - a 10% shrink is decent.  But
> I believe that with full uninlining of the ocfs2 logging code we can
> shrink the filesystem's footprint by 50%.
> 
> This code needs some pretty serious rework and rethink, perhaps
> involving a change to the emitted info.  I was hoping one of the ocfs2
> developers would take the bait, but they're all in hiding.

If it functions the same and doesn't have a major performance change, I'm
pretty sure it'll be fine. We sometimes ask customers to enable some of the
debugging if they are having an issue. I would ask that it be tested
on a live system - a local fs, no cluster or cluster config required.


> If you feel like undertaking such a rotorooting then go wild - that should
> wake 'em up ;)

Ok, I've taken the bait  :)
	--Mark

--
Mark Fasheh

Re: [PATCH next] ocfs2: Reduce object size of mlog uses

[Andrew Morton]

On Thu, 23 Apr 2015 16:04:18 -0700 Mark Fasheh <mfasheh@suse.de> wrote:

> > This code needs some pretty serious rework and rethink, perhaps
> > involving a change to the emitted info.  I was hoping one of the ocfs2
> > developers would take the bait, but they're all in hiding.
> 
> If it functions the same and doesn't have a major performance change, I'm
> pretty sure it'll be fine. We sometimes ask customers to enable some of the
> debugging if they are having an issue. I would ask that it be tested
> on a live system - a local fs, no cluster or cluster config required.

Is there a simpleton's guide to testing ocfs2 on a local disk?  One
which assumes a starting point of "knows how to type".

A few paragraphs in Documentation/filesystems/ocfs2.txt would be great
- then we can point non-ocfs2 people at it when they muck with stuff.

Re: [PATCH next] ocfs2: Reduce object size of mlog uses

[Richard Weinberger]

On Fri, Apr 24, 2015 at 1:19 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Thu, 23 Apr 2015 16:04:18 -0700 Mark Fasheh <mfasheh@suse.de> wrote:
>
>> > This code needs some pretty serious rework and rethink, perhaps
>> > involving a change to the emitted info.  I was hoping one of the ocfs2
>> > developers would take the bait, but they're all in hiding.
>>
>> If it functions the same and doesn't have a major performance change, I'm
>> pretty sure it'll be fine. We sometimes ask customers to enable some of the
>> debugging if they are having an issue. I would ask that it be tested
>> on a live system - a local fs, no cluster or cluster config required.
>
> Is there a simpleton's guide to testing ocfs2 on a local disk?  One
> which assumes a starting point of "knows how to type".

See http://docs.oracle.com/cd/E37670_01/E41138/html/ol_crlcl_ocfs2.html

-- 
Thanks,
//richard

Re: [Ocfs2-devel] [PATCH next] ocfs2: Reduce object size of mlog uses

[Mark Fasheh]

On Fri, Apr 24, 2015 at 01:37:34AM +0200, Richard Weinberger wrote:
> On Fri, Apr 24, 2015 at 1:19 AM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Thu, 23 Apr 2015 16:04:18 -0700 Mark Fasheh <mfasheh@suse.de> wrote:
> >
> >> > This code needs some pretty serious rework and rethink, perhaps
> >> > involving a change to the emitted info.  I was hoping one of the ocfs2
> >> > developers would take the bait, but they're all in hiding.
> >>
> >> If it functions the same and doesn't have a major performance change, I'm
> >> pretty sure it'll be fine. We sometimes ask customers to enable some of the
> >> debugging if they are having an issue. I would ask that it be tested
> >> on a live system - a local fs, no cluster or cluster config required.
> >
> > Is there a simpleton's guide to testing ocfs2 on a local disk?  One
> > which assumes a starting point of "knows how to type".
> 
> See http://docs.oracle.com/cd/E37670_01/E41138/html/ol_crlcl_ocfs2.html

As Richard points out, it's pretty straightforward, just use the '-M local'
switch to mkfs.ocfs2. From there mount.ocfs2 will do the right thing when
you ask it to load the file system.
	--Mark

--
Mark Fasheh

Re: [PATCH next] ocfs2: Reduce object size of mlog uses

[Joe Perches]

On Thu, 2015-04-23 at 16:04 -0700, Mark Fasheh wrote:
> On Wed, Apr 22, 2015 at 03:46:04PM -0700, Andrew Morton wrote:
> > If you feel like undertaking such a rotorooting then go wild - that should
> > wake 'em up ;)
> 
> Ok, I've taken the bait  :)

"Here fishy, fishy...", erm, "Here Fasheh, Fasheh..."

With that out of the way:

A couple of possibilities:

o I wonder whether or not file/func/line matter at all.
  I think they don't.
  Removing them would reduce code size ~90K
o There's a small logging improvement possible in tcp.c.

Both below:

----------------------------------------------------------

 fs/ocfs2/cluster/masklog.c | 7 +++----
 fs/ocfs2/cluster/masklog.h | 8 +++-----
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
index fc5e522..8b9816f 100644
--- a/fs/ocfs2/cluster/masklog.c
+++ b/fs/ocfs2/cluster/masklog.c
@@ -77,8 +77,7 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
 	_cpu;								\
 })
 
-void __mlog_printk(const u64 *mask, const char *func, int line,
-		   const char *fmt, ...)
+void __mlog_printk(const u64 *mask, const char *fmt, ...)
 {
 	struct va_format vaf;
 	va_list args;
@@ -103,9 +102,9 @@ void __mlog_printk(const u64 *mask, const char *func, int line,
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	printk("%s(%s,%u,%lu):%s:%d %s%pV",
+	printk("%s(%s,%u,%lu) %s%pV",
 	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
-	       func, line, prefix, &vaf);
+	       prefix, &vaf);
 
 	va_end(args);
 }
diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
index 308ea0e..9e93f19 100644
--- a/fs/ocfs2/cluster/masklog.h
+++ b/fs/ocfs2/cluster/masklog.h
@@ -162,9 +162,8 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
 
 #endif
 
-__printf(4, 5)
-void __mlog_printk(const u64 *m, const char *func, int line,
-		   const char *fmt, ...);
+__printf(2, 3)
+void __mlog_printk(const u64 *m, const char *fmt, ...);
 
 /*
  * Testing before the __mlog_printk call lets the compiler eliminate the
@@ -174,8 +173,7 @@ void __mlog_printk(const u64 *m, const char *func, int line,
 do {									\
 	u64 _m = MLOG_MASK_PREFIX | (mask);				\
 	if (_m & ML_ALLOWED_BITS)					\
-		__mlog_printk(&_m, __func__, __LINE__, fmt,		\
-			      ##__VA_ARGS__);				\
+		__mlog_printk(&_m, fmt, ##__VA_ARGS__);			\
 } while (0)
 
 #define mlog_errno(st) ({						\

----------------------------------------------------------

 fs/ocfs2/cluster/tcp.c | 60 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index 56c403a..2c74973 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -78,29 +78,43 @@
 			  &sc->sc_node->nd_ipv4_address,		\
 			  ntohs(sc->sc_node->nd_ipv4_port)
 
-/*
- * In the following two log macros, the whitespace after the ',' just
- * before ##args is intentional. Otherwise, gcc 2.95 will eat the
- * previous token if args expands to nothing.
- */
-#define msglog(hdr, fmt, args...) do {					\
-	typeof(hdr) __hdr = (hdr);					\
-	mlog(ML_MSG, "[mag %u len %u typ %u stat %d sys_stat %d "	\
-	     "key %08x num %u] " fmt,					\
-	     be16_to_cpu(__hdr->magic), be16_to_cpu(__hdr->data_len), 	\
-	     be16_to_cpu(__hdr->msg_type), be32_to_cpu(__hdr->status),	\
-	     be32_to_cpu(__hdr->sys_status), be32_to_cpu(__hdr->key),	\
-	     be32_to_cpu(__hdr->msg_num) ,  ##args);			\
-} while (0)
-
-#define sclog(sc, fmt, args...) do {					\
-	typeof(sc) __sc = (sc);						\
-	mlog(ML_SOCKET, "[sc %p refs %d sock %p node %u page %p "	\
-	     "pg_off %zu] " fmt, __sc,					\
-	     atomic_read(&__sc->sc_kref.refcount), __sc->sc_sock,	\
-	    __sc->sc_node->nd_num, __sc->sc_page, __sc->sc_page_off ,	\
-	    ##args);							\
-} while (0)
+__printf(2, 3)
+void msglog(struct o2net_msg *hdr, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	mlog(ML_MSG, "[mag %u len %u typ %u stat %d sys_stat %d key %08x num %u] %pV",
+	     be16_to_cpu(hdr->magic), be16_to_cpu(hdr->data_len),
+	     be16_to_cpu(hdr->msg_type), be32_to_cpu(hdr->status),
+	     be32_to_cpu(hdr->sys_status), be32_to_cpu(hdr->key),
+	     be32_to_cpu(hdr->msg_num), &vaf);
+
+	va_end(args);
+}
+
+__printf(2, 3)
+void sclog(struct o2net_sock_container *sc, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	mlog(ML_SOCKET, "[sc %p refs %d sock %p node %u page %p pg_off %zu] %pV",
+	     sc, atomic_read(&sc->sc_kref.refcount), sc->sc_sock,
+	     sc->sc_node->nd_num, sc->sc_page, sc->sc_page_off, &vaf);
+
+	va_end(args);
+}
 
 static DEFINE_RWLOCK(o2net_handler_lock);
 static struct rb_root o2net_handler_tree = RB_ROOT;

Re: [Ocfs2-devel] [PATCH next] ocfs2: Reduce object size of mlog uses

[Mark Fasheh]

On Thu, Apr 23, 2015 at 04:35:21PM -0700, Joe Perches wrote:
> On Thu, 2015-04-23 at 16:04 -0700, Mark Fasheh wrote:
> > On Wed, Apr 22, 2015 at 03:46:04PM -0700, Andrew Morton wrote:
> > > If you feel like undertaking such a rotorooting then go wild - that should
> > > wake 'em up ;)
> > 
> > Ok, I've taken the bait  :)
> 
> "Here fishy, fishy...", erm, "Here Fasheh, Fasheh..."
> 
> With that out of the way:

:)


> 
> A couple of possibilities:
> 
> o I wonder whether or not file/func/line matter at all.
>   I think they don't.
>   Removing them would reduce code size ~90K

They do to those that are debugging live Ocfs2 modules. I would like at
least one pair kept please.


> o There's a small logging improvement possible in tcp.c.

This part looks good to me though, thanks for it.
	--Mark

> 
> Both below:
> 
> ----------------------------------------------------------
> 
>  fs/ocfs2/cluster/masklog.c | 7 +++----
>  fs/ocfs2/cluster/masklog.h | 8 +++-----
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
> index fc5e522..8b9816f 100644
> --- a/fs/ocfs2/cluster/masklog.c
> +++ b/fs/ocfs2/cluster/masklog.c
> @@ -77,8 +77,7 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
>  	_cpu;								\
>  })
>  
> -void __mlog_printk(const u64 *mask, const char *func, int line,
> -		   const char *fmt, ...)
> +void __mlog_printk(const u64 *mask, const char *fmt, ...)
>  {
>  	struct va_format vaf;
>  	va_list args;
> @@ -103,9 +102,9 @@ void __mlog_printk(const u64 *mask, const char *func, int line,
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
>  
> -	printk("%s(%s,%u,%lu):%s:%d %s%pV",
> +	printk("%s(%s,%u,%lu) %s%pV",
>  	       level, current->comm, task_pid_nr(current), __mlog_cpu_guess,
> -	       func, line, prefix, &vaf);
> +	       prefix, &vaf);
>  
>  	va_end(args);
>  }
> diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
> index 308ea0e..9e93f19 100644
> --- a/fs/ocfs2/cluster/masklog.h
> +++ b/fs/ocfs2/cluster/masklog.h
> @@ -162,9 +162,8 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
>  
>  #endif
>  
> -__printf(4, 5)
> -void __mlog_printk(const u64 *m, const char *func, int line,
> -		   const char *fmt, ...);
> +__printf(2, 3)
> +void __mlog_printk(const u64 *m, const char *fmt, ...);
>  
>  /*
>   * Testing before the __mlog_printk call lets the compiler eliminate the
> @@ -174,8 +173,7 @@ void __mlog_printk(const u64 *m, const char *func, int line,
>  do {									\
>  	u64 _m = MLOG_MASK_PREFIX | (mask);				\
>  	if (_m & ML_ALLOWED_BITS)					\
> -		__mlog_printk(&_m, __func__, __LINE__, fmt,		\
> -			      ##__VA_ARGS__);				\
> +		__mlog_printk(&_m, fmt, ##__VA_ARGS__);			\
>  } while (0)
>  
>  #define mlog_errno(st) ({						\
> 
> ----------------------------------------------------------
> 
>  fs/ocfs2/cluster/tcp.c | 60 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
> index 56c403a..2c74973 100644
> --- a/fs/ocfs2/cluster/tcp.c
> +++ b/fs/ocfs2/cluster/tcp.c
> @@ -78,29 +78,43 @@
>  			  &sc->sc_node->nd_ipv4_address,		\
>  			  ntohs(sc->sc_node->nd_ipv4_port)
>  
> -/*
> - * In the following two log macros, the whitespace after the ',' just
> - * before ##args is intentional. Otherwise, gcc 2.95 will eat the
> - * previous token if args expands to nothing.
> - */
> -#define msglog(hdr, fmt, args...) do {					\
> -	typeof(hdr) __hdr = (hdr);					\
> -	mlog(ML_MSG, "[mag %u len %u typ %u stat %d sys_stat %d "	\
> -	     "key %08x num %u] " fmt,					\
> -	     be16_to_cpu(__hdr->magic), be16_to_cpu(__hdr->data_len), 	\
> -	     be16_to_cpu(__hdr->msg_type), be32_to_cpu(__hdr->status),	\
> -	     be32_to_cpu(__hdr->sys_status), be32_to_cpu(__hdr->key),	\
> -	     be32_to_cpu(__hdr->msg_num) ,  ##args);			\
> -} while (0)
> -
> -#define sclog(sc, fmt, args...) do {					\
> -	typeof(sc) __sc = (sc);						\
> -	mlog(ML_SOCKET, "[sc %p refs %d sock %p node %u page %p "	\
> -	     "pg_off %zu] " fmt, __sc,					\
> -	     atomic_read(&__sc->sc_kref.refcount), __sc->sc_sock,	\
> -	    __sc->sc_node->nd_num, __sc->sc_page, __sc->sc_page_off ,	\
> -	    ##args);							\
> -} while (0)
> +__printf(2, 3)
> +void msglog(struct o2net_msg *hdr, const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	mlog(ML_MSG, "[mag %u len %u typ %u stat %d sys_stat %d key %08x num %u] %pV",
> +	     be16_to_cpu(hdr->magic), be16_to_cpu(hdr->data_len),
> +	     be16_to_cpu(hdr->msg_type), be32_to_cpu(hdr->status),
> +	     be32_to_cpu(hdr->sys_status), be32_to_cpu(hdr->key),
> +	     be32_to_cpu(hdr->msg_num), &vaf);
> +
> +	va_end(args);
> +}
> +
> +__printf(2, 3)
> +void sclog(struct o2net_sock_container *sc, const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	mlog(ML_SOCKET, "[sc %p refs %d sock %p node %u page %p pg_off %zu] %pV",
> +	     sc, atomic_read(&sc->sc_kref.refcount), sc->sc_sock,
> +	     sc->sc_node->nd_num, sc->sc_page, sc->sc_page_off, &vaf);
> +
> +	va_end(args);
> +}
>  
>  static DEFINE_RWLOCK(o2net_handler_lock);
>  static struct rb_root o2net_handler_tree = RB_ROOT;
> 
> 
> 
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
--
Mark Fasheh

Re: [Ocfs2-devel] [PATCH next] ocfs2: Reduce object size of mlog uses

[Joe Perches]

On Tue, 2015-04-28 at 11:30 -0700, Mark Fasheh wrote:
> On Thu, Apr 23, 2015 at 04:35:21PM -0700, Joe Perches wrote:
> > On Thu, 2015-04-23 at 16:04 -0700, Mark Fasheh wrote:
> > > On Wed, Apr 22, 2015 at 03:46:04PM -0700, Andrew Morton wrote:
> > > > If you feel like undertaking such a rotorooting then go wild - that should
> > > > wake 'em up ;)
> > > Ok, I've taken the bait  :)
> > "Here fishy, fishy...", erm, "Here Fasheh, Fasheh..."
> > With that out of the way:
> :)

Yeah.  Sorry 'bout that.

My own name is vaguely aquatic too so I thought I
could get away with it.  Bad Joe...

> > A couple of possibilities:
> > 
> > o I wonder whether or not file/func/line matter at all.
> >   I think they don't.
> >   Removing them would reduce code size ~90K
> 
> They do to those that are debugging live Ocfs2 modules. I would like at
> least one pair kept please.

Your code, your choice.

> > o There's a small logging improvement possible in tcp.c.
> This part looks good to me though, thanks for it.

Please submit something like it whenever you deem
appropriate.

cheers, Joe