Re: [PATCH 7/7] writeback: Avoid iput() from flusher thread

[Jan Kara <jack@suse.cz>]

On Mon 30-04-12 11:30:27, Christoph Hellwig wrote:
> > - * Wait for writeback on an inode to complete.
> > + * Wait for writeback on an inode to complete. Called with i_lock held.
> > + * Caller must make sure inode cannot go away when we drop i_lock.
> >   */
> > -static void inode_wait_for_writeback(struct inode *inode)
> > +void inode_wait_for_writeback(struct inode *inode)
> >  {
> >  	DEFINE_WAIT_BIT(wq, &inode->i_state, __I_SYNC);
> >  	wait_queue_head_t *wqh;
> > @@ -340,6 +341,26 @@ static void inode_wait_for_writeback(struct inode *inode)
> >  	}
> >  }
> 
> This needs way better documentation, preferably using kernel doc.
  OK, will improve.

> I also suspect we should rename inode_wait_for_writeback to
> __inode_wait_for_writeback and make inode_wait_for_writeback the public
> wrapper that takes i_lock.
  Good idea. Will do.

> > diff --git a/fs/inode.c b/fs/inode.c
> > index d3ebdbe..3869714 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -510,7 +510,6 @@ void end_writeback(struct inode *inode)
> >  	BUG_ON(!list_empty(&inode->i_data.private_list));
> >  	BUG_ON(!(inode->i_state & I_FREEING));
> >  	BUG_ON(inode->i_state & I_CLEAR);
> > -	inode_sync_wait(inode);
> >  	/* don't need i_lock here, no concurrent mods to i_state */
> >  	inode->i_state = I_FREEING | I_CLEAR;
> >  }
> 
> end_writeback is really misnamed now and needs a better name.
  Yes. I think old good clear_inode() may be fine? The only thing we really
*do* in that function is setting of I_CLEAR. I guess I'll do the rename in
one big jumbo patch. Would that be OK? I don't want to spam with 50
one-line patches...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR