Date:	Tue, 28 Apr 2015 11:30:08 -0700
From:	Mark Fasheh <mfasheh@...e.de>
To:	Joe Perches <joe@...ches.com>
Cc:	ocfs2-devel@....oracle.com, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [Ocfs2-devel] [PATCH next] ocfs2: Reduce object size of mlog uses

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@....oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
--
Mark Fasheh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists