Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Przemek Kitszel]

On 12/6/23 17:46, Johannes Berg wrote:
> On Wed, 2023-12-06 at 08:44 -0800, Jakub Kicinski wrote:
>> On Wed,  6 Dec 2023 11:39:32 +0100 Johannes Berg wrote:
>>> As reported by Marc MERLIN, at least one driver (igc) wants or
>>> needs to acquire the RTNL inside suspend/resume ops, which can
>>> be called from here in ethtool if runtime PM is enabled.
>>>
>>> Allow this by doing runtime PM transitions without the RTNL
>>> held. For the ioctl to have the same operations order, this
>>> required reworking the code to separately check validity and
>>> do the operation. For the netlink code, this now has to do
>>> the runtime_pm_put a bit later.
>>
>> I was really, really hoping that this would serve as a motivation
>> for Intel to sort out the igb/igc implementation. The flow AFAICT
>> is ndo_open() starts the NIC, the calls pm_sus, which shuts the NIC
>> back down immediately (!?) then it schedules a link check from a work
>> queue, which opens it again (!?). It's a source of never ending bugs.
>>
> 
> Well, I work there, but ... WiFi something else entirely. Marc just got
> lucky I spotted an issue in the logs ;-)
> 
> I'll let you guys take it from here ...
> 
> johannes
> 

I have let know our igc TL, architect, and anybody that could be
interested via cc: IWL.
And I'm happy that this could be done at relaxed pace thanks to Johannes
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Jakub Kicinski]

On Thu, 7 Dec 2023 11:16:10 +0100 Przemek Kitszel wrote:
> I have let know our igc TL, architect, and anybody that could be
> interested via cc: IWL. And I'm happy that this could be done at
> relaxed pace thanks to Johannes

I think you may be expecting us to take Johannes's patch.
It's still on the table, but to make things clear -
upstream we prefer to wait for the "real fix", so if we agree
that fixing igb/igc is a better way (as Heiner pointed out on previous
version PM functions are called by the stack under rtnl elsewhere too,
just not while device is open) - we'll wait for that. Especially
that I'm 80% I complained about the PM in those drivers in
the past and nobody seemed to care. It's a constant source of rtnl
deadlocks.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Marc MERLIN]

On Thu, Dec 07, 2023 at 09:40:21AM -0800, Jakub Kicinski wrote:
> On Thu, 7 Dec 2023 11:16:10 +0100 Przemek Kitszel wrote:
> > I have let know our igc TL, architect, and anybody that could be
> > interested via cc: IWL. And I'm happy that this could be done at
> > relaxed pace thanks to Johannes
> 
> I think you may be expecting us to take Johannes's patch.
> It's still on the table, but to make things clear -
> upstream we prefer to wait for the "real fix", so if we agree
> that fixing igb/igc is a better way (as Heiner pointed out on previous
> version PM functions are called by the stack under rtnl elsewhere too,
> just not while device is open) - we'll wait for that. Especially
> that I'm 80% I complained about the PM in those drivers in
> the past and nobody seemed to care. It's a constant source of rtnl
> deadlocks.

For whatever it's worth, I want to be clear that all stock kernels
are 100% unusable on lenovo P17gen2 because of this deadlock and that
without the temporary patch, my laptop would be usuable.
It was also a risk of data loss due to repeated deadlocks and unclean
shutdowns.

I cannot say what the correct fix is, but I am definitely hoping you
will accept some solution for the next stable kernel.

Thank you
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
 
Home page: http://marc.merlins.org/                       | PGP 7F55D5F27AAF9D08
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Heiner Kallweit]

On 11.12.2023 05:52, Marc MERLIN wrote:
> On Thu, Dec 07, 2023 at 09:40:21AM -0800, Jakub Kicinski wrote:
>> On Thu, 7 Dec 2023 11:16:10 +0100 Przemek Kitszel wrote:
>>> I have let know our igc TL, architect, and anybody that could be
>>> interested via cc: IWL. And I'm happy that this could be done at
>>> relaxed pace thanks to Johannes
>>
>> I think you may be expecting us to take Johannes's patch.
>> It's still on the table, but to make things clear -
>> upstream we prefer to wait for the "real fix", so if we agree
>> that fixing igb/igc is a better way (as Heiner pointed out on previous
>> version PM functions are called by the stack under rtnl elsewhere too,
>> just not while device is open) - we'll wait for that. Especially
>> that I'm 80% I complained about the PM in those drivers in
>> the past and nobody seemed to care. It's a constant source of rtnl
>> deadlocks.
> 
> For whatever it's worth, I want to be clear that all stock kernels
> are 100% unusable on lenovo P17gen2 because of this deadlock and that
> without the temporary patch, my laptop would be usuable.
> It was also a risk of data loss due to repeated deadlocks and unclean
> shutdowns.
> 
Why don't you simply disable runtime pm for the affected device as a
workaround? This can be done via sysfs.

> I cannot say what the correct fix is, but I am definitely hoping you
> will accept some solution for the next stable kernel.
> 
> Thank you
> Marc

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Marc MERLIN]

On Fri, Dec 15, 2023 at 02:42:01PM +0100, Heiner Kallweit wrote:
> Why don't you simply disable runtime pm for the affected device as a
> workaround? This can be done via sysfs.

1) because I didn't know what the exact bug was and how to work around it :)
2) without power management, the battery use is not good, but yes not
good is better than laptop crashing :)

That said, if it's only affecting wired ethernet, I can also unload the
igc module until I actually need wired ethernet, which is sometimes but
not often.

Thanks for your suggestion
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
 
Home page: http://marc.merlins.org/                       | PGP 7F55D5F27AAF9D08
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Marc MERLIN]

On Fri, Dec 15, 2023 at 09:46:34AM -0800, Marc MERLIN wrote:
> On Fri, Dec 15, 2023 at 02:42:01PM +0100, Heiner Kallweit wrote:
> > Why don't you simply disable runtime pm for the affected device as a
> > workaround? This can be done via sysfs.
> 
> 1) because I didn't know what the exact bug was and how to work around it :)

Mmmh, so I need to test an ubuntu kernel (6.5.0-14) because of sound
issues in mainline TOT, and I can't boot the kernel to completion
without hititng this hang bug. I'm not exactly sure which part of the
boot triggers it.

I can't patch that kernel easily. How exactly do I disable runtime PM
from the kernel command line for "that device" which I'm not even sure
which one it is.  If it's the eth device, I already removed the igc
module to prevent it from loading, and I also removed the ethtool
binary, but I'm still getting the hang.

On the plus side, with 6.6.8 and the old patch which I understand is not
the ideal solution, I can confirm that I've been running problem free
until now, so thanks again for that interim patch.

Thanks,
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
 
Home page: http://marc.merlins.org/                       | PGP 7F55D5F27AAF9D08
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Heiner Kallweit]

On 24.12.2023 17:30, Marc MERLIN wrote:
> On Fri, Dec 15, 2023 at 09:46:34AM -0800, Marc MERLIN wrote:
>> On Fri, Dec 15, 2023 at 02:42:01PM +0100, Heiner Kallweit wrote:
>>> Why don't you simply disable runtime pm for the affected device as a
>>> workaround? This can be done via sysfs.
>>
>> 1) because I didn't know what the exact bug was and how to work around it :)
> 
> Mmmh, so I need to test an ubuntu kernel (6.5.0-14) because of sound
> issues in mainline TOT, and I can't boot the kernel to completion
> without hititng this hang bug. I'm not exactly sure which part of the
> boot triggers it.
> 
> I can't patch that kernel easily. How exactly do I disable runtime PM
> from the kernel command line for "that device" which I'm not even sure

Change <device>/power/control from "auto" to "on".

> which one it is.  If it's the eth device, I already removed the igc
> module to prevent it from loading, and I also removed the ethtool
> binary, but I'm still getting the hang.
> 
> On the plus side, with 6.6.8 and the old patch which I understand is not
> the ideal solution, I can confirm that I've been running problem free
> until now, so thanks again for that interim patch.
> 
> Thanks,
> Marc

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Sasha Neftin]

On 25/12/2023 1:12, Heiner Kallweit wrote:
> On 24.12.2023 17:30, Marc MERLIN wrote:
>> On Fri, Dec 15, 2023 at 09:46:34AM -0800, Marc MERLIN wrote:
>>> On Fri, Dec 15, 2023 at 02:42:01PM +0100, Heiner Kallweit wrote:
>>>> Why don't you simply disable runtime pm for the affected device as a
>>>> workaround? This can be done via sysfs.
>>>
>>> 1) because I didn't know what the exact bug was and how to work around it :)
>>
>> Mmmh, so I need to test an ubuntu kernel (6.5.0-14) because of sound
>> issues in mainline TOT, and I can't boot the kernel to completion
>> without hititng this hang bug. I'm not exactly sure which part of the
>> boot triggers it.
>>
>> I can't patch that kernel easily. How exactly do I disable runtime PM
>> from the kernel command line for "that device" which I'm not even sure
> 
> Change <device>/power/control from "auto" to "on".

Need to figure out your controller location in a file system via 
lspci/lspci -t and then change to "on"
For example: echo on > 
/sys/devices/pci0000\:00/0000\:00\:1c.0/0000\:ae\:00.0/power/control

We are starting to look at this problem, but I can't reproduce the 
problem on my machines yet.

> 
>> which one it is.  If it's the eth device, I already removed the igc
>> module to prevent it from loading, and I also removed the ethtool
>> binary, but I'm still getting the hang.
>>
>> On the plus side, with 6.6.8 and the old patch which I understand is not
>> the ideal solution, I can confirm that I've been running problem free
>> until now, so thanks again for that interim patch.
>>
>> Thanks,
>> Marc
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net v3] net: ethtool: do runtime PM outside RTNL

[Marc MERLIN]

On Mon, Dec 25, 2023 at 10:03:23AM +0200, Sasha Neftin wrote:
> > > I can't patch that kernel easily. How exactly do I disable runtime PM
> > > from the kernel command line for "that device" which I'm not even sure
> > 
> > Change <device>/power/control from "auto" to "on".
> 
> Need to figure out your controller location in a file system via lspci/lspci
> -t and then change to "on"
> For example: echo on >
> /sys/devices/pci0000\:00/0000\:00\:1c.0/0000\:ae\:00.0/power/control
> 
> We are starting to look at this problem, but I can't reproduce the problem
> on my machines yet.

Thanks. I realized it was going to be hard either way if the boot hangs
before I get to a command prompt, which was what was happening
yesterday.
I had to boot ubuntu to debug a sound issue, and it was very tricky
since most of the time it hung before I got to a command prompt, but I
was finally able to get it to work long enough to debug the sound issue
and go back to my self built kernel to port over the sound config I
needed.

I wish I could tell you exactly how to reproduct this in a more useful
way, sorry about that.

Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
 
Home page: http://marc.merlins.org/                       | PGP 7F55D5F27AAF9D08
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan