Re: delayed allocatiou result in Oops

[Dmitry Monakhov <dmonakhov@sw.ru>]

On 11:11 Втр 19 Июн     , Alex Tomas wrote:
> Mingming Cao wrote:
> >>From the comments it says the page->private is set to 1 to letting
> >commit_write know that it needs block reservation, but I don't see the
> >page->private value being checked in ext4_wb_commit_write(). Instead,
> >the PageMappedToDisk(page) flag is being checked.
> >
> >Alex, can you clarify the use of page->private and PagePrivate flag
> >here? Do we still need the page->private for delayed allocation, with
> >PageBooked flag and PageMappedToDisk page flag?
> 
> sorry for confusion, we need PagePrivate so that truncate calls our
> ->invalidatepage(). in turn this call is needed to drop unused reservation.
> 
> block_read_full_page() must not happen to pages being delayed allocated
> in the first place - it's uptodate by definition. I think the problem
> began when ext4_wb_commit_write() exited due to -ENOSPC but left not
> uptodate *and* with PG_private. then subsequent access to page turned to
> block_read_full_page() which relies on PG_private and meaningful private
> field. Dmitry, could you repeat the test with SetPagePrivate(page) moved
> to after that if() with ext4_wb_reserve_space_page(), please?
Yes it works. Bug not triggered now.

But whole approach based on using PagePrivate bit and page->private
ptr for special purposes is realy dengerous, and imho wrong,
because avery fs-related code assume that page->private points to
page_buffers. Especially this approach not work for blksize < pgsize.
The best way to store/pass any block related info is buffer_head flags.
This approach works for blksize < pgsize case  too.
> 
> 
> as i'm here ... status update: I've been reworking delayed allocation
> patches to support blocksize < PAGE_CACHE_SIZE (and address akpm's request
> for more generic implementation). the patch isn't ready for review, but
> hopefully will be in few days.
> 
> 
> thanks, Alex
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html