linux-kernel - Re: [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some duplicated lines

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120712032624.GC22640@shlinux2.ap.freescale.net>
Date:	Thu, 12 Jul 2012 11:26:25 +0800
From:	Dong Aisheng <b29396@...escale.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"benh@...nel.crashing.org" <benh@...nel.crashing.org>
Subject: Re: [RFC PATCH 2/2] irq: add irq_desc_initialize to remove some
 duplicated lines

Hi Thomas,

Thanks for the review firstly.

On Thu, Jul 12, 2012 at 06:19:18AM +0800, Thomas Gleixner wrote:
> On Wed, 20 Jun 2012, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@...aro.org>
> > 
> > There're two copies of irq_desc initialization code, reform them into
> > an irq_desc_initialize function to call.
> > 
> > Signed-off-by: Dong Aisheng <dong.aisheng@...aro.org>
> > ---
> >  kernel/irq/irqdesc.c |   51 +++++++++++++++++++++++++++----------------------
> >  1 files changed, 28 insertions(+), 23 deletions(-)
> 
> We add more code by removing redundant copies?
> 
I also had this strange question.
I looked the code a bit more, i guess the main problem is that the redundant copies
is not too big, so we can not see great savings.

Compare to the init code of irq_desc in original alloc_desc function,
the new irq_desc_initialize function saves 4 lines.
However, the new function also add 4 lines for defining extra function name,
parameter and etc.
And alloc_desc still needs to call irq_desc_initialize and checking return value
which needs extra 6 lines.
The main saving is another copy of irq_desc initialization in early_irq_init,
but this copy does not check any return values which cause we did not save too much,
only about 4 lines.
Plus extra blan lines added, so totally it does not save more than new added.

However, i was thinking it could make code looks a bit better.
So i sent out this RFC patch.
Do you think if it's reasonable?

BTW, there's an issue in my patch, should change like:
        if (alloc_masks(desc, GFP_KERNEL, node)) {
-               kfree(desc->kstat_irqs);
+               free_percpu(desc->kstat_irqs);
                return -ENOMEM;
        }

Regards
Dong Aisheng

--
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/