Re: [LTP] [PATCH] detach_device: Wait until the kernel frees the loop device

Re: [LTP] [PATCH] detach_device: Wait until the kernel frees the loop device

[Stanislav Kholmanskikh]

[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]



On 06/10/2015 11:11 AM, Stanislav Kholmanskikh wrote:
> On systems employing the "lazy loop device removal",
> kernel commit commit a1ecac3b0656a68259927c234e505804d33a7b83
> ("loop: Make explicit loop device destruction lazy"),
> ioctl(LOOP_CLR_FD) may return 0 when the loop device is in use,
> for example by udev.
>
> Therefore, there is a chance that a quick attach/detach may not
> give udev enough time to complete, like this:
>
> open("file.img", O_RDWR)           = 4
> ioctl(3, LOOP_SET_FD, 0x4)         = 0
> close(3)                           = 0
> close(4)                           = 0
> open("/dev/loop0", O_RDONLY)       = 3
> ioctl(3, LOOP_CLR_FD, 0)           = 0
> close(3)                           = 0
> open("/dev/loop0", O_RDWR)         = 3
> open("file.img", O_RDWR)           = 4
> ioctl(3, LOOP_SET_FD, 0x4)         = -1 EBUSY (Device or resource busy)
>
> So let's wait until the kernel frees the loop device, i.e.
> when ioctl(LOOP_CLR_FD) starts failing with ENXIO.
>
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>

I tested it the attached test case (plus you need to remove 'static' 
next to attach_device and detach_device).

Without the patch it fails with:

[root@kholmanskikh test]# ./loop_test
loop_test    1  TBROK  :  tst_device.c:144: ioctl(/dev/loop0, 
LOOP_SET_FD, file.img) failed: EBUSY
loop_test    2  TBROK  :  tst_device.c:144: Remaining cases broken


[-- Attachment #2: loop_test.c --]
[-- Type: text/x-csrc, Size: 592 bytes --]

#include "test.h"
#include "safe_macros.h"

char *TCID = "loop_test";
int TST_TOTAL = 1;

void attach_device(void (*cleanup_fn)(void),
		   const char *dev, const char *file);
void detach_device(void (*cleanup_fn)(void), const char *dev);

int main(void)
{
	int i, fd;

	tst_tmpdir();

	fd = SAFE_OPEN(NULL, "file.img", O_RDWR | O_CREAT, 0644);
	SAFE_FTRUNCATE(NULL, fd, 1024 * 1024UL);

	SAFE_CLOSE(NULL, fd);

	for (i = 0; i < 1000; i++) {
		attach_device(NULL, "/dev/loop0", "file.img");
		detach_device(NULL, "/dev/loop0");
	}

	tst_rmdir();

	tst_resm(TPASS, "Finished");
	tst_exit();
}

[-- Attachment #3: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #4: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] detach_device: Wait until the kernel frees the loop device

[Cyril Hrubis]

Hi!
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  lib/tst_device.c |   17 ++++++-----------
>  1 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 3bded53..9a92fc8 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -150,30 +150,25 @@ static void attach_device(void (*cleanup_fn)(void),
>  
>  static void detach_device(void (*cleanup_fn)(void), const char *dev)
>  {
> -	int dev_fd, err, i;
> +	int dev_fd, ret, i;
>  
>  	dev_fd = SAFE_OPEN(cleanup_fn, dev, O_RDONLY);
>  
> -	/* keep trying to clear LOOPDEV fd if EBUSY, a quick succession
> +	/* keep trying to clear LOOPDEV until we get ENXIO, a quick succession
>  	 * of attach/detach might not give udev enough time to complete */
>  	for (i = 0; i < 40; i++) {
> -		if (ioctl(dev_fd, LOOP_CLR_FD, 0) == 0) {
> +		ret = ioctl(dev_fd, LOOP_CLR_FD, 0);
> +
> +		if (ret && (errno == ENXIO)) {
>  			close(dev_fd);
>  			return;
>  		}
> -		if (errno != EBUSY) {
> -			err = errno;
> -			close(dev_fd);
> -			tst_brkm(TBROK, cleanup_fn,
> -				"ioctl(%s, LOOP_CLR_FD, 0) failed: %s",
> -				dev, tst_strerrno(err));
> -		}

I would be a bit more verbose and would print an TINFO or TWARN message
if we get error different from the expected EBUSY or ENXIO.

Otherwise it looks fine, acked.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] detach_device: Wait until the kernel frees the loop device

[Cyril Hrubis]

Hi!
> Do you mean printing of TINFO/TWARN messages right in the cycle?
> 
> 		if (errno != EBUSY) {
> 			err = errno;
> 			tst_resm(TWARN,
> 				"ioctl(%s, LOOP_CLR_FD, 0) unexpectedly failed with: %s",
> 				dev, tst_strerrno(err));
> 		}
> 
> 
> But in the worst case, we may end up with 40 messages generated.
> 
> Or do you mean saving the last errno and printing it to the final tst_brkm?

Either one works for me.

Having 40 messages is better than none. Because you will see these only
if something went wrong.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list