All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@digeo.com>
To: dipankar@gamebox.net
Cc: linux-kernel@vger.kernel.org,
	Rusty Russell <rusty@rustcorp.com.au>,
	Ravikiran G Thirumalai <kiran@in.ibm.com>
Subject: Re: [patch] kmalloc_percpu  -- 2 of 2
Date: Thu, 05 Dec 2002 14:15:39 -0800	[thread overview]
Message-ID: <3DEFD00B.555DE931@digeo.com> (raw)
In-Reply-To: 20021206025307.A20657@dikhow

Dipankar Sarma wrote:
> 
> On Thu, Dec 05, 2002 at 09:10:16PM +0100, Andrew Morton wrote:
> >
> > I'd suggest that you drop the new allocator until a compelling
> > need for it (in real, live 2.5/2.6 code) has been demonstrated.
> 
> Fine with me since atleast one workaround for fragmentation with small
> allocations is known. I can't see anything in 2.5 timeframe
> requiring small per-cpu allocations.
> 
> Would you like me to resubmit a simple kmalloc-only version ?
> 

I think that would be best.

BTW, looking at the snmp application of this work:

+#define ICMP_INC_STATS_USER_FIELD(offt)                                \
+       (*((unsigned long *) ((void *)                                  \
+                            per_cpu_ptr(icmp_statistics[1],            \
+                                        smp_processor_id())) + offt))++;

This guy is racy on preempt.  Just a little bit.  It is effectively:

	ptr = per_cpu_ptr(...);
	(*ptr)++;

On some architectures, `(*ptr)++' is not atomic wrt interrupts.  The
CPU could be preempted midway through the increment.

Surely it's not an issue for SNMP stats, but for some applications
such as struct page_state, such a permanent off-by-a-little-bit would
be a showstopper.

So some big loud comments which describe the worthiness of get_cpu_ptr(),
and the potential inaccuracy of per_cpu_ptr would be useful.

And as this is the first application of the kmalloc_precpu infrastructure,
it may be best to convert it to use get_cpu_ptr/put_cpu_ptr.

  reply	other threads:[~2002-12-05 22:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-12-04 12:12 [patch] kmalloc_percpu -- 1 of 2 Ravikiran G Thirumalai
2002-12-04 12:15 ` [patch] kmalloc_percpu -- 2 " Ravikiran G Thirumalai
2002-12-04 19:34   ` Andrew Morton
2002-12-05  3:42     ` Dipankar Sarma
2002-12-05  4:32       ` Andrew Morton
2002-12-05  4:47         ` William Lee Irwin III
2002-12-05 10:53         ` Dipankar Sarma
2002-12-05 11:23           ` yodaiken
2002-12-05 11:28             ` William Lee Irwin III
2002-12-05 12:41             ` Dipankar Sarma
2002-12-05 15:08               ` yodaiken
2002-12-05 20:02           ` Andrew Morton
2002-12-05 21:23             ` Dipankar Sarma
2002-12-05 22:15               ` Andrew Morton [this message]
2002-12-09  5:30             ` Ravikiran G Thirumalai
2002-12-09  5:57               ` Andrew Morton
2002-12-09 19:28               ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3DEFD00B.555DE931@digeo.com \
    --to=akpm@digeo.com \
    --cc=dipankar@gamebox.net \
    --cc=kiran@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.