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

[Ivan Shapovalov <intelfx100@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2805 bytes --]

On 2015-10-15 at 19:20 +0200, Edward Shishkin wrote:
> 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.

I've done a bit more testing, and the encountered panic at unmount does
not depend on my proposed fix. Hence I'll resend the initial patchset.

-- 
Ivan Shapovalov / intelfx /

> 
> 
> 
> 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) {
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 213 bytes --]