On 21/05/25 16:53, Nam Cao wrote:
> On Wed, May 21, 2025 at 04:25:00PM +0200, Valentin Schneider wrote:
>> > + llist_for_each_entry_safe(epi, tmp, txlist.first, rdllink) {
>> > + /*
>> > + * We are done iterating. Allow the items we took to be added back to the ready
>> > + * list.
>> > + */
>> > + xchg(&epi->link_locked, false);
>> > +
>> > + /*
>> > + * In the loop above, we may mark some items ready, and they should be added back.
>> > + *
>> > + * Additionally, someone else may also attempt to add the item to the ready list,
>> > + * but got blocked by us. Add those blocked items now.
>> > + */
>> > + if (smp_load_acquire(&epi->ready)) {
>> > ep_pm_stay_awake(epi);
>> > + epitem_ready(epi);
>> > }
>>
>> Isn't this missing a:
>>
>> list_del_init(&epi->rdllink);
>>
>> AFAICT we're always going to overwrite that link when next marking the item
>> as ready, but I'd say it's best to exit this with a clean state. That would
>> have to be before the clearing of link_locked so it doesn't race with a
>> concurrent epitem_ready() call.
>
> To confirm I understand you: there is no functional problem, and your
> comment is more of a "just to be safe"?
>
Yup, even if they're not accessed it's good to not have stray references to
stack addresses.
> Thanks so much for the review,
> Nam
