lkml.org 
[lkml]   [2004]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] fix sys cpumap for > 352 NR_CPUS
    From
    Date
    On Thu, 2004-06-03 at 18:27, Paul Jackson wrote:
    > > + BUILD_BUG_ON(NR_CPUS/4 > PAGE_SIZE/2);
    >
    > Interesting. This would be the only use in the entire kernel of
    > BUILD_BUG_ON().
    >
    > An alternative mechanism would be:
    >
    > #if(badthing) ... #error "darn" ... #endif

    Whatever, I like BUILD_BUG_ON().

    > #if ALIGN(NR_CPUS,32)*9/32 > PAGE_SIZE
    > #error "Need 9 bytes space per 32 CPUs in PAGE_SIZE buffer"
    > #endif

    Honestly, I dislike the static check altogether, since it's tied too
    closely to the implementation of independent code. Since glibc will
    break on > 1024 cpus, I'm not too concerned about the 10000+ CPU limit.

    > > + len = cpumask_scnprintf(buf, -1UL, mask);
    >
    > Why not instead:
    >
    > > + len = cpumask_scnprintf(buf, PAGE_SIZE-1, mask);
    >
    > I see no sense in giving cpumask_scnprintf() license to write past the
    > end of the buffer, independent of any build-time checks (the -1 is for
    > the trailing newline). And since the contract says "PAGE_SIZE", we
    > should code exactly to that value "PAGE_SIZE", for clarity as to our
    > understandings. Once again - I hate fuzz ;).

    Because now you have silently truncated, which is much worse than
    blowing up loudly. My version also works correctly when sysfs starts
    handing out two pages rather than one because something else needed it.

    If you want to do this, then please do:

    len = cpumask_scnprintf(buf, PAGE_SIZE, mask);
    if (len == PAGE_SIZE)
    return -ENOMEM;

    > > + BUG_ON(count > PAGE_SIZE);
    >
    > Only 'BUG_ON' ?? We have in hand almost certain proof of just
    > having scrogged kernel memory. Time to panic, no?

    That would be extremely unusual; we tend not to panic even when things
    are bad. In this case, things are bad because of a static configuration
    error, so I find it hard to really care...

    > ==
    >
    > What are we going to do about the removal of the node_dev->cpumap field,
    > and changing this node_read_cpumap() routine to display instead the
    > value of node_to_cpumask(node_dev->sysdev.id)?
    >
    > Should I do it, or you? Should it presume your patch above, or collide
    > with it, or replace and extend it?

    I'll do it; seems like we need negotiation with Greg-KH, too.

    > Since I am most impressed with your abilities, since you doubt my
    > abilities, and since I'm a lazy s.o.b., you're welcome to it.

    That's not true. I have faith in your abilities; I question anyone's
    ability to produce a perfectly balanced solution without any external
    input.

    Rusty.
    --
    Anyone who quotes me in their signature is an idiot -- Rusty Russell

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:03    [W:4.156 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site