Re: [PATCH blktests v2] loop/002: Regression testing for loop device flush

2017-08-18 Thread James Wang
No problem thank you sir

发自我的 iPhone

> 在 2017年8月18日,16:11,Omar Sandoval <osan...@osandov.com> 写道:
> 
>> On Tue, Jun 27, 2017 at 12:01:47PM +0800, James Wang wrote:
>> Add a regression testing for loop device. when an unbound device
>> be close that take too long time. kernel will consume serveral orders
>> of magnitude more wall time than it does for a mounted device.
> 
> James, sorry I forgot about this. I ended up committing a simplified
> version here:
> https://github.com/osandov/blktests/commit/bbe69c4cc83ddc36e5c82114890ac071d01c8ac5
> Thanks for the test!
> 



[PATCH blktests v2] loop/002: Regression testing for loop device flush

2017-06-26 Thread James Wang
Add a regression testing for loop device. when an unbound device
be close that take too long time. kernel will consume serveral orders
of magnitude more wall time than it does for a mounted device.

Signed-off-by: James Wang <jnw...@suse.com>
---
 tests/loop/002 | 63 ++
 tests/loop/002.out |  2 ++
 2 files changed, 65 insertions(+)
 create mode 100755 tests/loop/002
 create mode 100644 tests/loop/002.out

diff --git a/tests/loop/002 b/tests/loop/002
new file mode 100755
index 000..ef69729
--- /dev/null
+++ b/tests/loop/002
@@ -0,0 +1,63 @@
+#!/bin/bash
+#
+# Test if close()ing a unbound loop device is too slow
+# Copyright (C) 2017 James Wang <jnw...@suse.com>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+DESCRIPTION="Test if close()ing a unbound loop device is too slow"
+
+QUICK=1
+
+run_test() {
+   TIMEFORMAT='%5R'
+   time {
+   for ((i=0;i<200;i++)); do dd if=/dev/loop0 of=/dev/null  bs=512 
count=1 >/dev/null 2>&1; done
+   }
+}
+clean_up() {
+   if lsmod | grep loop >/dev/null 2>&1; then
+   if ! rmmod loop;then
+   return 2;
+   fi
+   fi
+}
+
+prepare() {
+   modprobe loop max_loop=1
+}
+
+
+test() {
+   echo "Running ${TEST_NAME}"
+
+   clean_up
+   prepare
+   SECONDS=0
+   run_test >/dev/null 2>&1
+   DURATION=${SECONDS}
+
+   clean_up
+   if ! clean_up; then
+   echo "Test complete"
+   return 2
+   fi
+   echo "Test complete"
+   if [[ "${DURATION}" -gt 1 ]]; then
+   echo "test took too long ($URATION seconds)"
+   return 1
+   else
+   return 0
+   fi
+}
diff --git a/tests/loop/002.out b/tests/loop/002.out
new file mode 100644
index 000..5c34a37
--- /dev/null
+++ b/tests/loop/002.out
@@ -0,0 +1,2 @@
+Running loop/002
+Test complete
-- 
2.12.3



Re: [PATCH blktests] loop/002: Regression testing for loop device flush

2017-06-26 Thread James Wang


On 06/27/2017 02:58 AM, Omar Sandoval wrote:
> Hi, James, thanks for sending this in. Sorry for the delay, I've been
> out of the office for a couple of weeks. A few comments below.
>
> On Thu, Jun 08, 2017 at 08:28:12PM +0800, James Wang wrote:
>> Add a regression testing for loop device. when an unbound device
>> be close that take too long time. kernel will consume serveral orders
>> of magnitude more wall time than it does for a mounted device.
>>
>> Signed-off-by: James Wang <jnw...@suse.com>
>> ---
>>  tests/loop/002 | 77 
>> ++
>>  tests/loop/002.out |  2 ++
>>  2 files changed, 79 insertions(+)
>>
>> diff --git a/tests/loop/002 b/tests/loop/002
>> new file mode 100755
>> index 000..fd607d1
>> --- /dev/null
>> +++ b/tests/loop/002
>> @@ -0,0 +1,77 @@
>> +#!/bin/bash
>> +#
>> +# Test if close()ing a unbound loop device is too slow
>> +# Copyright (C) 2017 James Wang
>> +#
>> +# This program is free software: you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation, either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +DESCRIPTION="Test if close()ing a unbound loop device is too slow"
>> +
>> +QUICK=1
>> +
>> +function run_test() {
> For consistency with everything else in blktests, please don't use
> "function" when defining a function.
I will fix it.
>> +TIMEFORMAT='%5R'
>> +time {
>> +for f in `ls /dev/loop[0-9]*|sort`; do dd if=$f of=/dev/null  
>> bs=512 count=1 >/dev/null 2>&1; done
>> +}
>> +}
>> +function clean_up() {
>> +if lsmod | grep loop >/dev/null 2>&1; then
>> +umount /dev/loop* >/dev/null 2>&1
>> +losetup -D
>> +sleep 5
>> +
>> +if ! rmmod loop;then
>> +return 2;
>> +fi
>> +fi
>> +}
>> +
>> +function prepare() {
>> +modprobe loop max_loop=64
> If loop is already loaded, this won't work, right?
Actually, I could use clean_up() first , but due to My testing machine
has a bug causes clean_up() very slow..
I use call clean_up() before prepare(), make sense?
>
>> +dd if=/dev/zero of=${TMPDIR}/disk bs=512 count=200K >/dev/null 2>&1
>> +for((i=0;i<4;i++))
>> +do
>> +losetup -f ${TMPDIR}/disk;
>> +done
>> +mkfs.ext4 -F /dev/loop0 >/dev/null 2>&1
> Hm, so if I happened to have something I care about on /dev/loop0,
> running blktests will destroy it? This is a no-go.
Yes, but due to our insert loop module and create a fake-disk and bound
to loop0, so format loop0 should doesn't matter.

>> +for((i=0;i<4;i++))
>> +do
>> +mkdir -p t$i;
>> +mount /dev/loop$i t$i;
>> +done
>> +
>> +}
>> +
>> +
>> +test() {
>> +echo "Running ${TEST_NAME}"
>> +
>> +prepare
>> +SECONDS=0
>> +run_test >/dev/null 2>&1
>> +DURATION=${SECONDS}
> Nifty, I didn't know about $SECONDS.
SECONDS is a built-in variable in bash, it will automatic increase.
>
>> +
>> +clean_up
>> +if ! clean_up; then
>> +echo "Test complete"
>> +return 2
>> +fi
>> +echo "Test complete"
>> +if [[ "${DURATION}" -gt 1 ]]; then
>> +return 1
>> +else
>> +return 0
>> +fi
> I'd really like a meaningful output if this test fails, so something
> like this instead of the if/else
>
> if [[ "${DURATION}" -gt 1 ]]; then
>   echo "test took too long ($DURATION seconds)"
> fi
I will fix this.
>> +}
>> diff --git a/tests/loop/002.out b/tests/loop/002.out
>> new file mode 100644
>> index 000..5c34a37
>> --- /dev/null
>> +++ b/tests/loop/002.out
>> @@ -0,0 +1,2 @@
>> +Running loop/002
>> +Test complete
>> -- 
>> 2.12.3
>>
> Overall, is there an easier way to test this than setting up 64 loop
> devices at modprobe time? E.g., can you losetup -f and run it on a
> single loop device many times to measure the same issue?
Use many loop devices for get a enough long time to compare with 1 second.
if we only create 1 loop device, I afraid it can't be measured.
In this scenario, I could get the duration of unbound and bound loop
device takes.
OK, I could try your suggestion.

I will send patch later.

James
>
> Thanks again!
>
>

-- 
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)



[PATCH blktests] loop/002: Regression testing for loop device flush

2017-06-08 Thread James Wang
Add a regression testing for loop device. when an unbound device
be close that take too long time. kernel will consume serveral orders
of magnitude more wall time than it does for a mounted device.

Signed-off-by: James Wang <jnw...@suse.com>
---
 tests/loop/002 | 77 ++
 tests/loop/002.out |  2 ++
 2 files changed, 79 insertions(+)

diff --git a/tests/loop/002 b/tests/loop/002
new file mode 100755
index 000..fd607d1
--- /dev/null
+++ b/tests/loop/002
@@ -0,0 +1,77 @@
+#!/bin/bash
+#
+# Test if close()ing a unbound loop device is too slow
+# Copyright (C) 2017 James Wang
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+DESCRIPTION="Test if close()ing a unbound loop device is too slow"
+
+QUICK=1
+
+function run_test() {
+   TIMEFORMAT='%5R'
+   time {
+   for f in `ls /dev/loop[0-9]*|sort`; do dd if=$f of=/dev/null  
bs=512 count=1 >/dev/null 2>&1; done
+   }
+}
+function clean_up() {
+   if lsmod | grep loop >/dev/null 2>&1; then
+   umount /dev/loop* >/dev/null 2>&1
+   losetup -D
+   sleep 5
+   
+   if ! rmmod loop;then
+   return 2;
+   fi
+   fi
+}
+
+function prepare() {
+   modprobe loop max_loop=64
+   dd if=/dev/zero of=${TMPDIR}/disk bs=512 count=200K >/dev/null 2>&1
+   for((i=0;i<4;i++))
+   do
+   losetup -f ${TMPDIR}/disk;
+   done
+   mkfs.ext4 -F /dev/loop0 >/dev/null 2>&1
+   for((i=0;i<4;i++))
+   do
+   mkdir -p t$i;
+   mount /dev/loop$i t$i;
+   done
+
+}
+
+
+test() {
+   echo "Running ${TEST_NAME}"
+
+   prepare
+   SECONDS=0
+   run_test >/dev/null 2>&1
+   DURATION=${SECONDS}
+
+   clean_up
+   if ! clean_up; then
+   echo "Test complete"
+   return 2
+   fi
+   echo "Test complete"
+   if [[ "${DURATION}" -gt 1 ]]; then
+   return 1
+   else
+   return 0
+   fi
+}
diff --git a/tests/loop/002.out b/tests/loop/002.out
new file mode 100644
index 000..5c34a37
--- /dev/null
+++ b/tests/loop/002.out
@@ -0,0 +1,2 @@
+Running loop/002
+Test complete
-- 
2.12.3



Re: [PATCH] Fix loop device flush before configure v3

2017-06-08 Thread James Wang


On 06/08/2017 03:53 PM, Johannes Thumshirn wrote:
> On 06/08/2017 08:52 AM, James Wang wrote:
>> Test method:
>> modprobe loop max_loop=64
>> dd if=/dev/zero of=disk bs=512 count=200K
>> for((i=0;i<4;i++))do losetup -f disk; done
>> mkfs.ext4 -F /dev/loop0
>> for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
>> for f in `ls /dev/loop[0-9]*|sort`; do \
>>  echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
>>  done
> I think Christoph already asked this, but can you send a patch for
> blktests [1] as well?
I have forked this project and I'm working on it.
due to framework is limited, so I'm adjusting my code.
> Thanks,
>   Johannes
>
> [1] https://github.com/osandov/blktests
>

-- 
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)



Re: [PATCH] Fix loop device flush before configure

2017-06-08 Thread James Wang


On 06/08/2017 02:56 PM, Christoph Hellwig wrote:
> On Thu, Jun 08, 2017 at 08:45:31AM +0800, James Wang wrote:
>> Ok I got it blktests is a suite. I'd like to contribute something. If you 
>> need, we adapt you,;-)!
>> But I have to learn some how to do that, need time.
> I haven't added test myself to blktests yet either, so I'd have to
> learn it as well.  Omar can probably help you though.
>
>
Ah, I have fork this project  in Github.  and write 1 script in 'loop' 
group.

Debuging


James

-- 
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)



[PATCH] Fix loop device flush before configure v3

2017-06-08 Thread James Wang
While installing SLES-12 (based on v4.4), I found that the installer
will stall for 60+ seconds during LVM disk scan.  The root cause was
determined to be the removal of a bound device check in loop_flush()
by commit b5dd2f6047ca ("block: loop: improve performance via blk-mq").

Restoring this check, examining ->lo_state as set by loop_set_fd()
eliminates the bad behavior.

Test method:
modprobe loop max_loop=64
dd if=/dev/zero of=disk bs=512 count=200K
for((i=0;i<4;i++))do losetup -f disk; done
mkfs.ext4 -F /dev/loop0
for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
for f in `ls /dev/loop[0-9]*|sort`; do \
echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
done

Test output:  stock  patched
/dev/loop018.1217e-058.3842e-05
/dev/loop1 6.1114e-050.000147979
/dev/loop100.414701  0.000116564
/dev/loop110.74746.7942e-05
/dev/loop120.747986  8.9082e-05
/dev/loop130.746532  7.4799e-05
/dev/loop140.480041  9.3926e-05
/dev/loop151.26453   7.2522e-05

Note that from loop10 onward, the device is not mounted, yet the
stock kernel consumes several orders of magnitude more wall time
than it does for a mounted device.
(Thanks for Mike Galbraith <efa...@gmx.de>, give a changelog review.)

Reviewed-by: Hannes Reinecke <h...@suse.com>
Reviewed-by: Ming Lei <ming....@redhat.com>
Signed-off-by: James Wang <jnw...@suse.com>
Fixes: b5dd2f6047ca ("block: loop: improve performance via blk-mq")
---
 drivers/block/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 48f6fa6f810e..2e5b8538760c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -625,6 +625,9 @@ static int loop_switch(struct loop_device *lo, struct file 
*file)
  */
 static int loop_flush(struct loop_device *lo)
 {
+   /* loop not yet configured, no running thread, nothing to flush */
+   if (lo->lo_state != Lo_bound)
+   return 0;
return loop_switch(lo, NULL);
 }
 
-- 
2.12.3



[PATCH] Fix loop device flush before configure

2017-06-08 Thread James Wang
While installing SLES-12 (based on v4.4), I found that the installer
will stall for 60+ seconds during LVM disk scan.  The root cause was
determined to be the removal of a bound device check in loop_flush()
by commit b5dd2f6047ca ("block: loop: improve performance via blk-mq").

Restoring this check, examining ->lo_state as set by loop_set_fd()
eliminates the bad behavior.

Test method:
modprobe loop max_loop=64
dd if=/dev/zero of=disk bs=512 count=200K
for((i=0;i<4;i++))do losetup -f disk; done
mkfs.ext4 -F /dev/loop0
for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
for f in `ls /dev/loop[0-9]*|sort`; do \
echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
done

Test output:  stock  patched
/dev/loop018.1217e-058.3842e-05
/dev/loop1 6.1114e-050.000147979
/dev/loop100.414701  0.000116564
/dev/loop110.74746.7942e-05
/dev/loop120.747986  8.9082e-05
/dev/loop130.746532  7.4799e-05
/dev/loop140.480041  9.3926e-05
/dev/loop151.26453   7.2522e-05

Note that from loop10 onward, the device is not mounted, yet the
stock kernel consumes several orders of magnitude more wall time
than it does for a mounted device.
(Thanks for Mike Galbraith <efa...@gmx.de>, give a changelog review.)

Reviewed-by: Hannes Reinecke <h...@suse.com>
Reviewed-by: Ming Lei <ming....@redhat.com>
Signed-off-by: James Wang <jnw...@suse.com>
Fixes: b5dd2f6047ca ("block: loop: improve performance via blk-mq")
---
 drivers/block/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 48f6fa6f810e..2e5b8538760c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -625,6 +625,9 @@ static int loop_switch(struct loop_device *lo, struct file 
*file)
  */
 static int loop_flush(struct loop_device *lo)
 {
+   /* loop not yet configured, no running thread, nothing to flush */
+   if (lo->lo_state != Lo_bound)
+   return 0;
return loop_switch(lo, NULL);
 }
 
-- 
2.12.3



Re: [PATCH] Fix loop device flush before configure v2

2017-06-07 Thread James Wang


On 06/08/2017 01:06 PM, Mike Galbraith wrote:
> On Thu, 2017-06-08 at 10:17 +0800, James Wang wrote:
>> This condition check was exist at before commit b5dd2f6047ca ("block: loop:
>> improve performance via blk-mq") When add MQ support to loop device, it be
>> removed because the member of '->lo_thread' be removed. And then upstream
>> add '->worker_task', I think they forget add it to here.
>>
>> When I install SLES-12 product is base on 4.4 kernel I found installer will
>> hang +60 second at scan disks. and I found LVM tools would take this action.
>> finally I found this problem is more obvious on AMD platform. This problem
>> will impact all scenarios that scan loop devcie.
>>
>> When the loop device didn't configure backing file or Request Queue, we
>> shouldn't to cost a lot of time to flush it.
> The changelog sounds odd to me, perhaps reword/condense a bit?...
>
> While installing SLES-12 (based on v4.4), I found that the installer
> will stall for 60+ seconds during LVM disk scan.  The root cause was
> determined to be the removal of a bound device check in loop_flush()
> by commit b5dd2f6047ca ("block: loop: improve performance via blk-mq").
>
> Restoring this check, examining ->lo_state as set by loop_set_fd()
> eliminates the bad behavior.
Thank you sir. I will rewrite this changelog.
> Test method:
> modprobe loop max_loop=64
> dd if=/dev/zero of=disk bs=512 count=200K
> for((i=0;i<4;i++))do losetup -f disk; done
> mkfs.ext4 -F /dev/loop0
> for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
> for f in `ls /dev/loop[0-9]*|sort`; do \
>   echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
>   done
>
> Test output:  stock  patched
> /dev/loop018.1217e-058.3842e-05
> /dev/loop1 6.1114e-050.000147979
> /dev/loop100.414701  0.000116564
> /dev/loop110.74746.7942e-05
> /dev/loop120.747986  8.9082e-05
> /dev/loop130.746532  7.4799e-05
> /dev/loop140.480041  9.3926e-05
> /dev/loop151.26453   7.2522e-05
>
> Note that from loop10 onward, the device is not mounted, yet the
> stock kernel consumes several orders of magnitude more wall time
> than it does for a mounted device.
>
> Reviewed-by: Hannes Reinecke <h...@suse.com>
> Signed-off-by: James Wang <jnw...@suse.com>
> Fixes: b5dd2f6047ca ("block: loop: improve performance via blk-mq")
> ---
>>  drivers/block/loop.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index 48f6fa6f810e..2e5b8538760c 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -625,6 +625,9 @@ static int loop_switch(struct loop_device *lo, struct 
>> file *file)
>>   */
>>  static int loop_flush(struct loop_device *lo)
>>  {
>> +/* loop not yet configured, no running thread, nothing to flush */
>> +if (lo->lo_state != Lo_bound)
>> +return 0;
>>  return loop_switch(lo, NULL);
>>  }
>>  
>

-- 
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)



[PATCH] Fix loop device flush before configure v2

2017-06-07 Thread James Wang
This condition check was exist at before commit b5dd2f6047ca ("block: loop:
improve performance via blk-mq") When add MQ support to loop device, it be
removed because the member of '->lo_thread' be removed. And then upstream
add '->worker_task', I think they forget add it to here.

When I install SLES-12 product is base on 4.4 kernel I found installer will
hang +60 second at scan disks. and I found LVM tools would take this action.
finally I found this problem is more obvious on AMD platform. This problem
will impact all scenarios that scan loop devcie.

When the loop device didn't configure backing file or Request Queue, we
shouldn't to cost a lot of time to flush it.

Testing steps are following:
modprobe loop max_loop=64
dd if=/dev/zero of=disk bs=512 count=200K
for((i=0;i<4;i++))do losetup -f disk; done
mkfs.ext4 -F /dev/loop0
for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
for f in `ls /dev/loop[0-9]*|sort`; do \
echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
done

Testing data is following:
/dev/loop0   <+patched>
8.1217e-05  8.3842e-05
/dev/loop1
6.1114e-05  0.000147979
/dev/loop10
0.4147010.000116564
/dev/loop11
0.7474  6.7942e-05
/dev/loop12
0.7479868.9082e-05
/dev/loop13
0.7465327.4799e-05
/dev/loop14
0.4800419.3926e-05
/dev/loop15
1.26453 7.2522e-05

>From /dev/loop10 start, loop isn't mounted. but it take more time than
mounted devices. And The data differ by several orders of magnitude.

Reviewed-by: Hannes Reinecke <h...@suse.com>

Signed-off-by: James Wang <jnw...@suse.com>
---
 drivers/block/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 48f6fa6f810e..2e5b8538760c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -625,6 +625,9 @@ static int loop_switch(struct loop_device *lo, struct file 
*file)
  */
 static int loop_flush(struct loop_device *lo)
 {
+   /* loop not yet configured, no running thread, nothing to flush */
+   if (lo->lo_state != Lo_bound)
+   return 0;
return loop_switch(lo, NULL);
 }
 
-- 
2.12.3



Re: [PATCH] Fix loop device flush before configure

2017-06-07 Thread James Wang
Ok I got it blktests is a suite. I'd like to contribute something. If you need, 
we adapt you,;-)!
But I have to learn some how to do that, need time.

James
 
>From subway.

在 2017年6月7日,21:17,Christoph Hellwig  写道:

>> Testing steps are following:
>> modprobe loop max_loop=64
>> dd if=/dev/zero of=disk bs=512 count=200K
>> for((i=0;i<4;i++))do losetup -f disk; done
>> mkfs.ext4 -F /dev/loop0
>> for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
>> for f in `ls /dev/loop[0-9]*|sort`; do \
>>echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
>>done
> 
> Can you write this up for blktests, please?
> 



[PATCH] Fix loop device flush before configure

2017-06-07 Thread James Wang
This condition check was exist at before commit b5dd2f6047ca ("block: loop:
improve performance via blk-mq") When add MQ support to loop device, it be
removed because the member of '->lo_thread' be removed. And then upstream
add '->worker_task', I think they forget add it to here.

When I install SLES-12 product is base on 4.4 kernel, I found installer will
hang +60 second at scan disks. and I found LVM tools would take this action.
finally I found this problem is more obvious on AMD platform. This problem
will impact all scenarios that scan loop devcies.

When the loop device didn't configure backing file or Request Queue, we
shouldn't to cost a lot of time to flush it.

Testing steps are following:
modprobe loop max_loop=64
dd if=/dev/zero of=disk bs=512 count=200K
for((i=0;i<4;i++))do losetup -f disk; done
mkfs.ext4 -F /dev/loop0
for((i=0;i<4;i++))do mkdir t$i; mount /dev/loop$i t$i;done
for f in `ls /dev/loop[0-9]*|sort`; do \
echo $f; dd if=$f of=/dev/null  bs=512 count=1; \
done

Testing data is following:
/dev/loop0   <+patched>
8.1217e-05  8.3842e-05
/dev/loop1
6.1114e-05  0.000147979
/dev/loop10
0.4147010.000116564
/dev/loop11
0.7474  6.7942e-05
/dev/loop12
0.7479868.9082e-05
/dev/loop13
0.7465327.4799e-05
/dev/loop14
0.4800419.3926e-05
/dev/loop15
1.26453 7.2522e-05

>From /dev/loop10 start, loop isn't mounted. but it take more time than
mounted devices. And The data differ by several orders of magnitude.

Reviewed-by: Hannes Reinecke <h...@suse.com>

Signed-off-by: James Wang <jnw...@suse.com>
---
 drivers/block/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 48f6fa6f810e..c1807e91db08 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -625,6 +625,9 @@ static int loop_switch(struct loop_device *lo, struct file 
*file)
  */
 static int loop_flush(struct loop_device *lo)
 {
+   /* loop not yet configured, no running thread, nothing to flush */
+   if (!lo->worker_task)
+   return 0;
return loop_switch(lo, NULL);
 }
 
-- 
2.12.3