Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init()

From: Julia Lawall
Date: Sun Aug 28 2016 - 05:29:01 EST

On Sat, 27 Aug 2016, Joe Perches wrote:

> On Sat, 2016-08-27 at 09:02 +0200, SF Markus Elfring wrote:
> > > If you _really wanted to clear up this code and make it more
> > > robust/better, it'd probably be nicer to convert the
> > > struct list_head **sn_irq_lh to a single struct list_head *
> > > That would be less data space overall given the alignment
> > > waste of the individual allocs.
> > Does this suggestion mean that I should drop my proposal
> > around the software components "IRQ" and "TLB" for the system
> > architecture "IA64" in such a questionable patch series?
> While elimination of code duplication should be good,
> what it means it you should avoid making changes that
> are merely mechanical and strive to make changes that
> improve code execution speed or reduce overall object
> size while not impacting overall execution speed.

I do think that there is some value in doing similar things in a uniform
way, using meaningful names, even if in a particular case it doesn't help
performance or reduce code size. Even duplicating code could be OK if it
is not in a critical path and it makes the code overall easier to
understand. But if the maintainer prefers the code not to be duplicated,
then of course it should not be duplicated.