----- Original Message -----
> From: "Stanislav Kholmanskikh" <stanislav.kholmansk...@oracle.com>
> To: "Cyril Hrubis" <chru...@suse.cz>, "Jan Stancek" <jstan...@redhat.com>
> Cc: ltp-list@lists.sourceforge.net, "vasily isaenko"
> <vasily.isae...@oracle.com>
> Sent: Tuesday, 9 June, 2015 1:51:46 PM
> Subject: Re: [LTP] [RFC PATCH 5/5] acl_test01: Wait until the loop device is
> detached
>
>
>
> On 06/09/2015 01:10 PM, Cyril Hrubis wrote:
> > Hi!
> >> If the test is executed on NFS, it may print:
> >>
> >> rm: cannot remove `/mnt/ltp-xeJYpESVKz/acltest01.fCogoo4gn0': Directory
> >> not empty
> >>
> >> It seems the following happen:
> >>
> >> 1. After 'unmount -d' 'acltest01.fCogoo4gn0/blkext3' is still in use by
> >> udev.
> >> See kernel commit a1ecac3b0656a68259927c234e505804d33a7b83
> >> ("loop: Make explicit loop device destruction lazy")
> >>
> >> 2. Due to the "NFS silly rename" feature, command 'rm -f
> >> acltest01.fCogoo4gn0/blkext3'
> >> renames 'blkext3' to '.nfs*'
> >>
> >> 3. The removal of 'acltest01.fCogoo4gn0' directory fails, because it
> >> still contains this '.nfs*' file.
> >>
> >> Intorucing a wait cycle should fix this problem.
> >
> > Hmm, we may want to add this into test.sh because otherwise we will need
> > to carry this snippet of code in each testcase that works with loop
> > devices.
> >
> > Would calling umount without -d and detach_device() (from tst_device.c
> > via apicmd) help here? (it seems to fix the very same problem, it this
> > function does not work, it needs to be fixed anyway)
> >
>
> I think it wouldn't. Please, see below.
>
> ## Problem 1 ##
> Rapid execution of 'losetup /dev/loop0' file + 'losetup -d /dev/loop0'
> may fail.
>
> Reproducer:
> for i in `seq 20`; do
> echo "attach"
> losetup /dev/loop0 file1.img
> echo "detach"
> losetup -d /dev/loop0
> done
>
> 1. Kernel versions without the above kernel patch should fail on the
> "detach" phase, i.e. on
> if (lo->lo_refcnt > 1) /* we needed one fd for the ioctl */
> return -EBUSY;
> in loop_clr_fd().
>
> However, I've not tested it by myself.
>
> And Jan's commit:
>
> commit 554b1793a1aa34f8918e8b1c04d8c6835f4a5710
> Author: Jan Stancek <jstan...@redhat.com>
> Date: Tue Jan 20 14:35:49 2015 +0100
>
> tst_device: sleep/retry if LOOP_CLR_FD fails with EBUSY
>
> Rapid succession of device attach/detach may lead to EBUSY
> from ioctl(LOOP_CLR_FD), because udev rules might still be
> running.
>
> Sleep/retry for a short period if LOOP_CLR_FD fails with EBUSY.
>
> Signed-off-by: Jan Stancek <jstan...@redhat.com>
> Acked-by: Cyril Hrubis <chru...@suse.cz>
>
> is to fix this scenario.
>
> 2. However, newer kernels (i.e. with the above kernel patch) will fail
> with EBUSY at the "attach" phase, because loop_clr_fd() will always
> return 0 and mark the loop device for auto clearing, but loop_set_fd()
> will go this path:
>
> error = -EBUSY;
> if (lo->lo_state != Lo_unbound)
> goto out_putf;
>
> /* ... */
>
> out_putf:
> fput(file);
> out:
> /* This is safe: open() is still holding a reference. */
> module_put(THIS_MODULE);
> return error;
>
> ## Problem 2 - NFS specific ##
>
> If we have the above kernel patch, even if 'losetup -d' executed without
> problems, it doesn't mean that the backend file is not in use.
>
> So we need to wait some undetermined time until udev finishes its
> playing with the loop device, and only after that we can safely remove
> the file without a risk to be affected by "NFS silly rename".
>
> ##
>
> I think that to nail down both the problems at once, we may need to do
> something like this:
>
> attach_device(const char *dev)
> {
> /* a cycle to verify that dev is not in use.
> * If it's in use after the cycle, call tst_brkm()
> */
> ioctl(dev_fd, LOOP_SET_FD);
> /* handle ioctl errors */
> }
>
> detach_device(const char *dev)
> {
> ioctl(dev_fd, LOOP_CLR_FD);
> /* ignore the return code */
>
> /* a cycle to verify that dev is not in use.
> * If it's still in use after the cycle, call tst_brkm()
> */
> }
>
> ioctl(LOOP_GET_STATUS64) looks like a good way to check whether the loop
> device is in use.
>
> What do you think?
A cycle in detach_device() to make sure it's no longer in use
on kernels which have lazy cleanup seems reasonable to me.
But I'm not following, why is it needed to have same cycle also in
attach_device()?
Regards,
Jan
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
