* Re: possible bug regarding pciehp
From: Tormen @ 2015-09-18 9:19 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <55E70945.6060802@diplomail.ch>
On 09/05/2015 05:38 AM, Greg KH wrote:
> On Fri, Sep 04, 2015 at 09:54:18AM +0200, Tormen wrote:
>> On 09/02/2015 05:27 PM, Greg KH wrote:
>>> On Wed, Sep 02, 2015 at 04:35:49PM +0200, Tormen wrote:
>> When the network driver is loaded it seems to connect as a "PCI Express"
>> device:
>> tg3 0000:0a:00.0 eth0: Tigon3 [partno(BCM957762) rev 57766000] (PCI
>> Express)
>
> That's separate from the thunderbolt controller.
>
>> When I unplug the adapter the first thing appearing in dmesg is pciehp:
>> [ 210.992938] usb 1-2: USB disconnect, device number 4
>> [ 1557.453818] pciehp 0000:06:03.0:pcie24: slot(3-2): Link Down event
>> [ 1557.453870] pciehp 0000:06:03.0:pcie24: Cannot remove display device
>> 0000:08:00.0
>> [ 1557.509812] pciehp 0000:06:03.0:pcie24: Card not present on Slot(3-2)
>> [ 1558.721344] tg3 0000:0a:00.0: tg3_abort_hw timed out, TX_MODE_ENABLE
>> will not clear MAC_TX_MODEÿffffff
>> [ 1558.934601] pciehp 0000:09:00.0:pcie24: unloading service driver pciehp
>> [ 1558.935366] pciehp 0000:09:00.0:pcie24: Timeout on hotplug command
>> 0x1038 (issued 1557532 msec ago)
>> [ 1558.943460] pci_bus 0000:0a: busn_res: [bus 0a] is released
>> [ 1558.958426] pci_bus 0000:09: busn_res: [bus 09-0a] is released
>> [ 1558.958432] acpiphp: Slot [1] unregistered
>> [ 1558.958451] acpiphp: Slot [2-1] unregistered
>> [ 1558.958464] acpiphp: Slot [3-1] unregistered
>> [ 1558.958475] acpiphp: Slot [4-1] unregistered
>> [ 1558.958486] acpiphp: Slot [5-1] unregistered
>>
>> But I am not a kernel, pciehp, thunderbolt specialist :(
>>
>> How can I determine why the pciehp hotplug command 0x1038 timed out ?
>
> No idea, sorry, this is very odd hardware that doesn't follow any spec.
> Try emailing the author of the thunderbolt code, they should be able to
> help you out more.
>
> good luck,
>
> greg k-h
>
Thanks a lot! - I did and he was very kind in answering me right away -
thanks a lot Andreas - and his suggestion did lead to a working patch:
https://bugzilla.kernel.org/show_bug.cgi?id\x100191#c18
which makes the thunderbolt module load on boot and thus - like you
assumed Greg - provides the missing hotplug functionality!
Thanks again !
Tormen
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Rafael J. Wysocki @ 2015-09-08 22:28 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Jiang Liu, mingo, Rafael J. Wysocki, Tejun Heo,
linux hotplug mailing, Linux Kernel Mailing List,
ACPI Devel Maling List
In-Reply-To: <20150908104008.GD3644@twins.programming.kicks-ass.net>
On Tuesday, September 08, 2015 12:40:08 PM Peter Zijlstra wrote:
> On Mon, Sep 07, 2015 at 11:33:21PM +0200, Rafael J. Wysocki wrote:
> > On Monday, September 07, 2015 11:11:19 AM Jiang Liu wrote:
> > Peter, Ingo, some help from lockdep expert is needed.
> >
> > We have a splat that almost certainly is a false positive (the original report
> > is here http://marc.info/?l=linux-kernel&m\x144109156901959&w=4) and no ideas
> > how to make it go away. Can you please have a look and advise?
>
> I can't even find the relevant code :/
>
> From that email I get kernfs_fop_write() which calls
> kernfs_get_active(), but that does _NOT_ call cpu_up(), so that
> callchain is shite.
>
> The actual lockdep splat is also not really helpful, and is spraying
> names over: acpi, device, sysfs and kernfs (do we really need that many
> layeres of obfuscation for a simple file?)
>
> So, please, start by explaining the thing proper such that simple people
> like me know what to look for.
OK, I'll try to get that later this week.
Or maybe Jiang Liu can beat me to doing that. :-)
Thanks,
Rafael
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Peter Zijlstra @ 2015-09-08 10:40 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Jiang Liu, mingo, Rafael J. Wysocki, Tejun Heo,
linux hotplug mailing, Linux Kernel Mailing List,
ACPI Devel Maling List
In-Reply-To: <2165815.z3RZSlC1oy@vostro.rjw.lan>
On Mon, Sep 07, 2015 at 11:33:21PM +0200, Rafael J. Wysocki wrote:
> On Monday, September 07, 2015 11:11:19 AM Jiang Liu wrote:
> Peter, Ingo, some help from lockdep expert is needed.
>
> We have a splat that almost certainly is a false positive (the original report
> is here http://marc.info/?l=linux-kernel&m\x144109156901959&w=4) and no ideas
> how to make it go away. Can you please have a look and advise?
I can't even find the relevant code :/
From that email I get kernfs_fop_write() which calls
kernfs_get_active(), but that does _NOT_ call cpu_up(), so that
callchain is shite.
The actual lockdep splat is also not really helpful, and is spraying
names over: acpi, device, sysfs and kernfs (do we really need that many
layeres of obfuscation for a simple file?)
So, please, start by explaining the thing proper such that simple people
like me know what to look for.
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Rafael J. Wysocki @ 2015-09-07 21:33 UTC (permalink / raw)
To: Jiang Liu, mingo, Peter Zijlstra
Cc: Rafael J. Wysocki, Tejun Heo, linux hotplug mailing,
Linux Kernel Mailing List, ACPI Devel Maling List
In-Reply-To: <55ED0057.1000806@linux.intel.com>
On Monday, September 07, 2015 11:11:19 AM Jiang Liu wrote:
> On 2015/9/4 22:16, Rafael J. Wysocki wrote:
> > Hi,
> >
> > On Fri, Sep 4, 2015 at 9:20 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> >> On 2015/9/4 4:08, Rafael J. Wysocki wrote:
> >>> Hi Tejun,
> >>>
> >>> On Thu, Sep 3, 2015 at 6:19 PM, Tejun Heo <tj@kernel.org> wrote:
> >>>> Hello, Rafael.
> >>>>
> >>>> On Thu, Sep 03, 2015 at 02:58:16AM +0200, Rafael J. Wysocki wrote:
> >>>>> So acpi_device_hotplug() calls lock_device_hotplug() which simply
> >>>>> acquires device_hotplug_lock. It is held throughout the entire
> >>>>> hot-add/hot-remove code path.
> >>>>>
> >>>>> Witing anything to /sys/devices/system/cpu/cpux/online goes through
> >>>>> online_store() in drivers/base/core.c and that does
> >>>>> lock_device_hotplug_sysfs() which then attempts to acquire
> >>>>> device_hotplug_lock using mutex_trylock(). And it only calls
> >>>>> either device_online() or device_offline() if it ends up with the
> >>>>> lock held.
> >>>>>
> >>>>> Quite frankly, I don't see how these particular two code paths can
> >>>>> deadlock in any way.
> >>>>>
> >>>>> So either a third code path is involved which is not executed
> >>>>> under device_hotplug_lock, or lockdep needs to be told to actually
> >>>>> take device_hotplug_lock into account in this case IMO.
> >>>>
> >>>> Hmm... all sysfs rw functions are protected from removal. ie. by
> >>>> default, removal of a sysfs file drains in-flight rw operations, so
> >>>> the hot plug path grabs a lock and then tries to remove a file and
> >>>> writing to the online file makes the file's write method to try to
> >>>> grab the same lock. It deadlocks if the hotunplug path already has
> >>>> the lock and trying to drain the online file for removal.
> >>>
> >>> My point is that you cannot get into that situation. If hotplug
> >>> already holds device_hotplug_lock, the write to "online" will end up
> >>> doing restart_syscall().
> >>>
> >>> If the "online" code path is holding the lock, hotplug cannot acquire
> >>> it and cannot proceed.
> >>>
> >>> Am I missing anything?
> >> Hi Rafael,
> >> I think your are right. The lock_device_hotplug_sysfs() has
> >> already provided a solution for such a deadlock scenario. And there's
> >> another related code path at boot as:
> >> smp_init()
> >> ->cpu_up()
> >> ->cpu_hotplug_begin()
> >> So it seems to be a false alarm. Any way to teach lockdep
> >> about this to get rid of the false alarm?
> >
> > Well, maybe we could call lock_device_hotplug() from that code path too?
> Hi Rafael,
> Adding lock_device_hotplug() to smp_init() doesn't solve the
> issue. So it seems to be an false alarm of lockdep, and I don't know
> how to get rid of such an lockdep false alarm:(
Peter, Ingo, some help from lockdep expert is needed.
We have a splat that almost certainly is a false positive (the original report
is here http://marc.info/?l=linux-kernel&m\x144109156901959&w=4) and no ideas
how to make it go away. Can you please have a look and advise?
Rafael
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Jiang Liu @ 2015-09-07 3:11 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Tejun Heo, Rafael J. Wysocki, linux hotplug mailing,
Linux Kernel Mailing List, ACPI Devel Maling List
In-Reply-To: <CAJZ5v0itBxg_gQzEe=nVfxOVpw5WBs9u8VDP7PHoyXyirnmdPQ@mail.gmail.com>
On 2015/9/4 22:16, Rafael J. Wysocki wrote:
> Hi,
>
> On Fri, Sep 4, 2015 at 9:20 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>> On 2015/9/4 4:08, Rafael J. Wysocki wrote:
>>> Hi Tejun,
>>>
>>> On Thu, Sep 3, 2015 at 6:19 PM, Tejun Heo <tj@kernel.org> wrote:
>>>> Hello, Rafael.
>>>>
>>>> On Thu, Sep 03, 2015 at 02:58:16AM +0200, Rafael J. Wysocki wrote:
>>>>> So acpi_device_hotplug() calls lock_device_hotplug() which simply
>>>>> acquires device_hotplug_lock. It is held throughout the entire
>>>>> hot-add/hot-remove code path.
>>>>>
>>>>> Witing anything to /sys/devices/system/cpu/cpux/online goes through
>>>>> online_store() in drivers/base/core.c and that does
>>>>> lock_device_hotplug_sysfs() which then attempts to acquire
>>>>> device_hotplug_lock using mutex_trylock(). And it only calls
>>>>> either device_online() or device_offline() if it ends up with the
>>>>> lock held.
>>>>>
>>>>> Quite frankly, I don't see how these particular two code paths can
>>>>> deadlock in any way.
>>>>>
>>>>> So either a third code path is involved which is not executed
>>>>> under device_hotplug_lock, or lockdep needs to be told to actually
>>>>> take device_hotplug_lock into account in this case IMO.
>>>>
>>>> Hmm... all sysfs rw functions are protected from removal. ie. by
>>>> default, removal of a sysfs file drains in-flight rw operations, so
>>>> the hot plug path grabs a lock and then tries to remove a file and
>>>> writing to the online file makes the file's write method to try to
>>>> grab the same lock. It deadlocks if the hotunplug path already has
>>>> the lock and trying to drain the online file for removal.
>>>
>>> My point is that you cannot get into that situation. If hotplug
>>> already holds device_hotplug_lock, the write to "online" will end up
>>> doing restart_syscall().
>>>
>>> If the "online" code path is holding the lock, hotplug cannot acquire
>>> it and cannot proceed.
>>>
>>> Am I missing anything?
>> Hi Rafael,
>> I think your are right. The lock_device_hotplug_sysfs() has
>> already provided a solution for such a deadlock scenario. And there's
>> another related code path at boot as:
>> smp_init()
>> ->cpu_up()
>> ->cpu_hotplug_begin()
>> So it seems to be a false alarm. Any way to teach lockdep
>> about this to get rid of the false alarm?
>
> Well, maybe we could call lock_device_hotplug() from that code path too?
Hi Rafael,
Adding lock_device_hotplug() to smp_init() doesn't solve the
issue. So it seems to be an false alarm of lockdep, and I don't know
how to get rid of such an lockdep false alarm:(
Thanks!
Gerry
^ permalink raw reply
* Re: possible bug regarding pciehp
From: Greg KH @ 2015-09-05 3:38 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <55E70945.6060802@diplomail.ch>
On Fri, Sep 04, 2015 at 09:54:18AM +0200, Tormen wrote:
> On 09/02/2015 05:27 PM, Greg KH wrote:
> > On Wed, Sep 02, 2015 at 04:35:49PM +0200, Tormen wrote:
> >> Hi,
> >>
> >> I am contacting you, hoping that either you can maybe help
> >> or you know someone who can help :)
> >>
> >> I opened a bug related to the tg3 driver when hot-plugging via pciehp:
> >> https://bugzilla.kernel.org/show_bug.cgi?id\x100191
> >>
> >> I am desperate because :
> >> (a) there is also a brcmfmac related driver.
> >> So both wireless and wired internet have problems:
> >> https://bugzilla.kernel.org/show_bug.cgi?id\x100201
> >> (b) there was no input by anybody so far. Maybe because I wrongly
> >> categorized the tg3 bug ?
> >>
> >> The bug is related to Broadcom tg3 driver and the pciehp hotplug framework.
> >> In order to sort out where the problem(s) lies, I wanted to kindly ask
> >> you if you could have a look at the bug :
> >> https://bugzilla.kernel.org/show_bug.cgi?id\x100191
> >> ?
> >> And/or if you could help me figuring out if pciehp is causing this
> >> problem and/or if tg3 is to blame.
> >
> > Odds are neither are the issue, this is a thunderbolt problem. Make
> > sure you have enabled thunderbolt properly in your kernel if you want
> > hot-add of a device to work.
> >
> > I think the code has only been tested on older macbooks, changes might
> > be needed for newer ones given that Apple's thunderbolt controller is
> > "special" (i.e. doesn't follow the specification.)
> >
> > good luck!
> >
> > greg k-h
> >
>
> Thanks a lot for your feedback !
>
> Is "the code" that you are referring to the thunderbolt kernel module ?
Yes.
> The kernel I am using uses:
> CONFIG_THUNDERBOLT=m -- The module will be called thunderbolt.
> But when I boot-up with the plugged network device, lsmod does not show
> thunderbolt.
Have you loaded that module? Please do so:
modprobe thunderbolt
> So not sure how I can check further if thunderbolt is properly in my kernel?
> Or is it possible that I would need to load the thunderbolt module
> manually in order to make hotplug work?
Maybe.
> When the network driver is loaded it seems to connect as a "PCI Express"
> device:
> tg3 0000:0a:00.0 eth0: Tigon3 [partno(BCM957762) rev 57766000] (PCI
> Express)
That's separate from the thunderbolt controller.
> When I unplug the adapter the first thing appearing in dmesg is pciehp:
> [ 210.992938] usb 1-2: USB disconnect, device number 4
> [ 1557.453818] pciehp 0000:06:03.0:pcie24: slot(3-2): Link Down event
> [ 1557.453870] pciehp 0000:06:03.0:pcie24: Cannot remove display device
> 0000:08:00.0
> [ 1557.509812] pciehp 0000:06:03.0:pcie24: Card not present on Slot(3-2)
> [ 1558.721344] tg3 0000:0a:00.0: tg3_abort_hw timed out, TX_MODE_ENABLE
> will not clear MAC_TX_MODEÿffffff
> [ 1558.934601] pciehp 0000:09:00.0:pcie24: unloading service driver pciehp
> [ 1558.935366] pciehp 0000:09:00.0:pcie24: Timeout on hotplug command
> 0x1038 (issued 1557532 msec ago)
> [ 1558.943460] pci_bus 0000:0a: busn_res: [bus 0a] is released
> [ 1558.958426] pci_bus 0000:09: busn_res: [bus 09-0a] is released
> [ 1558.958432] acpiphp: Slot [1] unregistered
> [ 1558.958451] acpiphp: Slot [2-1] unregistered
> [ 1558.958464] acpiphp: Slot [3-1] unregistered
> [ 1558.958475] acpiphp: Slot [4-1] unregistered
> [ 1558.958486] acpiphp: Slot [5-1] unregistered
>
> But I am not a kernel, pciehp, thunderbolt specialist :(
>
> How can I determine why the pciehp hotplug command 0x1038 timed out ?
No idea, sorry, this is very odd hardware that doesn't follow any spec.
Try emailing the author of the thunderbolt code, they should be able to
help you out more.
good luck,
greg k-h
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Rafael J. Wysocki @ 2015-09-04 14:16 UTC (permalink / raw)
To: Jiang Liu
Cc: Rafael J. Wysocki, Tejun Heo, Rafael J. Wysocki,
linux hotplug mailing, Linux Kernel Mailing List,
ACPI Devel Maling List
In-Reply-To: <55E94642.6020707@linux.intel.com>
Hi,
On Fri, Sep 4, 2015 at 9:20 AM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> On 2015/9/4 4:08, Rafael J. Wysocki wrote:
>> Hi Tejun,
>>
>> On Thu, Sep 3, 2015 at 6:19 PM, Tejun Heo <tj@kernel.org> wrote:
>>> Hello, Rafael.
>>>
>>> On Thu, Sep 03, 2015 at 02:58:16AM +0200, Rafael J. Wysocki wrote:
>>>> So acpi_device_hotplug() calls lock_device_hotplug() which simply
>>>> acquires device_hotplug_lock. It is held throughout the entire
>>>> hot-add/hot-remove code path.
>>>>
>>>> Witing anything to /sys/devices/system/cpu/cpux/online goes through
>>>> online_store() in drivers/base/core.c and that does
>>>> lock_device_hotplug_sysfs() which then attempts to acquire
>>>> device_hotplug_lock using mutex_trylock(). And it only calls
>>>> either device_online() or device_offline() if it ends up with the
>>>> lock held.
>>>>
>>>> Quite frankly, I don't see how these particular two code paths can
>>>> deadlock in any way.
>>>>
>>>> So either a third code path is involved which is not executed
>>>> under device_hotplug_lock, or lockdep needs to be told to actually
>>>> take device_hotplug_lock into account in this case IMO.
>>>
>>> Hmm... all sysfs rw functions are protected from removal. ie. by
>>> default, removal of a sysfs file drains in-flight rw operations, so
>>> the hot plug path grabs a lock and then tries to remove a file and
>>> writing to the online file makes the file's write method to try to
>>> grab the same lock. It deadlocks if the hotunplug path already has
>>> the lock and trying to drain the online file for removal.
>>
>> My point is that you cannot get into that situation. If hotplug
>> already holds device_hotplug_lock, the write to "online" will end up
>> doing restart_syscall().
>>
>> If the "online" code path is holding the lock, hotplug cannot acquire
>> it and cannot proceed.
>>
>> Am I missing anything?
> Hi Rafael,
> I think your are right. The lock_device_hotplug_sysfs() has
> already provided a solution for such a deadlock scenario. And there's
> another related code path at boot as:
> smp_init()
> ->cpu_up()
> ->cpu_hotplug_begin()
> So it seems to be a false alarm. Any way to teach lockdep
> about this to get rid of the false alarm?
Well, maybe we could call lock_device_hotplug() from that code path too?
Thanks,
Rafael
^ permalink raw reply
* Re: possible bug regarding pciehp
From: Tormen @ 2015-09-04 7:54 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <55E70945.6060802@diplomail.ch>
On 09/02/2015 05:27 PM, Greg KH wrote:
> On Wed, Sep 02, 2015 at 04:35:49PM +0200, Tormen wrote:
>> Hi,
>>
>> I am contacting you, hoping that either you can maybe help
>> or you know someone who can help :)
>>
>> I opened a bug related to the tg3 driver when hot-plugging via pciehp:
>> https://bugzilla.kernel.org/show_bug.cgi?id\x100191
>>
>> I am desperate because :
>> (a) there is also a brcmfmac related driver.
>> So both wireless and wired internet have problems:
>> https://bugzilla.kernel.org/show_bug.cgi?id\x100201
>> (b) there was no input by anybody so far. Maybe because I wrongly
>> categorized the tg3 bug ?
>>
>> The bug is related to Broadcom tg3 driver and the pciehp hotplug framework.
>> In order to sort out where the problem(s) lies, I wanted to kindly ask
>> you if you could have a look at the bug :
>> https://bugzilla.kernel.org/show_bug.cgi?id\x100191
>> ?
>> And/or if you could help me figuring out if pciehp is causing this
>> problem and/or if tg3 is to blame.
>
> Odds are neither are the issue, this is a thunderbolt problem. Make
> sure you have enabled thunderbolt properly in your kernel if you want
> hot-add of a device to work.
>
> I think the code has only been tested on older macbooks, changes might
> be needed for newer ones given that Apple's thunderbolt controller is
> "special" (i.e. doesn't follow the specification.)
>
> good luck!
>
> greg k-h
>
Thanks a lot for your feedback !
Is "the code" that you are referring to the thunderbolt kernel module ?
The kernel I am using uses:
CONFIG_THUNDERBOLT=m -- The module will be called thunderbolt.
But when I boot-up with the plugged network device, lsmod does not show
thunderbolt.
So not sure how I can check further if thunderbolt is properly in my kernel?
Or is it possible that I would need to load the thunderbolt module
manually in order to make hotplug work?
When the network driver is loaded it seems to connect as a "PCI Express"
device:
tg3 0000:0a:00.0 eth0: Tigon3 [partno(BCM957762) rev 57766000] (PCI
Express)
When I unplug the adapter the first thing appearing in dmesg is pciehp:
[ 210.992938] usb 1-2: USB disconnect, device number 4
[ 1557.453818] pciehp 0000:06:03.0:pcie24: slot(3-2): Link Down event
[ 1557.453870] pciehp 0000:06:03.0:pcie24: Cannot remove display device
0000:08:00.0
[ 1557.509812] pciehp 0000:06:03.0:pcie24: Card not present on Slot(3-2)
[ 1558.721344] tg3 0000:0a:00.0: tg3_abort_hw timed out, TX_MODE_ENABLE
will not clear MAC_TX_MODEÿffffff
[ 1558.934601] pciehp 0000:09:00.0:pcie24: unloading service driver pciehp
[ 1558.935366] pciehp 0000:09:00.0:pcie24: Timeout on hotplug command
0x1038 (issued 1557532 msec ago)
[ 1558.943460] pci_bus 0000:0a: busn_res: [bus 0a] is released
[ 1558.958426] pci_bus 0000:09: busn_res: [bus 09-0a] is released
[ 1558.958432] acpiphp: Slot [1] unregistered
[ 1558.958451] acpiphp: Slot [2-1] unregistered
[ 1558.958464] acpiphp: Slot [3-1] unregistered
[ 1558.958475] acpiphp: Slot [4-1] unregistered
[ 1558.958486] acpiphp: Slot [5-1] unregistered
But I am not a kernel, pciehp, thunderbolt specialist :(
How can I determine why the pciehp hotplug command 0x1038 timed out ?
Is the "unloading service driver pciehp" the reason why replugging
(hotplugging) the device goes completely unnoticed as far as dmesg goes ?
And if the problem is can (should?) be solved within pciehp ?
For the complete version of the dmesg outputs:
1. bootup: dmesg___boot-with-plugged-network-adapter.txt
2. unplug:
dmesg___boot-with-plugged-network-adapter___unplugged-adapter.txt
3. replug:
dmesg___boot-with-plugged-network-adapter___unplugged-adapter.txt
see:
https://gist.github.com/anonymous/6221168ddcebe37f0ec4
Tormen
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Jiang Liu @ 2015-09-04 7:20 UTC (permalink / raw)
To: Rafael J. Wysocki, Tejun Heo
Cc: Rafael J. Wysocki, linux hotplug mailing,
Linux Kernel Mailing List, ACPI Devel Maling List
In-Reply-To: <CAJZ5v0izmv8CycKd0hjEkSBUmPeRmWWMUMgfMHkXK-0uDJFPaQ@mail.gmail.com>
On 2015/9/4 4:08, Rafael J. Wysocki wrote:
> Hi Tejun,
>
> On Thu, Sep 3, 2015 at 6:19 PM, Tejun Heo <tj@kernel.org> wrote:
>> Hello, Rafael.
>>
>> On Thu, Sep 03, 2015 at 02:58:16AM +0200, Rafael J. Wysocki wrote:
>>> So acpi_device_hotplug() calls lock_device_hotplug() which simply
>>> acquires device_hotplug_lock. It is held throughout the entire
>>> hot-add/hot-remove code path.
>>>
>>> Witing anything to /sys/devices/system/cpu/cpux/online goes through
>>> online_store() in drivers/base/core.c and that does
>>> lock_device_hotplug_sysfs() which then attempts to acquire
>>> device_hotplug_lock using mutex_trylock(). And it only calls
>>> either device_online() or device_offline() if it ends up with the
>>> lock held.
>>>
>>> Quite frankly, I don't see how these particular two code paths can
>>> deadlock in any way.
>>>
>>> So either a third code path is involved which is not executed
>>> under device_hotplug_lock, or lockdep needs to be told to actually
>>> take device_hotplug_lock into account in this case IMO.
>>
>> Hmm... all sysfs rw functions are protected from removal. ie. by
>> default, removal of a sysfs file drains in-flight rw operations, so
>> the hot plug path grabs a lock and then tries to remove a file and
>> writing to the online file makes the file's write method to try to
>> grab the same lock. It deadlocks if the hotunplug path already has
>> the lock and trying to drain the online file for removal.
>
> My point is that you cannot get into that situation. If hotplug
> already holds device_hotplug_lock, the write to "online" will end up
> doing restart_syscall().
>
> If the "online" code path is holding the lock, hotplug cannot acquire
> it and cannot proceed.
>
> Am I missing anything?
Hi Rafael,
I think your are right. The lock_device_hotplug_sysfs() has
already provided a solution for such a deadlock scenario. And there's
another related code path at boot as:
smp_init()
->cpu_up()
->cpu_hotplug_begin()
So it seems to be a false alarm. Any way to teach lockdep
about this to get rid of the false alarm?
Thanks!
Gerry
>
> Thanks,
> Rafael
>
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Rafael J. Wysocki @ 2015-09-03 20:08 UTC (permalink / raw)
To: Tejun Heo
Cc: Rafael J. Wysocki, Jiang Liu, linux hotplug mailing,
Linux Kernel Mailing List, ACPI Devel Maling List
In-Reply-To: <20150903161904.GC10394@mtj.duckdns.org>
Hi Tejun,
On Thu, Sep 3, 2015 at 6:19 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Rafael.
>
> On Thu, Sep 03, 2015 at 02:58:16AM +0200, Rafael J. Wysocki wrote:
>> So acpi_device_hotplug() calls lock_device_hotplug() which simply
>> acquires device_hotplug_lock. It is held throughout the entire
>> hot-add/hot-remove code path.
>>
>> Witing anything to /sys/devices/system/cpu/cpux/online goes through
>> online_store() in drivers/base/core.c and that does
>> lock_device_hotplug_sysfs() which then attempts to acquire
>> device_hotplug_lock using mutex_trylock(). And it only calls
>> either device_online() or device_offline() if it ends up with the
>> lock held.
>>
>> Quite frankly, I don't see how these particular two code paths can
>> deadlock in any way.
>>
>> So either a third code path is involved which is not executed
>> under device_hotplug_lock, or lockdep needs to be told to actually
>> take device_hotplug_lock into account in this case IMO.
>
> Hmm... all sysfs rw functions are protected from removal. ie. by
> default, removal of a sysfs file drains in-flight rw operations, so
> the hot plug path grabs a lock and then tries to remove a file and
> writing to the online file makes the file's write method to try to
> grab the same lock. It deadlocks if the hotunplug path already has
> the lock and trying to drain the online file for removal.
My point is that you cannot get into that situation. If hotplug
already holds device_hotplug_lock, the write to "online" will end up
doing restart_syscall().
If the "online" code path is holding the lock, hotplug cannot acquire
it and cannot proceed.
Am I missing anything?
Thanks,
Rafael
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Tejun Heo @ 2015-09-03 16:19 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Jiang Liu, linux hotplug mailing, Linux Kernel Mailing List,
ACPI Devel Maling List
In-Reply-To: <12669324.bPBpI0mOPP@vostro.rjw.lan>
Hello, Rafael.
On Thu, Sep 03, 2015 at 02:58:16AM +0200, Rafael J. Wysocki wrote:
> So acpi_device_hotplug() calls lock_device_hotplug() which simply
> acquires device_hotplug_lock. It is held throughout the entire
> hot-add/hot-remove code path.
>
> Witing anything to /sys/devices/system/cpu/cpux/online goes through
> online_store() in drivers/base/core.c and that does
> lock_device_hotplug_sysfs() which then attempts to acquire
> device_hotplug_lock using mutex_trylock(). And it only calls
> either device_online() or device_offline() if it ends up with the
> lock held.
>
> Quite frankly, I don't see how these particular two code paths can
> deadlock in any way.
>
> So either a third code path is involved which is not executed
> under device_hotplug_lock, or lockdep needs to be told to actually
> take device_hotplug_lock into account in this case IMO.
Hmm... all sysfs rw functions are protected from removal. ie. by
default, removal of a sysfs file drains in-flight rw operations, so
the hot plug path grabs a lock and then tries to remove a file and
writing to the online file makes the file's write method to try to
grab the same lock. It deadlocks if the hotunplug path already has
the lock and trying to drain the online file for removal.
The same problem exists for "delete" files but that's already handled
from device core side.
Thanks.
--
tejun
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Rafael J. Wysocki @ 2015-09-03 0:58 UTC (permalink / raw)
To: Tejun Heo
Cc: Jiang Liu, linux hotplug mailing, Linux Kernel Mailing List,
ACPI Devel Maling List
In-Reply-To: <20150902161445.GI22326@mtj.duckdns.org>
On Wednesday, September 02, 2015 12:14:45 PM Tejun Heo wrote:
> On Tue, Sep 01, 2015 at 03:12:34PM +0800, Jiang Liu wrote:
> > Hi Rafael and Tejun,
> > When running CPU hotplug tests, it triggers an lockdep warning
> > as follow. The two possible deadlock paths are:
> > 1) echo x > /sys/devices/system/cpu/cpux/online
> > ->kernfs_fop_write()
> > ->kernfs_get_active()
> > 1.a) ->rwsem_acquire_read(&kn->dep_map, 0, 1, _RET_IP_);
> > ->cpu_up()
> > 1.b) ->cpu_hotplug_begin()[lock_map_acquire(&cpu_hotplug.dep_map)]
> > 2) hardware triggers hotplug evetns
> > ->acpi_device_hotplug()
> > ->acpi_processor_remove()
> > 2.a) ->cpu_hotplug_begin()[lock_map_acquire(&cpu_hotplug.dep_map)]
> > ->unregister_cpu()
> > ->device_del()
> > ->kernfs_remove_by_name_ns()
> > ->__kernfs_remove()
> > ->kernfs_drain()
> > 2.b) ->rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_)
> >
> > So there is a possible deadlock scenario among 1.a, 1.b, 2.a and 2.b.
> > I'm not familiar with kernfs, so could you please help to comment:
> > 1) whether is a real deadlock issue?
>
> Yes, it seems to be. It's highly unlikely but still possible.
Hmm.
So acpi_device_hotplug() calls lock_device_hotplug() which simply
acquires device_hotplug_lock. It is held throughout the entire
hot-add/hot-remove code path.
Witing anything to /sys/devices/system/cpu/cpux/online goes through
online_store() in drivers/base/core.c and that does
lock_device_hotplug_sysfs() which then attempts to acquire
device_hotplug_lock using mutex_trylock(). And it only calls
either device_online() or device_offline() if it ends up with the
lock held.
Quite frankly, I don't see how these particular two code paths can
deadlock in any way.
So either a third code path is involved which is not executed
under device_hotplug_lock, or lockdep needs to be told to actually
take device_hotplug_lock into account in this case IMO.
> > 2) any recommended way to get it fixed?
>
> This usually happens with "delete" files and it's worked around by
> performing special self-removal on the file before actually removing
> the device. I suppose on/offline files would need to turn off
> active_protection with kernfs_[un]break_active_protection() which
> should probably grow sysfs and device layer wrappers.
Thanks,
Rafael
^ permalink raw reply
* Re: Possible deadlock related to CPU hotplug and kernfs
From: Tejun Heo @ 2015-09-02 16:14 UTC (permalink / raw)
To: Jiang Liu
Cc: Rafael J. Wysocki, linux hotplug mailing,
Linux Kernel Mailing List, ACPI Devel Maling List
In-Reply-To: <55E54FE2.7030601@linux.intel.com>
On Tue, Sep 01, 2015 at 03:12:34PM +0800, Jiang Liu wrote:
> Hi Rafael and Tejun,
> When running CPU hotplug tests, it triggers an lockdep warning
> as follow. The two possible deadlock paths are:
> 1) echo x > /sys/devices/system/cpu/cpux/online
> ->kernfs_fop_write()
> ->kernfs_get_active()
> 1.a) ->rwsem_acquire_read(&kn->dep_map, 0, 1, _RET_IP_);
> ->cpu_up()
> 1.b) ->cpu_hotplug_begin()[lock_map_acquire(&cpu_hotplug.dep_map)]
> 2) hardware triggers hotplug evetns
> ->acpi_device_hotplug()
> ->acpi_processor_remove()
> 2.a) ->cpu_hotplug_begin()[lock_map_acquire(&cpu_hotplug.dep_map)]
> ->unregister_cpu()
> ->device_del()
> ->kernfs_remove_by_name_ns()
> ->__kernfs_remove()
> ->kernfs_drain()
> 2.b) ->rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_)
>
> So there is a possible deadlock scenario among 1.a, 1.b, 2.a and 2.b.
> I'm not familiar with kernfs, so could you please help to comment:
> 1) whether is a real deadlock issue?
Yes, it seems to be. It's highly unlikely but still possible.
> 2) any recommended way to get it fixed?
This usually happens with "delete" files and it's worked around by
performing special self-removal on the file before actually removing
the device. I suppose on/offline files would need to turn off
active_protection with kernfs_[un]break_active_protection() which
should probably grow sysfs and device layer wrappers.
Thanks.
--
tejun
^ permalink raw reply
* Re: possible bug regarding pciehp
From: Greg KH @ 2015-09-02 15:27 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <55E70945.6060802@diplomail.ch>
On Wed, Sep 02, 2015 at 04:35:49PM +0200, Tormen wrote:
> Hi,
>
> I am contacting you, hoping that either you can maybe help
> or you know someone who can help :)
>
> I opened a bug related to the tg3 driver when hot-plugging via pciehp:
> https://bugzilla.kernel.org/show_bug.cgi?id\x100191
>
> I am desperate because :
> (a) there is also a brcmfmac related driver.
> So both wireless and wired internet have problems:
> https://bugzilla.kernel.org/show_bug.cgi?id\x100201
> (b) there was no input by anybody so far. Maybe because I wrongly
> categorized the tg3 bug ?
>
> The bug is related to Broadcom tg3 driver and the pciehp hotplug framework.
> In order to sort out where the problem(s) lies, I wanted to kindly ask
> you if you could have a look at the bug :
> https://bugzilla.kernel.org/show_bug.cgi?id\x100191
> ?
> And/or if you could help me figuring out if pciehp is causing this
> problem and/or if tg3 is to blame.
Odds are neither are the issue, this is a thunderbolt problem. Make
sure you have enabled thunderbolt properly in your kernel if you want
hot-add of a device to work.
I think the code has only been tested on older macbooks, changes might
be needed for newer ones given that Apple's thunderbolt controller is
"special" (i.e. doesn't follow the specification.)
good luck!
greg k-h
^ permalink raw reply
* possible bug regarding pciehp
From: Tormen @ 2015-09-02 14:35 UTC (permalink / raw)
To: linux-hotplug
Hi,
I am contacting you, hoping that either you can maybe help
or you know someone who can help :)
I opened a bug related to the tg3 driver when hot-plugging via pciehp:
https://bugzilla.kernel.org/show_bug.cgi?id\x100191
I am desperate because :
(a) there is also a brcmfmac related driver.
So both wireless and wired internet have problems:
https://bugzilla.kernel.org/show_bug.cgi?id\x100201
(b) there was no input by anybody so far. Maybe because I wrongly
categorized the tg3 bug ?
The bug is related to Broadcom tg3 driver and the pciehp hotplug framework.
In order to sort out where the problem(s) lies, I wanted to kindly ask
you if you could have a look at the bug :
https://bugzilla.kernel.org/show_bug.cgi?id\x100191
?
And/or if you could help me figuring out if pciehp is causing this
problem and/or if tg3 is to blame.
Please let me know if you would need any further information.
If you can't help me, I would also appreciate a lot if you could point
me to someone that you deem relevant for this bug.
Thanks a lot in advance !
Tormen
^ permalink raw reply
* Possible deadlock related to CPU hotplug and kernfs
From: Jiang Liu @ 2015-09-01 7:12 UTC (permalink / raw)
To: Tejun Heo, Rafael J. Wysocki, linux hotplug mailing,
Linux Kernel Mailing List, ACPI Devel Maling List
Hi Rafael and Tejun,
When running CPU hotplug tests, it triggers an lockdep warning
as follow. The two possible deadlock paths are:
1) echo x > /sys/devices/system/cpu/cpux/online
->kernfs_fop_write()
->kernfs_get_active()
1.a) ->rwsem_acquire_read(&kn->dep_map, 0, 1, _RET_IP_);
->cpu_up()
1.b) ->cpu_hotplug_begin()[lock_map_acquire(&cpu_hotplug.dep_map)]
2) hardware triggers hotplug evetns
->acpi_device_hotplug()
->acpi_processor_remove()
2.a) ->cpu_hotplug_begin()[lock_map_acquire(&cpu_hotplug.dep_map)]
->unregister_cpu()
->device_del()
->kernfs_remove_by_name_ns()
->__kernfs_remove()
->kernfs_drain()
2.b) ->rwsem_acquire(&kn->dep_map, 0, 0, _RET_IP_)
So there is a possible deadlock scenario among 1.a, 1.b, 2.a and 2.b.
I'm not familiar with kernfs, so could you please help to comment:
1) whether is a real deadlock issue?
2) any recommended way to get it fixed?
Thanks!
Gerry
Full lockdep warnings:
[ 310.309391] [ INFO: possible circular locking dependency detected ]
[ 310.316462] 4.2.0-rc8+ #7 Not tainted
[ 310.320613] -------------------------------------------------------
[ 310.327684] kworker/u288:3/388 is trying to acquire lock:
[ 310.333780] (s_active#97){++++.+}, at: [<ffffffff812bd989>]
kernfs_remove_by_name_ns+0x49/0xb0
[ 310.343885]
[ 310.343885] but task is already holding lock:
[ 310.350466] (cpu_hotplug.lock#2){+.+.+.}, at: [<ffffffff81080aab>]
cpu_hotplug_begin+0x7b/0xc0
[ 310.360564]
[ 310.360564] which lock already depends on the new lock.
[ 310.360564]
[ 310.369766]
[ 310.369766] the existing dependency chain (in reverse order) is:
[ 310.378198]
[ 310.378198] -> #3 (cpu_hotplug.lock#2){+.+.+.}:
[ 310.383821] [<ffffffff810df04d>] lock_acquire+0xdd/0x2a0
[ 310.390591] [<ffffffff818644a0>] mutex_lock_nested+0x70/0x3e0
[ 310.397847] [<ffffffff81080aab>] cpu_hotplug_begin+0x7b/0xc0
[ 310.405004] [<ffffffff81080b61>] _cpu_up+0x31/0x140
[ 310.411285] [<ffffffff81080cec>] cpu_up+0x7c/0xa0
[ 310.417362] [<ffffffff821859cb>] smp_init+0x86/0x88
[ 310.423647] [<ffffffff82160181>] kernel_init_freeable+0x171/0x286
[ 310.431292] [<ffffffff8185228e>] kernel_init+0xe/0xe0
[ 310.437771] [<ffffffff81869e5f>] ret_from_fork+0x3f/0x70
[ 310.444540]
[ 310.444540] -> #2 (cpu_hotplug.lock){++++++}:
[ 310.449957] [<ffffffff810df04d>] lock_acquire+0xdd/0x2a0
[ 310.456714] [<ffffffff81080a9d>] cpu_hotplug_begin+0x6d/0xc0
[ 310.463871] [<ffffffff81080b61>] _cpu_up+0x31/0x140
[ 310.470143] [<ffffffff81080cec>] cpu_up+0x7c/0xa0
[ 310.476228] [<ffffffff821859cb>] smp_init+0x86/0x88
[ 310.482509] [<ffffffff82160181>] kernel_init_freeable+0x171/0x286
[ 310.490153] [<ffffffff8185228e>] kernel_init+0xe/0xe0
[ 310.496628] [<ffffffff81869e5f>] ret_from_fork+0x3f/0x70
[ 310.503393]
[ 310.503393] -> #1 (cpu_add_remove_lock){+.+.+.}:
[ 310.509099] [<ffffffff810df04d>] lock_acquire+0xdd/0x2a0
[ 310.515866] [<ffffffff811e1134>] __might_fault+0x84/0xb0
[ 310.522635] [<ffffffff812beb6f>] kernfs_fop_write+0x8f/0x190
[ 310.529793] [<ffffffff81233b68>] __vfs_write+0x28/0xe0
[ 310.536368] [<ffffffff812342ac>] vfs_write+0xac/0x1a0
[ 310.542833] [<ffffffff81235049>] SyS_write+0x49/0xb0
[ 310.549212] [<ffffffff818699f2>]
entry_SYSCALL_64_fastpath+0x16/0x7a
[ 310.557149]
[ 310.557149] -> #0 (s_active#97){++++.+}:
[ 310.562135] [<ffffffff810de269>] __lock_acquire+0x21b9/0x21c0
[ 310.569391] [<ffffffff810df04d>] lock_acquire+0xdd/0x2a0
[ 310.576159] [<ffffffff812bc7a1>] __kernfs_remove+0x231/0x330
[ 310.583318] [<ffffffff812bd989>]
kernfs_remove_by_name_ns+0x49/0xb0
[ 310.591154] [<ffffffff812bf3c5>] sysfs_remove_file_ns+0x15/0x20
[ 310.598594] [<ffffffff8157490e>] device_remove_attrs+0x3e/0x80
[ 310.605948] [<ffffffff815752a8>] device_del+0x138/0x270
[ 310.612617] [<ffffffff81575402>] device_unregister+0x22/0x70
[ 310.619767] [<ffffffff8157cfa9>] unregister_cpu+0x39/0x60
[ 310.626622] [<ffffffff81023e73>] arch_unregister_cpu+0x23/0x30
[ 310.633974] [<ffffffff814bab67>] acpi_processor_remove+0x91/0xca
[ 310.641524] [<ffffffff814b82e3>] acpi_bus_trim+0x5a/0x8d
[ 310.648292] [<ffffffff814b82c1>] acpi_bus_trim+0x38/0x8d
[ 310.655060] [<ffffffff814b8333>]
acpi_scan_device_not_present+0x1d/0x3d
[ 310.663312] [<ffffffff814b9e05>] acpi_scan_bus_check+0x29/0xa2
[ 310.670654] [<ffffffff814b9f17>] acpi_device_hotplug+0x99/0x3fa
[ 310.678103] [<ffffffff814b33ba>] acpi_hotplug_work_fn+0x1f/0x2b
[ 310.685555] [<ffffffff810a0241>] process_one_work+0x1f1/0x7c0
[ 310.692814] [<ffffffff810a0879>] worker_thread+0x69/0x480
[ 310.699677] [<ffffffff810a71af>] kthread+0x11f/0x140
[ 310.706046] [<ffffffff81869e5f>] ret_from_fork+0x3f/0x70
[ 310.712815]
[ 310.712815] other info that might help us debug this:
[ 310.712815]
[ 310.721907] Chain exists of:
[ 310.721907] s_active#97 --> cpu_hotplug.lock --> cpu_hotplug.lock#2
[ 310.721907]
[ 310.731680] Possible unsafe locking scenario:
[ 310.731680]
[ 310.738413] CPU0 CPU1
[ 310.743562] ---- ----
[ 310.748710] lock(cpu_hotplug.lock#2);
[ 310.753261] lock(cpu_hotplug.lock);
[ 310.760382] lock(cpu_hotplug.lock#2);
[ 310.767755] lock(s_active#97);
[ 310.771625]
[ 310.771625] *** DEADLOCK ***
[ 310.771625]
[ 310.778382] 7 locks held by kworker/u288:3/388:
[ 310.783530] #0: ("kacpi_hotplug"){.+.+.+}, at: [<ffffffff810a01b6>]
process_one_work+0x166/0x7c0
[ 310.793975] #1: ((&hpw->work)){+.+.+.}, at: [<ffffffff810a01b6>]
process_one_work+0x166/0x7c0
[ 310.804126] #2: (device_hotplug_lock){+.+.+.}, at:
[<ffffffff81575cc7>] lock_device_hotplug+0x17/0x20
[ 310.815057] #3: (acpi_scan_lock){+.+.+.}, at: [<ffffffff814b9eb4>]
acpi_device_hotplug+0x36/0x3fa
[ 310.825599] #4: (cpu_add_remove_lock){+.+.+.}, at:
[<ffffffff810807d7>] cpu_maps_update_begin+0x17/0x20
[ 310.836727] #5: (cpu_hotplug.lock){++++++}, at:
[<ffffffff81080a35>] cpu_hotplug_begin+0x5/0xc0
[ 310.847073] #6: (cpu_hotplug.lock#2){+.+.+.}, at:
[<ffffffff81080aab>] cpu_hotplug_begin+0x7b/0xc0
[ 310.857774]
[ 310.857774] stack backtrace:
[ 310.862754] CPU: 11 PID: 388 Comm: kworker/u288:3 Not tainted
4.2.0-rc8+ #7
[ 310.870628] Hardware name: Intel Corporation BRICKLAND/BRICKLAND,
BIOS BRHSXIN1.86B.0060.R02.1508171754 08/17/2015
[ 310.882326] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
[ 310.888499] ffffffff82a39b50 ffff88042b9a38d8 ffffffff8185f0b8
0000000000000011
[ 310.897130] ffffffff82afcab0 ffff88042b9a3928 ffffffff8185c183
0000000000000007
[ 310.905762] ffff88042b9a3998 ffff88042b9a3928 ffff88042b99ab08
ffff88042b99a980
[ 310.914393] Call Trace:
[ 310.917206] [<ffffffff8185f0b8>] dump_stack+0x4c/0x65
[ 310.923039] [<ffffffff8185c183>] print_circular_bug+0x20b/0x21c
[ 310.929843] [<ffffffff810de269>] __lock_acquire+0x21b9/0x21c0
[ 310.936455] [<ffffffff810260d8>] ? native_sched_clock+0x28/0x90
[ 310.943258] [<ffffffff810df04d>] lock_acquire+0xdd/0x2a0
[ 310.949382] [<ffffffff812bd989>] ? kernfs_remove_by_name_ns+0x49/0xb0
[ 310.956769] [<ffffffff812bc7a1>] __kernfs_remove+0x231/0x330
[ 310.963280] [<ffffffff812bd989>] ? kernfs_remove_by_name_ns+0x49/0xb0
[ 310.970669] [<ffffffff812bbd67>] ? kernfs_name_hash+0x17/0xa0
[ 310.977278] [<ffffffff812bcb81>] ? kernfs_find_ns+0x81/0x140
[ 310.983792] [<ffffffff812bd989>] kernfs_remove_by_name_ns+0x49/0xb0
[ 310.990986] [<ffffffff812bf3c5>] sysfs_remove_file_ns+0x15/0x20
[ 310.997791] [<ffffffff8157490e>] device_remove_attrs+0x3e/0x80
[ 311.004498] [<ffffffff815752a8>] device_del+0x138/0x270
[ 311.010524] [<ffffffff812bd995>] ? kernfs_remove_by_name_ns+0x55/0xb0
[ 311.017914] [<ffffffff81575402>] device_unregister+0x22/0x70
[ 311.024427] [<ffffffff8157cfa9>] unregister_cpu+0x39/0x60
[ 311.030646] [<ffffffff81023e73>] arch_unregister_cpu+0x23/0x30
[ 311.037354] [<ffffffff814bab67>] acpi_processor_remove+0x91/0xca
[ 311.044257] [<ffffffff814b82e3>] acpi_bus_trim+0x5a/0x8d
[ 311.050379] [<ffffffff814b82c1>] acpi_bus_trim+0x38/0x8d
[ 311.056501] [<ffffffff814b8333>] acpi_scan_device_not_present+0x1d/0x3d
[ 311.064085] [<ffffffff814b9e05>] acpi_scan_bus_check+0x29/0xa2
[ 311.070791] [<ffffffff814b9f17>] acpi_device_hotplug+0x99/0x3fa
[ 311.077596] [<ffffffff814b33ba>] acpi_hotplug_work_fn+0x1f/0x2b
[ 311.084402] [<ffffffff810a0241>] process_one_work+0x1f1/0x7c0
[ 311.091012] [<ffffffff810a01b6>] ? process_one_work+0x166/0x7c0
[ 311.097815] [<ffffffff810a0909>] ? worker_thread+0xf9/0x480
[ 311.104231] [<ffffffff810a0879>] worker_thread+0x69/0x480
[ 311.110451] [<ffffffff810a0810>] ? process_one_work+0x7c0/0x7c0
[ 311.117256] [<ffffffff810a71af>] kthread+0x11f/0x140
[ 311.122990] [<ffffffff810a7090>] ? kthread_create_on_node+0x260/0x260
[ 311.130379] [<ffffffff81869e5f>] ret_from_fork+0x3f/0x70
[ 311.136502] [<ffffffff810a7090>] ? kthread_create_on_node+0x260/0x260
^ permalink raw reply
* Re: [Patch V3 3/9] sgi-xp: Replace cpu_to_node() with cpu_to_mem() to support memoryless node
From: Jiang Liu @ 2015-08-20 6:36 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Mel Gorman, Mike Galbraith, Peter Zijlstra,
Rafael J . Wysocki, Tang Chen, Tejun Heo, Cliff Whickman,
Robin Holt, Tony Luck, linux-mm, linux-hotplug, linux-kernel, x86
In-Reply-To: <alpine.DEB.2.10.1508191701010.30666@chino.kir.corp.google.com>
On 2015/8/20 8:02, David Rientjes wrote:
> On Wed, 19 Aug 2015, Jiang Liu wrote:
>
>>> Why not simply fix build_zonelists_node() so that the __GFP_THISNODE
>>> zonelists are set up to reference the zones of cpu_to_mem() for memoryless
>>> nodes?
>>>
>>> It seems much better than checking and maintaining every __GFP_THISNODE
>>> user to determine if they are using a memoryless node or not. I don't
>>> feel that this solution is maintainable in the longterm.
>> Hi David,
>> There are some usage cases, such as memory migration,
>> expect the page allocator rejecting memory allocation requests
>> if there is no memory on local node. So we have:
>> 1) alloc_pages_node(cpu_to_node(), __GFP_THISNODE) to only allocate
>> memory from local node.
>> 2) alloc_pages_node(cpu_to_mem(), __GFP_THISNODE) to allocate memory
>> from local node or from nearest node if local node is memoryless.
>>
>
> Right, so do you think it would be better to make the default zonelists be
> setup so that cpu_to_node()->zonelists = cpu_to_mem()->zonelists and then
> individual callers that want to fail for memoryless nodes check
> populated_zone() themselves?
Hi David,
Great idea:) I think that means we are going to kill the
concept of memoryless node, and we only need to specially handle
a few callers who really care about whether there is memory on
local node.
Then I need some time to audit all usages of __GFP_THISNODE
and update you whether it's doable.
Thanks!
Gerry
^ permalink raw reply
* RE: [Intel-wired-lan] [Patch V3 5/9] i40e: Use numa_mem_id() to better support memoryless node
From: David Rientjes @ 2015-08-20 0:18 UTC (permalink / raw)
To: Patil, Kiran
Cc: Jiang Liu, Andrew Morton, Mel Gorman, Mike Galbraith,
Peter Zijlstra, Wysocki, Rafael J, Tang Chen, Tejun Heo,
Kirsher, Jeffrey T, Brandeburg, Jesse, Nelson, Shannon,
Wyborny, Carolyn, Skidmore, Donald C, Vick, Matthew,
Ronciak, John, Williams, Mitch A, Luck, Tony,
netdev@vger.kernel.org, x86@kernel.org,
linux-hotplug@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, intel-wired-lan@lists.osuosl.org
In-Reply-To: <4197C471DCF8714FBA1FE32565271C148FFFF4D3@ORSMSX103.amr.corp.intel.com>
On Wed, 19 Aug 2015, Patil, Kiran wrote:
> Acked-by: Kiran Patil <kiran.patil@intel.com>
Where's the call to preempt_disable() to prevent kernels with preemption
from making numa_node_id() invalid during this iteration?
^ permalink raw reply
* Re: [Patch V3 3/9] sgi-xp: Replace cpu_to_node() with cpu_to_mem() to support memoryless node
From: David Rientjes @ 2015-08-20 0:02 UTC (permalink / raw)
To: Jiang Liu
Cc: Andrew Morton, Mel Gorman, Mike Galbraith, Peter Zijlstra,
Rafael J . Wysocki, Tang Chen, Tejun Heo, Cliff Whickman,
Robin Holt, Tony Luck, linux-mm, linux-hotplug, linux-kernel, x86
In-Reply-To: <55D43C63.7060802@linux.intel.com>
On Wed, 19 Aug 2015, Jiang Liu wrote:
> > Why not simply fix build_zonelists_node() so that the __GFP_THISNODE
> > zonelists are set up to reference the zones of cpu_to_mem() for memoryless
> > nodes?
> >
> > It seems much better than checking and maintaining every __GFP_THISNODE
> > user to determine if they are using a memoryless node or not. I don't
> > feel that this solution is maintainable in the longterm.
> Hi David,
> There are some usage cases, such as memory migration,
> expect the page allocator rejecting memory allocation requests
> if there is no memory on local node. So we have:
> 1) alloc_pages_node(cpu_to_node(), __GFP_THISNODE) to only allocate
> memory from local node.
> 2) alloc_pages_node(cpu_to_mem(), __GFP_THISNODE) to allocate memory
> from local node or from nearest node if local node is memoryless.
>
Right, so do you think it would be better to make the default zonelists be
setup so that cpu_to_node()->zonelists = cpu_to_mem()->zonelists and then
individual callers that want to fail for memoryless nodes check
populated_zone() themselves?
^ permalink raw reply
* Re: [Patch V3 2/9] kernel/profile.c: Replace cpu_to_mem() with cpu_to_node()
From: David Rientjes @ 2015-08-20 0:00 UTC (permalink / raw)
To: Jiang Liu
Cc: Andrew Morton, Mel Gorman, Mike Galbraith, Peter Zijlstra,
Rafael J . Wysocki, Tang Chen, Tejun Heo, Tony Luck, linux-mm,
linux-hotplug, linux-kernel, x86
In-Reply-To: <55D42DE3.2040506@linux.intel.com>
On Wed, 19 Aug 2015, Jiang Liu wrote:
> On 2015/8/18 8:31, David Rientjes wrote:
> > On Mon, 17 Aug 2015, Jiang Liu wrote:
> >
> >> Function profile_cpu_callback() allocates memory without specifying
> >> __GFP_THISNODE flag, so replace cpu_to_mem() with cpu_to_node()
> >> because cpu_to_mem() may cause suboptimal memory allocation if
> >> there's no free memory on the node returned by cpu_to_mem().
> >>
> >
> > Why is cpu_to_node() better with regard to free memory and NUMA locality?
> Hi David,
> Thanks for review. This is a special case pointed out by Tejun.
> For the imagined topology, A<->B<->X<->C<->D, where A, B, C, D has
> memory and X is memoryless.
> Possible fallback lists are:
> B: [ B, A, C, D]
> X: [ B, C, A, D]
> C: [ C, D, B, A]
>
> cpu_to_mem(X) will either return B or C. Let's assume it returns B.
> Then we will use "B: [ B, A, C, D]" to allocate memory for X, which
> is not the optimal fallback list for X. And cpu_to_node(X) returns
> X, and "X: [ B, C, A, D]" is the optimal fallback list for X.
Ok, that makes sense, but I would prefer that this
alloc_pages_exact_node() change to alloc_pages_node() since, as you
mention in your commit message, __GFP_THISNODE is not set.
In the longterm, if we setup both zonelists correctly (no __GFP_THISNODE
and with __GFP_THISNODE), then I'm not sure there's any reason to ever use
cpu_to_mem() for alloc_pages().
^ permalink raw reply
* RE: [Intel-wired-lan] [Patch V3 5/9] i40e: Use numa_mem_id() to better support memoryless node
From: Patil, Kiran @ 2015-08-19 22:38 UTC (permalink / raw)
To: Jiang Liu, Andrew Morton, Mel Gorman, David Rientjes,
Mike Galbraith, Peter Zijlstra, Wysocki, Rafael J, Tang Chen,
Tejun Heo, Kirsher, Jeffrey T, Brandeburg, Jesse, Nelson, Shannon,
Wyborny, Carolyn, Skidmore, Donald C, Vick, Matthew,
Ronciak, John, Williams, Mitch A
Cc: Luck, Tony, netdev@vger.kernel.org, x86@kernel.org,
linux-hotplug@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, intel-wired-lan@lists.osuosl.org
In-Reply-To: <1439781546-7217-6-git-send-email-jiang.liu@linux.intel.com>
Acked-by: Kiran Patil <kiran.patil@intel.com>
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Jiang Liu
Sent: Sunday, August 16, 2015 8:19 PM
To: Andrew Morton; Mel Gorman; David Rientjes; Mike Galbraith; Peter Zijlstra; Wysocki, Rafael J; Tang Chen; Tejun Heo; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald C; Vick, Matthew; Ronciak, John; Williams, Mitch A
Cc: Luck, Tony; netdev@vger.kernel.org; x86@kernel.org; linux-hotplug@vger.kernel.org; linux-kernel@vger.kernel.org; linux-mm@kvack.org; intel-wired-lan@lists.osuosl.org; Jiang Liu
Subject: [Intel-wired-lan] [Patch V3 5/9] i40e: Use numa_mem_id() to better support memoryless node
Function i40e_clean_rx_irq() tries to reuse memory pages allocated from the nearest node. To better support memoryless node, use
numa_mem_id() instead of numa_node_id() to get the nearest node with memory.
This change should only affect performance.
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 9a4f2bc70cd2..a8f618cb8eb0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1516,7 +1516,7 @@ static int i40e_clean_rx_irq_ps(struct i40e_ring *rx_ring, int budget)
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
u16 rx_packet_len, rx_header_len, rx_sph, rx_hbo;
u16 cleaned_count = I40E_DESC_UNUSED(rx_ring);
- const int current_node = numa_node_id();
+ const int current_node = numa_mem_id();
struct i40e_vsi *vsi = rx_ring->vsi;
u16 i = rx_ring->next_to_clean;
union i40e_rx_desc *rx_desc;
--
1.7.10.4
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related
* Re: [Patch V3 3/9] sgi-xp: Replace cpu_to_node() with cpu_to_mem() to support memoryless node
From: Jiang Liu @ 2015-08-19 12:45 UTC (permalink / raw)
To: Robin Holt
Cc: Andrew Morton, Mel Gorman, David Rientjes, Mike Galbraith,
Peter Zijlstra, Rafael J . Wysocki, Tang Chen, Tejun Heo,
Cliff Whickman, Tony Luck, linux-mm, linux-hotplug, LKML, x86
In-Reply-To: <CAPp3RGoo3ZPTApwezua01Adjt1JaBraCTUCF0BcN=SKJfQO0iQ@mail.gmail.com>
On 2015/8/19 19:52, Robin Holt wrote:
> On Sun, Aug 16, 2015 at 10:19 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
>> Function xpc_create_gru_mq_uv() allocates memory with __GFP_THISNODE
>> flag set, which may cause permanent memory allocation failure on
>> memoryless node. So replace cpu_to_node() with cpu_to_mem() to better
>> support memoryless node. For node with memory, cpu_to_mem() is the same
>> as cpu_to_node().
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>> drivers/misc/sgi-xp/xpc_uv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
>> index 95c894482fdd..9210981c0d5b 100644
>> --- a/drivers/misc/sgi-xp/xpc_uv.c
>> +++ b/drivers/misc/sgi-xp/xpc_uv.c
>> @@ -238,7 +238,7 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
>>
>> mq->mmr_blade = uv_cpu_to_blade_id(cpu);
>>
>> - nid = cpu_to_node(cpu);
>> + nid = cpu_to_mem(cpu);
>
> I would recommend rejecting this. First, SGI's UV system does not and
> can not support memory-less nodes. Additionally the hardware _REALLY_
> wants the memory to be local to the CPU. We will register this memory
> region with the node firmware. That will set the hardware up to watch
> this memory block and raise an IRQ targeting the registered CPU when
> anything is written into the memory block. This is all part of how
> cross-partition communications expects to work.
>
> Additionally, the interrupt handler will read the memory region, so
> having node-local memory is extremely helpful.
Hi Robin,
Thanks for review, I will drop this patch in next version.
Actually, if SGI UV systems don't support memoryless node, cpu_to_mem()
is the same as cpu_to_node().
Thanks!
Gerry
>
> Thanks,
> Robin
>
^ permalink raw reply
* Re: [Patch V3 3/9] sgi-xp: Replace cpu_to_node() with cpu_to_mem() to support memoryless node
From: Robin Holt @ 2015-08-19 11:52 UTC (permalink / raw)
To: Jiang Liu
Cc: Andrew Morton, Mel Gorman, David Rientjes, Mike Galbraith,
Peter Zijlstra, Rafael J . Wysocki, Tang Chen, Tejun Heo,
Cliff Whickman, Tony Luck, linux-mm, linux-hotplug, LKML, x86
In-Reply-To: <1439781546-7217-4-git-send-email-jiang.liu@linux.intel.com>
On Sun, Aug 16, 2015 at 10:19 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote:
> Function xpc_create_gru_mq_uv() allocates memory with __GFP_THISNODE
> flag set, which may cause permanent memory allocation failure on
> memoryless node. So replace cpu_to_node() with cpu_to_mem() to better
> support memoryless node. For node with memory, cpu_to_mem() is the same
> as cpu_to_node().
>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
> drivers/misc/sgi-xp/xpc_uv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
> index 95c894482fdd..9210981c0d5b 100644
> --- a/drivers/misc/sgi-xp/xpc_uv.c
> +++ b/drivers/misc/sgi-xp/xpc_uv.c
> @@ -238,7 +238,7 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
>
> mq->mmr_blade = uv_cpu_to_blade_id(cpu);
>
> - nid = cpu_to_node(cpu);
> + nid = cpu_to_mem(cpu);
I would recommend rejecting this. First, SGI's UV system does not and
can not support memory-less nodes. Additionally the hardware _REALLY_
wants the memory to be local to the CPU. We will register this memory
region with the node firmware. That will set the hardware up to watch
this memory block and raise an IRQ targeting the registered CPU when
anything is written into the memory block. This is all part of how
cross-partition communications expects to work.
Additionally, the interrupt handler will read the memory region, so
having node-local memory is extremely helpful.
Thanks,
Robin
^ permalink raw reply
* Re: [Patch V3 3/9] sgi-xp: Replace cpu_to_node() with cpu_to_mem() to support memoryless node
From: Jiang Liu @ 2015-08-19 8:20 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Mel Gorman, Mike Galbraith, Peter Zijlstra,
Rafael J . Wysocki, Tang Chen, Tejun Heo, Cliff Whickman,
Robin Holt, Tony Luck, linux-mm, linux-hotplug, linux-kernel, x86
In-Reply-To: <alpine.DEB.2.10.1508171723290.5527@chino.kir.corp.google.com>
On 2015/8/18 8:25, David Rientjes wrote:
> On Mon, 17 Aug 2015, Jiang Liu wrote:
>
>> Function xpc_create_gru_mq_uv() allocates memory with __GFP_THISNODE
>> flag set, which may cause permanent memory allocation failure on
>> memoryless node. So replace cpu_to_node() with cpu_to_mem() to better
>> support memoryless node. For node with memory, cpu_to_mem() is the same
>> as cpu_to_node().
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>> drivers/misc/sgi-xp/xpc_uv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
>> index 95c894482fdd..9210981c0d5b 100644
>> --- a/drivers/misc/sgi-xp/xpc_uv.c
>> +++ b/drivers/misc/sgi-xp/xpc_uv.c
>> @@ -238,7 +238,7 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
>>
>> mq->mmr_blade = uv_cpu_to_blade_id(cpu);
>>
>> - nid = cpu_to_node(cpu);
>> + nid = cpu_to_mem(cpu);
>> page = alloc_pages_exact_node(nid,
>> GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
>> pg_order);
>
> Why not simply fix build_zonelists_node() so that the __GFP_THISNODE
> zonelists are set up to reference the zones of cpu_to_mem() for memoryless
> nodes?
>
> It seems much better than checking and maintaining every __GFP_THISNODE
> user to determine if they are using a memoryless node or not. I don't
> feel that this solution is maintainable in the longterm.
Hi David,
There are some usage cases, such as memory migration,
expect the page allocator rejecting memory allocation requests
if there is no memory on local node. So we have:
1) alloc_pages_node(cpu_to_node(), __GFP_THISNODE) to only allocate
memory from local node.
2) alloc_pages_node(cpu_to_mem(), __GFP_THISNODE) to allocate memory
from local node or from nearest node if local node is memoryless.
Not sure whether we could consolidate all callers specifying
__GFP_THISNODE flag into one case, need more investigating here.
Thanks!
Gerry
^ permalink raw reply
* Re: [Patch V3 0/9] Enable memoryless node support for x86
From: Jiang Liu @ 2015-08-19 8:09 UTC (permalink / raw)
To: Tang Chen, Andrew Morton, Mel Gorman, David Rientjes,
Mike Galbraith, Peter Zijlstra, Rafael J . Wysocki, Tejun Heo
Cc: Tony Luck, linux-mm, linux-hotplug, linux-kernel, x86
In-Reply-To: <55D302CA.9010703@cn.fujitsu.com>
On 2015/8/18 18:02, Tang Chen wrote:
>
> On 08/17/2015 11:18 AM, Jiang Liu wrote:
>> This is the third version to enable memoryless node support on x86
>> platforms. The previous version (https://lkml.org/lkml/2014/7/11/75)
>> blindly replaces numa_node_id()/cpu_to_node() with numa_mem_id()/
>> cpu_to_mem(). That's not the right solution as pointed out by Tejun
>> and Peter due to:
>> 1) We shouldn't shift the burden to normal slab users.
>> 2) Details of memoryless node should be hidden in arch and mm code
>> as much as possible.
>>
>> After digging into more code and documentation, we found the rules to
>> deal with memoryless node should be:
>> 1) Arch code should online corresponding NUMA node before onlining any
>> CPU or memory, otherwise it may cause invalid memory access when
>> accessing NODE_DATA(nid).
>> 2) For normal memory allocations without __GFP_THISNODE setting in the
>> gfp_flags, we should prefer numa_node_id()/cpu_to_node() instead of
>> numa_mem_id()/cpu_to_mem() because the latter loses hardware topology
>> information as pointed out by Tejun:
>> A - B - X - C - D
>> Where X is the memless node. numa_mem_id() on X would return
>> either B or C, right? If B or C can't satisfy the allocation,
>> the allocator would fallback to A from B and D for C, both of
>> which aren't optimal. It should first fall back to C or B
>> respectively, which the allocator can't do anymoe because the
>> information is lost when the caller side performs numa_mem_id().
>
> Hi Liu,
>
> BTW, how is this A - B - X - C - D problem solved ?
> I don't quite follow this.
>
> I cannot tell the difference between numa_node_id()/cpu_to_node() and
> numa_mem_id()/cpu_to_mem() on this point. Even with hardware topology
> info, how could it avoid this problem ?
>
> Isn't it still possible falling back to A from B and D for C ?
Hi Chen,
For the imagined topology, A<->B<->X<->C<->D, where A, B, C, D has
memory and X is memoryless.
Possible fallback lists are:
B: [ B, A, C, D]
X: [ B, C, A, D]
C: [ C, D, B, A]
cpu_to_mem(X) will either return B or C. Let's assume it returns B.
Then we will use "B: [ B, A, C, D]" to allocate memory for X, which
is not the optimal fallback list for X. And cpu_to_node(X) returns
X, and "X: [ B, C, A, D]" is the optimal fallback list for X.
Thanks!
Gerry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox