linux-kernel - Re: [PATCH -mm 1/2] irq_work, Use llist in irq

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <4E5EFA08.30205@intel.com>
Date:	Thu, 01 Sep 2011 11:20:40 +0800
From:	Huang Ying <ying.huang@...el.com>
To:	Huang Ying <ying.huang@...el.com>
CC:	Peter Zijlstra <peterz@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -mm 1/2] irq_work, Use llist in irq_work

On 09/01/2011 09:46 AM, Huang Ying wrote:
>>> -static void __irq_work_queue(struct irq_work *entry)
>>> +static void __irq_work_queue(struct irq_work *work)
>>>  {
>>> -	struct irq_work *next;
>>> +	struct irq_work_list *irq_work_list;
>>>  
>>> -	preempt_disable();
>>> +	irq_work_list = &get_cpu_var(irq_work_lists);
>>>  
>>> -	do {
>>> -		next = __this_cpu_read(irq_work_list);
>>> -		/* Can assign non-atomic because we keep the flags set. */
>>> -		entry->next = next_flags(next, IRQ_WORK_FLAGS);
>>> -	} while (this_cpu_cmpxchg(irq_work_list, next, entry) != next);
>>> +	llist_add(&work->llnode, &irq_work_list->llist);
>>>  
>>>  	/* The list was empty, raise self-interrupt to start processing. */
>>> -	if (!irq_work_next(entry))
>>> +	if (!test_and_set_bit(LIST_NONEMPTY_BIT, &irq_work_list->flags))
>>>  		arch_irq_work_raise();
>>
>> So why can't you simply test work->llnode->next?
> 
> Yes.  That is better.  Even if there may be a small race window, it is
> not a big issue to raise one extra self interrupt seldom.

Remember something about this.  I didn't test work->llnode->next here
because I didn't want expose the implementation details like that here.
 How about make llist_add() return whether list is empty before adding?
 Because it will be an inline function, that should be optimized out if
the caller do not need the information.

Best Regards,
Huang Ying
--
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/