[lkml]   [2003]   [Dec]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] another minor bit of cpumask cleanup
    In message <> you write:
    > I like your patch. Since your more substantial patch negates my
    > trivial patch to remove the old for_each_online_cpu(), I'll forget
    > about my patch.

    OK, thanks for reviewing!

    > Speaking of trivial patches, didn't you (Rusty) used to be the Trivial
    > Patch Monkey, and what has become of that esteemed role in 2.6 land?

    Yes, but while things were frozen I only checked that mailbox once a
    week or less, since things slowed to a crawl. Also, there are only so
    many genuinely trivial patches which aren't stupid 8)

    > I do have a more substantial patch that is yet widely published to
    > provide an alternative underlying implementation of the cpumask macros
    > with something that can be used for both cpu and node masks, that takes
    > one file to express instead of 5 or 6, and that has one base type
    > (struct of array of unsigned longs) rather than a choice of three or so
    > implementations.

    Hmm, sounds interesting.

    > For at least one architecture, sparc64 (IIRC), wli informs me that davem
    > is quite certain this alternative can't be used (resulting machine code
    > way too painful). But I am hopeful that we can make it cleaner source
    > code and just as good machine code, at least for the architectures that
    > can use recent gcc optimizing.

    I think we'll only be able to tell with the patch in front of us.
    Maybe Dave will be convinced: he's dogmatic, but rarely unreasonable.

    > > possible cpu ... online cpus
    > I'm not quite sure of the meaning to you of these terms.
    > Is it that possible cpus are the union of online and offline cpus?

    Yes. cpu_online(x) <= cpu_possible(x). For adding counters and the
    like, cpu_possible() is the test you want (most common usage). If
    you're using cpu_online(x), you need to either hold the cpucontrol
    lock, or register a callback for it changing, or both.

    > > noone uses them that way (except for arch/i386/mach-voyager, which
    > > D: uses for_each_cpu(cpu_online_mask)
    > What about the one remaining usage of for_each_cpu(), also in
    > voyager, but not using cpu_online_mask:
    > arch/i386/mach-voyager/voyager_smp.c:
    > =============================================================
    > #ifdef VOYAGER_DEBUG
    > ...
    > if((isr & (1<<irq) && !(status & IRQ_REPLAY)) == 0) {
    > ...
    > int mask;
    > printk("VOYAGER SMP: CPU%d lost interrupt %d\n",
    > cpu, irq);
    > for_each_cpu(real_cpu, mask) {
    > =============================================================
    > You noted that 'mask' needed initializing in a comment in your patch,
    > but I don't see that you change the calling signature of for_each_cpu(),
    > not that it is clear to what it should be changed ;(.

    I figured the code is broken as is: I've left it broken (with the
    benefit that it no longer even compiles). Someday someone will enable
    VOYAGER_DEBUG and they'll fix it.

    > > so the iterators are moved
    > > D: from linux/cpumask.h to linux/smp.h, where that is asm/smp.h is included.
    > This comment says the iterators are moved to smp.h, but the patch seems
    > to still show them in cpumask.h. I suspect that I prefer them in smp.h
    > better.

    Good catch: that comment is wrong. Moving broke too much stuff IIRC.
    Someone can do a separate patch if they want.

    > > D: Followup patches will convert users.
    > Looks to me like this here patch is converting some users, such as
    > in fork.c and sched.c. Is this the conversion you speak of, or is
    > there more to come in a followup?

    I did the users which are actually wrong, but didn't do the ones which
    are simply inefficient (ie. for (i = 0; i < NR_CPUS; i++)). The
    freeze came down, and I decided to go for minimal impact.

    In 2.7, my aim is to switch the rest of them, move more things to
    per-cpu rather than [NR_CPUS] arrays, add the more efficient dynamic
    per-cpu allocation, and spread the per-cpu religion by fire and the

    But I've said too much already...
    Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:59    [W:0.025 / U:46.012 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site