Re: [PATCH] 2.6.11-mm1 "nobh" support for ext3 writeback mode

[Andrew Morton]

Badari Pulavarty <pbadari@xxxxxxxxxx> wrote:
>
> @@ -1646,13 +1659,34 @@ static int ext3_block_truncate_page(hand
>  	unsigned blocksize, iblock, length, pos;
>  	struct inode *inode = mapping->host;
>  	struct buffer_head *bh;
> -	int err;
> +	int err = 0;
>  	void *kaddr;
>  
>  	blocksize = inode->i_sb->s_blocksize;
>  	length = blocksize - (offset & (blocksize - 1));
>  	iblock = index << (PAGE_CACHE_SHIFT - inode->i_sb->s_blocksize_bits);
>  
> +	if (test_opt(inode->i_sb, NOBH) && !page_has_buffers(page)) {
> +		if (!PageUptodate(page)) {
> +			err = mpage_readpage(page, ext3_get_block);
> +			if ((err == 0) && !PageUptodate(page))
> +				wait_on_page_locked(page);
> +			lock_page(page);
> +		}
> +		if (err == 0 && PageUptodate(page)) {
> +			kaddr = kmap_atomic(page, KM_USER0);
> +			memset(kaddr + offset, 0, length);
> +			flush_dcache_page(page);
> +			kunmap_atomic(kaddr, KM_USER0);
> +			set_page_dirty(page);
> +			goto unlock;
> +		}
> +		/* 

What's all this doing?  (It needs comments please - it's very unobvious).

Dropping and reacquiring the page lock in the middle of the truncate there
is a bit of a worry - need to think about that.

Err, yes, it's buggy - page reclaim can come in, grab the page lock and
whip the page off the mapping.  We end up with an anonymous page and we
then start trying to write it into the file and all sorts of things.


This bit:

			if ((err == 0) && !PageUptodate(page))
				wait_on_page_locked(page);

can simply be removed.  We're about to lock the page anyway.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/