linux-kernel - Re: [PATCH] percpu_counter: FBC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20081208001940.94737420.akpm@linux-foundation.org>
Date:	Mon, 8 Dec 2008 00:19:40 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Eric Dumazet <dada1@...mosbay.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Mike Travis <travis@....com>,
	"David S. Miller" <davem@...emloft.net>,
	linux kernel <linux-kernel@...r.kernel.org>,
	Christoph Lameter <cl@...ux-foundation.org>,
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH] percpu_counter: FBC_BATCH should be a variable

On Mon, 08 Dec 2008 09:05:04 +0100 Eric Dumazet <dada1@...mosbay.com> wrote:

> Andrew Morton a __crit :
> > On Sun, 07 Dec 2008 19:30:10 +0100 Eric Dumazet <dada1@...mosbay.com> wrote:
> > 
> >>> Do
> >>>
> >>> 	$EDITOR  $(grep -l hotcpu_notifier */*.c)
> >>>
> >>> and you'll see lots of code gets it right, and lots of code gets it wrong.
> >> I see nothing interesting, I must be blind.
> >>
> >> lib/percpu_counter.c: In function 'percpu_counter_startup':
> >> lib/percpu_counter.c:158: error: 'percpu_counter_hotcpu_callback' undeclared (first use in this function)
> >> lib/percpu_counter.c:158: error: (Each undeclared identifier is reported only once
> >> lib/percpu_counter.c:158: error: for each function it appears in.)
> >> make[1]: *** [lib/percpu_counter.o] Error 1
> > 
> > Perhaps you still had percpu_counter_hotcpu_callback inside #ifdef.
> > 
> > That a look at kernel/workqueue.c, fs/buffer.c.  No #ifdef CONFIG_HOTPLUG_CPU
> > needed at all.
> 
> We still need some #ifdef or we also must also delete them around "struct list_head list"
> in include/linux/percpu_counter.h and grow struct percpu_counter.
> 
> lib/percpu_counter.c: In function 'percpu_counter_hotcpu_callback':
> lib/percpu_counter.c:137: error: 'struct percpu_counter' has no member named 'list'
> lib/percpu_counter.c:137: warning: type defaults to 'int' in declaration of '__mptr'
> lib/percpu_counter.c:137: warning: initialization from incompatible pointer type
> ...
> 

Yes, this conditionality:

struct percpu_counter {
	spinlock_t lock;
	s64 count;
#ifdef CONFIG_HOTPLUG_CPU
	struct list_head list;	/* All percpu_counters are on a list */
#endif
	s32 *counters;
};

mucked up our nice scheme.  Oh well.

> +static void compute_batch_value(void)
> +{
> +	int nr = num_online_cpus();
> +
> +	percpu_counter_batch = max(32, nr*2);
> +}
> +
>  static int __cpuinit percpu_counter_hotcpu_callback(struct notifier_block *nb,
>  					unsigned long action, void *hcpu)
>  {
> +#ifdef CONFIG_HOTPLUG_CPU
>  	unsigned int cpu;
>  	struct percpu_counter *fbc;
>  
> +	compute_batch_value();
>  	if (action != CPU_DEAD)
>  		return NOTIFY_OK;
>  
> @@ -137,13 +146,14 @@ static int __cpuinit percpu_counter_hotcpu_callback(struct notifier_block *nb,
>  		spin_unlock_irqrestore(&fbc->lock, flags);
>  	}
>  	mutex_unlock(&percpu_counters_lock);
> +#endif
>  	return NOTIFY_OK;
>  }
>  
>  static int __init percpu_counter_startup(void)
>  {
> +	compute_batch_value();
>  	hotcpu_notifier(percpu_counter_hotcpu_callback, 0);
>  	return 0;
>  }
>  module_init(percpu_counter_startup);

compute_batch_value() can be __cpuinit, but I think we're close enough
here ;)

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