Re: [PATCH] cpumask 5/10 rewrite cpumask.h - single bitmap based implementation

[Paul Jackson <pj@sgi.com>]

William wrote:
> Ridiculous == ...

oh - ok - we agree

> Sounds like a glibc bug.

We agree.  Someone in glibc land doesn't.

> it's 1024 bits ... SGI may need to get that bumped up,
> but I doubt many others do.

SGI fits, for now.

Someday, someone, won't.

It's baked in to glibc, so someday, someone will have a bit of pain.  
Oh well ...  Good chance that someone will include me.  Not much I
can do about it now.

> +asmlinkage long compat_sched_getaffinity(compat_pid_t pid, ...

That looks more readable.  Thanks.

Do you see a sensible way to pass back an odd number of u32's?  If
NR_CPUS is say 8, then the 32 bit user code might expect to only need
one compat_ulong_t, not two.  If NR_CPUS is 48, then it should only need
3, not 4.   And so forth.

As I noted in another reply, perhaps your bitmap_to_u32_array() code
could be modified to handle this.

And I agree with Andrew's suggestion, that cpumask provide the
conversion, to and from kernel memory, separately from copying the
result to user space.

> This is trivial. Just like we needed ASCII marshalling ...

The implementation is trivial.

The API design choice was the point of my analysis.  Not how to do it,
but what to do.

Should the kernel support two flavors of bitmap marshalling, where it is
headed now, with the sched_setaffinity() format differing from the
perfctr format?  Or should we pick one, and demand that the other
change?

Since I like the perfctr format better, and since I suspect it is to
late to change the sched_setaffinity format, I am resigned to supporting
two binary bitmap formats, across the kernel/user API boundary, forever.

Actually, the two forms are close.  They differ just in the big endian
64 bit case.  And if you were able to handle an odd number of u32 dest
words in your bitmap_to_u32_array() code, then perhaps that single bit
of code could serve as the marshalling for both.  So we end up with two
variants of one flavor, differing only in whether you want 32 or 64 bit
chunks when running on a 64 bit arch, the 64 bit chunks being the native
kernel bitmap representation, for now at least.

That's probably about as good as we are going to do with this.

> The only case where this is distinguished at all from copy_to_user() is
> 64-bit bigendian with 32-bit userspace.

Yes - exactly.  Well, almost.  Either 32-bit userspace compatibility,
or 32 bit chunks for improved portability, such as perfctr has chosen,
if I understand them correctly.

> Index: irqaction-2.6.7-rc2/include/asm-generic/cpumask_array.h

Hmmm ... do you use Quilt too?  That's the only place I recall seeing
this "Index" line.  Cool.

If Andrew accepts my cpumask patches, then you will presumably have
to do your addition of cpus_to_u32_array().  Trivial, of course.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373