linux-kernel - Re: [PATCH v2] block, writeback: wait for writeback to finish before detaching wb

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: <CAAeU0aMZgR=Ed7xna+4M6PiTuB9UiSaSJ3CEhJOKBQv-A4JYvg@mail.gmail.com>
Date:   Thu, 9 Mar 2017 15:53:11 -0800
From:   Tahsin Erdogan <tahsin@...gle.com>
To:     Alexander Viro <viro@...iv.linux.org.uk>,
        Ilya Dryomov <idryomov@...il.com>, Jens Axboe <axboe@...com>,
        Raghavendra K T <raghavendra.kt@...ux.vnet.ibm.com>,
        Tejun Heo <tj@...nel.org>, Jan Kara <jack@...e.cz>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org, Tahsin Erdogan <tahsin@...gle.com>
Subject: Re: [PATCH v2] block, writeback: wait for writeback to finish before
 detaching wb

Hi Tejun,

>
> White line contaminations.

Fixed in v2.


> Given that there's write_inode_now(@sync == true) call right above,
> I'm not sure how waiting for I_SYNC once more time would make a
> difference.  Can you please explain how to trigger the issue?

bdev_write_inode() calls write_inode_now() only if I_DIRTY is set for
the inode. It is possible that I_DIRTY flags have been cleared by the
writeback thread, but it hasn't run node_sync_complete() yet.

I am able to reproduce the problem by adding articial delays in
__blkdev_put() and writeback_sb_inodes(). Please see the repro patch
below:

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 2eca00ec4370..e3cfbba19a6c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -33,6 +33,7 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/falloc.h>
 #include <linux/uaccess.h>
+#include <linux/delay.h>
 #include "internal.h"

 struct bdev_inode {
@@ -1875,6 +1876,8 @@ static void __blkdev_put(struct block_device
*bdev, fmode_t mode, int for_part)
                sync_blockdev(bdev);
                kill_bdev(bdev);

+               /* Allow time for writeback thread to clear I_DIRTY flags */
+               mdelay(200);
                bdev_write_inode(bdev);
                /*
                 * Detaching bdev inode from its wb in __destroy_inode()
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index ef600591d96f..55073cbc1823 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -28,6 +28,7 @@
 #include <linux/tracepoint.h>
 #include <linux/device.h>
 #include <linux/memcontrol.h>
+#include <linux/delay.h>
 #include "internal.h"

 /*
@@ -1585,6 +1586,10 @@ static long writeback_sb_inodes(struct super_block *sb,
                        cond_resched();
                }

+               /* Allow __blkdev_put() to run inode_detach_wb() */
+               if (inode->i_ino == 0)
+                       mdelay(200);
+
                /*
                 * Requeue @inode if still dirty.  Be careful as @inode may
                 * have been switched to another wb in the meantime.
diff --git a/wb-detach-repro.sh b/wb-detach-repro.sh
new file mode 100755
index 000000000000..66892d71cd20
--- /dev/null
+++ b/wb-detach-repro.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+set -ex
+
+echo 100 > /proc/sys/vm/dirty_expire_centisecs
+echo 100 > /proc/sys/vm/dirty_writeback_centisecs
+
+mount /dev/sdb /mnt/sdb
+sleep 0.9
+echo 10 > /proc/sys/vm/dirty_writeback_centisecs
+umount /dev/sdb