On Mon, Mar 7, 2011 at 7:52 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Mon, Mar 7, 2011 at 6:08 AM, Ted Ts'o <tytso@xxxxxxx> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
