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

Andrew Morton
2013-06-05T23:07:31Z

On Mon, 3 Jun 2013 23:29:32 +0400 Glauber Costa <glommer@openvz.org> wrote:

> From: Dave Chinner <dchinner@redhat.com>
>
> Before we split up the dcache_lru_lock, the unused dentry counter
> needs to be made independent of the global dcache_lru_lock. Convert
> it to per-cpu counters to do this.
>
> ...
>
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -118,8 +118,10 @@ struct dentry_stat_t dentry_stat = {
> };
>
> 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. */

That's a poor comment. It explains what the code does (which is dead
obvious) but fails to explain *why* the code does it.

> @@ -129,10 +131,20 @@ static long get_nr_dentry(void)
> return sum < 0 ? 0 : sum;
> }
>
> +static long get_nr_dentry_unused(void)
> +{
> + int i;
> + long sum = 0;
> + for_each_possible_cpu(i)
> + sum += per_cpu(nr_dentry_unused, i);
> + return sum < 0 ? 0 : sum;
> +}

And I'm sure we've asked and answered ad nauseum why this code needed
to open-code the counters instead of using the provided library code,
yet the answer to that *still* isn't in the code comments or even in
the changelog. It should be.

Given that the existing proc_nr_dentry() will suck mud rocks on
large-cpu-count machines (due to get_nr_dentry()), I guess we can
assume that nobody will be especially hurt by making proc_nr_dentry()
suck even harder...

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: email@kvack.org