From: Alex Chiang <achiang@hp.com>
To: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: jbarnes@virtuousgeek.org, kristen.c.accardi@intel.com,
matthew@wil.cx, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 00/15] PCI: let the core manage slot names
Date: Wed, 8 Oct 2008 22:19:05 -0600 [thread overview]
Message-ID: <20081009041905.GE4668@ldl.fc.hp.com> (raw)
In-Reply-To: <48EC53D3.9020203@jp.fujitsu.com>
* Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>:
> 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.
> - [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.
/ac
next prev parent reply other threads:[~2008-10-09 4:22 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-03 23:17 [PATCH v4 00/15] PCI: let the core manage slot names Alex Chiang
2008-10-03 23:17 ` [PATCH v4 01/15] PCI Hotplug core: add 'name' param pci_hp_register interface Alex Chiang
2008-10-06 9:04 ` Matthew Wilcox
2008-10-03 23:17 ` [PATCH v4 02/15] PCI Hotplug: serialize pci_hp_register/deregister Alex Chiang
2008-10-06 14:45 ` Matthew Wilcox
2008-10-09 4:09 ` Alex Chiang
2008-10-08 5:42 ` Kenji Kaneshige
2008-10-03 23:17 ` [PATCH v4 03/15] PCI: prevent duplicate slot names Alex Chiang
2008-10-08 6:00 ` Kenji Kaneshige
2008-10-09 4:12 ` Alex Chiang
2008-10-03 23:17 ` [PATCH v4 04/15] PCI, PCI Hotplug: introduce slot_name helpers Alex Chiang
2008-10-03 23:17 ` [PATCH v4 05/15] PCI: acpiphp: remove 'name' parameter Alex Chiang
2008-10-03 23:17 ` [PATCH v4 06/15] PCI: cpci_hotplug: stop managing hotplug_slot->name Alex Chiang
2008-10-03 23:18 ` [PATCH v4 07/15] PCI: cpqphp: " Alex Chiang
2008-10-03 23:18 ` [PATCH v4 08/15] PCI: fakephp: remove 'name' parameter Alex Chiang
2008-10-03 23:18 ` [PATCH v4 09/15] PCI: ibmphp: stop managing hotplug_slot->name Alex Chiang
2008-10-03 23:18 ` [PATCH v4 10/15] PCI: pciehp: remove 'name' parameter Alex Chiang
2008-10-03 23:18 ` [PATCH v4 11/15] PCI: rpaphp: kmalloc/kfree slot->name directly Alex Chiang
2008-10-06 6:44 ` Pekka Enberg
2008-10-09 4:05 ` Alex Chiang
2008-10-03 23:18 ` [PATCH v4 12/15] PCI: SGI Hotplug: stop managing bss_hotplug_slot->name Alex Chiang
2008-10-03 23:18 ` [PATCH v4 13/15] PCI: shcphp: remove 'name' parameter Alex Chiang
2008-10-03 23:18 ` [PATCH v4 14/15] PCI: Hotplug core: remove 'name' Alex Chiang
2008-10-03 23:18 ` [PATCH v4 15/15] PCI Hotplug: fakephp: add duplicate slot name debugging Alex Chiang
2008-10-08 6:31 ` [PATCH v4 00/15] PCI: let the core manage slot names Kenji Kaneshige
2008-10-08 6:33 ` [01/03] Sample patch for [PATCH v4 02/15] Kenji Kaneshige
2008-10-08 6:34 ` [02/03] Sample patch for [PATCH v4 03/15] Kenji Kaneshige
2008-10-08 6:36 ` [03/03] Sample patch for [PATCH v4 14/15] Kenji Kaneshige
2008-10-09 4:19 ` Alex Chiang [this message]
2008-10-09 5:01 ` [PATCH v4 00/15] PCI: let the core manage slot names Kenji Kaneshige
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=20081009041905.GE4668@ldl.fc.hp.com \
--to=achiang@hp.com \
--cc=jbarnes@virtuousgeek.org \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=kristen.c.accardi@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=matthew@wil.cx \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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