lkml.org 
[lkml]   [2013]   [Aug]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 06/11] bdi: add a new writeback list for sync
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@fromorbit.com


\
 
 \ /
  Last update: 2013-08-01 08:21    [W:0.226 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site