PERFORCE change 124529 for review

[Jung-uk Kim]

On Thursday 02 August 2007 03:29 pm, Roman Divacky wrote:
> On Thu, Aug 02, 2007 at 01:56:34PM -0400, Jung-uk Kim wrote:
> > On Thursday 02 August 2007 07:30 am, Roman Divacky wrote:
> > > +
> > > +/* XXX: fake one.. waiting for ssouhlal to commit his patch */
> > > +int
> > > +linux_sched_getaffinity(struct thread *td, struct
> > > linux_sched_getaffinity_args *args) +{
> > > +	int error;
> > > +	cpumask_t i = ~0;
> > > +
> > > +	if (args->len < sizeof(cpumask_t))
> > > +		return (EINVAL);
> > > +
> > > +	error = copyout(&i, args->user_mask_ptr, sizeof(cpumask_t));
> > > +	return (error);
> > > +}
> >
> > Er, shouldn't it be more like this?
> >
> > int
> > linux_sched_getaffinity(struct thread *td,
> > 	struct linux_sched_getaffinity_args *args)
> > {
> > 	uint8_t	*mask;
> > 	int	error;
> >
> > 	if (args->cpusetsize < sizeof(cpumask_t))
> > 		return (EINVAL);
> >
> > 	mask = malloc(args->cpusetsize, M_LINUX, M_WAITOK);
> > 	memset(mask, 0xff, args->cpusetsize);
> > 	error = copyout(mask, args->mask, args->cpusetsize);
> > 	free(mask, M_LINUX);
> >
> > 	return (error);
> > }
>
> hm.. I looked at it and in my version the cpumask_t (linux one) is
> defined to be bit array of configurable length. I dont know what is
> the default but I think its quite safe to assume that its 128.

Yes, it was but not any more.  Basically it depends on Linux kernel 
configuration option, i.e., maximum number of CPUs.  Since the bit 0 
is CPU 0, bit 1 is CPU 1, etc, you have to make sure the last bits 
are properly set.  If you really had to do i = ~0, you probably 
wanted to do casting first, e.g., i = ~(cpumask_t)0 to make sure.  Of 
course my version doesn't have to worry about it. ;-)

> but still.. the prototype is:
>
> asmlinkage long sys_sched_getaffinity(pid_t pid, unsigned int len,
>                                       unsigned long __user
> *user_mask_ptr)
>
> and the len is not used anywhere in the code to dynamically size
> it. I wonder how to deal with that.

The prototype I gave you was from manual page, not the kernel source:

http://www.linuxhowtos.org/manpages/2/sched_getaffinity.htm

While googling, I found many interesting sched_getaffinity() API 
issues but this seems to summarize it nicely:

http://jeff.squyres.com/journal/archives/2005/10/linux_processor.html

Interesting history...

AFAIK, cpumask_t is not dynamically sized as I said but cpu_set_t is.  
Glibc or application just defines cpu_set_t type via CPU_SETSIZE() 
macro and calls sched_getaffinity().  Then, the kernel returns 
sizeof(cpumask_t), so that user can determine the actual size of 
cpumask_t - thanks for catching it, BTW.

Jung-uk Kim