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

William Lee Irwin III
2004-06-06T16:45:33Z

On Sun, Jun 06, 2004 at 05:07:59PM +0200, Mikael Pettersson wrote:
> Notice how it emits the high int before the low int.
> (Which btw also is the native big-endian storage order,
> so the memcpy() would have done the same.)
> Now consider the location of bit 0, with mask value 1(*),
> on a 64-bit big-endian machine. The code above puts this
> in the second int, as bit 0 in *((char*)dst + 7).
> But a 32-bit user-space, or a 64-bit user-space that sees
> an array of ints not longs, wants it in the first int,
> as bit 0 in *((char*)dst + 3).

Feh. So swap the assignments.

On Sun, Jun 06, 2004 at 05:07:59PM +0200, Mikael Pettersson wrote:
> Perfctr's marshalling procedure for cpumask_t values
> (drivers/perfctr/init.c:cpus_copy_to_user() in recent -mm)
> is endian-neutral and converts each long by emitting the
> ints from least significant to most significant.
> Considering the API for retrieving an array of unknown size,
> perfctr's marshalling procedure does the following:
> > const unsigned int k_nrwords = PERFCTR_CPUMASK_NRLONGS*(sizeof(long)/sizeof(int));
> > unsigned int u_nrwords;
> > if (get_user(u_nrwords, &argp->nrwords))
> > return -EFAULT;
> > if (put_user(k_nrwords, &argp->nrwords))
> > return -EFAULT;
> > if (u_nrwords < k_nrwords)
> > return -EOVERFLOW;
> That is, it always tells user-space how much space is needed,
> and if user-space provided too little, it gets EOVERFLOW.
> Knowing the number of words in the encoded cpumask_t also
> avoids having to know the exact value of NR_CPUS in user-space.
> /Mikael
> (*) Normal bit order, not IBM POWER's reversed bit order.

I don't really care about the particular format exported to userspace,
but cpus_addr() is not a legitimate API. cpus_copy_to_user() is, but it
should belong to the core. Just shove the stuff that's doing cpus_addr()
for internals of cpumask_t into lib/bitmap.c etc. and it should be fine.

-- wli