2012/10/29 Steven Rostedt <rost...@goodmis.org>:
> On Mon, 2012-10-29 at 14:28 +0100, Frederic Weisbecker wrote:
>> On irq work initialization, let the user choose to define it
>> as "lazy" or not. "Lazy" means that we don't want to send
>> an IPI (provided the arch can anyway) when we enqueue this
>> work but we rather prefer to wait for the next timer tick
>> to execute our work if possible.
>>
>> This is going to be a benefit for non-urgent enqueuers
>> (like printk in the future) that may prefer not to raise
>> an IPI storm in case of frequent enqueuing on short periods
>> of time.
>>
>> Signed-off-by: Frederic Weisbecker <fweis...@gmail.com>
>> Cc: Peter Zijlstra <pet...@infradead.org>
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Ingo Molnar <mi...@kernel.org>
>> Cc: Andrew Morton <a...@linux-foundation.org>
>> Cc: Steven Rostedt <rost...@goodmis.org>
>> Cc: Paul Gortmaker <paul.gortma...@windriver.com>
>> ---
>> include/linux/irq_work.h | 31 ++++++++++++++++++++++++++
>> kernel/irq_work.c | 53
>> ++++++++++++++++++++++++++++++++-------------
>> kernel/time/tick-sched.c | 3 +-
>> 3 files changed, 70 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
>> index b39ea0b..7b60c87 100644
>> --- a/include/linux/irq_work.h
>> +++ b/include/linux/irq_work.h
>> @@ -4,6 +4,20 @@
>> #include <linux/llist.h>
>> #include <asm/irq_work.h>
>>
>> +/*
>> + * An entry can be in one of four states:
>> + *
>
> Can you add a comment to what the pointer value is. I know you just
> moved it to the header, but it's still confusing.
Which pointer value? You mean the flag? You mean the below need more
details or?
>
>> + * free NULL, 0 -> {claimed} : free to be used
>> + * claimed NULL, 3 -> {pending} : claimed to be enqueued
>> + * pending next, 3 -> {busy} : queued, pending callback
>> + * busy NULL, 2 -> {free, claimed} : callback in progress, can be
>> claimed
>> + */
>> +
>> +#define IRQ_WORK_PENDING 1UL
>> +#define IRQ_WORK_BUSY 2UL
>> +#define IRQ_WORK_FLAGS 3UL
>> +#define IRQ_WORK_LAZY 4UL /* Doesn't want IPI, wait for tick
>> */
[...]
>> @@ -66,10 +56,28 @@ static void __irq_work_queue(struct irq_work *work)
>> preempt_disable();
>>
>> empty = llist_add(&work->llnode, &__get_cpu_var(irq_work_list));
>> - /* The list was empty, raise self-interrupt to start processing. */
>> - if (empty)
>> +
>> + /*
>> + * In any case, raise an IPI if requested and possible in case
>> + * the queue is empty or it's filled with lazy works.
>> + */
>> + if (!(work->flags & IRQ_WORK_LAZY) && arch_irq_work_has_ipi()) {
>> arch_irq_work_raise();
>
> Doesn't this mean that now if we queue up a bunch of work (say in
> tracing), that we will send out an IPI for each queue? We only want to
> send out an IPI if the list isn't empty. Perhaps we should make two
> lists. One for lazy work and one for immediate work. Then, when adding a
> non-lazy work item, we can check the empty variable for that. No need to
> check the result for the lazy queue. That will be done during tick.
Indeed, if an IPI is pending while we queue another work, we'll raise
another one. I would prefer to avoid the complication of adding
another queue though. Perhaps a per cpu "ipi_pending" flag would be
enough. I'll try something.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
