Re: [patch 0/5] [PATCH,RFC] vfs: per-superblock unused dentrieslist (2nd version)

From: Andrew Morton
Date: Thu Jun 01 2006 - 21:01:57 EST


On Thu, 01 Jun 2006 11:51:25 +0200
jblunck@xxxxxxx wrote:

> This is an attempt to have per-superblock unused dentry lists.

Fairly significant clashes with
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.17-rc5/2.6.17-rc5-mm2/broken-out/fix-dcache-race-during-umount.patch

I guess Neil's patch will go into the 2.6.18 tree, so you'd be best off
working against that.


Also, you're making what appears to be a quite deep design change to a
pretty important part of the memory reclaim code and all the info we have
is this:


+ /*
+ * Try to be fair to the unused lists:
+ * sb_count/sb_unused ~ count/global_unused
+ *
+ * Additionally, if the age_limit of the
+ * superblock is expired shrink at least one
+ * dentry from the superblock
+ */
+ tmp = sb->s_dentry_stat.nr_unused /
+ ((unused / count) + 1);
+ if (!tmp && time_after(jiffies,
+ sb->s_dentry_unused_age))
+ tmp = 1;


Please, we'll need much much more description of what this is trying to
achieve, why it exists, analysis, testing results, etc, etc. Coz my
immediate reaction is "wtf is that, and what will that do to my computer?".

In particular, `jiffies' has near-to-zero correlation with the rate of
creation and reclaim of these objects, so it looks highly inappropriate
that it's in there. If anything can be used to measure "time" in this code
it is the number of scanned entries, not jiffies.

But I cannot say more, because I do not know what that code is doing, nor
what problem it is trying to solve. The patch changelog would be an
appropriate place for that info ;)


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/