[U-Boot] [PATCH] test: ums: Add sleep before unmount directory

[U-Boot] [PATCH] test: ums: Add sleep before unmount directory

[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

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 test/ums/ums_gadget_test.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/ums/ums_gadget_test.sh b/test/ums/ums_gadget_test.sh
index 56d4616..f862332 100755
--- a/test/ums/ums_gadget_test.sh
+++ b/test/ums/ums_gadget_test.sh
@@ -59,6 +59,7 @@ ums_test_file () {
     fi
 
     cp ./$1 $MNT_DIR
+    sleep 2
     umount $MNT_DIR
 
 
-- 
2.0.0.rc2

[U-Boot] [PATCH] test: ums: Add sleep before unmount directory

[Stephen Warren]

On 11/06/2014 03:23 AM, Lukasz Majewski wrote:
> 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

> diff --git a/test/ums/ums_gadget_test.sh b/test/ums/ums_gadget_test.sh

>       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

[U-Boot] [PATCH] test: ums: Add sleep before unmount directory

[Lukasz Majewski]

On Thu, 06 Nov 2014 10:00:12 -0700
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 11/06/2014 03:23 AM, Lukasz Majewski wrote:
> > 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
> 
> > diff --git a/test/ums/ums_gadget_test.sh
> > b/test/ums/ums_gadget_test.sh
> 
> >       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

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>

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[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

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
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(-)

diff --git a/test/ums/ums_gadget_test.sh b/test/ums/ums_gadget_test.sh
index 56d4616..9da486b 100755
--- a/test/ums/ums_gadget_test.sh
+++ b/test/ums/ums_gadget_test.sh
@@ -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
-- 
2.0.0.rc2

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Marek Vasut]

On Friday, November 07, 2014 at 02:05:55 PM, Lukasz Majewski wrote:
> 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 <l.majewski@samsung.com>

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

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Stephen Warren]

On 11/07/2014 06:05 AM, Lukasz Majewski wrote:
> 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 <swarren@nvidia.com>

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Lukasz Majewski]

Hi Marek,

> On Friday, November 07, 2014 at 02:05:55 PM, Lukasz Majewski wrote:
> > 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 <l.majewski@samsung.com>
> 
> 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?


> 
> Best regards,
> Marek Vasut


-- 
Best regards,

Lukasz Majewski

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

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Marek Vasut]

On Friday, November 07, 2014 at 05:52:27 PM, Lukasz Majewski wrote:
> Hi Marek,
> 
> > On Friday, November 07, 2014 at 02:05:55 PM, Lukasz Majewski wrote:
> > > 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 <l.majewski@samsung.com>
> > 
> > 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 ?

> 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 ...

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

That should be the default ...

Best regards,
Marek Vasut

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Wolfgang Denk]

Dear Lukasz,

In message <1415365555-9790-1-git-send-email-l.majewski@samsung.com> you wrote:
>
> -    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

-- 
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.

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Stephen Warren]

On 11/07/2014 02:54 PM, Wolfgang Denk wrote:
> Dear Lukasz,
>
> In message <1415365555-9790-1-git-send-email-l.majewski@samsung.com> you wrote:
>>
>> -    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.

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:-)

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Wolfgang Denk]

Dear Stephen,

In message <545D40E1.2030700@wwwdotorg.org> you wrote:
>
> > 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:-)

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

Best regards,

Wolfgang Denk

-- 
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.

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Lukasz Majewski]

Hi Wolfgang,

> Dear Stephen,
> 
> In message <545D40E1.2030700@wwwdotorg.org> you wrote:
> >
> > > 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:-)
> 
> 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.


> Best regards,
> 
> Wolfgang Denk
> 



-- 
Best regards,

Lukasz Majewski

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

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Wolfgang Denk]

Dear Lukasz,

In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
> 
> > 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!

> The problem is with umount() syscall:
...
> 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

-- 
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.

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Stephen Warren]

On 11/12/2014 09:45 AM, Wolfgang Denk wrote:
> Dear Lukasz,
>
> In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
>>
>>> 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!
>
>> The problem is with umount() syscall:
> ...
>> 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.

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Lukasz Majewski]

On Wed, 12 Nov 2014 11:11:33 -0700
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 11/12/2014 09:45 AM, Wolfgang Denk wrote:
> > Dear Lukasz,
> >
> > In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
> >>
> >>> 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!
> >
> >> The problem is with umount() syscall:
> > ...
> >> 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:

> 
> 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.

> _______________________________________________
> 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>

[U-Boot] [PATCH v2] test: ums: Add sleep before unmount directory

[Lukasz Majewski]

Hi Lukasz,

> On Wed, 12 Nov 2014 11:11:33 -0700
> Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
> > On 11/12/2014 09:45 AM, Wolfgang Denk wrote:
> > > Dear Lukasz,
> > >
> > > In message <20141112152949.1fe6ce3c@amdc2363> you wrote:
> > >>
> > >>> 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!
> > >
> > >> The problem is with umount() syscall:
> > > ...
> > >> 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:
> 
> > 
> > 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.
> 
> > _______________________________________________
> > 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.

-- 
Best regards,

Lukasz Majewski

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