Hi Yongqiang, On Tue 26-07-11 09:20:28, Yongqiang Yang wrote: > I have been thinking if we can handle fiemap much simpler for a while. > Current code is very ugly due to page cache look up. I have a > thought on simplifying these code. The reason leading us to looking > up page cache is that delayed extents are not in extents tree. I > think we can add an in-memory delayed extents list in inode, and we > can delete entries in the list after we allocate blocks for them. > There is no limit on length of extents in the list, this way can an > entry contain as many blocks as they are contiguous logically. > > What's your opinion? Yes, that should be doable and shouldn't have too big overhead. It's just stupid we'll do all this stuff only for fiemap call which is relatively rare. Honza > On Mon, Jul 25, 2011 at 11:58 PM, Jan Kara <jack@xxxxxxx> wrote: > > Hello, > > > > I just had a look at the code checking delayed allocated buffers in > > ext4_ext_fiemap_cb(). I believe the checks there could use some elimiation > > of common patterns but that's just a minor thing. The main problem is that > > the code can easily crash the kernel when it races with page reclaim. You > > just cannot access most of the page contents (and for buffers it is > > especially true) without locking the page. Getting a reference via > > find_get_pages_tag() guarantees you the structure cannot go away but mm is > > still free to detach the page from the mapping at any moment. So you must > > always lock a page and check that it still belongs to the desired mapping > > before you check 'page_has_buffers()'. > > > > Honza > > -- > > Jan Kara <jack@xxxxxxx> > > SUSE Labs, CR > > > > > > -- > Best Wishes > Yongqiang Yang -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- 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
