test: ums: Add sleep before unmount directory

PATCH Superseded Lukasz Majewski 5 participants 16 messages Nov 6, 2014 – Nov 14, 2014

lore

Share & tools

lore

Lukasz Majewski

author

2014-11-06 10:23 UTC #0 lore raw

PATCH 1/1 test: ums: Add sleep before unmount directory

This change helps to run script on machines with quite long uptime. Without this the following error emerges:

File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test

Signed-off-by Lukasz Majewski <[email protected]>

---
 test/ums/ums_gadget_test.sh | 1 +
 1 file changed, 1 insertion(+)

test/ums/ums_gadget_test.sh +1 -0

index 56d4616..f862332 100755
@@ -59,6 +59,7 @@ ums_test_file () {
     fi
 
     cp ./$1 $MNT_DIR
+    sleep 2
     umount $MNT_DIR

Signature · 1 line

2.0.0.rc2

Stephen Warren

2014-11-06 17:00 UTC #1 lore raw

PATCH 1/1 test: ums: Add sleep before unmount directory

Quoted 12 lines · Lukasz Majewski

This change helps to run script on machines with quite long uptime. Without this the following error emerges:
File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test

test/ums/ums_gadget_test.sh ums_test_file () { · line 63

Quoted 4 lines

test/ums/ums_gadget_test.sh +1 -0

 
      cp ./$1 $MNT_DIR
+    sleep 2
      umount $MNT_DIR

I don't think there's any guarantee the "2" is the exact correct amount of time to sleep; I presume you derived the value by trying some values until you found one that works, and so the actual value required may vary from system to system. How about something like the following, which I use in a script that copies a new kernel to an SD card, where I experienced a similar issue:

while true; do
sudo umount /mnt/C-ROOT/
if [ $? -eq 0 ]; then
break
fi
echo sleeping to wait for umount...
sleep 1
done

Lukasz Majewski

2014-11-06 21:30 UTC #2 lore raw

PATCH 1/1 test: ums: Add sleep before unmount directory

Quoted 27 lines · On Thu, 06 Nov 2014 10:00:12 -0700 Stephen Warren <[email protected]> wrote:

Quoted 14 lines · Lukasz Majewski
This change helps to run script on machines with quite long uptime. Without this the following error emerges:
File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test
test/ums/ums_gadget_test.sh ums_test_file () { · line 63
Quoted 4 lines
test/ums/ums_gadget_test.sh +1 -0
 
      cp ./$1 $MNT_DIR
+    sleep 2
      umount $MNT_DIR
I don't think there's any guarantee the "2" is the exact correct amount of time to sleep; I presume you derived the value by trying some values until you found one that works, and so the actual value required may vary from system to system. How about something like the following, which I use in a script that copies a new kernel to an SD card, where I experienced a similar issue:

Quoted 8 lines

while true; do
sudo umount /mnt/C-ROOT/
if [ $? -eq 0 ]; then
break
fi
echo sleeping to wait for umount...
sleep 1
done

Stephen, It is exactly what I'm looking for.

Thanks a lot for support.

Best regards,
Lukasz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141106/b2751ed4/attachment.pgp>

Lukasz Majewski

author

2014-11-07 13:05 UTC #3 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

In reply to Lukasz Majewski

This change helps to run script on machines with quite long uptime. Without this the following error emerges:

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v2:
- Check umount return status code and sleep if needed

---
 test/ums/ums_gadget_test.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

test/ums/ums_gadget_test.sh +9 -1

index 56d4616..9da486b 100755
@@ -11,6 +11,7 @@ clear
 
 COLOUR_RED="\33[31m"
 COLOUR_GREEN="\33[32m"
+COLOUR_ORANGE="\33[33m"
 COLOUR_DEFAULT="\33[0m"
 
 DIR=./

@@ -59,8 +60,15 @@ ums_test_file () {
     fi
 
     cp ./$1 $MNT_DIR
-    umount $MNT_DIR
 
+    while true; do
+	umount $MNT_DIR > /dev/null 2>&1
+	if [ $? -eq 0 ]; then
+	    break
+	fi
+	printf "$COLOUR_ORANGE\tSleeping to wait for umount...$COLOUR_DEFAULT\n"
+	sleep 1
+    done
 
     echo -n "TX: "
     calculate_md5sum $1

Signature · 1 line

2.0.0.rc2

Marek Vasut

2014-11-07 15:28 UTC #4 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Quoted 11 lines · Lukasz Majewski

This change helps to run script on machines with quite long uptime. Without this the following error emerges:
File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test

Quoted 1 line

Signed-off-by Lukasz Majewski <[email protected]>

Why exactly does mount not block until it can unmount the device anyway ?

Best regards,
Marek Vasut

Lukasz Majewski

author

2014-11-07 16:52 UTC #5 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Hi Marek,

Quoted 13 lines · Lukasz Majewski

Quoted 12 lines
This change helps to run script on machines with quite long uptime. Without this the following error emerges:
File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test

Quoted 4 lines

Quoted 1 line
Signed-off-by Lukasz Majewski <[email protected]>
Why exactly does mount not block until it can unmount the device anyway ?

Has mount or umount expose any blocking behavior?

As fair as I remember you cannot umount directory when any process holds reference to it.

Here it looks like process which copy data queues data for writing and exit from cp.

Presumably, when we call umount there is still some pending data for write.

Hence the error.

Maybe there is a switch for mount/umount/cp which can correct such behavior?

Quoted 3 lines

Best regards,
Marek Vasut

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Marek Vasut

2014-11-07 21:02 UTC #6 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Quoted 18 lines · Lukasz Majewski

Hi Marek,
Quoted 16 lines · Lukasz Majewski
Quoted 15 lines
This change helps to run script on machines with quite long uptime. Without this the following error emerges:
File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Lukasz Majewski <[email protected]>
Why exactly does mount not block until it can unmount the device anyway ?
Has mount or umount expose any blocking behavior?

I would expect umount to be blocking by default in the first place. ie. umount /mnt/foo should wait until all data are synced on device under /mnt/foo and only then it should be unmounted.

Can you check (with strace or something) what exact return value will the umount() syscall return when it does NOT unmount the mountpoint and returns to the shell ? Is it EBUSY ?

Quoted 10 lines

As fair as I remember you cannot umount directory when any process holds reference to it.
Here it looks like process which copy data queues data for writing and exit from cp.
Presumably, when we call umount there is still some pending data for write.
Hence the error.

In case there are data which are not yet sync'd to the device, the umount() syscall should block until those data are written and return only after that. Why would it return -EBUSY ... hmmmmm ...

Quoted 2 lines

Maybe there is a switch for mount/umount/cp which can correct such behavior?

That should be the default ...

Best regards,
Marek Vasut

Stephen Warren

2014-11-07 15:33 UTC #7 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

In reply to Lukasz Majewski

Quoted 10 lines · Lukasz Majewski

This change helps to run script on machines with quite long uptime. Without this the following error emerges:
File: ./dat_14M.img
umount: /mnt/tmp-ums-test: device is busy.
(In some cases useful info about processes that use
the device is found by lsof(8) or fuser(1))
TX: md5sum:083d3d22b542d3ecba61b12d17e03f9f
mount: /dev/sdd6 already mounted or /mnt/tmp-ums-test busy
mount: according to mtab, /dev/sdd6 is already mounted on /mnt/tmp-ums-test

Reviewed-by Stephen Warren <[email protected]>

Wolfgang Denk

2014-11-07 21:54 UTC #8 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

In reply to Lukasz Majewski

Dear Lukasz,

test/ums/ums_gadget_test.sh ums_test_file () { · line 71

Quoted 11 lines · In message <[email protected]> you wrote:

test/ums/ums_gadget_test.sh +8 -1

-    umount $MNT_DIR
 
+    while true; do
+	umount $MNT_DIR > /dev/null 2>&1
+	if [ $? -eq 0 ]; then
+	    break
+	fi
+	printf "$COLOUR_ORANGE\tSleeping to wait for umount...$COLOUR_DEFAULT\n"
+	sleep 1
+    done

As Marek already pointed out, umount (both the command (8) and the syscall (2)) are supposed to be blocking (except for typical error cases like no permission, mountpoint does not exist, no file system mounted, etc.).

If you really ever see the umount entering above loop, then there is some bug somewhere. It would be indeed interesting to see the strace log for such umount calls. Also, can you please add details about the exact execution environment (host system / architecture, kernel version, distro, and _especially_ which sort of USB driver is involved here.

My gut feeling is that there might be some USB driver error involved here.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If the hours are long enough and the pay  is  short  enough,  someone
will say it's women's work.

Stephen Warren

2014-11-07 22:00 UTC #9 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Quoted 27 lines · Wolfgang Denk

Dear Lukasz,
test/ums/ums_gadget_test.sh ums_test_file () { · line 71
Quoted 11 lines · In message <[email protected]> you wrote:
test/ums/ums_gadget_test.sh +8 -1
-    umount $MNT_DIR
 
+    while true; do
+	umount $MNT_DIR > /dev/null 2>&1
+	if [ $? -eq 0 ]; then
+	    break
+	fi
+	printf "$COLOUR_ORANGE\tSleeping to wait for umount...$COLOUR_DEFAULT\n"
+	sleep 1
+    done
As Marek already pointed out, umount (both the command (8) and the syscall (2)) are supposed to be blocking (except for typical error cases like no permission, mountpoint does not exist, no file system mounted, etc.).
If you really ever see the umount entering above loop, then there is some bug somewhere. It would be indeed interesting to see the strace log for such umount calls. Also, can you please add details about the exact execution environment (host system / architecture, kernel version, distro, and _especially_ which sort of USB driver is involved here.

Quoted 2 lines

My gut feeling is that there might be some USB driver error involved here.

Where I've seen this is writing to an SD card in a USB-based SD card reader.

I have a fairly regular amd64 machine running Ubuntu. I put the loop above into my own scripts that mount an SD card, copy new data to it, and then immediately unmount it for the same reason that Lukasz made this patch; umount doesn't always unmount. I did this so long ago that I don't recall which Ubuntu version I had at the time, nor the kernel version. It was *probably* Ubuntu 10.04 or 12.10 though. Now I'm on 14.04, but have no idea if I still need the loop since it's already in place:-)

Wolfgang Denk

2014-11-07 23:03 UTC #10 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Dear Stephen,

Quoted 6 lines · In message <[email protected]> you wrote:

Quoted 3 lines
My gut feeling is that there might be some USB driver error involved here.
Where I've seen this is writing to an SD card in a USB-based SD card reader.

Quoted 8 lines

I have a fairly regular amd64 machine running Ubuntu. I put the loop above into my own scripts that mount an SD card, copy new data to it, and then immediately unmount it for the same reason that Lukasz made this patch; umount doesn't always unmount. I did this so long ago that I don't recall which Ubuntu version I had at the time, nor the kernel version. It was *probably* Ubuntu 10.04 or 12.10 though. Now I'm on 14.04, but have no idea if I still need the loop since it's already in place:-)

But this is a bug. Papering over is not a good idea. It should be analyzed, reported, and finally fixed.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Our management frequently gets lost in thought.   That's because it's
unfamiliar territory.

Lukasz Majewski

author

2014-11-12 14:29 UTC #11 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Hi Wolfgang,

Quoted 19 lines

Dear Stephen,
Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 3 lines
My gut feeling is that there might be some USB driver error involved here.
Where I've seen this is writing to an SD card in a USB-based SD card reader.
Quoted 8 lines
I have a fairly regular amd64 machine running Ubuntu. I put the loop above into my own scripts that mount an SD card, copy new data to it, and then immediately unmount it for the same reason that Lukasz made this patch; umount doesn't always unmount. I did this so long ago that I don't recall which Ubuntu version I had at the time, nor the kernel version. It was *probably* Ubuntu 10.04 or 12.10 though. Now I'm on 14.04, but have no idea if I still need the loop since it's already in place:-)

Quoted 3 lines

But this is a bug. Papering over is not a good idea. It should be analyzed, reported, and finally fixed.

I've debugged the script with strace.

The problem is with umount() syscall:

[pid 23296] umount("/mnt/tmp-ums-test", 0) = -1 EBUSY (Device or resource busy)

It is somehow strange, because with strace attached I occasionally see correct behaviour:

[pid 23776] umount("/mnt/tmp-ums-test", 0) = 0

I need to debug the syscall in the kernel.

Information about my system:

x86_64:
Linux 3.12.0 #2 SMP PREEMPT Wed Nov 27 10:26:22 CET 2013 x86_64 GNU/Linux

Debian 7.4.

Quoted 4 lines

Best regards,
Wolfgang Denk

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Wolfgang Denk

2014-11-12 16:45 UTC #12 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Dear Lukasz,

Quoted 5 lines · In message <20141112152949.1fe6ce3c@amdc2363> you wrote:

Quoted 3 lines
But this is a bug. Papering over is not a good idea. It should be analyzed, reported, and finally fixed.
I've debugged the script with strace.

Thanks a lot for that!

Quoted 1 line

The problem is with umount() syscall:

...

Quoted 1 line

I need to debug the syscall in the kernel.

I have a gut feeling that this might be a problem somewhere with the USB stuff that is involved here. Maybe enabling some degug messages for the USB subsystem gives some more information?

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The universe is all a spin-off of the Big Bang.

Stephen Warren

2014-11-12 18:11 UTC #13 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Quoted 11 lines · Wolfgang Denk

Dear Lukasz,
Quoted 5 lines · In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
Quoted 3 lines
But this is a bug. Papering over is not a good idea. It should be analyzed, reported, and finally fixed.
I've debugged the script with strace.
Thanks a lot for that!

Quoted 7 lines

Quoted 1 line
The problem is with umount() syscall:
...
Quoted 1 line
I need to debug the syscall in the kernel.
I have a gut feeling that this might be a problem somewhere with the USB stuff that is involved here. Maybe enabling some degug messages for the USB subsystem gives some more information?

If this really is related to USB only, it may be worth asking on the linux-usb mailing list to see if anyone already unerstands the issue.

Irrespective of all this though, even if there is a kernel issue and it can be fixed, I still think we should apply this patch, because the issue currently affects people. At the very least it will until some kernel fix is rolled out to everyone's distro, which will be a while no matter what.

Lukasz Majewski

2014-11-12 21:49 UTC #14 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Quoted 14 lines · On Wed, 12 Nov 2014 11:11:33 -0700 Stephen Warren <[email protected]> wrote:

Quoted 13 lines · Wolfgang Denk
Dear Lukasz,
Quoted 5 lines · In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
Quoted 3 lines
But this is a bug. Papering over is not a good idea. It should be analyzed, reported, and finally fixed.
I've debugged the script with strace.
Thanks a lot for that!
Quoted 1 line
The problem is with umount() syscall:
...

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
I need to debug the syscall in the kernel.
I have a gut feeling that this might be a problem somewhere with the USB stuff that is involved here. Maybe enabling some degug messages for the USB subsystem gives some more information?
If this really is related to USB only, it may be worth asking on the linux-usb mailing list to see if anyone already unerstands the issue.

Frankly speaking, I'm personally curious why such things happen. I may look into the issue in my "spare :-)" time.

In spite of this, I agree with Stephen:

Quoted 6 lines

Irrespective of all this though, even if there is a kernel issue and it can be fixed, I still think we should apply this patch, because the issue currently affects people. At the very least it will until some kernel fix is rolled out to everyone's distro, which will be a while no matter what.

The problem seems to be with long time running linux. The error seems to be orthogonal to our u-boot's UMS work.

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141112/78c141d1/attachment.pgp>

Lukasz Majewski

author

2014-11-14 10:43 UTC #15 lore raw

PATCH v2 1/1 test: ums: Add sleep before unmount directory

Hi Lukasz,

Quoted 44 lines

Quoted 16 lines · On Wed, 12 Nov 2014 11:11:33 -0700 Stephen Warren <[email protected]> wrote:
Quoted 15 lines · Wolfgang Denk
Dear Lukasz,
Quoted 5 lines · In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
Quoted 3 lines
But this is a bug. Papering over is not a good idea. It should be analyzed, reported, and finally fixed.
I've debugged the script with strace.
Thanks a lot for that!
Quoted 1 line
The problem is with umount() syscall:
...
Quoted 1 line
I need to debug the syscall in the kernel.
Quoted 7 lines
Quoted 3 lines
I have a gut feeling that this might be a problem somewhere with the USB stuff that is involved here. Maybe enabling some degug messages for the USB subsystem gives some more information?
If this really is related to USB only, it may be worth asking on the linux-usb mailing list to see if anyone already unerstands the issue.
Frankly speaking, I'm personally curious why such things happen. I may look into the issue in my "spare :-)" time.
In spite of this, I agree with Stephen:
Quoted 6 lines
Irrespective of all this though, even if there is a kernel issue and it can be fixed, I still think we should apply this patch, because the issue currently affects people. At the very least it will until some kernel fix is rolled out to everyone's distro, which will be a while no matter what.
+1
The problem seems to be with long time running linux. The error seems to be orthogonal to our u-boot's UMS work.

Quoted 7 lines

Quoted 4 lines
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Best regards,
Lukasz Majewski

Applied to u-boot-dfu tree. Thanks.

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group