Re: [PATCH v4 00/15] PCI: let the core manage slot names

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Alex Chiang wrote:
* Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>:
Hi Alex-san,

I have some ideas and made some patches for comments I sent about
your [PATCH v4 02/15] and [PATCH v4 03/15]. Please take a look.
There are three patches.

Hi Kenji-san,

Thanks for doing this work.

I tested your patches, but found a problem with refcounting in
your 02/03, and the slot directories in sysfs remained, even
after rmmod of all drivers.

I decided that things are getting too complicated with all these
new interfaces, so I got rid of them and updated the
pci_create_slot API instead, to take a 'rename' parameter. That
way, creating a slot and overriding its name can become an atomic
operation.

That should make the race conditions go away, and the code is
much easier to understand as well.


OK.
I guess one of the thing that will bother us is how to handle
the case pci_slot created by pci_create_slot() already has its
hotplug callbacks in pci_hp_register(). Current code calls
pci_destroy_slot() and return -EBUSY in this case. With the
new API which takes 'rename' parameterIn addtion to that, we
need to rename the slot as it used to be, in addition.


 - [01/03] Sample patch for [PATCH v4 02/15]

This is a good patch by itself. I think you should submit it to
Jesse. I did not need it after reworking to my new design.

 - [02/03] Sample patch for [PATCH v4 03/15]
   NOTE:This doesn't target the comment about changing exported
        symbol name.

I had to stare at this patch for a long time to understand it,
and finally saw that you were changing the rename logic to detect
if we were trying to rename an existing slot.

Unfortunately, it had some problem with the refcounting, and in
this scenario:

	- pci_slot loaded
	- fakephp dup_slots=1 loaded
	- pci_slot unloaded

The slots claimed by pci_slot (but _not_ by fakephp) were never
released.

By the time I got this far, I was already thinking about a
redesign, so I did not try and debug further...

 - [03/03] Sample patch for [PATCH v4 14/15]
   This is needed because above two patches make your [PATCH v4
   14/15] can not be applied.

Not needed, since I re-designed the approach.

Note: I made those patches as replacement of your corresponding
ones. So those patches are NOT for applying on top your original
patches.

Again, thank you very much for all the review and hard work, and
sorry for causing so much churn. :-/

I'll be sending out the new patch series shortly.


No problem.
I'm looking forward to looking at new patches.

Thanks,
Kenji Kaneshige



--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux