From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754890AbcH0HDQ (ORCPT ); Sat, 27 Aug 2016 03:03:16 -0400 Received: from mout.web.de ([212.227.15.14]:51252 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752533AbcH0HDO (ORCPT ); Sat, 27 Aug 2016 03:03:14 -0400 Subject: Re: [PATCH 1/5] IA64-IRQ: Use kmalloc_array() in sn_irq_lh_init() To: Joe Perches References: <349bbfb4-bada-628e-2981-ca2a315299fc@users.sourceforge.net> <2e046b40-1c8e-717f-68b1-534c3125724c@users.sourceforge.net> <1472245341.4914.79.camel@perches.com> Cc: Julia Lawall , linux-ia64@vger.kernel.org, Fenghua Yu , Tony Luck , LKML , kernel-janitors@vger.kernel.org, Paolo Bonzini From: SF Markus Elfring Message-ID: <894bf885-4cf0-fcaa-e040-35d9add64acc@users.sourceforge.net> Date: Sat, 27 Aug 2016 09:02:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: <1472245341.4914.79.camel@perches.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K0:vvWULKHbjK9Y0sD3IqWX7SVmp70830HPuLj7HXrLETsxshTqYrW m/PEjAAVivyJdCgTcsEOHh8FZcwYnYMsoQhwWJWW9OG7HdhqSGoTw6m4GY7JxDE5JbhQoJj D4sTmIAJ44eyZkRi6NSZRC9zHx2J12R/hNurFVYbh4aDo4vfGU8fijaKbBvAus8PYlBSr42 UKNRCR85ufVNns8+J70+Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:ISVB20/Pwyg=:QoehAI169ELeW3hQrR2P/y WqNMsIob3y+Q+RY0qIOrxNuGcW9RcGgAbRzNZw9+GspkVEA3YE/twpfeZIFRYuWby0ZEKhbhP r+3UmtPJfOP1qyTuPBVEe/5mJ5KGYXOYr+XbiEva4jaJAEaHiiaS0pBcomE6rmYGcdeacKraL +kxIfn2auC3274B5YgTtzFUh/kvpydozaLVX1twi0rOo9k4hon66KQgsuRF4I3EEwbfd4eB1H 49EL6YpMrd7pcxNbQvnSxTLAU4lS5UL3TtWLgBBDEHZdWzJga5OCP9U4NjF7+xgLB1PH0QxjT aTh29nott7ibBrjKgNUfM39lHKkHM2YvgiVlFh8I47IYPJgzCvFy0eGFd7kdd15Ex9MSAOLI4 gxPokCkya5ZrTHB6qD9oKlS/P4M6KY611SN2aYnOTBw98AZG7omv9FYhgtfT9DviMNPKddrN2 wc4NR70pjxxl9iZXwnM4vn7HOMH9OudcRm0ClF4QppYEBL8A7o9TEGu/QeHuc5M1YU/c7uiIR naur4niVylXahah4q7lVps53XuUqASxKLOadQ1nlv3oui6VfhSXJss2GjQmz308KaSsW3VS9S kszpEZTSY/21PXhB1KAQPJ8GQn2FhL1TcbKicfZyXY76y85Tk6JeAtE57uNhgH3dPmmwXy2c7 8c2Pqu345Jry3HVPg+E2RsUFp/0qdtbNWtnrAGwn2oiQ+zeQUxrEWtUTDmL52jwcNqCi6OBuJ gJZDD/D/n/jUEka6VjHeFENEvX7PP2P0n+OzwaNSm8RW7JRm4586MgrYH+DdoJcZEhBoTUvy7 O0LQnQp Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >>> @@ -474,12 +474,12 @@ void __init sn_irq_lh_init(void) >>> { >>> int i; >>> >>> - sn_irq_lh = kmalloc(sizeof(struct list_head *) * NR_IRQS, GFP_KERNEL); >>> + sn_irq_lh = kmalloc_array(NR_IRQS, sizeof(*sn_irq_lh), GFP_KERNEL); >>> if (!sn_irq_lh) >>> panic("SN PCI INIT: Failed to allocate memory for PCI init\n"); >>> >>> for (i = 0; i < NR_IRQS; i++) { >>> - sn_irq_lh[i] = kmalloc(sizeof(struct list_head), GFP_KERNEL); >>> + sn_irq_lh[i] = kmalloc(*sn_irq_lh[i], GFP_KERNEL); >> >> Did a sizeof get lost here? > > Yes, thanks Julia. Unfortunately, another copy mistake happened during a bit of source code editing. > This is why adding the generating spatch code is always good. I find that this broken update suggestion can point a few details out for further considerations. I dared to combine some software aspects once more in this use case. Such a combination (join point) shows interesting challenges, doesn't it? > And Markus, please always compile test your code using the > appropriate cross-compilers available here: > https://www.kernel.org/pub/tools/crosstool/ Thanks for your link. > And btw: using sizeof(*pp[i]) or sizeof(**pp) is not always > clearer or better than using sizeof(type) Do you express a target conflict between your expectations and the evolving Linux coding style documentation here? Would any software developers insist to see the corresponding data type directly instead of "evaluating" a pointer expression? > 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? Regards, Markus