Re: [PATCH v10 03/35] dcache: convert dentry_stat.nr_unused to per-cpu counters

Glauber Costa
2013-06-06T12:40:42Z

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

On 06/06/2013 06:48 AM, Andrew Morton wrote:
> On Thu, 6 Jun 2013 11:45:09 +1000 Dave Chinner <david@fromorbit.com> wrote:
>
>> Andrew, if you want to push the changes back to generic per-cpu
>> counters through to Linus, then I'll write the patches for you. But
>> - and this is a big but - I'll only do this if you are going to deal
>> with the "performance trumps all other concerns" fanatics over
>> whether it should be merged or not. I have better things to do
>> with my time have a flamewar over trivial details like this.
>
> Please view my comments as a critique of the changelog, not of the code.
>
> There are presumably good (but undisclosed) reasons for going this way,
> but this question is so bleeding obvious that the decision should have
> been addressed up-front and in good detail.
>
> And, preferably, with benchmark numbers. Because it might have been
> the wrong decision - stranger things have happened.
>

I have folded the attached patch here. Let me know if it still needs
more love.

[-- Attachment #2: 3.patch --]
[-- Type: text/x-patch, Size: 1013 bytes --]

diff --git a/fs/dcache.c b/fs/dcache.c
index 9f2aa96..0466dbd 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -121,7 +121,19 @@ static DEFINE_PER_CPU(long, nr_dentry);
static DEFINE_PER_CPU(long, nr_dentry_unused);

#if defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS)
-/* scan possible cpus instead of online and avoid worrying about CPU hotplug. */
+
+/*
+ * Here we resort to our own counters instead of using generic per-cpu counters
+ * for consistency with what the vfs inode code does. We are expected to harvest
+ * better code and performance by having our own specialized counters.
+ *
+ * Please note that the loop is done over all possible CPUs, not over all online
+ * CPUs. The reason for this is that we don't want to play games with CPUs going
+ * on and off. If one of them goes off, we will just keep their counters.
+ *
+ * glommer: See cffbc8a for details, and if you ever intend to change this,
+ * please update all vfs counters to match.
+ */
static long get_nr_dentry(void)
{
int i;