[PATCH v4 9/9] loop: Add LOOP_CONFIGURE ioctl
LOOP_CONFIGURE is added to Linux 5.8
This allows userspace to completely setup a loop device with a single
ioctl, removing the in-between state where the device can be partially
configured - eg the loop device has a backing file associated with it,
but is reading from the wrong offset.
https://lwn.net/Articles/820408/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
Builders can use config to choose which algorithm to build into their busybox
binary
kernel version >= 5.8, choice CONFIG_LOOP_CONFIGURE
function old new delta
set_loop 716 639 -77
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-77) Total:
-77 bytes
kernel version < 5.8, choice CONFIG_NO_LOOP_CONFIGURE
function old new delta
--
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total:
0 bytes
kernel version is unknown, choice CONFIG_TRY_LOOP_CONFIGURE
function old new delta
set_loop 716 832+116
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 116/0) Total:
116 bytes
Signed-off-by: Xiaoming Ni
---
libbb/Config.src | 22 ++
libbb/loop.c | 45 +
2 files changed, 67 insertions(+)
diff --git a/libbb/Config.src b/libbb/Config.src
index 66a3ffa23..b7f9ddab4 100644
--- a/libbb/Config.src
+++ b/libbb/Config.src
@@ -369,3 +369,25 @@ config UNICODE_PRESERVE_BROKEN
For example, this means that entering 'l', 's', ' ', 0xff, [Enter]
at shell prompt will list file named 0xff (single char name
with char value 255), not file named '?'.
+
+choice
+ prompt "LOOP_CONFIGURE or LOOP_SET_FD + LOOP_SET_STATUS"
+ default TRY_LOOP_CONFIGURE
+ help
+ LOOP_CONFIGURE is added to Linux 5.8
+ https://lwn.net/Articles/820408/
+ This allows userspace to completely setup a loop device with a single
+ ioctl, removing the in-between state where the device can be partially
+ configured - eg the loop device has a backing file associated with it,
+ but is reading from the wrong offset.
+
+config LOOP_CONFIGURE
+ bool "always uses LOOP_CONFIGURE, kernel version >= 5.8"
+
+config NO_LOOP_CONFIGURE
+ bool "never uses LOOP_CONFIGURE, kernel version < 5.8"
+
+config TRY_LOOP_CONFIGURE
+ bool "try LOOP_CONFIGURE, kernel version is unknown"
+
+endchoice
diff --git a/libbb/loop.c b/libbb/loop.c
index 03b73e658..9316c6b44 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -129,6 +129,7 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+#ifndef CONFIG_LOOP_CONFIGURE
static int set_loop_fd_and_status(int ffd, int lfd, const bb_loop_info
*loopinfo)
{
int rc;
@@ -152,6 +153,46 @@ static int set_loop_fd_and_status(int ffd, int lfd, const
bb_loop_info *loopinfo
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
return -1;
}
+#endif
+
+#ifndef CONFIG_NO_LOOP_CONFIGURE
+
+#ifndef LOOP_CONFIGURE
+#define LOOP_CONFIGURE 0x4C0A
+struct loop_config {
+ uint32_t fd;
+ uint32_t block_size;
+ struct loop_info64 info;
+ uint64_t __reserved[8];
+};
+#endif
+
+/*
+ * linux v5.8.0
+ * loop: Add LOOP_CONFIGURE ioctl
+ * https://lwn.net/Articles/820408/
+ *
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
+ */
+static int set_loop_configure(int ffd, int lfd, const bb_loop_info *loopinfo)
+{
+ int rc;
+ struct loop_config config;
+
+ memset(, 0, sizeof(config));
+ config.fd = ffd;
+ memcpy(, loopinfo, sizeof(config.info));
+
+ rc = ioctl(lfd, LOOP_CONFIGURE, );
+ if (rc == 0) {
+ return lfd;
+ }
+#ifdef CONFIG_TRY_LOOP_CONFIGURE
+ if (rc == -1 && errno == EINVAL) /* The system may not support
LOOP_CONFIGURE. */
+ return set_loop_fd_and_status(ffd, lfd, loopinfo);
+#endif
+ return -1;
+}
+#endif
static int set_loop_info(int ffd, int lfd, const bb_loop_info *loopinfo)
{
@@ -162,7 +203,11 @@ static int set_loop_info(int ffd, int lfd, const
bb_loop_info *loopinfo)
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
+#ifdef CONFIG_NO_LOOP_CONFIGURE
return set_loop_fd_and_status(ffd, lfd, loopinfo);
+#else
+ return set_loop_configure(ffd
[PATCH v4 7/9] loop:refactor: Extract subfunction do_stat_and_mknod()
Step 7 of micro-refactoring the set_loop():
Extract subfunction do_stat_and_mknod()
function old new delta
set_loop 720 700 -20
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20) Total: -20 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 38 --
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 5f0db4b18..ec4fcf883 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -183,6 +183,27 @@ static void init_bb_loop_info(bb_loop_info *loopinfo,
const char *file,
loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
}
+static int do_stat_and_mknod(const char *dev, const char *device, int i)
+{
+ struct stat statbuf;
+
+ IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
+ if (stat(dev, ) == 0 && S_ISBLK(statbuf.st_mode))
+ return 0;
+ if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
+ && errno == ENOENT
+ && (!device)
+ ) {
+ /* Node doesn't exist, try to create it */
+ if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0) {
+ return 0;
+ }
+ }
+ /* Ran out of block devices, return failure */
+ return -1;
+}
+
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -192,7 +213,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- struct stat statbuf;
int i, lfd, ffd, mode, rc;
bb_loop_info loopinfo;
@@ -219,18 +239,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
}
- IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
- if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
- if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
-&& errno == ENOENT
-&& (!*device)
- ) {
- /* Node doesn't exist, try to create it */
- if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
- goto open_lfd;
- }
- /* Ran out of block devices, return failure */
- rc = -1;
+ rc = do_stat_and_mknod(try, *device, i);
+ if (rc == -1) {
break;
}
open_lfd:
@@ -252,7 +262,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc == lfd) {
/* SUCCESS! */
if (!*device)
- *device = xstrdup(dev);
+ *device = xstrdup(try);
break;
}
close(lfd);
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v4 5/9] loop:refactor: extract subfunction set_loop_configure()
Step 5 of micro-refactoring the set_loop():
Extract subfunction set_loop_configure()
function old new delta
set_loop 700 708 +8
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 8/0) Total: 8 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 63 ++--
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 5dd4d4e59..0d4d0398c 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -129,6 +129,41 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_configure(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return -1;
+}
+
static int set_loop_info(int ffd, int lfd, const char *file,
unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
{
@@ -139,33 +174,7 @@ static int set_loop_info(int ffd, int lfd, const char
*file,
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- return -1;
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
- /* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- }
- if (rc == 0) {
- return lfd;
- }
- /* failure, undo LOOP_SET_FD */
- ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
}
return -1;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v4 6/9] loop:refactor: Use a structure to reduce parameters
Step 6 of micro-refactoring the set_loop():
Use structs to avoid transferring a large number of parameters
in set_loop_configure() and set_loop_info()
function old new delta
set_loop 708 720 +12
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0) Total: 12 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 54 +---
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 0d4d0398c..5f0db4b18 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -129,32 +129,21 @@ static int open_file(const char *file, unsigned flags,
int *mode)
return ffd;
}
-static int set_loop_configure(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_fd_and_status(int ffd, int lfd, const bb_loop_info
*loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info loopinfo2;
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
return -1;
}
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
+ if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ memcpy(, loopinfo, sizeof(*loopinfo));
/* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
/* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
}
if (rc == 0) {
return lfd;
@@ -164,21 +153,36 @@ static int set_loop_configure(int ffd, int lfd, const
char *file,
return -1;
}
-static int set_loop_info(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_info(int ffd, int lfd, const bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info tmp;
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
+ return set_loop_fd_and_status(ffd, lfd, loopinfo);
}
return -1;
}
+static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ memset(loopinfo, 0, sizeof(*loopinfo));
+ safe_strncpy((char *)loopinfo->lo_file_name, file, LO_NAME_SIZE);
+ loopinfo->lo_offset = offset;
+ loopinfo->lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -190,12 +194,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
char *try;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
+ bb_loop_info loopinfo;
ffd = open_file(file, flags, );
if (ffd < 0) {
return -errno;
}
+ init_bb_loop_info(, file, offset, sizelimit, flags);
try = *device;
if (!try) {
try = dev;
@@ -242,7 +248,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
[PATCH v4 2/9] loop:refactor: extract subfunction get_next_free_loop()
Step 2 of micro-refactoring the set_loop function ()
Extract subfunction get_next_free_loop() from set_loop()
Also fix miss free(try) when stat(try) and mknod fail
function old new delta
set_loop 758 734 -24
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24 bytes
Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 58 +---
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index c517ceb13..6c28e683a 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -9,6 +9,7 @@
*/
#include "libbb.h"
#include
+#include
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
@@ -96,6 +97,22 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int get_next_free_loop(char *dev, __attribute__((unused)) size_t
dev_size, int id)
+{
+ int loopdevno;
+ assert(dev_size >= LOOP_NAMESIZE);
+ loopdevno = get_free_loop();
+ if (loopdevno >= 0) {
+ sprintf(dev, LOOP_FORMAT, loopdevno);
+ return 1; /* use /dev/loop-control */
+ }
+ if (loopdevno == -2) {
+ sprintf(dev, LOOP_FORMAT, id);
+ return 2;
+ }
+ return -1; /* no free loop devices */
+}
+
static int open_file(const char *file, unsigned flags, int *mode)
{
int ffd;
@@ -132,30 +149,26 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
try = *device;
if (!try) {
- get_free_loopN:
- i = get_free_loop();
- if (i == -1) {
- close(ffd);
- return -1; /* no free loop devices */
- }
- if (i >= 0) {
- try = xasprintf(LOOP_FORMAT, i);
- goto open_lfd;
- }
- /* i == -2: no /dev/loop-control. Do an old-style search for a
free device */
try = dev;
}
/* Find a loop device */
/* 0xf is a max possible minor number in Linux circa 2010 */
for (i = 0; i <= 0xf; i++) {
- sprintf(dev, LOOP_FORMAT, i);
+ if (!*device) {
+ rc = get_next_free_loop(dev, sizeof(dev), i);
+ if (rc == -1) {
+ break; /* no free loop devices */
+ } else if (rc == 1) {
+ goto open_lfd;
+ }
+ }
IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
&& errno == ENOENT
-&& try == dev
+&& (!*device)
) {
/* Node doesn't exist, try to create it */
if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
@@ -188,13 +201,10 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
/* Ouch. Are we racing with other mount? */
- if (!*device /* yes */
-&& try != dev /* tried a _kernel-offered_
loopN? */
- ) {
- free(try);
+ if (!*device) {
close(lfd);
//TODO: add "if (--failcount != 0) ..."?
- goto get_free_loopN;
+ continue;
}
goto close_and_try_next_loopN;
}
@@ -218,8 +228,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
if (rc == 0) {
/* SUCCESS! */
- if (try != dev) /* tried a kernel-offered free
loopN? */
- *device = try; /* malloced */
if (!*device) /* was looping in search of
free "/dev/loopN"? */
*device = xstrdup(dev);
rc = lfd; /* return this */
@@ -227,16 +235,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsi
[PATCH v4 8/9] loop:refactor: extract subfunction set_loop_dev()
Step 8 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop_dev()
function old new delta
set_loop 700 716 +16
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 16/0) Total: 16 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 39 ---
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index ec4fcf883..03b73e658 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -203,6 +203,26 @@ static int do_stat_and_mknod(const char *dev, const char
*device, int i)
return -1;
}
+static int set_loop_dev(const char *dev, int *mode, int ffd, const
bb_loop_info *loopinfo)
+{
+ int rc;
+ /* Open the sucker and check its loopiness */
+ int lfd = open(dev, *mode);
+ if (lfd < 0 && errno == EROFS) {
+ *mode = O_RDONLY;
+ lfd = open(dev, *mode);
+ }
+ if (lfd < 0) {
+ return lfd;
+ }
+ rc = set_loop_info(ffd, lfd, loopinfo);
+ if (rc == lfd) {
+ return lfd;
+ }
+ close(lfd);
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
@@ -213,7 +233,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- int i, lfd, ffd, mode, rc;
+ int i, ffd, mode, rc;
bb_loop_info loopinfo;
ffd = open_file(file, flags, );
@@ -244,30 +264,19 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
break;
}
open_lfd:
- /* Open the sucker and check its loopiness */
- lfd = rc = open(try, mode);
- if (lfd < 0 && errno == EROFS) {
- mode = O_RDONLY;
- lfd = rc = open(try, mode);
- }
- if (lfd < 0) {
+ rc = set_loop_dev(try, , ffd, );
+ if (rc == -1) {
if (errno == ENXIO) {
/* Happens if loop module is not loaded */
/* rc is -1; */
break;
}
- goto try_next_loopN;
- }
- rc = set_loop_info(ffd, lfd, );
- if (rc == lfd) {
+ } else {
/* SUCCESS! */
if (!*device)
*device = xstrdup(try);
break;
}
- close(lfd);
- try_next_loopN:
- rc = -1;
if (*device) /* was looking for a particular "/dev/loopN"? */
break; /* yes, do not try other names */
} /* for() */
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v4 4/9] loop:refactor: extract subfunction set_loop_info()
Step 4 of micro-refactoring the set_loop():
Extract subfunction set_loop_info() from set_loop()
function old new delta
set_loop 734 700 -34
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-34) Total: -34 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 91 +++-
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index c7687052b..5dd4d4e59 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -129,6 +129,47 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_info(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+
+ /* If device is free, try to claim it */
+ if (rc && errno == ENXIO) {
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ }
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -138,7 +179,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- bb_loop_info loopinfo;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
@@ -193,49 +233,12 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
-
- /* If device is free, try to claim it */
- if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- close(lfd);
- /* Ouch. Are we racing with other mount? */
- if (!*device) {
-//TODO: add "if (--failcount != 0) ..."?
- continue;
- } else {
- break;
- }
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is
controlled by open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before
mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags &
BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support
LO_FLAGS_AUTOCLEAR? */
-
[PATCH v4 0/9] loop: Micro-refactoring set_loop() and add LOOP_CONFIGURE
LOOP_CONFIGURE is added to Linux 5.8 This allows userspace to completely setup a loop device with a single ioctl, removing the in-between state where the device can be partially configured - eg the loop device has a backing file associated with it, but is reading from the wrong offset. https://lwn.net/Articles/820408/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5 patch 1-8: To facilitate the addition of new functions, the code of set_loop() is micro-refactored. function old new delta set_loop 760 716 -44 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44) Total: -44 bytes patch 9: add LOOP_CONFIGURE Builders can use config to choose which algorithm to build into their busybox binary --- v4: Fine-tuned based on review comments patch2: add assert(dev_size >= LOOP_NAMESIZE) in get_next_free_loop(). patch6,8,9: The const modifier is added to the "bb_loop_info *loopinfo" parameter. v3: http://lists.busybox.net/pipermail/busybox/2022-November/090009.html Fine-tuned based on review comments patch2: Rename local variables, use "loopdevno" instead of "i" in get_next_free_loop(). patch7: Reduce indentation using guard statements in do_stat_and_mknod(). patch9: Rename set_loop_configure_old() to set_loop_fd_and_status() v2: http://lists.busybox.net/pipermail/busybox/2022-November/089990.html changes in patch9: add config for builder to select v1: http://lists.busybox.net/pipermail/busybox/2022-November/089969.html --- Xiaoming Ni (9): loop:refactor: extract subfunction open_file() loop:refactor: extract subfunction get_next_free_loop() loop:refactor: del close_and_try_next_loopN loop:refactor: extract subfunction set_loop_info() loop:refactor: extract subfunction set_loop_configure() loop:refactor: Use a structure to reduce parameters loop:refactor: Extract subfunction do_stat_and_mknod() loop:refactor: extract subfunction set_loop_dev() loop: Add LOOP_CONFIGURE ioctl libbb/Config.src | 22 libbb/loop.c | 295 ++- 2 files changed, 214 insertions(+), 103 deletions(-) -- 2.27.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v4 1/9] loop:refactor: extract subfunction open_file()
Step 1 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop()
function old new delta
set_loop 760 758 -2
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2) Total: -2 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 750642ade..c517ceb13 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,22 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int open_file(const char *file, unsigned flags, int *mode)
+{
+ int ffd;
+ /* open the file. barf if this doesn't work. */
+ *mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
+ retry_open_ffd:
+ ffd = open(file, *mode);
+ if (ffd < 0) {
+ if (*mode != O_RDONLY) {
+ *mode = O_RDONLY;
+ goto retry_open_ffd;
+ }
+ }
+ return ffd;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -109,15 +125,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
struct stat statbuf;
int i, lfd, ffd, mode, rc;
- /* Open the file. Barf if this doesn't work. */
- mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
- open_ffd:
- ffd = open(file, mode);
+ ffd = open_file(file, flags, );
if (ffd < 0) {
- if (mode != O_RDONLY) {
- mode = O_RDONLY;
- goto open_ffd;
- }
return -errno;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v4 3/9] loop:refactor: del close_and_try_next_loopN
Step 3 of micro-refactoring the set_loop() function:
Delete close_and_try_next_loopN.
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total: 0 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 6c28e683a..c7687052b 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -200,13 +200,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc && errno == ENXIO) {
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ close(lfd);
/* Ouch. Are we racing with other mount? */
if (!*device) {
- close(lfd);
//TODO: add "if (--failcount != 0) ..."?
continue;
+ } else {
+ break;
}
- goto close_and_try_next_loopN;
}
memset(, 0, sizeof(loopinfo));
safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
@@ -236,7 +237,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
}
- close_and_try_next_loopN:
close(lfd);
try_next_loopN:
rc = -1;
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2 2/9] loop:refactor: extract subfunction get_next_free_loop()
On 2022/11/21 15:50, Kang-Che Sung wrote:
On Mon, Nov 21, 2022 at 9:19 AM Xiaoming Ni wrote:
static int get_next_free_loop(char *dev, size_t dev_size, int id)
{
int loopdevno = get_free_loop();
if (loopdevno >= 0) {
snprintf(dev, dev_size, LOOP_FORMAT, loopdevno);
return 1; /* use /dev/loop-control */
}
if (loopdevno == -2) {
snprintf(dev, dev_size, LOOP_FORMAT, id);
return 2;
}
return -1; /* no free loop devices */
}
If the dev_size parameter is added to get_next_free_loop(), the code
size increases, Is it worth?
function old new delta
set_loop 734 744 +10
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 10/0) Total: 10
bytes
No, that isn't what I mean. sprintf() is faster than snprintf() when
we are sure the string buffer would never overflow.
Just keep using sprintf() here but add a statement before it:
`assert(dev_size >= LOOP_NAMESIZE);`
.
thanks
I'll add const modifiers in the next version.
Thanks
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2 6/9] loop:refactor: Use a structure to reduce parameters
On 2022/11/21 12:28, Kang-Che Sung wrote:
On Mon, Nov 21, 2022 at 9:31 AM Xiaoming Ni wrote:
Also, it is unclear why there is the need to clone the loopinfo buffer.
> /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
> /* (this code path is not tested) */
> - loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> - rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> + loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> + rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> }
> if (rc == 0) {
> return lfd;
...
Pardon for my ignorance, but does the LOOP_SET_STATUS64 ioctl modify
the `loopinfo` object internally?
in linux kernel, drivers/block/loop.c:
static int
loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
static int
loop_set_status64(struct loop_device *lo, const struct loop_info64
__user *arg)
If the answer is yes, then it might not be a good idea to pass the
`loopinfo` structure to set_loop_configure().
I think it might be better to create the object on the fly (i.e. drop
this patch).
Otherwise, let set_loop_configure pass in a `const bb_loop_info *`
object, when we are sure it would never be modified internally.
thanks
I'll add const modifiers in the next version.
Thanks
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v3 6/9] loop:refactor: Use a structure to reduce parameters
Step 6 of micro-refactoring the set_loop():
Use structs to avoid transferring a large number of parameters
in set_loop_configure() and set_loop_info()
function old new delta
set_loop 708 720 +12
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0) Total: 12 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 54 +---
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index dc8ecc9a8..0b1336a02 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,32 +126,21 @@ static int open_file(const char *file, unsigned flags,
int *mode)
return ffd;
}
-static int set_loop_configure(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info loopinfo2;
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
return -1;
}
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
+ if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ memcpy(, loopinfo, sizeof(*loopinfo));
/* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
/* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
}
if (rc == 0) {
return lfd;
@@ -161,21 +150,36 @@ static int set_loop_configure(int ffd, int lfd, const
char *file,
return -1;
}
-static int set_loop_info(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info tmp;
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
+ return set_loop_configure(ffd, lfd, loopinfo);
}
return -1;
}
+static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ memset(loopinfo, 0, sizeof(*loopinfo));
+ safe_strncpy((char *)loopinfo->lo_file_name, file, LO_NAME_SIZE);
+ loopinfo->lo_offset = offset;
+ loopinfo->lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -187,12 +191,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
char *try;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
+ bb_loop_info loopinfo;
ffd = open_file(file, flags, );
if (ffd < 0) {
return -errno;
}
+ init_bb_loop_info(, file, offset, sizelimit, flags);
try = *device;
if (!try) {
try = dev;
@@ -239,7 +245,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
[PATCH v3 8/9] loop:refactor: extract subfunction set_loop_dev()
Step 8 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop_dev()
function old new delta
set_loop 700 716 +16
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 16/0) Total: 16 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 39 ---
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 8b207b929..799936765 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -200,6 +200,26 @@ static int do_stat_and_mknod(const char *dev, const char
*device, int i)
return -1;
}
+static int set_loop_dev(const char *dev, int *mode, int ffd, bb_loop_info
*loopinfo)
+{
+ int rc;
+ /* Open the sucker and check its loopiness */
+ int lfd = open(dev, *mode);
+ if (lfd < 0 && errno == EROFS) {
+ *mode = O_RDONLY;
+ lfd = open(dev, *mode);
+ }
+ if (lfd < 0) {
+ return lfd;
+ }
+ rc = set_loop_info(ffd, lfd, loopinfo);
+ if (rc == lfd) {
+ return lfd;
+ }
+ close(lfd);
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
@@ -210,7 +230,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- int i, lfd, ffd, mode, rc;
+ int i, ffd, mode, rc;
bb_loop_info loopinfo;
ffd = open_file(file, flags, );
@@ -241,30 +261,19 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
break;
}
open_lfd:
- /* Open the sucker and check its loopiness */
- lfd = rc = open(try, mode);
- if (lfd < 0 && errno == EROFS) {
- mode = O_RDONLY;
- lfd = rc = open(try, mode);
- }
- if (lfd < 0) {
+ rc = set_loop_dev(try, , ffd, );
+ if (rc == -1) {
if (errno == ENXIO) {
/* Happens if loop module is not loaded */
/* rc is -1; */
break;
}
- goto try_next_loopN;
- }
- rc = set_loop_info(ffd, lfd, );
- if (rc == lfd) {
+ } else {
/* SUCCESS! */
if (!*device)
*device = xstrdup(try);
break;
}
- close(lfd);
- try_next_loopN:
- rc = -1;
if (*device) /* was looking for a particular "/dev/loopN"? */
break; /* yes, do not try other names */
} /* for() */
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v3 9/9] loop: Add LOOP_CONFIGURE ioctl
LOOP_CONFIGURE is added to Linux 5.8
This allows userspace to completely setup a loop device with a single
ioctl, removing the in-between state where the device can be partially
configured - eg the loop device has a backing file associated with it,
but is reading from the wrong offset.
https://lwn.net/Articles/820408/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
Builders can use config to choose which algorithm to build into their busybox
binary
kernel version >= 5.8, choice CONFIG_LOOP_CONFIGURE
function old new delta
set_loop 716 639 -77
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-77) Total:
-77 bytes
kernel version < 5.8, choice CONFIG_NO_LOOP_CONFIGURE
function old new delta
--
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total:
0 bytes
kernel version is unknown, choice CONFIG_TRY_LOOP_CONFIGURE
function old new delta
set_loop 716 832+116
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 116/0) Total:
116 bytes
Signed-off-by: Xiaoming Ni
---
libbb/Config.src | 22 ++
libbb/loop.c | 47 ++-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/libbb/Config.src b/libbb/Config.src
index 66a3ffa23..b7f9ddab4 100644
--- a/libbb/Config.src
+++ b/libbb/Config.src
@@ -369,3 +369,25 @@ config UNICODE_PRESERVE_BROKEN
For example, this means that entering 'l', 's', ' ', 0xff, [Enter]
at shell prompt will list file named 0xff (single char name
with char value 255), not file named '?'.
+
+choice
+ prompt "LOOP_CONFIGURE or LOOP_SET_FD + LOOP_SET_STATUS"
+ default TRY_LOOP_CONFIGURE
+ help
+ LOOP_CONFIGURE is added to Linux 5.8
+ https://lwn.net/Articles/820408/
+ This allows userspace to completely setup a loop device with a single
+ ioctl, removing the in-between state where the device can be partially
+ configured - eg the loop device has a backing file associated with it,
+ but is reading from the wrong offset.
+
+config LOOP_CONFIGURE
+ bool "always uses LOOP_CONFIGURE, kernel version >= 5.8"
+
+config NO_LOOP_CONFIGURE
+ bool "never uses LOOP_CONFIGURE, kernel version < 5.8"
+
+config TRY_LOOP_CONFIGURE
+ bool "try LOOP_CONFIGURE, kernel version is unknown"
+
+endchoice
diff --git a/libbb/loop.c b/libbb/loop.c
index 799936765..14919b318 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,7 +126,8 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
-static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
+#ifndef CONFIG_LOOP_CONFIGURE
+static int set_loop_fd_and_status(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
bb_loop_info loopinfo2;
@@ -149,6 +150,46 @@ static int set_loop_configure(int ffd, int lfd,
bb_loop_info *loopinfo)
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
return -1;
}
+#endif
+
+#ifndef CONFIG_NO_LOOP_CONFIGURE
+
+#ifndef LOOP_CONFIGURE
+#define LOOP_CONFIGURE 0x4C0A
+struct loop_config {
+ uint32_t fd;
+ uint32_t block_size;
+ struct loop_info64 info;
+ uint64_t __reserved[8];
+};
+#endif
+
+/*
+ * linux v5.8.0
+ * loop: Add LOOP_CONFIGURE ioctl
+ * https://lwn.net/Articles/820408/
+ *
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
+ */
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
+{
+ int rc;
+ struct loop_config config;
+
+ memset(, 0, sizeof(config));
+ config.fd = ffd;
+ memcpy(, loopinfo, sizeof(config.info));
+
+ rc = ioctl(lfd, LOOP_CONFIGURE, );
+ if (rc == 0) {
+ return lfd;
+ }
+#ifdef CONFIG_TRY_LOOP_CONFIGURE
+ if (rc == -1 && errno == EINVAL) /* The system may not support
LOOP_CONFIGURE. */
+ return set_loop_fd_and_status(ffd, lfd, loopinfo);
+#endif
+ return -1;
+}
+#endif
static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
{
@@ -159,7 +200,11 @@ static int set_loop_info(int ffd, int lfd, bb_loop_info
*loopinfo)
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
+#ifdef CONFIG_NO_LOOP_CONFIGURE
+ return set_l
[PATCH v3 2/9] loop:refactor: extract subfunction get_next_free_loop()
Step 2 of micro-refactoring the set_loop function ()
Extract subfunction get_next_free_loop() from set_loop()
Also fix miss free(try) when stat(try) and mknod fail
function old new delta
set_loop 758 734 -24
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24 bytes
Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 55
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index c517ceb13..baa02dabb 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int get_next_free_loop(char *dev, int id)
+{
+ int loopdevno = get_free_loop();
+ if (loopdevno >= 0) {
+ sprintf(dev, LOOP_FORMAT, loopdevno);
+ return 1; /* use /dev/loop-control */
+ }
+ if (loopdevno == -2) {
+ sprintf(dev, LOOP_FORMAT, id);
+ return 2;
+ }
+ return -1; /* no free loop devices */
+}
+
static int open_file(const char *file, unsigned flags, int *mode)
{
int ffd;
@@ -132,30 +146,26 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
try = *device;
if (!try) {
- get_free_loopN:
- i = get_free_loop();
- if (i == -1) {
- close(ffd);
- return -1; /* no free loop devices */
- }
- if (i >= 0) {
- try = xasprintf(LOOP_FORMAT, i);
- goto open_lfd;
- }
- /* i == -2: no /dev/loop-control. Do an old-style search for a
free device */
try = dev;
}
/* Find a loop device */
/* 0xf is a max possible minor number in Linux circa 2010 */
for (i = 0; i <= 0xf; i++) {
- sprintf(dev, LOOP_FORMAT, i);
+ if (!*device) {
+ rc = get_next_free_loop(dev, i);
+ if (rc == -1) {
+ break; /* no free loop devices */
+ } else if (rc == 1) {
+ goto open_lfd;
+ }
+ }
IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
&& errno == ENOENT
-&& try == dev
+&& (!*device)
) {
/* Node doesn't exist, try to create it */
if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
@@ -188,13 +198,10 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
/* Ouch. Are we racing with other mount? */
- if (!*device /* yes */
-&& try != dev /* tried a _kernel-offered_
loopN? */
- ) {
- free(try);
+ if (!*device) {
close(lfd);
//TODO: add "if (--failcount != 0) ..."?
- goto get_free_loopN;
+ continue;
}
goto close_and_try_next_loopN;
}
@@ -218,8 +225,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
if (rc == 0) {
/* SUCCESS! */
- if (try != dev) /* tried a kernel-offered free
loopN? */
- *device = try; /* malloced */
if (!*device) /* was looping in search of
free "/dev/loopN"? */
*device = xstrdup(dev);
rc = lfd; /* return this */
@@ -227,16 +232,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
- } else {
-
[PATCH v3 4/9] loop:refactor: extract subfunction set_loop_info()
Step 4 of micro-refactoring the set_loop():
Extract subfunction set_loop_info() from set_loop()
function old new delta
set_loop 734 700 -34
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-34) Total: -34 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 91 +++-
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 147597ab9..34e2dce7a 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,47 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_info(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+
+ /* If device is free, try to claim it */
+ if (rc && errno == ENXIO) {
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ }
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -135,7 +176,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- bb_loop_info loopinfo;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
@@ -190,49 +230,12 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
-
- /* If device is free, try to claim it */
- if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- close(lfd);
- /* Ouch. Are we racing with other mount? */
- if (!*device) {
-//TODO: add "if (--failcount != 0) ..."?
- continue;
- } else {
- break;
- }
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is
controlled by open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before
mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags &
BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support
LO_FLAGS_AUTOCLEAR? */
-
[PATCH v3 5/9] loop:refactor: extract subfunction set_loop_configure()
Step 5 of micro-refactoring the set_loop():
Extract subfunction set_loop_configure()
function old new delta
set_loop 700 708 +8
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 8/0) Total: 8 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 63 ++--
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 34e2dce7a..dc8ecc9a8 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,41 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_configure(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return -1;
+}
+
static int set_loop_info(int ffd, int lfd, const char *file,
unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
{
@@ -136,33 +171,7 @@ static int set_loop_info(int ffd, int lfd, const char
*file,
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- return -1;
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
- /* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- }
- if (rc == 0) {
- return lfd;
- }
- /* failure, undo LOOP_SET_FD */
- ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
}
return -1;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v3 7/9] loop:refactor: Extract subfunction do_stat_and_mknod()
Step 7 of micro-refactoring the set_loop():
Extract subfunction do_stat_and_mknod()
function old new delta
set_loop 720 700 -20
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20) Total: -20 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 38 --
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 0b1336a02..8b207b929 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -180,6 +180,27 @@ static void init_bb_loop_info(bb_loop_info *loopinfo,
const char *file,
loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
}
+static int do_stat_and_mknod(const char *dev, const char *device, int i)
+{
+ struct stat statbuf;
+
+ IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
+ if (stat(dev, ) == 0 && S_ISBLK(statbuf.st_mode))
+ return 0;
+ if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
+ && errno == ENOENT
+ && (!device)
+ ) {
+ /* Node doesn't exist, try to create it */
+ if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0) {
+ return 0;
+ }
+ }
+ /* Ran out of block devices, return failure */
+ return -1;
+}
+
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -189,7 +210,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- struct stat statbuf;
int i, lfd, ffd, mode, rc;
bb_loop_info loopinfo;
@@ -216,18 +236,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
}
- IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
- if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
- if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
-&& errno == ENOENT
-&& (!*device)
- ) {
- /* Node doesn't exist, try to create it */
- if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
- goto open_lfd;
- }
- /* Ran out of block devices, return failure */
- rc = -1;
+ rc = do_stat_and_mknod(try, *device, i);
+ if (rc == -1) {
break;
}
open_lfd:
@@ -249,7 +259,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc == lfd) {
/* SUCCESS! */
if (!*device)
- *device = xstrdup(dev);
+ *device = xstrdup(try);
break;
}
close(lfd);
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v3 3/9] loop:refactor: del close_and_try_next_loopN
Step 3 of micro-refactoring the set_loop() function:
Delete close_and_try_next_loopN.
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total: 0 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index baa02dabb..147597ab9 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -197,13 +197,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc && errno == ENXIO) {
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ close(lfd);
/* Ouch. Are we racing with other mount? */
if (!*device) {
- close(lfd);
//TODO: add "if (--failcount != 0) ..."?
continue;
+ } else {
+ break;
}
- goto close_and_try_next_loopN;
}
memset(, 0, sizeof(loopinfo));
safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
@@ -233,7 +234,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
}
- close_and_try_next_loopN:
close(lfd);
try_next_loopN:
rc = -1;
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v3 1/9] loop:refactor: extract subfunction open_file()
Step 1 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop()
function old new delta
set_loop 760 758 -2
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2) Total: -2 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 750642ade..c517ceb13 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,22 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int open_file(const char *file, unsigned flags, int *mode)
+{
+ int ffd;
+ /* open the file. barf if this doesn't work. */
+ *mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
+ retry_open_ffd:
+ ffd = open(file, *mode);
+ if (ffd < 0) {
+ if (*mode != O_RDONLY) {
+ *mode = O_RDONLY;
+ goto retry_open_ffd;
+ }
+ }
+ return ffd;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -109,15 +125,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
struct stat statbuf;
int i, lfd, ffd, mode, rc;
- /* Open the file. Barf if this doesn't work. */
- mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
- open_ffd:
- ffd = open(file, mode);
+ ffd = open_file(file, flags, );
if (ffd < 0) {
- if (mode != O_RDONLY) {
- mode = O_RDONLY;
- goto open_ffd;
- }
return -errno;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v3 0/9] loop: Micro-refactoring set_loop() and add LOOP_CONFIGURE
LOOP_CONFIGURE is added to Linux 5.8 This allows userspace to completely setup a loop device with a single ioctl, removing the in-between state where the device can be partially configured - eg the loop device has a backing file associated with it, but is reading from the wrong offset. https://lwn.net/Articles/820408/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5 patch 1-8: To facilitate the addition of new functions, the code of set_loop() is micro-refactored. function old new delta set_loop 760 716 -44 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44) Total: -44 bytes patch 9: add LOOP_CONFIGURE Builders can use config to choose which algorithm to build into their busybox binary --- v3: Fine-tuned based on review comments patch2: Rename local variables, use "loopdevno" instead of "i" in get_next_free_loop(). patch7: Reduce indentation using guard statements in do_stat_and_mknod(). patch9: Rename set_loop_configure_old() to set_loop_fd_and_status() v2: http://lists.busybox.net/pipermail/busybox/2022-November/089990.html changes in patch9: add config for builder to select v1: http://lists.busybox.net/pipermail/busybox/2022-November/089969.html --- *** BLURB HERE *** Xiaoming Ni (9): loop:refactor: extract subfunction open_file() loop:refactor: extract subfunction get_next_free_loop() loop:refactor: del close_and_try_next_loopN loop:refactor: extract subfunction set_loop_info() loop:refactor: extract subfunction set_loop_configure() loop:refactor: Use a structure to reduce parameters loop:refactor: Extract subfunction do_stat_and_mknod() loop:refactor: extract subfunction set_loop_dev() loop: Add LOOP_CONFIGURE ioctl libbb/Config.src | 22 libbb/loop.c | 292 ++- 2 files changed, 211 insertions(+), 103 deletions(-) -- 2.27.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: AW: [PATCH v2 5/9] loop:refactor: extract subfunction set_loop_configure()
On 2022/11/18 20:36, Walter Harms wrote:
A little tweak make it more readable (i hope)
putting if (rc) direkt behind
rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
if (rc == 0)
return lfd;
// no need to check rc again
if ( (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
// try again without BB_LO_FLAGS_AUTOCLEAR
// does it realy work that way ? no ~ ?
loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
}
if (rc == 0)
return lfd;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
In this case, duplicate "if(rc==0)" occurs.
Is that worse?
Thanks
Xiaoming Ni
Von: busybox im Auftrag von Xiaoming Ni
Gesendet: Freitag, 18. November 2022 13:14:44
An: busybox@busybox.net; vda.li...@googlemail.com; c...@gmx.com;
explore...@gmail.com
Cc: wang...@huawei.com
Betreff: [PATCH v2 5/9] loop:refactor: extract subfunction set_loop_configure()
Step 5 of micro-refactoring the set_loop():
Extract subfunction set_loop_configure()
function old new delta
set_loop 700 708 +8
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 8/0) Total: 8 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 63 ++--
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 89adc4f94..914af57b2 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,41 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_configure(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return -1;
+}
+
static int set_loop_info(int ffd, int lfd, const char *file,
unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
{
@@ -136,33 +171,7 @@ static int set_loop_info(int ffd, int lfd, const char
*file,
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- return -1;
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
- /* (this cod
Re: [PATCH v2 9/9] loop: Add LOOP_CONFIGURE ioctl
On 2022/11/19 2:22, Kang-Che Sung wrote:
On Friday, November 18, 2022, Xiaoming Ni <mailto:nixiaom...@huawei.com>> wrote:
> LOOP_CONFIGURE is added to Linux 5.8
>
> This allows userspace to completely setup a loop device with a single
> ioctl, removing the in-between state where the device can be partially
> configured - eg the loop device has a backing file associated with it,
> but is reading from the wrong offset.
>
> https://lwn.net/Articles/820408/ <https://lwn.net/Articles/820408/>
>
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5>
>
> kernel version >= 5.8, choice CONFIG_LOOP_CONFIGURE
>     function                       old
 new  delta
>     set_loop                       716
 832  +116
>
--
> Â Â Â Â (add/remove: 0/0 grow/shrink: 1/0 up/down: 116/0)
 Total: 116 bytes
> kernel version < 5.8, choice CONFIG_NO_LOOP_CONFIGURE
>     function                       old
 new  delta
>     set_loop                       760
 716   -44
>
--
> Â Â Â Â (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44)
 Total: -44 bytes
> kernel version is unknown, choice CONFIG_TRY_LOOP_CONFIGURE
>     function                       old
 new  delta
>
--
> Â Â Â Â (add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0)
  Total: 0 bytes
>
> Signed-off-by: Xiaoming Ni <mailto:nixiaom...@huawei.com>>
> ---
> Â libbb/Config.src | 22 ++
>  libbb/loop.c   | 47 ++-
> Â 2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/libbb/Config.src b/libbb/Config.src
> index 66a3ffa23..b7f9ddab4 100644
> --- a/libbb/Config.src
> +++ b/libbb/Config.src
> @@ -369,3 +369,25 @@ config UNICODE_PRESERVE_BROKEN
> Â Â Â Â For example, this means that entering 'l', 's', ' ', 0xff,
[Enter]
> Â Â Â Â at shell prompt will list file named 0xff (single char name
> Â Â Â Â with char value 255), not file named '?'.
> +
> +choice
> +Â Â Â Â prompt "LOOP_CONFIGURE or LOOP_SET_FD + LOOP_SET_STATUS"
> +Â Â Â Â default TRY_LOOP_CONFIGURE
> +Â Â Â Â help
> +Â Â Â Â LOOP_CONFIGURE is added to Linux 5.8
> + https://lwn.net/Articles/820408/ <https://lwn.net/Articles/820408/>
> +Â Â Â Â This allows userspace to completely setup a loop device with
a single
> +Â Â Â Â ioctl, removing the in-between state where the device can be
partially
> +Â Â Â Â configured - eg the loop device has a backing file associated
with it,
> +Â Â Â Â but is reading from the wrong offset.
> +
> +config LOOP_CONFIGURE
> +Â Â Â Â bool "always uses LOOP_CONFIGURE, kernel version >= 5.8"
> +
> +config NO_LOOP_CONFIGURE
> +Â Â Â Â bool "never uses LOOP_CONFIGURE, kernel version < 5.8"
> +
> +config TRY_LOOP_CONFIGURE
> +Â Â Â Â bool "try LOOP_CONFIGURE, kernel version is unknown"
> +
> +endchoice
> diff --git a/libbb/loop.c b/libbb/loop.c
> index ddb92fce3..be592bc4b 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -126,7 +126,8 @@ static int open_file(const char *file, unsigned
flags, int *mode)
> Â Â Â Â return ffd;
> Â }
>
> -static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
> +#ifndef CONFIG_LOOP_CONFIGURE
> +static int set_loop_configure_old(int ffd, int lfd, bb_loop_info
*loopinfo)
> Â {
> Â Â Â Â int rc;
> Â Â Â Â bb_loop_info loopinfo2;
> @@ -149,6 +150,46 @@ static int set_loop_configure(int ffd, int lfd,
bb_loop_info *loopinfo)
> Â Â Â Â ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
> Â Â Â Â return -1;
> Â }
> +#endif
> +
> +#ifndef CONFIG_NO_LOOP_CONFIGURE
> +
> +#ifndef LOOP_CONFIGURE
> +#define LOOP_CONFIGURE 0x4C0A
> +struct loop_config {
> +Â Â Â Â uint32_t fd;
> +Â Â Â Â uint32_t block_size;
> +Â Â Â Â struct loop_info64 info;
> +Â Â Â Â uint64_t __reserved[8];
> +};
> +#endif
> +
> +/*
> + * linux v5.8.0
> + * loop: Add LOOP_CONFIGURE ioctl
> + * https://lwn.net/Articles/820408/ <https://lwn.net/Articles/820408/>
> + *
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i
Re: [PATCH v2 6/9] loop:refactor: Use a structure to reduce parameters
On 2022/11/19 2:06, Kang-Che Sung wrote:
On Friday, November 18, 2022, Xiaoming Ni <mailto:nixiaom...@huawei.com>> wrote:
> Step 6 of micro-refactoring the set_loop():
> Â Â Â Â Use structs to avoid transferring a large number of parameters
> Â Â Â Â in set_loop_configure() and set_loop_info()
>
> function                       old   new  delta
> set_loop                       708   720   +12
>
--
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0)Â Â Â Â Â Â Â Â Total:
12 bytes
>
> Signed-off-by: Xiaoming Ni <mailto:nixiaom...@huawei.com>>
> ---
> Â libbb/loop.c | 54 +---
> Â 1 file changed, 30 insertions(+), 24 deletions(-)
>
...
> -Â Â Â Â */
> -Â Â Â Â loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
> -Â Â Â Â rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> -Â Â Â Â if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
> +Â Â Â Â rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
> +Â Â Â Â if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
> +Â Â Â Â Â Â Â Â memcpy(, loopinfo, sizeof(*loopinfo));
Just use `loopinfo2 = loopinfo;`
Tested on my PC, there was no difference in code size between your
modified scheme and my current scheme.
function old new delta
--
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total: 0
bytes
Also, it is unclear why there is the need to clone the loopinfo buffer.
> Â Â Â Â Â Â Â Â /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
> Â Â Â Â Â Â Â Â /* (this code path is not tested) */
> -Â Â Â Â Â Â Â Â loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> -Â Â Â Â Â Â Â Â rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> +Â Â Â Â Â Â Â Â loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> +Â Â Â Â Â Â Â Â rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> Â Â Â Â }
> Â Â Â Â if (rc == 0) {
> Â Â Â Â Â Â Â Â return lfd;
...
>
> +static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
> +Â Â Â Â Â Â Â Â unsigned long long offset, unsigned long long
sizelimit, unsigned flags)
> +{
> +Â Â Â Â memset(loopinfo, 0, sizeof(*loopinfo));
Would it reduce code size by doing the initialization like this?
```
    bb_loop_info empty = {0};
    loopinfo = empty;
```
Tested on my PC, there was no difference in code size between your
modified scheme and my current scheme.
function old new delta
------
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total: 0
bytes
Thanks
Xiaoming Ni
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2 2/9] loop:refactor: extract subfunction get_next_free_loop()
On 2022/11/19 1:40, Kang-Che Sung wrote:
On Friday, November 18, 2022, Xiaoming Ni <mailto:nixiaom...@huawei.com>> wrote:
> Step 2 of micro-refactoring the set_loop function ()
> Â Â Â Â Extract subfunction get_next_free_loop() from set_loop()
>
> Also fix miss free(try) when stat(try) and mknod fail
>
> function                       old   new  delta
> set_loop                       758   734   -24
>
--
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24)Â Â Â Â Â Â Â Total:
-24 bytes
>
> Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
> Signed-off-by: Xiaoming Ni <mailto:nixiaom...@huawei.com>>
> ---
> Â libbb/loop.c | 55
> Â 1 file changed, 25 insertions(+), 30 deletions(-)
>
> diff --git a/libbb/loop.c b/libbb/loop.c
> index c517ceb13..71fd8c1bc 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void)
> Â Â Â Â return loopdevno; /* can be -1 if error */
> Â }
>
> +static int get_next_free_loop(char *dev, int id)
> +{
> +Â Â Â Â int i = get_free_loop();
> +Â Â Â Â if (i >= 0) {
> +Â Â Â Â Â Â Â Â sprintf(dev, LOOP_FORMAT, i);
> +Â Â Â Â Â Â Â Â return 1; /* use /dev/loop-control */
> +Â Â Â Â } else if (i == -2) {
> +Â Â Â Â Â Â Â Â sprintf(dev, LOOP_FORMAT, id);
> +Â Â Â Â Â Â Â Â return 2;
> +Â Â Â Â } else {
> +Â Â Â Â Â Â Â Â return -1; /* no free loop devices */
> +Â Â Â Â }
> +}
I'm a little nervous when the buffer length of `dev` is not passed into
this function. Yes I know the buffer is large enough for the loop device
path that would be printed. But I just wish there would be an assertion
in this function, so that if the function is reused somewhere else, the
developer would know what he is doing.
static int get_next_free_loop(char *dev, size_t dev_size, int id)
{
int loopdevno = get_free_loop();
if (loopdevno >= 0) {
snprintf(dev, dev_size, LOOP_FORMAT, loopdevno);
return 1; /* use /dev/loop-control */
}
if (loopdevno == -2) {
snprintf(dev, dev_size, LOOP_FORMAT, id);
return 2;
}
return -1; /* no free loop devices */
}
If the dev_size parameter is added to get_next_free_loop(), the code
size increases, Is it worth?
function old new delta
set_loop 734 744 +10
------
(add/remove: 0/0 grow/shrink: 1/0 up/down: 10/0) Total: 10
bytes
Thanks
Xiaoming Ni
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: AW: [PATCH v2 7/9] loop:refactor: Extract subfunction do_stat_and_mknod()
On 2022/11/18 20:39, Walter Harms wrote:
again try to safe indent level
if (stat(dev, ) == 0 && S_ISBLK(statbuf.st_mode))
return 0;
jm2c
Thanks, will be modified in v3
Von: busybox im Auftrag von Xiaoming Ni
Gesendet: Freitag, 18. November 2022 13:14:46
An: busybox@busybox.net; vda.li...@googlemail.com; c...@gmx.com;
explore...@gmail.com
Cc: wang...@huawei.com
Betreff: [PATCH v2 7/9] loop:refactor: Extract subfunction do_stat_and_mknod()
Step 7 of micro-refactoring the set_loop():
Extract subfunction do_stat_and_mknod()
function old new delta
set_loop 720 700 -20
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20) Total: -20 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 39 +--
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 2200ccb9a..67e16ddb0 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -180,6 +180,28 @@ static void init_bb_loop_info(bb_loop_info *loopinfo,
const char *file,
loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
}
+static int do_stat_and_mknod(const char *dev, const char *device, int i)
+{
+ struct stat statbuf;
+
+ IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
+ if (stat(dev, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
+ if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
+ && errno == ENOENT
+ && (!device)
+ ) {
+ /* Node doesn't exist, try to create it */
+ if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0) {
+ return 0;
+ }
+ }
+ /* Ran out of block devices, return failure */
+ return -1;
+ }
+ return 0;
+}
+
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -189,7 +211,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- struct stat statbuf;
int i, lfd, ffd, mode, rc;
bb_loop_info loopinfo;
@@ -216,18 +237,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
}
- IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
- if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
- if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
-&& errno == ENOENT
-&& (!*device)
- ) {
- /* Node doesn't exist, try to create it */
- if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
- goto open_lfd;
- }
- /* Ran out of block devices, return failure */
- rc = -1;
+ rc = do_stat_and_mknod(try, *device, i);
+ if (rc == -1) {
break;
}
open_lfd:
@@ -249,7 +260,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc == lfd) {
/* SUCCESS! */
if (!*device)
- *device = xstrdup(dev);
+ *device = xstrdup(try);
break;
}
close(lfd);
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: AW: [PATCH v2 4/9] loop:refactor: extract subfunction set_loop_info()
On 2022/11/18 20:27, Walter Harms wrote:
on other minor
if (rc && errno == ENXIO)
turn it on its head safes one indent level
if ( !rc || errno != ENXIO )
return -1; // failed to get loopinfo
jm2c
Here's for copy-paste consistency
Simplification has been implemented in patch 5 by extracting functions
Von: busybox im Auftrag von Xiaoming Ni
Gesendet: Freitag, 18. November 2022 13:14:43
An: busybox@busybox.net; vda.li...@googlemail.com; c...@gmx.com;
explore...@gmail.com
Cc: wang...@huawei.com
Betreff: [PATCH v2 4/9] loop:refactor: extract subfunction set_loop_info()
Step 4 of micro-refactoring the set_loop():
Extract subfunction set_loop_info() from set_loop()
function old new delta
set_loop 734 700 -34
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-34) Total: -34 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 91 +++-
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 91c3a45c4..89adc4f94 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,47 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_info(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+
+ /* If device is free, try to claim it */
+ if (rc && errno == ENXIO) {
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ }
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -135,7 +176,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- bb_loop_info loopinfo;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
@@ -190,49 +230,12 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
-
- /* If device is free, try to claim it */
- if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- close(lfd);
- /* Ouch. Are we racing with other mount? */
- if (!*device) {
-//TODO: add "if (--failcount != 0) ..."?
- continue;
- } else {
- break;
- }
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-
Re: AW: [PATCH 2/9] loop:refactor: extract subfunction get_next_free_loop()
On 2022/11/18 20:20, Walter Harms wrote:
hi,
just a minor comment.
do not use i as name for return value,
most ppl use it as loop counter, triggers the
wrong circuits in the brain. (rule of least surprise).
just name it err or what you like,
and please untangle the if()
if (err>=0)
return
if (err==-2)
return
return -1
just my 2 cents ...
Thanks, I'll send a v3 patch later to replace "i" with "loopdevno",
refer to the variable name of get_free_loop().
Also, I made a mistake in patch9's commit msg.
The log of the code size change is misposted.
I will send patch v3 later
Thanks
Von: busybox im Auftrag von Xiaoming Ni
Gesendet: Freitag, 18. November 2022 02:01:51
An: busybox@busybox.net; vda.li...@googlemail.com
Cc: wang...@huawei.com
Betreff: [PATCH 2/9] loop:refactor: extract subfunction get_next_free_loop()
Step 2 of micro-refactoring the set_loop function ()
Extract subfunction get_next_free_loop() from set_loop()
Also fix miss free(try) when stat(try) and mknod fail
function old new delta
set_loop 758 734 -24
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24 bytes
Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 55
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index c517ceb13..71fd8c1bc 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int get_next_free_loop(char *dev, int id)
+{
+ int i = get_free_loop();
+ if (i >= 0) {
+ sprintf(dev, LOOP_FORMAT, i);
+ return 1; /* use /dev/loop-control */
+ } else if (i == -2) {
+ sprintf(dev, LOOP_FORMAT, id);
+ return 2;
+ } else {
+ return -1; /* no free loop devices */
+ }
+}
+
static int open_file(const char *file, unsigned flags, int *mode)
{
int ffd;
@@ -132,30 +146,26 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
try = *device;
if (!try) {
- get_free_loopN:
- i = get_free_loop();
- if (i == -1) {
- close(ffd);
- return -1; /* no free loop devices */
- }
- if (i >= 0) {
- try = xasprintf(LOOP_FORMAT, i);
- goto open_lfd;
- }
- /* i == -2: no /dev/loop-control. Do an old-style search for a
free device */
try = dev;
}
/* Find a loop device */
/* 0xf is a max possible minor number in Linux circa 2010 */
for (i = 0; i <= 0xf; i++) {
- sprintf(dev, LOOP_FORMAT, i);
+ if (!*device) {
+ rc = get_next_free_loop(dev, i);
+ if (rc == -1) {
+ break; /* no free loop devices */
+ } else if (rc == 1) {
+ goto open_lfd;
+ }
+ }
IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
&& errno == ENOENT
-&& try == dev
+&& (!*device)
) {
/* Node doesn't exist, try to create it */
if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
@@ -188,13 +198,10 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
/* Ouch. Are we racing with other mount? */
- if (!*device /* yes */
-&& try != dev /* tried a _kernel-offered_
loopN? */
- ) {
- free(try);
+ if (!*device) {
close(lfd);
//TODO: add "if (--failcount != 0) ..."?
- goto get_free_loopN;
+ continue;
}
goto close_and_try_next_loopN;
}
@@ -218,8 +225,6
[PATCH v2 4/9] loop:refactor: extract subfunction set_loop_info()
Step 4 of micro-refactoring the set_loop():
Extract subfunction set_loop_info() from set_loop()
function old new delta
set_loop 734 700 -34
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-34) Total: -34 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 91 +++-
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 91c3a45c4..89adc4f94 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,47 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_info(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+
+ /* If device is free, try to claim it */
+ if (rc && errno == ENXIO) {
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ }
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -135,7 +176,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- bb_loop_info loopinfo;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
@@ -190,49 +230,12 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
-
- /* If device is free, try to claim it */
- if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- close(lfd);
- /* Ouch. Are we racing with other mount? */
- if (!*device) {
-//TODO: add "if (--failcount != 0) ..."?
- continue;
- } else {
- break;
- }
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is
controlled by open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before
mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags &
BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support
LO_FLAGS_AUTOCLEAR? */
-
[PATCH v2 6/9] loop:refactor: Use a structure to reduce parameters
Step 6 of micro-refactoring the set_loop():
Use structs to avoid transferring a large number of parameters
in set_loop_configure() and set_loop_info()
function old new delta
set_loop 708 720 +12
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0) Total: 12 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 54 +---
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 914af57b2..2200ccb9a 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,32 +126,21 @@ static int open_file(const char *file, unsigned flags,
int *mode)
return ffd;
}
-static int set_loop_configure(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info loopinfo2;
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
return -1;
}
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
+ if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ memcpy(, loopinfo, sizeof(*loopinfo));
/* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
/* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
}
if (rc == 0) {
return lfd;
@@ -161,21 +150,36 @@ static int set_loop_configure(int ffd, int lfd, const
char *file,
return -1;
}
-static int set_loop_info(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info tmp;
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
+ return set_loop_configure(ffd, lfd, loopinfo);
}
return -1;
}
+static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ memset(loopinfo, 0, sizeof(*loopinfo));
+ safe_strncpy((char *)loopinfo->lo_file_name, file, LO_NAME_SIZE);
+ loopinfo->lo_offset = offset;
+ loopinfo->lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -187,12 +191,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
char *try;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
+ bb_loop_info loopinfo;
ffd = open_file(file, flags, );
if (ffd < 0) {
return -errno;
}
+ init_bb_loop_info(, file, offset, sizelimit, flags);
try = *device;
if (!try) {
try = dev;
@@ -239,7 +245,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
[PATCH v2 2/9] loop:refactor: extract subfunction get_next_free_loop()
Step 2 of micro-refactoring the set_loop function ()
Extract subfunction get_next_free_loop() from set_loop()
Also fix miss free(try) when stat(try) and mknod fail
function old new delta
set_loop 758 734 -24
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24 bytes
Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 55
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index c517ceb13..71fd8c1bc 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int get_next_free_loop(char *dev, int id)
+{
+ int i = get_free_loop();
+ if (i >= 0) {
+ sprintf(dev, LOOP_FORMAT, i);
+ return 1; /* use /dev/loop-control */
+ } else if (i == -2) {
+ sprintf(dev, LOOP_FORMAT, id);
+ return 2;
+ } else {
+ return -1; /* no free loop devices */
+ }
+}
+
static int open_file(const char *file, unsigned flags, int *mode)
{
int ffd;
@@ -132,30 +146,26 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
try = *device;
if (!try) {
- get_free_loopN:
- i = get_free_loop();
- if (i == -1) {
- close(ffd);
- return -1; /* no free loop devices */
- }
- if (i >= 0) {
- try = xasprintf(LOOP_FORMAT, i);
- goto open_lfd;
- }
- /* i == -2: no /dev/loop-control. Do an old-style search for a
free device */
try = dev;
}
/* Find a loop device */
/* 0xf is a max possible minor number in Linux circa 2010 */
for (i = 0; i <= 0xf; i++) {
- sprintf(dev, LOOP_FORMAT, i);
+ if (!*device) {
+ rc = get_next_free_loop(dev, i);
+ if (rc == -1) {
+ break; /* no free loop devices */
+ } else if (rc == 1) {
+ goto open_lfd;
+ }
+ }
IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
&& errno == ENOENT
-&& try == dev
+&& (!*device)
) {
/* Node doesn't exist, try to create it */
if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
@@ -188,13 +198,10 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
/* Ouch. Are we racing with other mount? */
- if (!*device /* yes */
-&& try != dev /* tried a _kernel-offered_
loopN? */
- ) {
- free(try);
+ if (!*device) {
close(lfd);
//TODO: add "if (--failcount != 0) ..."?
- goto get_free_loopN;
+ continue;
}
goto close_and_try_next_loopN;
}
@@ -218,8 +225,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
if (rc == 0) {
/* SUCCESS! */
- if (try != dev) /* tried a kernel-offered free
loopN? */
- *device = try; /* malloced */
if (!*device) /* was looping in search of
free "/dev/loopN"? */
*device = xstrdup(dev);
rc = lfd; /* return this */
@@ -227,16 +232,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
- } else {
- /* device is not free (rc =
[PATCH v2 0/9] loop: Micro-refactoring set_loop() and add LOOP_CONFIGURE
LOOP_CONFIGURE is added to Linux 5.8 This allows userspace to completely setup a loop device with a single ioctl, removing the in-between state where the device can be partially configured - eg the loop device has a backing file associated with it, but is reading from the wrong offset. https://lwn.net/Articles/820408/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5 patch 1-8: To facilitate the addition of new functions, the code of set_loop() is micro-refactored. function old new delta set_loop 760 703 -57 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-57) Total: -57 bytes patch 9: add LOOP_CONFIGURE Builders can use config to choose which algorithm to build into their busybox binary --- v2: add config for builder to select v1: http://lists.busybox.net/pipermail/busybox/2022-November/089969.html --- Xiaoming Ni (9): loop:refactor: extract subfunction open_file() loop:refactor: extract subfunction get_next_free_loop() loop:refactor: del close_and_try_next_loopN loop:refactor: extract subfunction set_loop_info() loop:refactor: extract subfunction set_loop_configure() loop:refactor: Use a structure to reduce parameters loop:refactor: Extract subfunction do_stat_and_mknod() loop:refactor: extract subfunction set_loop_dev() loop: Add LOOP_CONFIGURE ioctl libbb/Config.src | 22 libbb/loop.c | 293 ++- 2 files changed, 212 insertions(+), 103 deletions(-) -- 2.27.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2 7/9] loop:refactor: Extract subfunction do_stat_and_mknod()
Step 7 of micro-refactoring the set_loop():
Extract subfunction do_stat_and_mknod()
function old new delta
set_loop 720 700 -20
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20) Total: -20 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 39 +--
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 2200ccb9a..67e16ddb0 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -180,6 +180,28 @@ static void init_bb_loop_info(bb_loop_info *loopinfo,
const char *file,
loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
}
+static int do_stat_and_mknod(const char *dev, const char *device, int i)
+{
+ struct stat statbuf;
+
+ IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
+ if (stat(dev, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
+ if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
+ && errno == ENOENT
+ && (!device)
+ ) {
+ /* Node doesn't exist, try to create it */
+ if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0) {
+ return 0;
+ }
+ }
+ /* Ran out of block devices, return failure */
+ return -1;
+ }
+ return 0;
+}
+
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -189,7 +211,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- struct stat statbuf;
int i, lfd, ffd, mode, rc;
bb_loop_info loopinfo;
@@ -216,18 +237,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
}
- IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
- if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
- if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
-&& errno == ENOENT
-&& (!*device)
- ) {
- /* Node doesn't exist, try to create it */
- if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
- goto open_lfd;
- }
- /* Ran out of block devices, return failure */
- rc = -1;
+ rc = do_stat_and_mknod(try, *device, i);
+ if (rc == -1) {
break;
}
open_lfd:
@@ -249,7 +260,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc == lfd) {
/* SUCCESS! */
if (!*device)
- *device = xstrdup(dev);
+ *device = xstrdup(try);
break;
}
close(lfd);
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2 5/9] loop:refactor: extract subfunction set_loop_configure()
Step 5 of micro-refactoring the set_loop():
Extract subfunction set_loop_configure()
function old new delta
set_loop 700 708 +8
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 8/0) Total: 8 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 63 ++--
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 89adc4f94..914af57b2 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,41 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_configure(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return -1;
+}
+
static int set_loop_info(int ffd, int lfd, const char *file,
unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
{
@@ -136,33 +171,7 @@ static int set_loop_info(int ffd, int lfd, const char
*file,
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- return -1;
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
- /* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- }
- if (rc == 0) {
- return lfd;
- }
- /* failure, undo LOOP_SET_FD */
- ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
}
return -1;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2 3/9] loop:refactor: del close_and_try_next_loopN
Step 3 of micro-refactoring the set_loop() function:
Delete close_and_try_next_loopN.
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total: 0 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 71fd8c1bc..91c3a45c4 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -197,13 +197,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc && errno == ENXIO) {
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ close(lfd);
/* Ouch. Are we racing with other mount? */
if (!*device) {
- close(lfd);
//TODO: add "if (--failcount != 0) ..."?
continue;
+ } else {
+ break;
}
- goto close_and_try_next_loopN;
}
memset(, 0, sizeof(loopinfo));
safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
@@ -233,7 +234,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
}
- close_and_try_next_loopN:
close(lfd);
try_next_loopN:
rc = -1;
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2 9/9] loop: Add LOOP_CONFIGURE ioctl
LOOP_CONFIGURE is added to Linux 5.8
This allows userspace to completely setup a loop device with a single
ioctl, removing the in-between state where the device can be partially
configured - eg the loop device has a backing file associated with it,
but is reading from the wrong offset.
https://lwn.net/Articles/820408/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
kernel version >= 5.8, choice CONFIG_LOOP_CONFIGURE
function old new delta
set_loop 716 832+116
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 116/0) Total:
116 bytes
kernel version < 5.8, choice CONFIG_NO_LOOP_CONFIGURE
function old new delta
set_loop 760 716 -44
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44) Total:
-44 bytes
kernel version is unknown, choice CONFIG_TRY_LOOP_CONFIGURE
function old new delta
--
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total:
0 bytes
Signed-off-by: Xiaoming Ni
---
libbb/Config.src | 22 ++
libbb/loop.c | 47 ++-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/libbb/Config.src b/libbb/Config.src
index 66a3ffa23..b7f9ddab4 100644
--- a/libbb/Config.src
+++ b/libbb/Config.src
@@ -369,3 +369,25 @@ config UNICODE_PRESERVE_BROKEN
For example, this means that entering 'l', 's', ' ', 0xff, [Enter]
at shell prompt will list file named 0xff (single char name
with char value 255), not file named '?'.
+
+choice
+ prompt "LOOP_CONFIGURE or LOOP_SET_FD + LOOP_SET_STATUS"
+ default TRY_LOOP_CONFIGURE
+ help
+ LOOP_CONFIGURE is added to Linux 5.8
+ https://lwn.net/Articles/820408/
+ This allows userspace to completely setup a loop device with a single
+ ioctl, removing the in-between state where the device can be partially
+ configured - eg the loop device has a backing file associated with it,
+ but is reading from the wrong offset.
+
+config LOOP_CONFIGURE
+ bool "always uses LOOP_CONFIGURE, kernel version >= 5.8"
+
+config NO_LOOP_CONFIGURE
+ bool "never uses LOOP_CONFIGURE, kernel version < 5.8"
+
+config TRY_LOOP_CONFIGURE
+ bool "try LOOP_CONFIGURE, kernel version is unknown"
+
+endchoice
diff --git a/libbb/loop.c b/libbb/loop.c
index ddb92fce3..be592bc4b 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,7 +126,8 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
-static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
+#ifndef CONFIG_LOOP_CONFIGURE
+static int set_loop_configure_old(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
bb_loop_info loopinfo2;
@@ -149,6 +150,46 @@ static int set_loop_configure(int ffd, int lfd,
bb_loop_info *loopinfo)
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
return -1;
}
+#endif
+
+#ifndef CONFIG_NO_LOOP_CONFIGURE
+
+#ifndef LOOP_CONFIGURE
+#define LOOP_CONFIGURE 0x4C0A
+struct loop_config {
+ uint32_t fd;
+ uint32_t block_size;
+ struct loop_info64 info;
+ uint64_t __reserved[8];
+};
+#endif
+
+/*
+ * linux v5.8.0
+ * loop: Add LOOP_CONFIGURE ioctl
+ * https://lwn.net/Articles/820408/
+ *
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
+ */
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
+{
+ int rc;
+ struct loop_config config;
+
+ memset(, 0, sizeof(config));
+ config.fd = ffd;
+ memcpy(, loopinfo, sizeof(config.info));
+
+ rc = ioctl(lfd, LOOP_CONFIGURE, );
+ if (rc == 0) {
+ return lfd;
+ }
+#ifdef CONFIG_TRY_LOOP_CONFIGURE
+ if (rc == -1 && errno == EINVAL) /* The system may not support
LOOP_CONFIGURE. */
+ return set_loop_configure_old(ffd, lfd, loopinfo);
+#endif
+ return -1;
+}
+#endif
static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
{
@@ -159,7 +200,11 @@ static int set_loop_info(int ffd, int lfd, bb_loop_info
*loopinfo)
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
+#ifdef CONFIG_NO_LOOP_CONFIGURE
+ return set_loop_configure_old(ffd, lfd, loopinfo);
+#else
return set_loo
[PATCH v2 8/9] loop:refactor: extract subfunction set_loop_dev()
Step 8 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop_dev()
function old new delta
set_loop 700 716 +16
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 16/0) Total: 16 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 39 ---
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 67e16ddb0..ddb92fce3 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -201,6 +201,26 @@ static int do_stat_and_mknod(const char *dev, const char
*device, int i)
return 0;
}
+static int set_loop_dev(const char *dev, int *mode, int ffd, bb_loop_info
*loopinfo)
+{
+ int rc;
+ /* Open the sucker and check its loopiness */
+ int lfd = open(dev, *mode);
+ if (lfd < 0 && errno == EROFS) {
+ *mode = O_RDONLY;
+ lfd = open(dev, *mode);
+ }
+ if (lfd < 0) {
+ return lfd;
+ }
+ rc = set_loop_info(ffd, lfd, loopinfo);
+ if (rc == lfd) {
+ return lfd;
+ }
+ close(lfd);
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
@@ -211,7 +231,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- int i, lfd, ffd, mode, rc;
+ int i, ffd, mode, rc;
bb_loop_info loopinfo;
ffd = open_file(file, flags, );
@@ -242,30 +262,19 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
break;
}
open_lfd:
- /* Open the sucker and check its loopiness */
- lfd = rc = open(try, mode);
- if (lfd < 0 && errno == EROFS) {
- mode = O_RDONLY;
- lfd = rc = open(try, mode);
- }
- if (lfd < 0) {
+ rc = set_loop_dev(try, , ffd, );
+ if (rc == -1) {
if (errno == ENXIO) {
/* Happens if loop module is not loaded */
/* rc is -1; */
break;
}
- goto try_next_loopN;
- }
- rc = set_loop_info(ffd, lfd, );
- if (rc == lfd) {
+ } else {
/* SUCCESS! */
if (!*device)
*device = xstrdup(try);
break;
}
- close(lfd);
- try_next_loopN:
- rc = -1;
if (*device) /* was looking for a particular "/dev/loopN"? */
break; /* yes, do not try other names */
} /* for() */
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v2 1/9] loop:refactor: extract subfunction open_file()
Step 1 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop()
function old new delta
set_loop 760 758 -2
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2) Total: -2 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 750642ade..c517ceb13 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,22 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int open_file(const char *file, unsigned flags, int *mode)
+{
+ int ffd;
+ /* open the file. barf if this doesn't work. */
+ *mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
+ retry_open_ffd:
+ ffd = open(file, *mode);
+ if (ffd < 0) {
+ if (*mode != O_RDONLY) {
+ *mode = O_RDONLY;
+ goto retry_open_ffd;
+ }
+ }
+ return ffd;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -109,15 +125,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
struct stat statbuf;
int i, lfd, ffd, mode, rc;
- /* Open the file. Barf if this doesn't work. */
- mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
- open_ffd:
- ffd = open(file, mode);
+ ffd = open_file(file, flags, );
if (ffd < 0) {
- if (mode != O_RDONLY) {
- mode = O_RDONLY;
- goto open_ffd;
- }
return -errno;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 9/9] loop: Add LOOP_CONFIGURE ioctl
On 2022/11/18 17:13, Kang-Che Sung wrote: On Fri, Nov 18, 2022 at 9:04 AM Xiaoming Ni wrote: LOOP_CONFIGURE is added to Linux 5.8 This allows userspace to completely setup a loop device with a single ioctl, removing the in-between state where the device can be partially configured - eg the loop device has a backing file associated with it, but is reading from the wrong offset. https://lwn.net/Articles/820408/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5 Hello. 1. Are the patches 1 to 8 you proposed in this set _not_ relevant to the new LOOP_CONFIGURE ioctl? I.e. They are general code structure improvements to the busybox loop device code and do not add any feature? Patches 1-8 are micro-refactoring, not new features. Note that according to the bloat-o-meter results you included in the patches, some of them increase code size, and you should explain why the code size increases there, and why you think it is okay for the patch to increase code size. In patches 1-8, each patch increases or decreases the code size, but the end result is a reduction in the code size. see [PATCH 0/9] loop: Micro-refactoring set_loop() and add LOOP_CONFIGURE 2. I think LOOP_CONFIGURE support can be made into a config option, so that builders can have choice on which algorithm should be built into their busybox binary. * Always use LOOP_CONFIGURE ioctl, or * Support LOOP_CONFIGURE, but keep the old code for runtime fallback, or * Not support LOOP_CONFIGURE at all (always use the old code) . Thanks, I will add the switch control in the v2 patch. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 9/9] loop: Add LOOP_CONFIGURE ioctl
On 2022/11/18 14:34, Lauri Kasanen wrote:
On Fri, 18 Nov 2022 09:01:58 +0800
Xiaoming Ni wrote:
LOOP_CONFIGURE is added to Linux 5.8
...
diff --git a/libbb/loop.c b/libbb/loop.c
index 9a7ca666d..d4f4906b0 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,30 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,8,0)
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
+#else
This assumes compile-time matches runtime. Ie, if you build on a 5.8
system, but run on an earlier kernel, the resulting busybox would be
unable to mount loops at all.
Please put it backward runtime compatibility, probably by having the
new-kernel code in the same func.
+ rc = ioctl(lfd, LOOP_CONFIGURE, );
+ if (rc == 0) {
+ return lfd;
+ }
+ if (rc == -1 && errno == EINVAL) /* The system may not support
RING_CONFIGURE. */
+ return set_loop_configure_old(ffd, lfd, loopinfo);
+ return -1;
Is this ok?
- Lauri
.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 8/9] loop:refactor: extract subfunction set_loop_dev()
Step 8 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop_dev()
function old new delta
set_loop 700 703 +3
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 3/0) Total: 3 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 41 -
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 67e16ddb0..9a7ca666d 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -201,6 +201,26 @@ static int do_stat_and_mknod(const char *dev, const char
*device, int i)
return 0;
}
+static int set_loop_dev(const char *dev, int *mode, int ffd, bb_loop_info
*loopinfo)
+{
+ int rc;
+ /* Open the sucker and check its loopiness */
+ int lfd = open(dev, *mode);
+ if (lfd < 0 && errno == EROFS) {
+ *mode = O_RDONLY;
+ lfd = open(dev, *mode);
+ }
+ if (lfd < 0) {
+ return lfd;
+ }
+ rc = set_loop_info(ffd, lfd, loopinfo);
+ if (rc == lfd) {
+ return lfd;
+ }
+ close(lfd);
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
@@ -211,7 +231,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- int i, lfd, ffd, mode, rc;
+ int i, ffd, mode, rc;
bb_loop_info loopinfo;
ffd = open_file(file, flags, );
@@ -242,32 +262,19 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
break;
}
open_lfd:
- /* Open the sucker and check its loopiness */
- lfd = rc = open(try, mode);
- if (lfd < 0 && errno == EROFS) {
- mode = O_RDONLY;
- lfd = rc = open(try, mode);
- }
- if (lfd < 0) {
+ rc = set_loop_dev(try, , ffd, );
+ if (rc == -1) {
if (errno == ENXIO) {
/* Happens if loop module is not loaded */
/* rc is -1; */
break;
}
- goto try_next_loopN;
- }
- rc = set_loop_info(ffd, lfd, );
- if (rc == lfd) {
+ } else {
/* SUCCESS! */
if (!*device)
*device = xstrdup(try);
break;
}
- close(lfd);
- try_next_loopN:
- rc = -1;
- if (*device) /* was looking for a particular "/dev/loopN"? */
- break; /* yes, do not try other names */
} /* for() */
close(ffd);
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/9] loop:refactor: extract subfunction open_file()
Step 1 of micro-refactoring the set_loop():
Extract subfunction open_file() from set_loop()
function old new delta
set_loop 760 758 -2
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2) Total: -2 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 750642ade..c517ceb13 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,22 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int open_file(const char *file, unsigned flags, int *mode)
+{
+ int ffd;
+ /* open the file. barf if this doesn't work. */
+ *mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
+ retry_open_ffd:
+ ffd = open(file, *mode);
+ if (ffd < 0) {
+ if (*mode != O_RDONLY) {
+ *mode = O_RDONLY;
+ goto retry_open_ffd;
+ }
+ }
+ return ffd;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -109,15 +125,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
struct stat statbuf;
int i, lfd, ffd, mode, rc;
- /* Open the file. Barf if this doesn't work. */
- mode = (flags & BB_LO_FLAGS_READ_ONLY) ? O_RDONLY : O_RDWR;
- open_ffd:
- ffd = open(file, mode);
+ ffd = open_file(file, flags, );
if (ffd < 0) {
- if (mode != O_RDONLY) {
- mode = O_RDONLY;
- goto open_ffd;
- }
return -errno;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 9/9] loop: Add LOOP_CONFIGURE ioctl
LOOP_CONFIGURE is added to Linux 5.8
This allows userspace to completely setup a loop device with a single
ioctl, removing the in-between state where the device can be partially
configured - eg the loop device has a backing file associated with it,
but is reading from the wrong offset.
https://lwn.net/Articles/820408/
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
LINUX_VERSION_CODE >= KERNEL_VERSION(5,8,0):
function old new delta
set_loop 703 626 -77
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-77) Total:
-77 bytes
else:
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/libbb/loop.c b/libbb/loop.c
index 9a7ca666d..d4f4906b0 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,30 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,8,0)
+
+/*
+ * loop: Add LOOP_CONFIGURE ioctl
+ * https://lwn.net/Articles/820408/
+ *
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
+ */
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
+{
+ int rc;
+ struct loop_config config;
+
+ memset(, 0, sizeof(config));
+ config.fd = ffd;
+ memcpy(, loopinfo, sizeof(config.info));
+
+ rc = ioctl(lfd, LOOP_CONFIGURE, );
+ if (rc == 0) {
+ return lfd;
+ }
+ return -1;
+}
+
+#else
static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
@@ -149,6 +173,7 @@ static int set_loop_configure(int ffd, int lfd,
bb_loop_info *loopinfo)
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
return -1;
}
+#endif
static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
{
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 3/9] loop:refactor: del close_and_try_next_loopN
Step 3 of micro-refactoring the set_loop() function:
Delete close_and_try_next_loopN.
(add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0) Total: 0 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 71fd8c1bc..91c3a45c4 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -197,13 +197,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc && errno == ENXIO) {
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ close(lfd);
/* Ouch. Are we racing with other mount? */
if (!*device) {
- close(lfd);
//TODO: add "if (--failcount != 0) ..."?
continue;
+ } else {
+ break;
}
- goto close_and_try_next_loopN;
}
memset(, 0, sizeof(loopinfo));
safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
@@ -233,7 +234,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
}
- close_and_try_next_loopN:
close(lfd);
try_next_loopN:
rc = -1;
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 7/9] loop:refactor: Extract subfunction do_stat_and_mknod()
Step 7 of micro-refactoring the set_loop():
Extract subfunction do_stat_and_mknod()
function old new delta
set_loop 720 700 -20
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20) Total: -20 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 39 +--
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 2200ccb9a..67e16ddb0 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -180,6 +180,28 @@ static void init_bb_loop_info(bb_loop_info *loopinfo,
const char *file,
loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
}
+static int do_stat_and_mknod(const char *dev, const char *device, int i)
+{
+ struct stat statbuf;
+
+ IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
+ if (stat(dev, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
+ if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
+ && errno == ENOENT
+ && (!device)
+ ) {
+ /* Node doesn't exist, try to create it */
+ if (mknod(dev, S_IFBLK|0644, makedev(7, i)) == 0) {
+ return 0;
+ }
+ }
+ /* Ran out of block devices, return failure */
+ return -1;
+ }
+ return 0;
+}
+
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -189,7 +211,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- struct stat statbuf;
int i, lfd, ffd, mode, rc;
bb_loop_info loopinfo;
@@ -216,18 +237,8 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
}
- IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
- if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
- if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
-&& errno == ENOENT
-&& (!*device)
- ) {
- /* Node doesn't exist, try to create it */
- if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
- goto open_lfd;
- }
- /* Ran out of block devices, return failure */
- rc = -1;
+ rc = do_stat_and_mknod(try, *device, i);
+ if (rc == -1) {
break;
}
open_lfd:
@@ -249,7 +260,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
if (rc == lfd) {
/* SUCCESS! */
if (!*device)
- *device = xstrdup(dev);
+ *device = xstrdup(try);
break;
}
close(lfd);
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 5/9] loop:refactor: extract subfunction set_loop_configure()
Step 5 of micro-refactoring the set_loop():
Extract subfunction set_loop_configure()
function old new delta
set_loop 700 708 +8
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 8/0) Total: 8 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 63 ++--
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 89adc4f94..914af57b2 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,41 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_configure(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return -1;
+}
+
static int set_loop_info(int ffd, int lfd, const char *file,
unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
{
@@ -136,33 +171,7 @@ static int set_loop_info(int ffd, int lfd, const char
*file,
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- return -1;
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
- /* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- }
- if (rc == 0) {
- return lfd;
- }
- /* failure, undo LOOP_SET_FD */
- ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
}
return -1;
}
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/9] loop:refactor: extract subfunction get_next_free_loop()
Step 2 of micro-refactoring the set_loop function ()
Extract subfunction get_next_free_loop() from set_loop()
Also fix miss free(try) when stat(try) and mknod fail
function old new delta
set_loop 758 734 -24
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24 bytes
Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 55
1 file changed, 25 insertions(+), 30 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index c517ceb13..71fd8c1bc 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void)
return loopdevno; /* can be -1 if error */
}
+static int get_next_free_loop(char *dev, int id)
+{
+ int i = get_free_loop();
+ if (i >= 0) {
+ sprintf(dev, LOOP_FORMAT, i);
+ return 1; /* use /dev/loop-control */
+ } else if (i == -2) {
+ sprintf(dev, LOOP_FORMAT, id);
+ return 2;
+ } else {
+ return -1; /* no free loop devices */
+ }
+}
+
static int open_file(const char *file, unsigned flags, int *mode)
{
int ffd;
@@ -132,30 +146,26 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
try = *device;
if (!try) {
- get_free_loopN:
- i = get_free_loop();
- if (i == -1) {
- close(ffd);
- return -1; /* no free loop devices */
- }
- if (i >= 0) {
- try = xasprintf(LOOP_FORMAT, i);
- goto open_lfd;
- }
- /* i == -2: no /dev/loop-control. Do an old-style search for a
free device */
try = dev;
}
/* Find a loop device */
/* 0xf is a max possible minor number in Linux circa 2010 */
for (i = 0; i <= 0xf; i++) {
- sprintf(dev, LOOP_FORMAT, i);
+ if (!*device) {
+ rc = get_next_free_loop(dev, i);
+ if (rc == -1) {
+ break; /* no free loop devices */
+ } else if (rc == 1) {
+ goto open_lfd;
+ }
+ }
IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
if (stat(try, ) != 0 || !S_ISBLK(statbuf.st_mode)) {
if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
&& errno == ENOENT
-&& try == dev
+&& (!*device)
) {
/* Node doesn't exist, try to create it */
if (mknod(dev, S_IFBLK|0644, makedev(7, i)) ==
0)
@@ -188,13 +198,10 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
/* Ouch. Are we racing with other mount? */
- if (!*device /* yes */
-&& try != dev /* tried a _kernel-offered_
loopN? */
- ) {
- free(try);
+ if (!*device) {
close(lfd);
//TODO: add "if (--failcount != 0) ..."?
- goto get_free_loopN;
+ continue;
}
goto close_and_try_next_loopN;
}
@@ -218,8 +225,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
if (rc == 0) {
/* SUCCESS! */
- if (try != dev) /* tried a kernel-offered free
loopN? */
- *device = try; /* malloced */
if (!*device) /* was looping in search of
free "/dev/loopN"? */
*device = xstrdup(dev);
rc = lfd; /* return this */
@@ -227,16 +232,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
- } else {
- /* device is not free (rc =
[PATCH 4/9] loop:refactor: extract subfunction set_loop_info()
Step 4 of micro-refactoring the set_loop():
Extract subfunction set_loop_info() from set_loop()
function old new delta
set_loop 734 700 -34
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-34) Total: -34 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 91 +++-
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 91c3a45c4..89adc4f94 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,6 +126,47 @@ static int open_file(const char *file, unsigned flags, int
*mode)
return ffd;
}
+static int set_loop_info(int ffd, int lfd, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ int rc;
+ bb_loop_info loopinfo;
+
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+
+ /* If device is free, try to claim it */
+ if (rc && errno == ENXIO) {
+ /* Associate free loop device with file */
+ if (ioctl(lfd, LOOP_SET_FD, ffd)) {
+ return -1;
+ }
+ memset(, 0, sizeof(loopinfo));
+ safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
+ loopinfo.lo_offset = offset;
+ loopinfo.lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by
open type of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
+ /* (this code path is not tested) */
+ loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ }
+ if (rc == 0) {
+ return lfd;
+ }
+ /* failure, undo LOOP_SET_FD */
+ ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
+ }
+ return -1;
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -135,7 +176,6 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
{
char dev[LOOP_NAMESIZE];
char *try;
- bb_loop_info loopinfo;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
@@ -190,49 +230,12 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
-
- /* If device is free, try to claim it */
- if (rc && errno == ENXIO) {
- /* Associate free loop device with file */
- if (ioctl(lfd, LOOP_SET_FD, ffd)) {
- close(lfd);
- /* Ouch. Are we racing with other mount? */
- if (!*device) {
-//TODO: add "if (--failcount != 0) ..."?
- continue;
- } else {
- break;
- }
- }
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file,
LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is
controlled by open type of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before
mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags &
BB_LO_FLAGS_AUTOCLEAR)) {
- /* Old kernel, does not support
LO_FLAGS_AUTOCLEAR? */
-
[PATCH 6/9] loop:refactor: Use a structure to reduce parameters
Step 6 of micro-refactoring the set_loop():
Use structs to avoid transferring a large number of parameters
in set_loop_configure() and set_loop_info()
function old new delta
set_loop 708 720 +12
--
(add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0) Total: 12 bytes
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 54 +---
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index 914af57b2..2200ccb9a 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -126,32 +126,21 @@ static int open_file(const char *file, unsigned flags,
int *mode)
return ffd;
}
-static int set_loop_configure(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info loopinfo2;
/* Associate free loop device with file */
if (ioctl(lfd, LOOP_SET_FD, ffd)) {
return -1;
}
- memset(, 0, sizeof(loopinfo));
- safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
- loopinfo.lo_offset = offset;
- loopinfo.lo_sizelimit = sizelimit;
- /*
-* Used by mount to set LO_FLAGS_AUTOCLEAR.
-* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
-* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
-* is wrong (would free the loop device!)
-*/
- loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
- if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
+ if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
+ memcpy(, loopinfo, sizeof(*loopinfo));
/* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
/* (this code path is not tested) */
- loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
- rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
+ loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
+ rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
}
if (rc == 0) {
return lfd;
@@ -161,21 +150,36 @@ static int set_loop_configure(int ffd, int lfd, const
char *file,
return -1;
}
-static int set_loop_info(int ffd, int lfd, const char *file,
- unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
{
int rc;
- bb_loop_info loopinfo;
+ bb_loop_info tmp;
- rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
+ rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
/* If device is free, try to claim it */
if (rc && errno == ENXIO) {
- return set_loop_configure(ffd, lfd, file, offset, sizelimit,
flags);
+ return set_loop_configure(ffd, lfd, loopinfo);
}
return -1;
}
+static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
+ unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
+{
+ memset(loopinfo, 0, sizeof(*loopinfo));
+ safe_strncpy((char *)loopinfo->lo_file_name, file, LO_NAME_SIZE);
+ loopinfo->lo_offset = offset;
+ loopinfo->lo_sizelimit = sizelimit;
+ /*
+* Used by mount to set LO_FLAGS_AUTOCLEAR.
+* LO_FLAGS_READ_ONLY is not set because RO is controlled by open type
of the file.
+* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
+* is wrong (would free the loop device!)
+*/
+ loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
+}
+
/* Returns opened fd to the loop device, <0 on error.
* *device is loop device to use, or if *device==NULL finds a loop device to
* mount it on and sets *device to a strdup of that loop device name.
@@ -187,12 +191,14 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
char *try;
struct stat statbuf;
int i, lfd, ffd, mode, rc;
+ bb_loop_info loopinfo;
ffd = open_file(file, flags, );
if (ffd < 0) {
return -errno;
}
+ init_bb_loop_info(, file, offset, sizelimit, flags);
try = *device;
if (!try) {
try = dev;
@@ -239,7 +245,7 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
goto try_next_loopN;
}
-
[PATCH 0/9] loop: Micro-refactoring set_loop() and add LOOP_CONFIGURE
LOOP_CONFIGURE is added to Linux 5.8 This allows userspace to completely setup a loop device with a single ioctl, removing the in-between state where the device can be partially configured - eg the loop device has a backing file associated with it, but is reading from the wrong offset. https://lwn.net/Articles/820408/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5 To facilitate the addition of new functions, the code of set_loop() is micro-refactored. LINUX_VERSION_CODE >= KERNEL_VERSION(5,8,0): function old new delta set_loop 760 626-134 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-134) Total: -134 bytes else function old new delta set_loop 760 703 -57 -- (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-57) Total: -57 bytes ---- Xiaoming Ni (9): loop:refactor: extract subfunction open_file() loop:refactor: extract subfunction get_next_free_loop() loop:refactor: del close_and_try_next_loopN loop:refactor: extract subfunction set_loop_info() loop:refactor: extract subfunction set_loop_configure() loop:refactor: Use a structure to reduce parameters loop:refactor: Extract subfunction do_stat_and_mknod() loop:refactor: extract subfunction set_loop_dev() loop: Add LOOP_CONFIGURE ioctl libbb/loop.c | 275 +++ 1 file changed, 170 insertions(+), 105 deletions(-) -- 2.27.0 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
ping //Re: [PATCH] loop: fix a race when a free loop device is snatched
ping
On 2022/10/21 15:10, Xiaoming Ni wrote:
When /dev/loop-control exists and *device is empty, the mounting fails
due to concurrent contention.
Code Execution Flow:
try = xasprintf(LOOP_FORMAT, i);
for (i = 0; i <= 0xf; i++) { // The value of "try" is not changed.
...
lfd = rc = open(try, mode);
...
rc = repeat_on_eagain(ioctl(lfd, BB_LOOP_GET_STATUS,
));
// Because of race, the value of "rc" is 0. and the value of
"try" is not changed
...
close(lfd);
}
add/remove: 0/0 grow/shrink: 1/0 up/down: 5/0 (5)
Function old new delta
set_loop 773 778 +5
Fixes: 4bc59a4cf ("mount: fix a race when a free loop device is snatched
under us by another mount")
Fiexe: 3b69ba799 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index cb8fa2442..845565d7b 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -218,8 +218,13 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
+ } else {
+ if (rc == 0 && *device == NULL && try != dev) {
+ free(try);
+ close(lfd);
+ goto get_free_loopN;
+ }
}
- /* else: device is not free (rc == 0) or error other than ENXIO
*/
close_and_try_next_loopN:
close(lfd);
try_next_loopN:
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] loop: fix a race when a free loop device is snatched
When /dev/loop-control exists and *device is empty, the mounting fails
due to concurrent contention.
Code Execution Flow:
try = xasprintf(LOOP_FORMAT, i);
for (i = 0; i <= 0xf; i++) { // The value of "try" is not changed.
...
lfd = rc = open(try, mode);
...
rc = repeat_on_eagain(ioctl(lfd, BB_LOOP_GET_STATUS,
));
// Because of race, the value of "rc" is 0. and the value of
"try" is not changed
...
close(lfd);
}
add/remove: 0/0 grow/shrink: 1/0 up/down: 5/0 (5)
Function old new delta
set_loop 773 778 +5
Fixes: 4bc59a4cf ("mount: fix a race when a free loop device is snatched
under us by another mount")
Fiexe: 3b69ba799 ("mount,losetup: use /dev/loop-control is it exists")
Signed-off-by: Xiaoming Ni
---
libbb/loop.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libbb/loop.c b/libbb/loop.c
index cb8fa2442..845565d7b 100644
--- a/libbb/loop.c
+++ b/libbb/loop.c
@@ -218,8 +218,13 @@ int FAST_FUNC set_loop(char **device, const char *file,
unsigned long long offse
}
/* failure, undo LOOP_SET_FD */
ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is
unnecessary
+ } else {
+ if (rc == 0 && *device == NULL && try != dev) {
+ free(try);
+ close(lfd);
+ goto get_free_loopN;
+ }
}
- /* else: device is not free (rc == 0) or error other than ENXIO
*/
close_and_try_next_loopN:
close(lfd);
try_next_loopN:
--
2.27.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
