From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 6/8] vfs: Make sys_sync writeout also block device inodes Date: Wed, 20 Jun 2012 22:03:36 +0200 Message-ID: <20120620200336.GF3435@quack.suse.cz> References: <1325807186-3397-1-git-send-email-jack@suse.cz> <1325807186-3397-7-git-send-email-jack@suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Jan Kara , linux-fsdevel@vger.kernel.org, Christoph Hellwig , Al Viro To: Curt Wohlgemuth Return-path: Received: from cantor2.suse.de ([195.135.220.15]:41287 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753833Ab2FTUDj (ORCPT ); Wed, 20 Jun 2012 16:03:39 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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. >=20 > 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 wrote: > > In case block device does not have filesystem mounted on it, sys_sy= nc will just > > ignore it and doesn't writeout its dirty pages. This is because wri= teback code > > avoids writing inodes from superblock without backing device and > > blockdev_superblock is such a superblock. =A0Since 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 bloc= kdev_super > > instead of iterating over all superblocks when syncing block device= s. > > > > Reviewed-by: Christoph Hellwig > > Signed-off-by: Jan Kara > > --- > > =A0fs/sync.c | =A0 58 +++++++++++++++++++++++++++++++++++++++++++++= ++++++------- > > =A01 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) > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sb->s_op->sync_fs(sb, *(int *)arg); > > =A0} > > > > -static void sync_blkdev_one_sb(struct super_block *sb, void *arg) > > +/* > > + * We go through all existing block devices so that even devices w= ithout > > + * filesystem mounted are synced. > > + */ > > +static void sync_all_bdevs(int wait) > > =A0{ > > - =A0 =A0 =A0 if (!(sb->s_flags & MS_RDONLY)) > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 __sync_blockdev(sb->s_bdev, *(int *)a= rg); > > + =A0 =A0 =A0 struct inode *inode, *old_inode =3D NULL; > > + > > + =A0 =A0 =A0 spin_lock(&inode_sb_list_lock); > > + =A0 =A0 =A0 list_for_each_entry(inode, &blockdev_superblock->s_in= odes, i_sb_list) { > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct address_space *mapping =3D ino= de->i_mapping; > > + > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock(&inode->i_lock); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (inode->i_state & (I_FREEING|I_WIL= L_FREE|I_NEW) || > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mapping->nrpages =3D=3D 0) { > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock(&inode->i= _lock); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue; > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 __iget(inode); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock(&inode->i_lock); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock(&inode_sb_list_lock); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* We hold a reference to 'inode' s= o it couldn't have been > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* removed from s_inodes list while= we dropped the > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* inode_sb_list_lock. =A0We cannot= iput the inode now as we can > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* be holding the last reference an= d we cannot iput it under > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* inode_sb_list_lock. So we keep t= he reference and iput it > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* later. > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/ > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 iput(old_inode); > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 old_inode =3D inode; > > + > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 __sync_blockdev(I_BDEV(inode), wait); > > + > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_lock(&inode_sb_list_lock); > > + =A0 =A0 =A0 } > > + =A0 =A0 =A0 spin_unlock(&inode_sb_list_lock); > > + =A0 =A0 =A0 iput(old_inode); > > +} > > + > > +static void flush_one_bdev(struct block_device *bdev, void *arg) > > +{ > > + =A0 =A0 =A0 __sync_blockdev(bdev, 0); > > +} > > + > > +static void sync_one_bdev(struct block_device *bdev, void *arg) > > +{ > > + =A0 =A0 =A0 sync_blockdev(bdev); > > =A0} > > > > =A0/* > > @@ -103,10 +147,10 @@ SYSCALL_DEFINE0(sync) > > =A0 =A0 =A0 =A0wakeup_flusher_threads(0, WB_REASON_SYNC); > > =A0 =A0 =A0 =A0iterate_supers(writeback_inodes_one_sb, NULL); > > =A0 =A0 =A0 =A0iterate_supers(sync_fs_one_sb, &nowait); > > - =A0 =A0 =A0 iterate_supers(sync_blkdev_one_sb, &nowait); > > + =A0 =A0 =A0 iterate_bdevs(flush_one_bdev, NULL); > > =A0 =A0 =A0 =A0iterate_supers(sync_inodes_one_sb, NULL); > > =A0 =A0 =A0 =A0iterate_supers(sync_fs_one_sb, &wait); > > - =A0 =A0 =A0 iterate_supers(sync_blkdev_one_sb, &wait); > > + =A0 =A0 =A0 iterate_bdevs(sync_one_bdev, NULL); > > =A0 =A0 =A0 =A0if (unlikely(laptop_mode)) > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0laptop_sync_completion(); > > =A0 =A0 =A0 =A0return 0; > > @@ -122,10 +166,10 @@ static void do_sync_work(struct work_struct *= work) > > =A0 =A0 =A0 =A0 */ > > =A0 =A0 =A0 =A0iterate_supers(sync_inodes_one_sb, &nowait); > > =A0 =A0 =A0 =A0iterate_supers(sync_fs_one_sb, &nowait); > > - =A0 =A0 =A0 iterate_supers(sync_blkdev_one_sb, &nowait); > > + =A0 =A0 =A0 sync_all_bdevs(nowait); > > =A0 =A0 =A0 =A0iterate_supers(sync_inodes_one_sb, &nowait); > > =A0 =A0 =A0 =A0iterate_supers(sync_fs_one_sb, &nowait); > > - =A0 =A0 =A0 iterate_supers(sync_blkdev_one_sb, &nowait); > > + =A0 =A0 =A0 sync_all_bdevs(nowait); > > =A0 =A0 =A0 =A0printk("Emergency Sync complete\n"); > > =A0 =A0 =A0 =A0kfree(work); > > =A0} > > -- > > 1.7.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-fsd= evel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm= l --=20 Jan Kara 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