[Toybox] [PATCH] df: -a should even show mounts we couldn't stat.
Yi-yo Chiang
yochiang at google.com
Thu Feb 18 08:58:35 PST 2021
> it's not clear to me whether yi-yo is happy yet?
Finally got some time to thoroughly test the *enhanced* df! I'm pretty
happy with the output now, where `df <filesystem>` shows more accurate
results than that of coreutils for over-mounted filesystems, and `df -a`
shows all mounts no matter what (best effort output), so I can use `df -a |
grep <filesystem-ish>` to get a pretty good grasp of the system state.
I also spot another case where toybox diverge from coreutils, and I think
in this case coreutils' output makes more sense:
$ mkdir -p aaa bbb
$ dd if=/dev/zero of=ext4fs.img bs=4k count=2000
$ mkfs.ext4 ext4fs.img
$ sudo mount ext4fs.img aaa
$ sudo mount --bind aaa bbb
$ df -a | grep loop0
/dev/loop0 6721 76 6085 2%
/external/toybox/aaa
/dev/loop0 6721 76 6085 2%
/external/toybox/bbb
$ ./toybox df -a | grep loop0
/dev/loop0 6721 76 6085 2%
/external/toybox/aaa
/dev/loop0 - - - -
/external/toybox/bbb
toybox `df -a` cannot tell "failed stat" and bind-mount apart, they look
the same, "-". I think we should add this back, so we can show bind-mounts
as duplicated lines (Since stat() didn't fail, we shouldn't show "-"):
@@ -177,8 +177,10 @@ void df_main(void)
if (mt->stat.st_dev == mt2->stat.st_dev) {
// For --bind mounts, show earliest mount
if (!strcmp(mt->device, mt2->device)) {
- mt3->stat.st_dev = 0;
- mt3 = mt2;
+ if (!FLAG(a)) {
+ mt3->stat.st_dev = 0;
+ mt3 = mt2;
+ }
} else mt2->stat.st_dev = 0;
}
}
On Thu, Feb 18, 2021 at 6:00 AM Rob Landley <rob at landley.net> wrote:
> On 2/17/21 1:40 PM, enh wrote:
> >
> >
> > On Wed, Feb 17, 2021 at 10:54 AM Rob Landley <rob at landley.net
> > <mailto:rob at landley.net>> wrote:
> >
> > On 2/16/21 3:24 PM, enh wrote:
> > > There's a problem where if we can't stat it, we can't tell
> which one
> > it is out
> > > of /proc/mounts because we can't match the device. And you
> can't do an
> > exact
> > > match on the filename because "df ." is a directory under the
> mount
> > point. I
> > > would have to add plumbing to xabspath() and then do a longest
> match
> > > fileunderdir(), which assumes that the /proc/mounts entry is
> always a
> > cannonical
> > > absolute path? (I _think_ that's true in current kernels but
> would
> > have to read
> > > the source)...
> > >
> > > is this a place where it would help if we switch from /proc/mounts
> to
> > > /proc/self/mountinfo?
> >
> > Maybe, but I need more bandwidth to stare at that than I have
> available right
> > now. (Look up when it showed up in the kernel,
> >
> >
> > it passes that test at least, or i wouldn't have mentioned it: Linux
> 2.6.26 :-)
> >
> >
> > find the Documentation file
> > explaining what the fields are, evaluate whether the new fields are
> useful,
> >
> >
> > for context, https://man7.org/linux/man-pages/man5/procfs.5.html
> mentions mount
> > and parent mount unique ids which is what i thought might be useful.
> there's a
> > ton of other stuff too, and links
> > to https://man7.org/linux/man-pages/man7/mount_namespaces.7.html to
> explain some
> > of it, which in turn -- despite being pretty long -- redirects readers
> > to Documentation/filesystems/sharedsubtree.txt for more!
>
> I meant
>
> https://github.com/torvalds/linux/blob/master/Documentation/filesystems/proc.rst#35-----procpidmountinfo---information-about-mounts
> but that's nice too.
>
> > so, yeah, there does seem to be a large can of worms here.
>
> Oh yeah. I historically know rather a lot of it, but would need to
> refresh...
>
> > figure out what impact if any this has on BSD or macos,
> >
> > they're already using completely different code, that already doesn't
> actually
> > work (it doesn't give the same results as the macOS df, and i haven't
> had chance
> > to look at why [since i don't know of anyone actually using it, and it's
> not in
> > macos_defconfig anyway]). i think it's fine to break macOS a bit more
> for now,
> > and i can worry about that later.
>
> Someday I hope to have bandwidth to reopen the bsd ps/top can of worms...
>
> > and then compare against
> > just _always_ doing the xabspath() string comparison and if that
> simplifies
> > anything while providing sufficient behavior, plus the test suite
> needs
> > updating...)
> >
> > it's not clear to me whether yi-yo is happy yet?
>
> There's a pending issue df on a /dev/node and st_rdev, I might need to dig
> into
> the kernel to see when rdev is set and when I can trust it to be zero, but
> I
> think one pass over everything where I return the first st_dev match (from
> end)
> unless there's an st_rdev match. So one pass over the list that caches the
> first
> st_dev hit and returns it at the end of there was no st_rdev match.
>
> Oh here, let me just do that... commit 9c7085f484db
>
> > for my debian (strictly https://en.wikipedia.org/wiki/GLinux) desktop,
> the only
> > difference between ToT toybox and coreutils right now is:
> >
> > [coreutils]
> > /dev/mapper/glinux_20200515-root 425941416 120332512 283902564 30% /
> >
> > versus:
> >
> > [toybox]
> > /dev/dm-1 425941416 120333508 283901568 30% /
> >
> > which i think is the opposite --- toybox seems to have dereferenced a
> symlink
> > that coreutils _didn't_?
>
> That was the part I wasn't sure about with the absolute path matching
> stuff.
> (Nobody said that the fstab entries were absolute paths, you can have two
> device
> nodes with the same major:minor, there's those uuid aliases... what does
> the
> kernel do with this stuff? No idea, gotta test and dig...) That's why I
> didn't
> do it that way to begin with. :)
>
> > ~$ ls -l /dev/mapper/glinux_20200515-root
> > lrwxrwxrwx 1 root root 7 Feb 16 14:02 /dev/mapper/glinux_20200515-root
> -> ../dm-1
> >
> > removing the xabspath() that's in df.c right now makes toybox match
> coreutils.
>
> Thus confirming that the kernel does _not_ return an absolute path. (I
> remember
> there was a way mount could have the kernel return a _relative_ path, from
> a
> current directory it hadn't recorded, which was annoying...)
>
> I forget, can you --bind mount a symlink? (I know I tried it, I don't
> remember
> the result...)
>
> > (okay, there's one other difference --- coreutils says "-" rather than
> "0%" for
> > filesystems of size 0. so all the /sys or /proc stuff has "-" in the Use%
> > column, not "0%", while still showing 0 in the other columns [because it
> did
> > stat(), but stat() said that the fs was zero-sized, and that's a special
> case?].)
> >
> > happy to send a patch/patches for either/both of those...
>
> Sure, send the patch. But also ask "is matching coreutils exactly the right
> thing to do"? It would be so nice if there were meaningful standards...
>
> Implementing stuff is generally easy. Working out the design goals and
> ramifications thereof (and doing tests to illuminate the corner cases, and
> AUTOMATING those tests in the test suite) is where all the effort goes...
>
> Rob
>
--
Yi-yo Chiang
Software Engineer
yochiang at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20210219/cc5cf7db/attachment-0001.html>
More information about the Toybox
mailing list