linux-ext4 - Re: [PATCH 8/9] e2scrub_all: refactor device probe loop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 21 Mar 2019 14:09:08 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Lukas Czerner <lczerner@...hat.com>
Cc:     Theodore Ts'o <tytso@....edu>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        darrick.wong@...cle.com
Subject: Re: [PATCH 8/9] e2scrub_all: refactor device probe loop

On Mar 21, 2019, at 9:57 AM, Lukas Czerner <lczerner@...hat.com> wrote:
> 
> So the good news is that it's not really a problem. What you really want
> is the mountpoint if it exists and the device otherwise right ?
> 
> Then my suggestion will do just that becuase awk will separate the fieds
> with FS (field separator) which by default is a single space. But space
> is special in awk becuase the fields will be separated by a runs of
> space, so if you pick the fileds just right you'll get what you want.
> 
> So if I run this on my system
> 
> # lsblk -o MOUNTPOINT,NAME,FSTYPE -p -n -l `lvs -o lv_path -S lv_active=active,lv_role=public,lv_role!=snapshot,vg_free\>256 --noheadings`
> 
> I'll get
> 
>            /dev/mapper/vg_perpetuum-ext4--suck
>            /dev/mapper/vg_perpetuum-fedora29
>            /dev/mapper/vg_perpetuum-fedora29--2
>            /dev/mapper/vg_perpetuum-freebsd
> /mnt/kernel /dev/mapper/vg_perpetuum-kernel      ext4
> /home       /dev/mapper/vg_perpetuum-lv_home     xfs
> /           /dev/mapper/vg_perpetuum-lv_root     xfs
> [SWAP]      /dev/mapper/vg_perpetuum-lv_swap     swap
> /var        /dev/mapper/vg_perpetuum-lv_var      xfs
>            /dev/mapper/vg_perpetuum-lvol001     ext4
>            /dev/mapper/vg_perpetuum-overlay     ext4
>            /dev/mapper/vg_perpetuum-rhel7
>            /dev/mapper/vg_perpetuum-rhel8
>            /dev/mapper/vg_perpetuum-test0       ext3
>            /dev/mapper/vg_perpetuum-testing     ext4
>            /dev/mapper/vg_perpetuum-testing1    xfs
>            /dev/mapper/vg_perpetuum-thinvolume
> 
> now if I add
> 
> | awk '{print $1}'
> 
> I'll get
> 
> /dev/mapper/vg_perpetuum-ext4--suck
> /dev/mapper/vg_perpetuum-fedora29
> /dev/mapper/vg_perpetuum-fedora29--2
> /dev/mapper/vg_perpetuum-freebsd
> /mnt/kernel
> /home
> /
> [SWAP]
> /var
> /dev/mapper/vg_perpetuum-lvol001
> /dev/mapper/vg_perpetuum-overlay
> /dev/mapper/vg_perpetuum-rhel7
> /dev/mapper/vg_perpetuum-rhel8
> /dev/mapper/vg_perpetuum-test0
> /dev/mapper/vg_perpetuum-testing
> /dev/mapper/vg_perpetuum-testing1
> /dev/mapper/vg_perpetuum-thinvolume
> 
> 
> hence I get mountpoin where the volume is mounted and the device where
> it is not. That's what we need right ?
> 
> What I did not consider was [SWAP] but we can get rid of that easily.
> 
> grep -v '[SWAP]'
> 
> So in the end we'll have something like
> 
> # lsblk -o MOUNTPOINT,NAME,FSTYPE -p -n -l `lvs -o lv_path -S lv_active=active,lv_role=public,lv_role!=snapshot,vg_free\>256 --noheadings` | grep -v '[SWAP]' | grep ' ext[234]$' | awk '{print $1}'

This doesn't need to skip [SWAP] explicitly because it will not have an ext[234]
filesystem mounted on it, so would be excluded by the next "grep" anyway.

However, if you are using "awk" then there is no need for the extra grep(2) I think.
Something like the following looks like it will work:

  lsblk -o MOUNTPOINT,NAME,FSTYPE -p -n -l $(lvs -o lv_path \
      -S lv_active=active,lv_role=public,lv_role!=snapshot,vg_free\>256 --noheadings) |
      awk '($NF > 1 && $2 == ext[234]) || ($NF > 2 && $3 == ext[234]) { print $1 }'

The "$NF" checks appear to be needed because "$3 = ext[234]" seems to match everything
if "$3" is empty, though I can't understand why that would be desirable.

> and so on my system this gives me
> 
> /mnt/kernel
> /dev/mapper/vg_perpetuum-lvol001
> /dev/mapper/vg_perpetuum-overlay
> /dev/mapper/vg_perpetuum-test0
> /dev/mapper/vg_perpetuum-test10
> /dev/mapper/vg_perpetuum-test11
> /dev/mapper/vg_perpetuum-test3
> /dev/mapper/vg_perpetuum-test4
> /dev/mapper/vg_perpetuum-test9
> /dev/mapper/vg_perpetuum-testing
> 
> And leaving out mounted file systems is again as simple as
> 
> grep -v '^/dev/'
> 
> Have I missed something ?
> 
> 
> Now for the performance. I drop caches, then run each twice
> 
> the ls_scan_targets function runs
> 
> real	0m0.575s
> user	0m0.086s
> sys	0m0.179s
> 
> real	0m0.241s
> user	0m0.088s
> sys	0m0.169s
> 
> 
> My one-line suggestion runs
> 
> real	0m0.275s
> user	0m0.027s
> sys	0m0.047s
> 
> real	0m0.069s
> user	0m0.015s
> sys	0m0.052s
> 
> 
> So the difference is significant, although I would not consider it
> meaningful since it's really low anyway, but some people complained so I
> guess someone cares and people do have different systems so...
> 
>> 
>> :-)
>> 
>> We could have used awk to select the field, but that still doesn't
>> deal with the mountpoint column being empty when it is unmounted.  I
>> did briefly consider using lsblk -J, but I didn't want to add a
>> dependency on the jq[1] package (and I didn't even know if RHEL/Fedora
>> packages jq).
>> 
>> [1] https://packages.debian.org/stretch/jq
> 
> Yes Fedora does have it, but I agree that adding this dependency is not
> ideal.
> 
> Thanks!
> -Lukas
> 
>> 
>> Cheers,
>> 
>> 					- Ted


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)