linux-ext4 - Re: ditching e4b->alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTima8ZC99JAQvcZQ=WuCyZgWCdVzg3qkLpZ7+_eq@mail.gmail.com>
Date:	Thu, 17 Mar 2011 18:13:49 +0200
From:	Amir Goldstein <amir73il@...il.com>
To:	"Ted Ts'o" <tytso@....edu>
Cc:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: ditching e4b->alloc_semp

On Mon, Mar 7, 2011 at 7:52 AM, Amir Goldstein <amir73il@...il.com> wrote:
> On Mon, Mar 7, 2011 at 6:08 AM, Ted Ts'o <tytso@....edu> wrote:
>> On Sun, Mar 06, 2011 at 10:15:41PM +0200, Amir Goldstein wrote:
>>>
>>> That sounds about right, but why do I need a new bit?
>>> Why can't I use EXT4_GROUP_INFO_NEED_INIT_BIT to tell me the exact
>>> same thing?
>>
>> The current meaning of NEED_INIT_BIT is that it indicates that the
>> group has been initialized once since the file system has been
>> mounted.  It is used by ext4_mb_good_group() to know whether it can
>> rely on ext4_group_info->bb_free, ext4_group_info->bb_fragments,
>> ext4_group_info->bb_largest_free_order, et. al, without needing to
>> reload the buddy bitmap.
>>
>> We added this so that even if memory pressure has forced the buddy
>> bitmap and block allocation bitmaps out of memory, we have enough
>> information in the ext4_group_info summary array that we can quickly
>> decide whether or not a group is a likely good candidate to be
>> examined more closely to have the necessary free blocks.  Without this
>> (relatively recent) change, the mballoc code might potentially need to
>> read in tens if not hundreds of block allocation bitmaps only to find
>> that it didn't have enough contiguous blocks, and then the memory
>> pressure would push the block bitmap out of memory again.... and file
>> system performance would go into the toilet.
>>
>
> Right... we need it.
> I also wanted to examine if clearing the NEED_INIT_BIT on add_group_blocks
> is really necessary.
> Couldn't the buddy bitmap of partial group be initialized with all
> blocks at the end
> "used", similar to the block bitmap itself?
> Then add_group_blocks() could just "free" the extra added blocks.
>

I have started to look into removing down_write(&grp->alloc_sem) from
ext4_add_group_blocks(), because until I remove it, I won't be able to get rid
of down_read(&e4b->alloc_semp) and I don't see how I can use the page_lock()
and grp->bb_state bits semantics alone.

As long as the NEED_INIT_BIT is a clear-only flag the semantics work,
but when ext4_add_group_blocks() sets the bit, we may have a task holding
a reference to buddy page and another trying to initialize it (under page_lock).
I don't suppose you meant holding page_lock() for the entire allocation.

So unless you have a better idea, I will try to implement the "add group blocks
by freeing them" paradigm.

Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html