Le lundi 22 septembre 2008 à 14:17 +0530, Aneesh Kumar K.V a écrit : > On Mon, Sep 22, 2008 at 10:09:57AM +0200, Frédéric Bohé wrote: > > Le samedi 20 septembre 2008 à 20:44 -0400, Theodore Tso a écrit : > > > On Thu, Sep 18, 2008 at 03:45:14PM +0200, Frédéric Bohé wrote: > > > > The issue here is that you can't use all inode of the second group of > > > > the fs. > > > > > > > > This happens because resize2fs make a call to ext2fs_read_bitmaps. This > > > > function reads all bitmaps while paying attention not to read the > > > > uninited bitmap. This works well as long as the fs block size is equal > > > > to the page size. But in the above test case, the fs use 1k blocks and > > > > we have an issue. > > > > > > > > That's because the "read" function issued by ext2fs_read_bitmaps is a > > > > call to kernel's block_read_full_page function. So when a single bitmap > > > > block is asked for, 4 blocks (for 1k blocks fs on x86) are actually read > > > > (including the uninited ones) and their respective buffer set to > > > > uptodate. > > > > > > > > As we rely on the buffer's uptodate flags to initialize or not this > > > > buffer, it may happen that certain bitmap blocks are not initialized at > > > > all. So their buffer contains the random garbage that was present on the > > > > disk prior to the mkfs ( In the above test case, the inode bitmap of the > > > > second group is full a random bits so I can't use all of its inodes ). > > > > > > Actually that's the problem. We shouldn't be relying on the buffer's > > > uptodate flags as a hint to tell mballoc to reload the buddy bitmaps. > > > Unfortunately I didn't notice this problem by not carefully auditing > > > commit 5f21b0e6 before it went in, but it's seriously buggy by trying > > > to overload the use of the buffer's uptodate flag for anything other > > > than error handling. > > > > > > > Maybe I missed something, but I thought the bug I am talking about here, > > is neither related to buddy nor directly to mballoc. Sorry, I was not > > clear enough. In fact, it happens even without using mballoc. It is > > related to uninit feature with filesystems using blocks which are > > smaller than page size. If any userland process call ext2fs_read_bitmaps > > function (or try to read a bitmap block directly), you may end up with > > those buffers full of garbage. It concerns either block bitmap buffers > > or inode bitmap buffers. > > > > > > > > > > I am a bit lost on how to fix this. Aneesh was right, I think it's an > > > > ext2fs_read_bitmaps bug, not a kernel bug. I guess we need a userland > > > > function to read a single block whatever the block size and page size > > > > are. I've made a try using O_DIRECT flag but I was unsuccessful. Any > > > > ideas/suggestions ? > > > > > > No!!!! Think about it. It's always fair for userspace to read from > > > the block device. If this causes the kernel to blow up, then it's a > > > kernel bug, not a userspace bug. And it is a *perfect* demonstration > > > why overloading the uptodate flag by using it for *anything* other > > > than error signalling from the buffer I/O layer is wrong and horribly > > > fragile. > > > > You are probably right, so maybe the patch I sent at the beginning of > > this thread makes sense ? > > > > What you can do is make ext4_group_info generic for both mballoc and > oldalloc. We can then add bg_flag to the in memory ext4_group_info > that would indicate whether the group is initialized or not. Here > initialized for an UNINIT_GROUP indicate we have done > ext4_init_block_bitmap on the buffer_head. Then > instead of depending on the buffer_head uptodate flag we can check > for the ext4_group_info bg_flags and decided whether the block/inode > bitmap need to be initialized. > That makes sense ! I agree with you, we need an additional in-memory flag to know whether buffers are initialized or not. Anyway, making ext4_group_info generic will lead to unneeded memory consumption for oldalloc. Maybe a simple independent bits array could do the trick. Is there any advantage to re-use ext4_group_info ? -- 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
