Re: [PATCH RESEND] fs: Move bh_cachep to the __read_mostly section
From: Vlad Zolotarov
Date: Wed Jul 04 2012 - 05:08:10 EST
On Tuesday 03 July 2012 14:23:48 Andrew Morton wrote:
> On Mon, 2 Jul 2012 23:54:09 +0530
> Chinmay V S <cvs268@xxxxxxxxx> wrote:
> > Hi Vlad,
> > I suppose we both are looking at opposite sides of the same coin.
> > While i am wary of the potential pitfalls,
> > you have focused on the benefits of using __read_mostly.
> > At this point i would like to send out a shout to everyone concerned to
> > please clarify the status of __read_mostly:
> > 1. Is it being actively pursued? (systems that *will* clearly benefit from
> > it) 2. Any actual results on real world use-cases? (w/ & w/o
> > __read_mostly) 3. Is __read_mostly being accepted in non-architecture
> > specific kernel code? 4. Is anyone working on a potential replacement for
> > it?
Andrew, thanks for joining! :)
> I don't recall ever having seen any serious work justifying or
> condemning __read_mostly.
Back in the thread I've pointed out that __read_mostly variables (are meant
to) have actually the same nature as the constants except the fact that they
are initialized with the some value that may change from one boot (of the
kernel) to another: e.g. kalloc() returned value, values initialized in __init
Have u ever seen any serious work justifying or condemning the usage for
If u did, (could u, pls., point us to it) the same reasoning hold in the
context of __read_mostly.
If not - do u think somebody should perform such a work? Or maybe we all
understand that if there a constant in my code I should define it as const?
And we all agree on const, why do we still arguing about __read_mostly which
are actually the same?
> It's one of those things which seemed a good idea at the time, got added and
> nobody did anything further with it, as far as I know.
Hmmm... ~1300 __read_mostly appearances in the kernel doesn't sound like
nothing to me... ;) People use it! ;)
> I've always been rather wobbly about it, for reasons discussed up-thread.
Could u, pls., comment on the following statements that conclude this thread:
1) There are no "pitfalls" in the __read_mostly usage/infrastructure - there
is a bad code that is being revealed by the usage of __read_mostly.
2) The bad code mentioned above is bad and may regress the performance
regardless the usage of __read_mostly, thus it must be fixed.
3) From (1) and (2) above follows that __read_mostly is a tool that helps to
discover the bad code thus it should be used as much as possible to do so.
4) To make the discovery of bad code easy __read_mostly qualifiers should be
added one-by-one despite the natural desire to replace all variables of a
certain type/nature (like "struct kmem_cache") in one shot.
5) __read_mostly as it's implemented today for x86 arch is good for any SMP
architecture that have L1 cache. More than that, the less is the level of the
associativity in the L1 cache the more this platform's performance is
susceptible to the bad code mentioned in (1) and (2). Therefore according to
(3) these platforms should use __read_mostly both to gain from straight
forward benefits of the __read_mostly mechanism and to locate the bad code
mentioned above and fix it. ARM is one of such platforms and there must have
been REALLY GOOD reason not to use it.
> If some problem has indeed been observed then an alternative to
> __read_mostly is to pad bh_cachep to a cacheline boundary with
> ____cacheline_aligned_in_smp or similar. Or, perhaps better, identify
> the variable which bh_cachep is sharing with, and pad *that* variable
> to a cacheline so it can't cause cache thrashing with something else in
> the future. And once this mystery variable is identified, we can
> perhaps beneficially group it into the same cacheline with some other
> variables which are related to it.
Again, I don't understand it - we are talking about a constant here. Why to
pad it or do anything else except for defining it as a constant? The one who
originally wrote this line (with bh_cachep) either missed that fact or there
wasn't __read_mostly infrastructure at that time. Either way this patch fixes
it. What else should be told? Why do we have to justify declaring the const
object as const?
Thanks in advance for your comments.
> But I'm madly guessing and can't say aything dispositive or even
> useful, because we haven't been told what the problem was. Still.
> 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/
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/