lkml.org 
[lkml]   [2014]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4] irqchip: gic: Allow gic_arch_extn hooks to call into scheduler
    On Sun, Aug 17, 2014 at 03:04:34PM -0400, Jason Cooper wrote:
    > Quoting Nico:
    >
    > "Of course it would be good to clarify things wrt Russell's remark
    > independently from this patch."
    >
    > I took 'independently' to mean "This patch is ok, *and* we need to
    > address Russell's concerns in a follow-up patch."
    >
    > Nico's Reviewed-by with that comment was sent August 13th. The most
    > recent activity on this thread was also August 13th. After four days, I
    > reasoned there were no objections to his comment.

    Right, during the merge window, and during merge windows, I tend to
    ignore almost all email now because people don't stop developing, and
    they don't take any notice where the mainline cycle is. In fact, I go
    off and do non-kernel work during a merge window and only briefly scan
    for bug fixes.

    However, I have other concerns with this patch, which I've yet to air.
    For example, I don't like this crappy conditional locking that people
    keep dreaming up - that kind of stuff makes the kernel much harder to
    statically check that everything is correct. It's an anti-lockdep
    strategy.

    Secondly, I don't like this:

    + raw_spin_lock(&gic_sgi_lock);
    + /*
    + * Ensure that the gic_cpu_map update above is seen in
    + * gic_raise_softirq() before we redirect any pending SGIs that
    + * may have been raised for the outgoing CPU (cur_cpu_id)
    + */
    + smp_mb__after_unlock_lock();
    + raw_spin_unlock(&gic_sgi_lock);

    That goes against the principle of locking, that you lock the data,
    not the code.

    I have no problem with changing gic_raise_softirq() to use a different
    lock, which gic_migrate_target(), and gic_set_affinity() can also use.
    There's no need for horrid locking here, because the only thing we're
    protecting is gic_map[] and the write to the register to trigger an
    IPI - and nothing using gic_arch_extn has any business knowing about
    SGIs.

    No need for these crappy sgi_map_lock() macros and all the ifdeffery.

    --
    FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
    according to speedtest.net.


    \
     
     \ /
      Last update: 2014-08-18 00:21    [W:2.705 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site