Re: [PATCH 11/18] fs: Introduce per-bucket inode hash locks

Nick Piggin
2010-10-26T00:06:29Z

On Sun, Oct 24, 2010 at 10:41 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 25 Oct 2010, Nick Piggin wrote:
>> You do not want to add a bloated mutex to each inode hash bucket and
>> think you can just dust off your hands and walk away.  You would
>> probably make a smaller auxiliary hash of locks, sanely sized, and
>> protect it with that.
>>
>> So it would be wrong to just bloat hlist_bl by a factor of several times
>> (how big is a mutex in -rt?) without doing anything else.
>
> Let me worry about it.

No, because you simply should almost never turn the hlist locking into anything
big and bloated, whether it is for -rt or anything else. It is most
likely going to be
used as a per-bucket hash lock (or bit of metadata), so anything
larger than a bit
(which is essentially free) is way overkill.

You would instead have an auxiliary small hash of locks, not tens of megs of
mutexes.

>> Although a sane locking macro and structure like I had, would perfectly
>> allow you to switch locks in a single place just the same.
>
> And a locking macro/structure is better in self documenting than a
> helper function which was proposed by Christoph?

Yes, because you still have the problem that you need to go through and fix up
all call sites.

With my abstraction, there is a small inode function for locking an inode hash
bucket. You have to change 2 places (lock and unlock) to look up an aux hash
of locks and you're done.