From: Felipe Balbi <balbi@kernel.org>
To: Wesley Cheng <wcheng@codeaurora.org>
Cc: gregkh@linuxfoundation.org, peter.chen@kernel.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
jackp@codeaurora.org
Subject: Re: [PATCH v2] usb: dwc3: gadget: Replace list_for_each_entry_safe() if using giveback
Date: Wed, 28 Jul 2021 08:53:49 +0300 [thread overview]
Message-ID: <87pmv3t24y.fsf@kernel.org> (raw)
In-Reply-To: <3c55dd4f-5ebd-3730-7428-cb15235465a7@codeaurora.org>
Hi Wesley,
Wesley Cheng <wcheng@codeaurora.org> writes:
>>>>> Sorry for the late response. So I tried with a list_replace_init() to
>>>>> within the list_for_each_entry_safe() loop to update tmp w/ the
>>>>> cancelled_list list head, but the issue was still observed. This is
>>>>> because we can't replace the reference the loop already has stored in
>>>>> tmp, which is simply updated as the current item on the next iteration.
>>>>>
>>>>> I believe this is what you were trying to achieve?
>>>>>
>>>> Was wondering if you had any further inputs on this change? As
>>>> mentioned, I tried a few things with list_replace_init(), which did not
>>>> work.
>>>>
>>>
>>> Sorry for the ping. Is this change OK to add as is? We've been running
>>> into this instance pretty frequently during our testing, so just wanted
>>> to close on the proper changes being merged upstream.
>>
>> The idea is this:
>>
>> struct list_head local;
>>
>> spin_lock_irq(&lock);
>> list_replace_init(&dwc->cancelled_list, &local);
>> spin_unlock_irq(&lock);
>>
>> list_for_each_entry_safe(req, tmp, &local, list) {
>> /* ... */
>> }
>>
>> It looks to me this should work fine, no? You can also follow what
>> drivers/usb/core/hcd.c is doing in usb_giveback_urb_bh() and restarting
>> if dwc->cancelled_list is not empty after list_for_each_entry_safe().
>>
>> Can you give that one a shot?
>>
>
> Great, thanks for this suggestion! Now I understand what you were
> referring to. I gave this a try and it works well. Will prepare a
> change to replace both places with list_replace_init()
this is great news :-) Thanks for trying it out
--
balbi
prev parent reply other threads:[~2021-07-28 5:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-11 7:03 [PATCH v2] usb: dwc3: gadget: Replace list_for_each_entry_safe() if using giveback Wesley Cheng
2021-05-11 8:13 ` Felipe Balbi
2021-05-19 8:52 ` Wesley Cheng
2021-06-09 20:57 ` Wesley Cheng
2021-07-21 6:14 ` Wesley Cheng
2021-07-21 6:30 ` Felipe Balbi
2021-07-27 21:00 ` Wesley Cheng
2021-07-28 5:53 ` Felipe Balbi [this message]
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=87pmv3t24y.fsf@kernel.org \
--to=balbi@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jackp@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=peter.chen@kernel.org \
--cc=wcheng@codeaurora.org \
/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;
as well as URLs for NNTP newsgroup(s).