All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Blanchard <anton@samba.org>
To: Theodore Tso <tytso@MIT.EDU>
Cc: Tejun Heo <tj@kernel.org>, Eric Dumazet <eric.dumazet@gmail.com>,
	adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] percpu_counter: Put a reasonable upper bound on percpu_counter_batch
Date: Wed, 7 Sep 2011 21:08:55 +1000	[thread overview]
Message-ID: <20110907210855.414d417e@kryten> (raw)
In-Reply-To: <EEB2FCE2-3C14-406E-BD0E-FD27D091C492@mit.edu>


Hi Ted,

> Um, this was an ext4 patch and I pointed out it could cause
> problems.  (Specifically, data loss…)

I'm a bit confused. While the comment mentions ext4, the patch is just
putting an upper bound on the size of percpu_counter_batch and it is
useful for percpu_counter_compare() too:

 static void compute_batch_value(void)
 {
        int nr = num_online_cpus();

-       percpu_counter_batch = max(32, nr*2);
+       /*
+        * The cutoff point for the percpu_counter_compare() fast path grows
+        * at num_online_cpus^2 and on a big enough machine it will be
+        * unlikely to hit.
+        * We clamp the batch value to 1024 so the cutoff point only grows
+        * linearly past 512 CPUs.
+        */
+       percpu_counter_batch = clamp(nr*2, 32, 1024);
 }

The batch value should be opaque to the rest of the kernel. If ext4
requires a specific batch value we can use the functions that take
an explicit one (eg __percpu_counter_add).

Anton
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Anton Blanchard <anton@samba.org>
To: Theodore Tso <tytso@MIT.EDU>
Cc: Tejun Heo <tj@kernel.org>, Eric Dumazet <eric.dumazet@gmail.com>,
	adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] percpu_counter: Put a reasonable upper bound on percpu_counter_batch
Date: Wed, 7 Sep 2011 21:08:55 +1000	[thread overview]
Message-ID: <20110907210855.414d417e@kryten> (raw)
In-Reply-To: <EEB2FCE2-3C14-406E-BD0E-FD27D091C492@mit.edu>


Hi Ted,

> Um, this was an ext4 patch and I pointed out it could cause
> problems.  (Specifically, data loss…)

I'm a bit confused. While the comment mentions ext4, the patch is just
putting an upper bound on the size of percpu_counter_batch and it is
useful for percpu_counter_compare() too:

 static void compute_batch_value(void)
 {
        int nr = num_online_cpus();

-       percpu_counter_batch = max(32, nr*2);
+       /*
+        * The cutoff point for the percpu_counter_compare() fast path grows
+        * at num_online_cpus^2 and on a big enough machine it will be
+        * unlikely to hit.
+        * We clamp the batch value to 1024 so the cutoff point only grows
+        * linearly past 512 CPUs.
+        */
+       percpu_counter_batch = clamp(nr*2, 32, 1024);
 }

The batch value should be opaque to the rest of the kernel. If ext4
requires a specific batch value we can use the functions that take
an explicit one (eg __percpu_counter_add).

Anton

  parent reply	other threads:[~2011-09-07 17:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-25 21:26 [PATCH 1/2] ext4: EXT4_FREEBLOCKS_WATERMARK is overly pessimistic Anton Blanchard
2011-08-25 21:29 ` [PATCH 2/2] percpu_counter: Put a reasonable upper bound on percpu_counter_batch Anton Blanchard
2011-08-26  8:39   ` Eric Dumazet
2011-08-26  8:39     ` Eric Dumazet
2011-08-29 11:46     ` [PATCH] " Anton Blanchard
2011-09-06  3:48       ` Tejun Heo
2011-09-06 13:30         ` Theodore Tso
2011-09-06 13:30           ` Theodore Tso
2011-09-06 16:44           ` Tejun Heo
2011-09-06 16:44             ` Tejun Heo
2011-09-07 11:08           ` Anton Blanchard [this message]
2011-09-07 11:08             ` Anton Blanchard
2011-08-26  9:00   ` [PATCH 2/2] " Tejun Heo
2011-08-26 11:48   ` Theodore Tso
2011-08-29 11:54     ` Anton Blanchard
2011-08-29 11:54       ` Anton Blanchard
2011-08-29 13:27     ` Ted Ts'o

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=20110907210855.414d417e@kryten \
    --to=anton@samba.org \
    --cc=adilger.kernel@dilger.ca \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=tytso@MIT.EDU \
    /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.