* [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd
@ 2024-08-15 6:37 Jan Stancek
2024-08-15 7:29 ` Petr Vorel
2024-08-16 7:00 ` Li Wang
0 siblings, 2 replies; 4+ messages in thread
From: Jan Stancek @ 2024-08-15 6:37 UTC (permalink / raw)
To: ltp; +Cc: liwan, gulam.mohamed
Since commit 18048c1af783 ("loop: Fix a race between loop detach and loop open")
detach operation is deferred to the last close of the device.
Make tst_detach_device_by_fd() also close dev_fd, and leave it up to
caller to re-open it for further use.
Reported-by: Gulam Mohamed <gulam.mohamed@oracle.com>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
include/tst_device.h | 4 +++-
lib/tst_device.c | 10 ++++++++--
testcases/kernel/syscalls/ioctl/ioctl09.c | 1 +
testcases/kernel/syscalls/ioctl/ioctl_loop01.c | 1 +
testcases/kernel/syscalls/ioctl/ioctl_loop02.c | 1 +
testcases/kernel/syscalls/ioctl/ioctl_loop04.c | 1 +
testcases/kernel/syscalls/ioctl/ioctl_loop06.c | 5 ++++-
testcases/kernel/syscalls/ioctl/ioctl_loop07.c | 2 ++
8 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/include/tst_device.h b/include/tst_device.h
index 391fb4e568f1..2597fb4e235e 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -67,7 +67,9 @@ int tst_attach_device(const char *dev_path, const char *file_path);
uint64_t tst_get_device_size(const char *dev_path);
/*
- * Detaches a file from a loop device fd.
+ * Detaches a file from a loop device fd. @dev_fd needs to be the
+ * last descriptor opened. Call to this function will close it,
+ * it is up to caller to open it again for further usage.
*
* @dev_path Path to the loop device e.g. /dev/loop0
* @dev_fd a open fd for the loop device
diff --git a/lib/tst_device.c b/lib/tst_device.c
index d659b54cfdee..723f6ca06375 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -243,17 +243,23 @@ int tst_detach_device_by_fd(const char *dev, int dev_fd)
/* keep trying to clear LOOPDEV until we get ENXIO, a quick succession
* of attach/detach might not give udev enough time to complete
+ *
+ * Since 18048c1af783 ("loop: Fix a race between loop detach and loop open")
+ * device is detached only after last close.
*/
for (i = 0; i < 40; i++) {
ret = ioctl(dev_fd, LOOP_CLR_FD, 0);
- if (ret && (errno == ENXIO))
+ if (ret && (errno == ENXIO)) {
+ SAFE_CLOSE(NULL, dev_fd);
return 0;
+ }
if (ret && (errno != EBUSY)) {
tst_resm(TWARN,
"ioctl(%s, LOOP_CLR_FD, 0) unexpectedly failed with: %s",
dev, tst_strerrno(errno));
+ SAFE_CLOSE(NULL, dev_fd);
return 1;
}
@@ -262,6 +268,7 @@ int tst_detach_device_by_fd(const char *dev, int dev_fd)
tst_resm(TWARN,
"ioctl(%s, LOOP_CLR_FD, 0) no ENXIO for too long", dev);
+ SAFE_CLOSE(NULL, dev_fd);
return 1;
}
@@ -276,7 +283,6 @@ int tst_detach_device(const char *dev)
}
ret = tst_detach_device_by_fd(dev, dev_fd);
- close(dev_fd);
return ret;
}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl09.c b/testcases/kernel/syscalls/ioctl/ioctl09.c
index ca682ee9ad71..9c79210864b8 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl09.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl09.c
@@ -88,6 +88,7 @@ static void verify_ioctl(void)
check_partition(2, true);
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
attach_flag = 0;
}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
index a4d191a2e982..2f0df3b27972 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -93,6 +93,7 @@ static void verify_ioctl_loop(void)
check_loop_value(0, LO_FLAGS_PARTSCAN, 0);
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
attach_flag = 0;
}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
index 380fd10069ab..fa193924a448 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
@@ -103,6 +103,7 @@ static void verify_ioctl_loop(unsigned int n)
SAFE_CLOSE(file_fd);
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
attach_flag = 0;
}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop04.c b/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
index 5b7506ea44df..f1021cc77d7b 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
@@ -63,6 +63,7 @@ static void verify_ioctl_loop(void)
SAFE_CLOSE(file_fd);
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
unlink("test.img");
attach_flag = 0;
}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
index 64800b4ee44b..317f693a0458 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
@@ -57,8 +57,10 @@ static void verify_ioctl_loop(unsigned int n)
if (TST_RET == 0) {
tst_res(TFAIL, "Set block size succeed unexpectedly");
- if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
+ if (tcases[n].ioctl_flag == LOOP_CONFIGURE) {
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+ }
return;
}
if (TST_ERR == EINVAL)
@@ -87,6 +89,7 @@ static void run(unsigned int n)
}
if (attach_flag) {
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
attach_flag = 0;
}
loopconfig.block_size = *(tc->setvalue);
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop07.c b/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
index d44f36212f5b..68db79558fb4 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
@@ -73,6 +73,7 @@ static void verify_ioctl_loop(unsigned int n)
/*Reset*/
if (tc->ioctl_flag == LOOP_CONFIGURE) {
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
} else {
loopinfo.lo_sizelimit = 0;
TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
@@ -101,6 +102,7 @@ static void run(unsigned int n)
}
if (attach_flag) {
tst_detach_device_by_fd(dev_path, dev_fd);
+ dev_fd = SAFE_OPEN(dev_path, O_RDWR);
attach_flag = 0;
}
loopconfig.info.lo_sizelimit = tc->set_sizelimit;
--
2.43.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd
2024-08-15 6:37 [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd Jan Stancek
@ 2024-08-15 7:29 ` Petr Vorel
2024-08-19 13:11 ` Jan Stancek
2024-08-16 7:00 ` Li Wang
1 sibling, 1 reply; 4+ messages in thread
From: Petr Vorel @ 2024-08-15 7:29 UTC (permalink / raw)
To: Jan Stancek; +Cc: liwan, gulam.mohamed, ltp
Hi Jan,
thanks a lot for fixing this!
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>
Closes: https://github.com/linux-test-project/ltp/issues/1175
> Since commit 18048c1af783 ("loop: Fix a race between loop detach and loop open")
> detach operation is deferred to the last close of the device.
> Make tst_detach_device_by_fd() also close dev_fd, and leave it up to
> caller to re-open it for further use.
6.11.0-rc3-2.g00af0c0-default
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd
2024-08-15 6:37 [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd Jan Stancek
2024-08-15 7:29 ` Petr Vorel
@ 2024-08-16 7:00 ` Li Wang
1 sibling, 0 replies; 4+ messages in thread
From: Li Wang @ 2024-08-16 7:00 UTC (permalink / raw)
To: Jan Stancek; +Cc: gulam.mohamed, ltp
On Thu, Aug 15, 2024 at 2:37 PM Jan Stancek <jstancek@redhat.com> wrote:
> Since commit 18048c1af783 ("loop: Fix a race between loop detach and loop
> open")
> detach operation is deferred to the last close of the device.
>
> Make tst_detach_device_by_fd() also close dev_fd, and leave it up to
> caller to re-open it for further use.
>
> Reported-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>
Reviewed-by: Li Wang <liwang@redhat.com>
---
> include/tst_device.h | 4 +++-
> lib/tst_device.c | 10 ++++++++--
> testcases/kernel/syscalls/ioctl/ioctl09.c | 1 +
> testcases/kernel/syscalls/ioctl/ioctl_loop01.c | 1 +
> testcases/kernel/syscalls/ioctl/ioctl_loop02.c | 1 +
> testcases/kernel/syscalls/ioctl/ioctl_loop04.c | 1 +
> testcases/kernel/syscalls/ioctl/ioctl_loop06.c | 5 ++++-
> testcases/kernel/syscalls/ioctl/ioctl_loop07.c | 2 ++
> 8 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 391fb4e568f1..2597fb4e235e 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -67,7 +67,9 @@ int tst_attach_device(const char *dev_path, const char
> *file_path);
> uint64_t tst_get_device_size(const char *dev_path);
>
> /*
> - * Detaches a file from a loop device fd.
> + * Detaches a file from a loop device fd. @dev_fd needs to be the
> + * last descriptor opened. Call to this function will close it,
> + * it is up to caller to open it again for further usage.
> *
> * @dev_path Path to the loop device e.g. /dev/loop0
> * @dev_fd a open fd for the loop device
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index d659b54cfdee..723f6ca06375 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -243,17 +243,23 @@ int tst_detach_device_by_fd(const char *dev, int
> dev_fd)
>
> /* keep trying to clear LOOPDEV until we get ENXIO, a quick
> succession
> * of attach/detach might not give udev enough time to complete
> + *
> + * Since 18048c1af783 ("loop: Fix a race between loop detach and
> loop open")
> + * device is detached only after last close.
> */
> for (i = 0; i < 40; i++) {
> ret = ioctl(dev_fd, LOOP_CLR_FD, 0);
>
> - if (ret && (errno == ENXIO))
> + if (ret && (errno == ENXIO)) {
> + SAFE_CLOSE(NULL, dev_fd);
> return 0;
> + }
>
> if (ret && (errno != EBUSY)) {
> tst_resm(TWARN,
> "ioctl(%s, LOOP_CLR_FD, 0) unexpectedly
> failed with: %s",
> dev, tst_strerrno(errno));
> + SAFE_CLOSE(NULL, dev_fd);
> return 1;
> }
>
> @@ -262,6 +268,7 @@ int tst_detach_device_by_fd(const char *dev, int
> dev_fd)
>
> tst_resm(TWARN,
> "ioctl(%s, LOOP_CLR_FD, 0) no ENXIO for too long", dev);
> + SAFE_CLOSE(NULL, dev_fd);
> return 1;
> }
>
> @@ -276,7 +283,6 @@ int tst_detach_device(const char *dev)
> }
>
> ret = tst_detach_device_by_fd(dev, dev_fd);
> - close(dev_fd);
> return ret;
> }
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl09.c
> b/testcases/kernel/syscalls/ioctl/ioctl09.c
> index ca682ee9ad71..9c79210864b8 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl09.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl09.c
> @@ -88,6 +88,7 @@ static void verify_ioctl(void)
> check_partition(2, true);
>
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> index a4d191a2e982..2f0df3b27972 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
> @@ -93,6 +93,7 @@ static void verify_ioctl_loop(void)
> check_loop_value(0, LO_FLAGS_PARTSCAN, 0);
>
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> index 380fd10069ab..fa193924a448 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
> @@ -103,6 +103,7 @@ static void verify_ioctl_loop(unsigned int n)
>
> SAFE_CLOSE(file_fd);
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> index 5b7506ea44df..f1021cc77d7b 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop04.c
> @@ -63,6 +63,7 @@ static void verify_ioctl_loop(void)
>
> SAFE_CLOSE(file_fd);
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> unlink("test.img");
> attach_flag = 0;
> }
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> index 64800b4ee44b..317f693a0458 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> @@ -57,8 +57,10 @@ static void verify_ioctl_loop(unsigned int n)
>
> if (TST_RET == 0) {
> tst_res(TFAIL, "Set block size succeed unexpectedly");
> - if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
> + if (tcases[n].ioctl_flag == LOOP_CONFIGURE) {
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> + }
> return;
> }
> if (TST_ERR == EINVAL)
> @@ -87,6 +89,7 @@ static void run(unsigned int n)
> }
> if (attach_flag) {
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
> loopconfig.block_size = *(tc->setvalue);
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> b/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> index d44f36212f5b..68db79558fb4 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop07.c
> @@ -73,6 +73,7 @@ static void verify_ioctl_loop(unsigned int n)
> /*Reset*/
> if (tc->ioctl_flag == LOOP_CONFIGURE) {
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> } else {
> loopinfo.lo_sizelimit = 0;
> TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo),
> TST_RETVAL_EQ0);
> @@ -101,6 +102,7 @@ static void run(unsigned int n)
> }
> if (attach_flag) {
> tst_detach_device_by_fd(dev_path, dev_fd);
> + dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> attach_flag = 0;
> }
> loopconfig.info.lo_sizelimit = tc->set_sizelimit;
> --
> 2.43.0
>
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd
2024-08-15 7:29 ` Petr Vorel
@ 2024-08-19 13:11 ` Jan Stancek
0 siblings, 0 replies; 4+ messages in thread
From: Jan Stancek @ 2024-08-19 13:11 UTC (permalink / raw)
To: Petr Vorel; +Cc: liwan, gulam.mohamed, ltp
On Thu, Aug 15, 2024 at 9:29 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Jan,
>
> thanks a lot for fixing this!
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
> Closes: https://github.com/linux-test-project/ltp/issues/1175
Pushed.
>
> > Since commit 18048c1af783 ("loop: Fix a race between loop detach and loop open")
> > detach operation is deferred to the last close of the device.
>
> > Make tst_detach_device_by_fd() also close dev_fd, and leave it up to
> > caller to re-open it for further use.
>
> 6.11.0-rc3-2.g00af0c0-default
>
> Kind regards,
> Petr
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-08-19 13:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15 6:37 [LTP] [PATCH] lib: make tst_detach_device_by_fd() also close dev_fd Jan Stancek
2024-08-15 7:29 ` Petr Vorel
2024-08-19 13:11 ` Jan Stancek
2024-08-16 7:00 ` Li Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox