On Wed, Jul 31, 2013 at 05:11:14PM +0200, Jan Kara wrote:
> On Wed 31-07-13 14:15:45, Dave Chinner wrote:
> > /*
> > + * mark an inode as under writeback on the given bdi
> > + */
> > +void bdi_mark_inode_writeback(struct backing_dev_info *bdi, struct inode *inode)
> > +{
> > + WARN_ON_ONCE(bdi != inode_to_bdi(inode));
> > + if (list_empty(&inode->i_wb_list)) {
> > + spin_lock(&bdi->wb.list_lock);
> > + if (list_empty(&inode->i_wb_list))
> > + list_add_tail(&inode->i_wb_list, &bdi->wb.b_writeback);
> > + spin_unlock(&bdi->wb.list_lock);
> > + }
> > +}
> > +
> > +/*
> > + * clear an inode as under writeback on the given bdi
> > + */
> > +static void bdi_clear_inode_writeback(struct backing_dev_info *bdi,
> > + struct inode *inode)
> > +{
> > + WARN_ON_ONCE(bdi != inode_to_bdi(inode));
> > + if (!list_empty(&inode->i_wb_list)) {
> > + spin_lock(&bdi->wb.list_lock);
> > + list_del_init(&inode->i_wb_list);
> > + spin_unlock(&bdi->wb.list_lock);
> > + }
> > +}
> Umm, are these list_empty() checks without lock really safe?
I think they are.....
> Looking into
> the code in more detail, it seems that mapping->tree_lock saves us from
> races between insert & removal but it definitely deserves a comment (or maybe
> even an assertion) that the function requires it.
> bdi_clear_inode_writeback() is safe only because it is called only when the
> inode is practically dead. Again, I think it deserves a comment...
Ok.
> > @@ -1264,9 +1330,21 @@ static void wait_sb_inodes(struct super_block *sb)
> >
> > cond_resched();
> >
> > - spin_lock(&sb->s_inode_list_lock);
> > + /*
> > + * the inode has been written back now, so check whether we
> > + * still have pages under IO and move it back to the primary
> > + * list if necessary
> > + */
> > + spin_lock_irq(&mapping->tree_lock);
> > + spin_lock(&bdi->wb.list_lock);
> > + if (mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK)) {
> > + WARN_ON(list_empty(&inode->i_wb_list));
> > + list_move(&inode->i_wb_list, &bdi->wb.b_writeback);
> > + } else
> > + list_del_init(&inode->i_wb_list);
> > + spin_unlock_irq(&mapping->tree_lock);
> Whitespace is damaged in the above hunk...
Bizarre. I'll fix it.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
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
