lkml.org 
[lkml]   [2011]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH -mm 1/2] irq_work, Use llist in irq_work
    From
    Date
    On Thu, 2011-09-01 at 09:46 +0800, Huang Ying wrote:

    > You mean we should not use cpu_relax before the first cmpxchg?

    Yeah, that's just wasting time for no reason..

    > You suggest something as follow?
    >
    > void llist_add(struct llist_node *new, struct llist_head *head)
    > {
    > struct llist_node *entry, *old_entry;
    >
    > #ifndef CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG
    > BUG_ON(in_nmi());
    > #endif
    >
    > entry = head->first;
    > for (;;) {
    > old_entry = entry;
    > new->next = entry;
    > entry = cmpxchg(&head->first, old_entry, new);
    > if (entry == old_entry)
    > break;
    > cpu_relax();
    > }
    > }

    If you insist on having cpu_relax(), then yes that's lots better. Also
    avoids the assignment in your conditional. Thing with cpu_relax() is
    that its only beneficial in the highly contended case and degrade
    light/un-contended loads.

    Also, just noticed, why do you have different list_head/list_node
    structures? They're the same, a single pointer.

    > > and loose the get/put
    > > cpu muck? The existing preempt_disable/enable() are already superfluous
    > > and could be removed, you just made all this way more horrid than need
    > > be.
    >
    > Will it cause race condition to remove preempt_disable/enable?
    > Considering something as follow:
    >
    > - get irq_work_list of CPU A
    > - queue irq_work into irq_work_list of CPU A
    > - preempted and resumed execution on CPU B
    > - arch_irq_work_raise on CPU B
    >
    > irq_work_run on CPU B will do nothing. While irq_work need to wait for
    > next timer interrupt. Isn't it an issue?

    Yes that's unfortunate, the current version would work just fine without
    preempt but that's because of the this_cpu_* ops foo.

    Not sure it would make sense to add a special this_cpu_llist_add() or
    so.. esp seeing that this_cpu_* is basically x86-only.


    \
     
     \ /
      Last update: 2011-09-01 09:59    [W:2.703 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site