On 2015-10-12 at 11:07 +0200, Edward Shishkin wrote: > On 10/10/2015 12:44 PM, Ivan Shapovalov wrote: > [...] > > I didn't check the code yet; I'll probably try with that assertion > > converted into warning and split into two > > (one for formatted and another for unformatted nodes), so that I > > could check what type of nodes is responsible > > for generating the final oops in set_page_writeback(). > > I suppose that oops happens on unformatted nodes, because > all formatted nodes have the same host - a special "fake" inode > with number 1, which gets i_wb at mount time (init_fake_inode() > calls inode_attach_wb()). > > Edward. 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. -- Ivan Shapovalov / intelfx / > > > > > > For unformatted nodes only code review > > > can help. Normally, all modifications of unformatted nodes should > > > look like the following: > > > > > > struct page *page = jnode_page(node); > > > lock_page(page); > > > char *data = kmap(page); > > > /* modifications are going here */ > > > kunmap(page); > > > set_page_dirty_nobuffers(page); /* somebody forgets to do this */ > > > unlock_page(page); > > > > > > Modifications of formatted nodes should look like the following: > > > > > > longterm_lock_znode(node); > > > zload(node); > > > /* modifications are going here */ > > > zrelse(node); > > > znode_make_dirty(node); /* somebody forgets to do this */ > > > longterm_unlock_znode(); > > > > > > Anyway, we can use your patch 3 as a temporal fixup. > > The most persistent things are those conseived as the most > > temporary > > ones... ;)
Attachment:
signature.asc
Description: This is a digitally signed message part
