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@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html