linux-kernel - Re: [PATCH] async: fix insert entry in ascending list

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <52B1C092.1010704@oracle.com>
Date:	Wed, 18 Dec 2013 23:34:42 +0800
From:	Vaughan Cao <vaughan.cao@...cle.com>
To:	Tejun Heo <tj@...nel.org>
CC:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] async: fix insert entry in ascending list


On 2013年12月18日 20:25, Tejun Heo wrote:
> Hello,
>
> On Wed, Dec 18, 2013 at 11:15:23AM +0800, Vaughan Cao wrote:
>> I suppose there is a fault in the patch of https://lkml.org/lkml/2013/1/16/546.
>> I know you made a new patch for latest kernel which don't move the entry
>> between pending and running list that remove the code I mentioned, but our
>> kernel is based on v3.8.13 that has the code.
>>
>> In my understanding, both pending and running list are sorted ascendingly by
>> cookie value. To find the correct postion to insert the entry into running
>> list, we traverse reversely to the head. When a node with a smaller cookie is
>> found, we break out and add the new entry after it. But the origin code tries
>> to find a larger cookie and insert itself before that node, it won't result in
>> a sorted list in any direction...
> Yeah, I should have used list_for_each_entry() there.  LOL, I'm an
> idiot.
I guess your original intention to use _reverse is that would take less 
steps to find the right position:)

>
>> I don't know if my understanding about the async mechanism is right, so here
>> to have a check with you. Thanks.
>>
>> Signed-off-by: Vaughan Cao <vaughan.cao@...cle.com>
>> ---
>>   kernel/async.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/async.c b/kernel/async.c
>> index 6f34904..596c5e7 100644
>> --- a/kernel/async.c
>> +++ b/kernel/async.c
>> @@ -135,9 +135,9 @@ static void async_run_entry_fn(struct work_struct *work)
>>   	/* 1) move self to the running queue, make sure it stays sorted */
>>   	spin_lock_irqsave(&async_lock, flags);
>>   	list_for_each_entry_reverse(pos, &running->domain, list)
>> -		if (entry->cookie < pos->cookie)
>> +		if (entry->cookie > pos->cookie)
>>   			break;
>> -	list_move_tail(&entry->list, &pos->list);
>> +	list_move(&entry->list, &pos->list);
> Hmmm... sadly, upstream doesn't have the ability to backport this.
> The relevant code path is gone and -stable doesn't backport patches
> which aren't mainline first.  The only way would be backporting
> through distros, I guess.  But, again, this problem shouldn't be
> noticeable with modern userland and it has been broekn without anyone
> noticing for long enough, so maybe we can just leave it alone?
>
> Thanks.
>
Got it. I'll consider pulling your patch of leaving node in pending list 
into our kernel. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/