Re: Automatic deletion of loopback device upon umount?

[Denys Vlasenko]

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

Re: [PATCH] dd: call fsync() only once before exiting if conv=fsync is specified

[Ari Sundholm]

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

[Rostislav Skudnov]

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

[Maxime Coste]

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

[Grant Edwards]

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

[Lauri Kasanen]

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