Re: [PATCH 9/9] loop: Add LOOP_CONFIGURE ioctl
On Fri, 18 Nov 2022 17:11:17 +0800
Xiaoming Ni wrote:
> >> +#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?
Probably, if it works (I don't remember what errno an invalid ioctl
returns). Also wrong comment (RING_CONFIGURE)
- Lauri
___
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 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? 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. 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) ___ 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
Re: [PATCH 9/9] loop: Add LOOP_CONFIGURE ioctl
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. - Lauri ___ 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
