Re: [PATCH] fix cpumask ncpus parameter handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 25, 2008 at 03:33:52PM -0500, Cliff Wickman wrote:
> 
> In Neil's patch:
> > +       int ncpumask = (1<<(sysconf(_SC_NPROCESSORS_CONF)))-1;
> >         /*
> >          * The maximum line size consists of the string at the beginning plus
> >          * a digit for each 4 cpus and a comma for each 64 cpus.
> > @@ -480,10 +481,22 @@ set_thread_constraints(void)
> >         fclose(f);
> >         free (buffer);
> > 
> > -       if (maxprocnode < 0) {
> > +       /*
> > +        * Cps_allowed in the kenrel can be defined to all f's
> > +        * i.e. it may be a superset of the actual available processors
> > +        * as such lets reduce maxproccpu with a mask of the actual
> > +        * available cpus
> > +        */
> > +       maxproccpu &= ncpumask;
> 
> I agree with the fix as long as the number of cpus does not exceed
> the number of bits in an int. But that's a pretty small constraint.
> I propose to trim maxprocnode to the number of configured cpus - 1.
> (And fix the numa_all_cpus_ptr map of all cpus as well)
> 
> 
> Then, in testing, I found that read_mask() was buggy.  (the regression
> tests don't uncover that).  So that needed some tweaks as well.
> 
> Can you give that a try?
> 
> Signed-off-by: Cliff Wickman <cpw@xxxxxxx>
> 
I've got a pretty full weekend planned, but I'll test this first thing in the
AM on monday.  Thanks!

Neil

> ---
>  libnuma.c |   30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> Index: numactl-dev/libnuma.c
> ===================================================================
> --- numactl-dev.orig/libnuma.c
> +++ numactl-dev/libnuma.c
> @@ -394,6 +394,7 @@ int
>  read_mask(char *s, struct bitmask *bmp)
>  {
>  	char *end = s;
> +	char *prevend;
>  	unsigned int *start = (unsigned int *)bmp->maskp;
>  	unsigned int *p = start;
>  	unsigned int *q;
> @@ -402,9 +403,13 @@ read_mask(char *s, struct bitmask *bmp)
>  
>  	i = strtoul(s, &end, 16);
>  
> +	prevend = end;
>  	/* Skip leading zeros */
> -	while (!i && *end++ == ',')
> +	while (!i && *end++ == ',') {
> +		prevend = end;
>  		i = strtoul(end, &end, 16);
> +	}
> +	end = prevend;
>  
>  	if (!i)
>  		/* End of string. No mask */
> @@ -436,7 +441,7 @@ read_mask(char *s, struct bitmask *bmp)
>  	/*
>  	 * Return the last bit set
>  	 */
> -	return sizeof(unsigned int) * n + i;
> +	return ((sizeof(unsigned int)*8) * n) + i;
>  }
>  
>  /*
> @@ -447,7 +452,9 @@ void
>  set_thread_constraints(void)
>  {
>  	int max_cpus = numa_num_possible_cpus();
> +	int hicpu = sysconf(_SC_NPROCESSORS_CONF)-1;
>  	int buflen;
> +	int i;
>  	char *buffer;
>  	FILE *f;
>  	/*
> @@ -480,6 +487,25 @@ set_thread_constraints(void)
>  	fclose(f);
>  	free (buffer);
>  
> +	/*
> +	 * Cpus_allowed in the kernel can be defined to all f's
> +	 * i.e. it may be a superset of the actual available processors.
> +	 * As such let's reduce maxproccpu to the number of actual
> +	 * available cpus - 1.
> +	 */
> +	if (maxproccpu > hicpu) {
> +		maxproccpu = hicpu;
> +		for (i=hicpu+1; i<numa_all_cpus_ptr->size; i++) {
> +			numa_bitmask_clearbit(numa_all_cpus_ptr, i);
> +		}
> +	}
> +
> +	/*
> +	 * Sanity checks
> +	 */
> +	if (maxproccpu == 0)
> +		numa_warn(W_cpumap, "Available cpus are empty set");
> +
>  	if (maxprocnode < 0) {
>  		numa_warn(W_cpumap, "Cannot parse %s", mask_size_file);
>  		return;

-- 
/****************************************************
 * Neil Horman <nhorman@xxxxxxxxxxxxx>
 * Software Engineer, Red Hat
 ****************************************************/
--
To unsubscribe from this list: send the line "unsubscribe linux-numa" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]     [Devices]

  Powered by Linux