| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Jun 2023 21:14:29 +1100
From: wang xiaolei <xiaolei.wang@...driver.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: peter.chen@...nel.org, pawell@...ence.com, rogerq@...nel.org,
a-govindraju@...com, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the
spin lock
On 6/15/23 8:49 PM, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, Jun 14, 2023 at 11:16:05AM +0800, wangxiaolei wrote:
>> On 5/5/23 6:33 PM, Greg KH wrote:
>>> CAUTION: This email comes from a non Wind River email account!
>>> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>> On Thu, May 04, 2023 at 09:16:02PM +0800, Xiaolei Wang wrote:
>>>> The device may be scheduled during the resume process,
>>>> so this cannot appear in atomic operations. Since
>>>> pm_runtime_set_active will resume suppliers, put set
>>>> active outside the spin lock, which is only used to
>>>> protect the struct cdns data structure, otherwise the
>>>> kernel will report the following warning:
>>>>
>>>> BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1163
>>>> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
>>>> preempt_count: 1, expected: 0
>>>> RCU nest depth: 0, expected: 0
>>>> CPU: 0 PID: 651 Comm: sh Tainted: G WC 6.1.20 #1
>>>> Hardware name: Freescale i.MX8QM MEK (DT)
>>>> Call trace:
>>>> dump_backtrace.part.0+0xe0/0xf0
>>>> show_stack+0x18/0x30
>>>> dump_stack_lvl+0x64/0x80
>>>> dump_stack+0x1c/0x38
>>>> __might_resched+0x1fc/0x240
>>>> __might_sleep+0x68/0xc0
>>>> __pm_runtime_resume+0x9c/0xe0
>>>> rpm_get_suppliers+0x68/0x1b0
>>>> __pm_runtime_set_status+0x298/0x560
>>>> cdns_resume+0xb0/0x1c0
>>>> cdns3_controller_resume.isra.0+0x1e0/0x250
>>>> cdns3_plat_resume+0x28/0x40
>>>>
>>>> Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
>>>> ---
>>>> drivers/usb/cdns3/cdns3-plat.c | 3 ++-
>>>> drivers/usb/cdns3/cdnsp-pci.c | 3 ++-
>>>> drivers/usb/cdns3/core.c | 14 +++++++++++---
>>>> drivers/usb/cdns3/core.h | 3 ++-
>>>> 4 files changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>>>> index 2bc5d094548b..726b2e4f67e4 100644
>>>> --- a/drivers/usb/cdns3/cdns3-plat.c
>>>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>>>> @@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
>>>> cdns3_set_platform_suspend(cdns->dev, false, false);
>>>>
>>>> spin_lock_irqsave(&cdns->lock, flags);
>>>> - cdns_resume(cdns, !PMSG_IS_AUTO(msg));
>>>> + cdns_resume(cdns);
>>>> cdns->in_lpm = false;
>>>> spin_unlock_irqrestore(&cdns->lock, flags);
>>>> + cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
>>>> if (cdns->wakeup_pending) {
>>>> cdns->wakeup_pending = false;
>>>> enable_irq(cdns->wakeup_irq);
>>>> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>>>> index 7b151f5af3cc..0725668ffea4 100644
>>>> --- a/drivers/usb/cdns3/cdnsp-pci.c
>>>> +++ b/drivers/usb/cdns3/cdnsp-pci.c
>>>> @@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct device *dev)
>>>> int ret;
>>>>
>>>> spin_lock_irqsave(&cdns->lock, flags);
>>>> - ret = cdns_resume(cdns, 1);
>>>> + ret = cdns_resume(cdns);
>>>> spin_unlock_irqrestore(&cdns->lock, flags);
>>>> + cdns_set_active(cdns, 1);
>>>>
>>>> return ret;
>>>> }
>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>> index dbcdf3b24b47..661d5597fb68 100644
>>>> --- a/drivers/usb/cdns3/core.c
>>>> +++ b/drivers/usb/cdns3/core.c
>>>> @@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)
>>>> }
>>>> EXPORT_SYMBOL_GPL(cdns_suspend);
>>>>
>>>> -int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> +int cdns_resume(struct cdns *cdns)
>>>> {
>>>> struct device *dev = cdns->dev;
>>>> enum usb_role real_role;
>>>> @@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> if (cdns->roles[cdns->role]->resume)
>>>> cdns->roles[cdns->role]->resume(cdns, cdns_power_is_lost(cdns));
>>>>
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cdns_resume);
>>>> +
>>>> +void cdns_set_active(struct cdns *cdns, u8 set_active)
>>>> +{
>>>> + struct device *dev = cdns->dev;
>>>> +
>>>> if (set_active) {
>>>> pm_runtime_disable(dev);
>>>> pm_runtime_set_active(dev);
>>>> pm_runtime_enable(dev);
>>>> }
>>>>
>>>> - return 0;
>>>> + return;
>>>> }
>>>> -EXPORT_SYMBOL_GPL(cdns_resume);
>>>> +EXPORT_SYMBOL_GPL(cdns_set_active);
>>>> #endif /* CONFIG_PM_SLEEP */
>>>>
>>>> MODULE_AUTHOR("Peter Chen <peter.chen@....com>");
>>>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>>>> index 2d332a788871..0f429042f997 100644
>>>> --- a/drivers/usb/cdns3/core.h
>>>> +++ b/drivers/usb/cdns3/core.h
>>>> @@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);
>>>> int cdns_remove(struct cdns *cdns);
>>>>
>>>> #ifdef CONFIG_PM_SLEEP
>>>> -int cdns_resume(struct cdns *cdns, u8 set_active);
>>>> +int cdns_resume(struct cdns *cdns);
>>>> int cdns_suspend(struct cdns *cdns);
>>>> +void cdns_set_active(struct cdns *cdns, u8 set_active);
>>>> #else /* CONFIG_PM_SLEEP */
>>>> static inline int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> { return 0; }
>>>> --
>>>> 2.25.1
>>>>
>>> Hi,
>>>
>>> This is the friendly semi-automated patch-bot of Greg Kroah-Hartman.
>>> You have sent him a patch that has triggered this response.
>>>
>>> Right now, the development tree you have sent a patch for is "closed"
>>> due to the timing of the merge window. Don't worry, the patch(es) you
>>> have sent are not lost, and will be looked at after the merge window is
>>> over (after the -rc1 kernel is released by Linus).
>>>
>>> So thank you for your patience and your patches will be reviewed at this
>>> later time, you do not have to do anything further, this is just a short
>>> note to let you know the patch status and so you don't worry they didn't
>>> make it through.
>> Can someone help review this patch?
> It breaks the build:
>
> drivers/usb/cdns3/core.c: In function ‘cdns_resume’:
> drivers/usb/cdns3/core.c:527:24: error: unused variable ‘dev’ [-Werror=unused-variable]
> 527 | struct device *dev = cdns->dev;
> | ^~~
>
>
> How did you test it?
Oh, sorry, this may be because I did not set the compilation parameters,
which caused me to ignore this warning. I will delete the unused code
and send the v2 version
thanks
xiaolei
>
> thanks,
>
> greg k-h
Powered by blists - more mailing lists