Re: [PATCH 2/2] Handle error in sync_sb_inodes()

[Guillaume Chazarain <guichaz@yahoo.fr>]

Andrew Morton a écrit :
>> @@ -365,7 +366,8 @@ sync_sb_inodes(struct super_block *sb, s
>>  		BUG_ON(inode->i_state & I_FREEING);
>>  		__iget(inode);
>>  		pages_skipped = wbc->pages_skipped;
>> -		__writeback_single_inode(inode, wbc);
>> +		ret = __writeback_single_inode(inode, wbc);
>> +		mapping_set_error(mapping, ret);
>>  		if (wbc->sync_mode == WB_SYNC_HOLD) {
>>  			inode->dirtied_when = jiffies;
>>  			list_move(&inode->i_list, &sb->s_dirty);
>>     
> --- a/fs/buffer.c~a
> +++ a/fs/buffer.c
> @@ -1739,6 +1739,7 @@ recover:
>  		}
>  	} while ((bh = bh->b_this_page) != head);
>  	SetPageError(page);
> +	mapping_set_error(page->mapping, err);
>  	BUG_ON(PageWriteback(page));
>  	set_page_writeback(page);
>  	unlock_page(page);
>   


Unfortunately, with your patch and not mine, the problem is still 
present: msync()
does not return the error. Both pieces of code (yours and mine) are 
called for the
same mapping though, albeit yours more frequently.

My interpretation (based more on imagination than experience) is that
__writeback_single_inode() ends up calling __block_write_full_page() which
sets the page flags (your patch), then calls wait_on_page_writeback_range()
which clears the flags but returns the error as its return value. And this
error code is dropped by sync_sb_inodes() (my patch not applied).

With my patch, wait_on_page_writeback_range() would get the error code
by some other mean, and sync_sb_inodes() would re-put it in the flags for
msync() later.

Sorry, for the speculation, but I would need some hint to debug this ;-)

Thanks.

-- 
Guillaume