Re: Automatic deletion of loopback device upon umount?
On Tue, Mar 7, 2017 at 6:12 PM, Mirko Vogt wrote: >> "foo" is a cramfs filesystem file which might help illustrate the issue >> >> 1) loopback mount "foo" to mount /bar >> 2) umount /bar >> 3) append new files and re-generate the "foo" cramfs image >> 4) loopback mount "foo" to mount /bar >> 5) the contents of /bar are the same as in 1) and not 3) >> >> the reason for this is because busybox has not deleted the "foo" <-> >> loopback device mapping, so when we attempt to mount again "foo", the >> mapping is already there, so the loopback device is not deleted then >> re-created. >> >> Obviously using umount -d in 2) fixes the issue, but I was wondering >> whether it would not be preferable to unconditionnaly delete the >> loopback device upon umount? util-linux does this actually, so other >> users might also be puzzled by such a case. > > I'm quoting most of the mail since it has been a while.. this "feature" > hit me today, however from a different angle. > > It seems LOOP_CLR_FD called on a loop-*partition* removes the mapping of > the whole *device* - which results in the following: > > root@LEDE:/# loop=$(losetup -f) > root@LEDE:/# echo ${loop} > /dev/loop2 > root@LEDE:/# losetup ${loop} /IMAGE > root@LEDE:/# ls -l ${loop}* > brw--- 1 root root 7, 2 Mar 6 20:09 /dev/loop2 > root@LEDE:/# partprobe ${loop} > root@LEDE:/# ls -l ${loop}* > brw--- 1 root root7, 2 Mar 6 20:09 /dev/loop2 > brw--- 1 root root 259, 8 Mar 6 21:59 /dev/loop2p1 > brw--- 1 root root 259, 9 Mar 6 21:59 /dev/loop2p2 > brw--- 1 root root 259, 10 Mar 6 21:59 /dev/loop2p3 > brw--- 1 root root 259, 11 Mar 6 21:59 /dev/loop2p4 > brw--- 1 root root 259, 12 Mar 6 21:59 /dev/loop2p5 > brw--- 1 root root 259, 13 Mar 6 21:59 /dev/loop2p6 > brw--- 1 root root 259, 14 Mar 6 21:59 /dev/loop2p7 > brw--- 1 root root 259, 15 Mar 6 21:59 /dev/loop2p8 > root@LEDE:/# mount ${loop}p8 /MOUNT # mount loop partition > root@LEDE:/# losetup -a | grep $loop # loop dev mapping still there > /dev/loop2: 0 /mnt/IMAGE > root@LEDE:/# strace umount /MOUNT 2> /log # unmount loop partition > root@LEDE:/# losetup -a | grep ${loop}# loop device mapping is gone > root@LEDE:/# grep -i loop /log > open("/dev/loop2p7", O_RDONLY|O_LARGEFILE) = 3 > ioctl(3, LOOP_CLR_FD) = 0 > root@LEDE:/# > > The strace was done to figure out, if maybe umount wrongly ioctl()'s the > parent device instead of the partition - it doesn't. > > I already wasn't a fan of umount implicitly removing the mapping in the > first place (as I usually setup and release loop devices with `losetup` > and scripts needed to call umount differently in order to work and > outside busybox). > > However taking above (kernel-)behaviour into account - umount calling > ioctl(LOOP_CLR_FD) unconditionally potentially causes some nasty side > effects, why I'd like to re-open that discussion. What a surprise. No one foresaw this to bite, right? ;) [to understand the snark, read the entire thread] Now seriously. You can use umount -D to suppress this. Unfortunately, util-linux's umount does not have that option, thus your scripts would become incompatible with util-linux if you go that route. Their manpage says: LOOP DEVICE The umount command will free the loop device associated with a mount when it finds the option loop=... in /etc/mtab, or when the -d option was given. Any still associated loop devices can be freed by using losetup -d; see losetup(8). We can try to match this description. However, it is lying: util-linux's umount somehow detects automatic loop mounts even when /etc/mtab does not contain "loop". First, I'm using util-linux's mount on a 1mbyte ext2 image: # mount -oloop z zz # ls -l /etc/mtab lrwxrwxrwx. 1 root root 19 May 24 2016 /etc/mtab -> ../proc/self/mounts # cat /etc/mtab | grep loop /dev/loop0 /home/srcdevel/bbox/fix/busybox.t6/zz ext2 rw,relatime,block_validity,barrier,user_xattr,acl 0 0 ^^^ no "loop" # umount zz # cat /etc/mtab | grep loop # losetup : /dev/loop0 freed Now the same with bbox mount: # ./busybox mount -oloop z zz # cat /etc/mtab | grep loop /dev/loop0 /home/srcdevel/bbox/fix/busybox.t6/zz ext4 rw,relatime,block_validity,delalloc,barrier,user_xattr,acl 0 0 ^^^ no "loop" # umount zz # losetup NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE DIO /dev/loop0 0 0 0 0 /home/srcdevel/bbox/fix/busybox.t6/z 0 Clearly, util-linux umount looks somewhere else to determine whether to free the loop device. Hmm... # mount -oloop z zz # losetup NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE DIO /dev/loop0 0 0 1 0 /home/srcdevel/bbox/fix/busybox.t6/z 0 It's probably the "AUTOCLEAR" thing. ___ busybox mailing list busybox@busybox.net http://lis
Re: [PATCH] dd: call fsync() only once before exiting if conv=fsync is specified
Hi! On 03/14/2017 06:44 PM, Rostislav Skudnov wrote: Signed-off-by: Rostislav Skudnov It would probably be useful to tell why this change was made: it makes the behavior consistent with coreutils dd and fixes performance problems caused by calling fsync() over and over again. --- coreutils/dd.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/coreutils/dd.c b/coreutils/dd.c index 5e68087..d89c0ae 100644 --- a/coreutils/dd.c +++ b/coreutils/dd.c @@ -532,17 +532,17 @@ int dd_main(int argc UNUSED_PARAM, char **argv) if (write_and_stats(ibuf, n, obs, outfile)) goto out_status; } - - if (G.flags & FLAG_FSYNC) { - if (fsync(ofd) < 0) - goto die_outfile; - } } if (ENABLE_FEATURE_DD_IBS_OBS && oc) { if (write_and_stats(obuf, oc, obs, outfile)) goto out_status; } + + if (G.flags & FLAG_FSYNC) { + if (fsync(ofd) < 0) + goto die_outfile; + } if (close(ifd) < 0) { die_infile: bb_simple_perror_msg_and_die(infile); Maybe a blank line between the closing brace and the next if statement to make the look more consistent? Best regards, Ari Sundholm a...@tuxera.com ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] dd: call fsync() only once before exiting if conv=fsync is specified
Signed-off-by: Rostislav Skudnov --- coreutils/dd.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/coreutils/dd.c b/coreutils/dd.c index 5e68087..d89c0ae 100644 --- a/coreutils/dd.c +++ b/coreutils/dd.c @@ -532,17 +532,17 @@ int dd_main(int argc UNUSED_PARAM, char **argv) if (write_and_stats(ibuf, n, obs, outfile)) goto out_status; } - - if (G.flags & FLAG_FSYNC) { - if (fsync(ofd) < 0) - goto die_outfile; - } } if (ENABLE_FEATURE_DD_IBS_OBS && oc) { if (write_and_stats(obuf, oc, obs, outfile)) goto out_status; } + + if (G.flags & FLAG_FSYNC) { + if (fsync(ofd) < 0) + goto die_outfile; + } if (close(ifd) < 0) { die_infile: bb_simple_perror_msg_and_die(infile); -- 2.1.4 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] add paste implementation
On Mon, Mar 13, 2017 at 09:30:56AM -0400, Jody Lee Bruchon wrote: > Given that BusyBox is intended to be as small as possible since it is > targeted at embedded platforms, I'd say that declaring variables inside of a > code block which are used only within a code block is better. Not only does > it de-clutter the top of the function, it also tells the compiler that the > variable is local to that code block which can help with optimization. The > compiler may be smart enough to notice this anyway, but it doesn't hurt to > remove all doubt. > > The only instance I can think of where this wouldn't be proper is if you use > a throwaway variable like 'i' for counting something and reuse it later in > the function (after its first use won't ever be needed again, of course) to > save on stack variable allocations. It makes more sense to me for embedded > platform code to reuse such variables instead of ending up with a pile of i, > j, k, etc. consuming more space unnecessarily. However for a single use in a > loop it should be declared within the code block. Even for that, I'd expect modern compilers to reuse the same stack space for these, as it can very easily see their use dont overlap. Regarding the point of declaration of variables, I am a bit confused now, I see there are different opinions, but I dont know who has the last word on such matter. I'd be happy to do whichever change is needed once its decided, along with preparing a proper commit message. Is there anything else preventing this from being pushed upstream ? Cheers, Maxime. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Problem with cat
On 2017-03-14, Lauri Kasanen wrote: > On Mon, 13 Mar 2017 19:24:17 + (UTC) > Grant Edwards wrote: > >> That paragraph is wrong (or at least misleading). I've checked the >> source code, and 2.6.33 does not update the output file postition (at >> least not the version I downloaded from kernels.org nor the version I >> got from Atmel). Perhaps the author of that man page had a 2.6.33 with >> some 3.0 stuff backported. > > Please report the bug to them. I've found the man-pages project quite > responsive. Done... thanks for reminding me. Unfortunately, I can't build/test kernels between 2.6.33.7 (sendfile->file broken), and 3.2 (works) to find the exact point where the change that correctly sets the output file position happened. -- Grant Edwards grant.b.edwardsYow! My face is new, my at license is expired, and I'm gmail.comunder a doctor's care ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Problem with cat
On Mon, 13 Mar 2017 19:24:17 + (UTC) Grant Edwards wrote: > That paragraph is wrong (or at least misleading). I've checked the > source code, and 2.6.33 does not update the output file postition (at > least not the version I downloaded from kernels.org nor the version I > got from Atmel). Perhaps the author of that man page had a 2.6.33 with > some 3.0 stuff backported. Please report the bug to them. I've found the man-pages project quite responsive. - Lauri ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox