Subject: Re: [PATCH 4/4] drivers/pci/hotplug: Support surprise hotplug

From: Michael Ellerman <[email-masked]>
Date: Thu, 22 Sep 2016 20:51:28 +1000

Bjorn Helgaas <[email-masked]> 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