lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150622174648.GA2965@redhat.com>
Date:	Mon, 22 Jun 2015 13:46:48 -0400
From:	Vivek Goyal <vgoyal@...hat.com>
To:	Christoph Hellwig <hch@....de>
Cc:	Mike Snitzer <snitzer@...hat.com>, dm-devel@...hat.com,
	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org,
	Tejun Heo <tj@...nel.org>
Subject: Re: WARNING: at fs/block_dev.c:5 when removing LV on removed device

On Fri, Jun 19, 2015 at 08:47:21AM +0200, Christoph Hellwig wrote:
> On Thu, Jun 18, 2015 at 03:28:21PM -0400, Mike Snitzer wrote:
> > > WARN_ON_ONCE(write_inode_now(inode, true))
> > > 
> > > If we failed to write back inode, then warning about it sounds right?
> > 
> > A warning is fine.. not a WARN_ON().  Pretty alarming backtrace spew but
> > maybe I'm missing something and DM's blkdev refcount mgmt couldn't
> > trigger this WARN_ON()?  I fail to see how to avoid it given the device
> > isn't thre so write_inode_now() fails.
> > 
> > > What's wrong with that? Should it be just a kernel log of level KERN_WARN
> > > instead?
> > 
> > Ideally, but I honestly don't have all the details paged in my head to
> > say definitively.  First need to answer how vitrio-blk isn't hitting
> > this (and DM is).  Could it be that __blkdev_put isn't getting called
> > for virtio-blk!?
> 
> Just a warnings if fine.  In fact we can probably remove that as well
> as it will happen after a hot removal all the time.

[CC Tejun]

Does following patch look good?

Thanks
Vivek


Subject: fs/block_dev.c: Warn on inode writeback failure instead of WARN_ON()

If a block device is hot removed and later last reference to devices
is put, we try to writeback the dirty inode. But device is gone and
that writeback fails.

Currently we do a WARN_ON() which does not seem to be the right thing.
Convert it to a ratelimited kernel warning.

Reported-by: Andi Kleen <andi@...stfloor.org>
Signed-off-by: Vivek Goyal <vgoyal@...hat.com>
---
 fs/block_dev.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: rhvgoyal-linux/fs/block_dev.c
===================================================================
--- rhvgoyal-linux.orig/fs/block_dev.c	2015-06-18 15:54:52.339383237 -0400
+++ rhvgoyal-linux/fs/block_dev.c	2015-06-22 12:55:47.642504742 -0400
@@ -48,12 +48,17 @@ inline struct block_device *I_BDEV(struc
 }
 EXPORT_SYMBOL(I_BDEV);
 
-static void bdev_write_inode(struct inode *inode)
+static void bdev_write_inode(struct block_device *bdev)
 {
+	struct inode *inode = bdev->bd_inode;
+
 	spin_lock(&inode->i_lock);
 	while (inode->i_state & I_DIRTY) {
 		spin_unlock(&inode->i_lock);
-		WARN_ON_ONCE(write_inode_now(inode, true));
+		if (write_inode_now(inode, true)) {
+			char name[BDEVNAME_SIZE] = "";
+			pr_warn_ratelimited("VFS: Dirty inode writeback failed for block device %s.\n", bdevname(bdev, name));
+		}
 		spin_lock(&inode->i_lock);
 	}
 	spin_unlock(&inode->i_lock);
@@ -1489,7 +1494,7 @@ static void __blkdev_put(struct block_de
 		 * ->release can cause the queue to disappear, so flush all
 		 * dirty data before.
 		 */
-		bdev_write_inode(bdev->bd_inode);
+		bdev_write_inode(bdev);
 	}
 	if (bdev->bd_contains == bdev) {
 		if (disk->fops->release)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ