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

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

 



On 06/06/2013 06:48 AM, Andrew Morton wrote:
> On Thu, 6 Jun 2013 11:45:09 +1000 Dave Chinner <david@xxxxxxxxxxxxx> 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.

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;

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]