From: Michael Ellerman <mpe@ellerman•id.au>
To: Bjorn Helgaas <helgaas@kernel•org>,
Gavin Shan <gwshan@linux•vnet.ibm.com>
Cc: linux-pci@vger•kernel.org, linuxppc-dev@ozlabs•org, bhelgaas@google•com
Subject: Re: [PATCH 4/4] drivers/pci/hotplug: Support surprise hotplug
Date: Thu, 22 Sep 2016 20:51:28 +1000 [thread overview]
Message-ID: <877fa48axb.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <20160921165703.GA17457@localhost>
Bjorn Helgaas <helgaas@kernel•org> writes:
> Hi Gavin,
>
> You don't need my ack for any of these, and I assume you'll merge them
> through the powerpc tree.
Thanks Bjorn, I wasn't sure if you wanted to ack it or not. I'll take
the whole series via the powerpc tree.
> Minor comments below, feel free to ignore them.
>
> On Wed, Sep 21, 2016 at 10:15:30PM +1000, Gavin Shan wrote:
>> ...
>> @@ -536,9 +565,16 @@ static struct pnv_php_slot *pnv_php_alloc_slot(struct device_node *dn)
>> if (unlikely(!php_slot))
>> return NULL;
>>
>> + php_slot->event = kzalloc(sizeof(struct pnv_php_event), GFP_KERNEL);
>> + if (unlikely(!php_slot->event)) {
>> + kfree(php_slot);
>> + return NULL;
>> + }
>
> Since you *always* allocate the event when allocating the php_slot,
> making the event a member of php_slot (instead of keeping a pointer to
> it) would simplify your memory management a bit.
> It seems to be the style in this file to use "unlikely" liberally, but
> I really doubt there's any performance consideration in this code. To
> me it adds more clutter than usefulness.
Agreed on both counts. Gavin can you do another spin with those changes,
thanks.
cheers
next prev parent reply other threads:[~2016-09-22 10:51 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-21 12:15 [PATCH 0/4] powerpc/powernv: PCI Surprise Hotplug Support Gavin Shan
2016-09-21 12:15 ` [PATCH 1/4] powerpc/eeh: Allow to freeze PE in eeh_pe_set_option() Gavin Shan
2016-09-21 12:15 ` [PATCH 2/4] powerpc/eeh: Export eeh_pe_state_mark() Gavin Shan
2016-09-21 12:15 ` [PATCH 3/4] powerpc/powernv: Unfreeze PE on allocation Gavin Shan
2016-09-21 12:15 ` [PATCH 4/4] drivers/pci/hotplug: Support surprise hotplug Gavin Shan
2016-09-21 16:57 ` Bjorn Helgaas
2016-09-22 10:51 ` Michael Ellerman [this message]
2016-09-26 13:08 ` Gavin Shan
2016-09-26 13:11 ` Bjorn Helgaas
-- strict thread matches above, loose matches on Subject: below --
2016-09-15 2:07 [PATCH 0/4] powerpc/powernv: PCI Surprise Hotplug Support Gavin Shan
2016-09-15 2:07 ` [PATCH 4/4] drivers/pci/hotplug: Support surprise hotplug Gavin Shan
2016-09-21 4:08 ` Michael Ellerman
2016-09-21 12:20 ` Gavin Shan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877fa48axb.fsf@concordia.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=bhelgaas@google$(echo .)com \
--cc=gwshan@linux$(echo .)vnet.ibm.com \
--cc=helgaas@kernel$(echo .)org \
--cc=linux-pci@vger$(echo .)kernel.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox