Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031214AbbDWXEV (ORCPT ); Thu, 23 Apr 2015 19:04:21 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52032 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754504AbbDWXEU (ORCPT ); Thu, 23 Apr 2015 19:04:20 -0400 Date: Thu, 23 Apr 2015 16:04:18 -0700 From: Mark Fasheh To: Andrew Morton Cc: Joe Perches , Joel Becker , ocfs2-devel@oss.oracle.com, LKML Subject: Re: [PATCH next] ocfs2: Reduce object size of mlog uses Message-ID: <20150423230418.GR17170@wotan.suse.de> Reply-To: Mark Fasheh References: <1429255070.2850.84.camel@perches.com> <20150422154604.3cb62467b66b1e313a30132c@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150422154604.3cb62467b66b1e313a30132c@linux-foundation.org> Organization: SUSE Labs User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3107 Lines: 88 On Wed, Apr 22, 2015 at 03:46:04PM -0700, Andrew Morton wrote: > On Fri, 17 Apr 2015 00:17:50 -0700 Joe Perches 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/