Re: [RFC][PATCH] ext4: Convert uninitialized extent to initialized extent in case of file system full

[Mingming Cao <cmm@us.ibm.com>]

Aneesh Kumar K.V wrote:
> On Fri, Feb 22, 2008 at 08:01:28PM +0530, Aneesh Kumar K.V wrote:
>>>> +
>>>> +		/* Now write all the buffer_heads in the page */
>>>> +		do {
>>>> +			set_buffer_uptodate(bh);
>>>> +			if (ext4_should_journal_data(inode)) {
>>>> +				err = ext4_journal_get_write_access(handle, bh);
>>>> +				/* do we have that many credits ??*/
>>>> +				if (err)
>>>> +					goto err_out;
>>>> +			}
>>>> +			zero_user(page, offset, blocksize);
>>> Ah oh, you are trying to zero out the pages in the page cache, that's
>>> seems wrong to me. By the time get_block() is called from writepages(),
>>> the pages should have meaningful content that needs to flush to disk,
>>> zero the pages out will lost the data.
>>>
>> It is writebegin.  In case of writebegin the pages doesn't have the content. By the
>> time we reach write begin the page is supposed to have buffer heads that
>> are alreayd mapped. So we won't end up calling get_blk. Even in case of
>> mmap with page_mkwrite change we would have called writebegin equivalent
>> before the writepage.
> 
> I guess the above para is confusing.
> 
> The callback is actually writebegin.In case of writebegin the page
> doesn't have the content. With respect to writepage by the time we call the
> callback the buffer_heads related to the page would already be mapped.
> So we won't end up calling get_blk.
> 
> 

Ah, right, the callback at this moment is from write_begin(),as 
get_block() with create==1 is called then (with the recently fix:-)).

But I am thinking from delayed allocation view, since I am looking at it 
recently.:-) get_block() with create ==1 will be defered at writepages 
time, then I am afraid this will broken.

I could be wrong but the code seems only working for buffered IO. What 
about DIO writes to the uninit extents? Since there is no mapping in the 
pagecache, then DIO starts calling get_block() with create ==1. What 
happened in this case? I had a feeling this also broken, isn't it?

Regards,

Mingmng