Re: [PATCH 6/8] vfs: Make sys_sync writeout also block device inodes

[Jan Kara <jack@suse.cz>]

  Hi Curt,

On Wed 20-06-12 07:23:56, Curt Wohlgemuth wrote:
> Was there ever a resolution for this patch and the series?  I looked
> for but found no ack/nack for V4 of your "Cleanup and improve sync"
> patch series.
> 
> We've been carrying a crappy hack/patch to sync all block device
> inodes in order to allow lilo to not break, and it'd be nice to have a
> real fix for it upstream.
  The patch set works, just I'm missing some final review and someone (Al?)
has to pick it up and merge it. Anyway I plan to repost the series soon to
hopefully get things moving again...

								Honza

> On Thu, Jan 5, 2012 at 3:46 PM, Jan Kara <jack@suse.cz> wrote:
> > In case block device does not have filesystem mounted on it, sys_sync will just
> > ignore it and doesn't writeout its dirty pages. This is because writeback code
> > avoids writing inodes from superblock without backing device and
> > blockdev_superblock is such a superblock.  Since it's unexpected that sync
> > doesn't writeout dirty data for block devices be nice to users and change the
> > behavior to do so. So now we iterate over all block devices on blockdev_super
> > instead of iterating over all superblocks when syncing block devices.
> >
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/sync.c |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  1 files changed, 51 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/sync.c b/fs/sync.c
> > index e62a57b..ccaaa1b 100644
> > --- a/fs/sync.c
> > +++ b/fs/sync.c
> > @@ -86,10 +86,54 @@ static void sync_fs_one_sb(struct super_block *sb, void *arg)
> >                sb->s_op->sync_fs(sb, *(int *)arg);
> >  }
> >
> > -static void sync_blkdev_one_sb(struct super_block *sb, void *arg)
> > +/*
> > + * We go through all existing block devices so that even devices without
> > + * filesystem mounted are synced.
> > + */
> > +static void sync_all_bdevs(int wait)
> >  {
> > -       if (!(sb->s_flags & MS_RDONLY))
> > -               __sync_blockdev(sb->s_bdev, *(int *)arg);
> > +       struct inode *inode, *old_inode = NULL;
> > +
> > +       spin_lock(&inode_sb_list_lock);
> > +       list_for_each_entry(inode, &blockdev_superblock->s_inodes, i_sb_list) {
> > +               struct address_space *mapping = inode->i_mapping;
> > +
> > +               spin_lock(&inode->i_lock);
> > +               if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW) ||
> > +                   mapping->nrpages == 0) {
> > +                       spin_unlock(&inode->i_lock);
> > +                       continue;
> > +               }
> > +               __iget(inode);
> > +               spin_unlock(&inode->i_lock);
> > +               spin_unlock(&inode_sb_list_lock);
> > +               /*
> > +                * We hold a reference to 'inode' so it couldn't have been
> > +                * removed from s_inodes list while we dropped the
> > +                * inode_sb_list_lock.  We cannot iput the inode now as we can
> > +                * be holding the last reference and we cannot iput it under
> > +                * inode_sb_list_lock. So we keep the reference and iput it
> > +                * later.
> > +                */
> > +               iput(old_inode);
> > +               old_inode = inode;
> > +
> > +               __sync_blockdev(I_BDEV(inode), wait);
> > +
> > +               spin_lock(&inode_sb_list_lock);
> > +       }
> > +       spin_unlock(&inode_sb_list_lock);
> > +       iput(old_inode);
> > +}
> > +
> > +static void flush_one_bdev(struct block_device *bdev, void *arg)
> > +{
> > +       __sync_blockdev(bdev, 0);
> > +}
> > +
> > +static void sync_one_bdev(struct block_device *bdev, void *arg)
> > +{
> > +       sync_blockdev(bdev);
> >  }
> >
> >  /*
> > @@ -103,10 +147,10 @@ SYSCALL_DEFINE0(sync)
> >        wakeup_flusher_threads(0, WB_REASON_SYNC);
> >        iterate_supers(writeback_inodes_one_sb, NULL);
> >        iterate_supers(sync_fs_one_sb, &nowait);
> > -       iterate_supers(sync_blkdev_one_sb, &nowait);
> > +       iterate_bdevs(flush_one_bdev, NULL);
> >        iterate_supers(sync_inodes_one_sb, NULL);
> >        iterate_supers(sync_fs_one_sb, &wait);
> > -       iterate_supers(sync_blkdev_one_sb, &wait);
> > +       iterate_bdevs(sync_one_bdev, NULL);
> >        if (unlikely(laptop_mode))
> >                laptop_sync_completion();
> >        return 0;
> > @@ -122,10 +166,10 @@ static void do_sync_work(struct work_struct *work)
> >         */
> >        iterate_supers(sync_inodes_one_sb, &nowait);
> >        iterate_supers(sync_fs_one_sb, &nowait);
> > -       iterate_supers(sync_blkdev_one_sb, &nowait);
> > +       sync_all_bdevs(nowait);
> >        iterate_supers(sync_inodes_one_sb, &nowait);
> >        iterate_supers(sync_fs_one_sb, &nowait);
> > -       iterate_supers(sync_blkdev_one_sb, &nowait);
> > +       sync_all_bdevs(nowait);
> >        printk("Emergency Sync complete\n");
> >        kfree(work);
> >  }
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html