Re: [PATCH 3/3] reiser4: in our own sync writes, mark pages dirty before marking them writeback.

[Edward Shishkin <edward.shishkin@gmail.com>]

Hello,

So the tail conversion is the culprit?
It's quite realistic...
I am currently on vacations,
I'll take a look at this at the end of October
more carefully.

Thanks!
Edward.



On 10/14/2015 09:06 PM, Ivan Shapovalov wrote:
> On 2015-10-14 at 13:55 +0300, Ivan Shapovalov wrote:
>> On 2015-10-14 at 13:05 +0300, Ivan Shapovalov wrote:
>>> On 2015-10-12 at 11:07 +0200, Edward Shishkin wrote:
>>>> [...]
>>> So, I've added three non-fatal checks right before
>>> set_page_writeback():
>>> - PageDirty(pg)
>>> - JF_ISSET(cur, JNODE_DIRTY)
>>> - pg->mapping->host->i_wb != NULL
>>>
>>> In almost all times when either of the checks is triggered,
>>> the first two checks are triggered at once (i. e. neither page nor
>>> jnode is dirty, but i_wb is not NULL). This happens with both
>>> formatted
>>> nodes (jnode_is_znode), unformatted nodes (jnode_is_unformatted)
>>> and
>>> other nodes.
>>>
>>> And, finally, there are very few warnings where jnode _is_ dirty,
>>> but
>>> the page _isn't_. On the first such warning i_wb is also NULL. And,
>>> just as you've suspected, this happens with an unformatted node.
>>>
>>> I'll try to add a backtrace buffer into struct jnode and generate a
>>> backtrace there on each jnode dirtying attempt... and then print it
>>> for
>>> the problematic jnode.
>>>
>> So, here it is. On all occurrences the backtrace is the same.
>>
>> [...]
>
> Hmm. This obvious fix "works",  but causes an assertion on rootfs unmount:
>
> "list_empty(&get_super_private(super)->all_jnodes)" in init_super.c:55 (no maintainer-id).
>
> Why?
>
> ---
>   fs/reiser4/plugin/file/tail_conversion.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/fs/reiser4/plugin/file/tail_conversion.c b/fs/reiser4/plugin/file/tail_conversion.c
> index 6ffa900..3b4e77d 100644
> --- a/fs/reiser4/plugin/file/tail_conversion.c
> +++ b/fs/reiser4/plugin/file/tail_conversion.c
> @@ -175,6 +175,7 @@ static int replace(struct inode *inode, struct page **pages, unsigned nr_pages,
>   								i_mapping));
>   		if (result)
>   			break;
> +		set_page_dirty_notag(pages[i]);
>   		unlock_page(pages[i]);
>   		result = find_or_create_extent(pages[i]);
>   		if (result) {