On Thu, Mar 21, 2019 at 02:24:56PM -0400, Theodore Ts'o wrote:
> On Thu, Mar 21, 2019 at 04:57:03PM +0100, Lukas Czerner wrote:
> >
> > hence I get mountpoin where the volume is mounted and the device where
> > it is not. That's what we need right ?
>
> Well, except by default we need to be able to determine whether or not
> the volume is mounted, since by default e2scrub_all only runs on
> mounted file systems (unless -A) is specified.
Right, I did mention it later in the reply. It can be filtered
grep -v '^/dev/'
>
> What I'm now doing is this, which I think is the simplest way to do things:
>
> ls_scan_targets() {
> for NAME in $(lvs -o lv_path --noheadings \
> -S "lv_active=active,lv_role=public,lv_role!=snapshot,vg_free>${snap_size_mb}") ; do
> # Skip non-ext[234]
> case "$(blkid -o value -s TYPE ${NAME})" in
> ext[234]) ;;
> *) continue;;
> esac
>
> if [ "${scrub_all}" -eq 1 ]; then
> echo ${NAME}
> else
> MOUNTPOINT="$(lsblk -o MOUNTPOINT --noheadings ${NAME})"
>
> if [ -n "${MOUNTPOINT}" ]; then
> echo "${MOUNTPOINT}"
> fi
> fi
> done | sort | uniq
> }
>
> This way we only bother to fetch the mountpoints for ext[234] file
> systems, and only when -A is _not_ specified.
>
> In fact, I'm actually thinking that we should just *always* just
> return the device pathname in which case we can make this even
> simpler:
>
> ls_scan_targets() {
> for NAME in $(lvs -o lv_path --noheadings \
> -S "lv_active=active,lv_role=public,lv_role!=snapshot,vg_free>${snap_size_mb}") ; do
> # Skip non-ext[234]
> case "$(blkid -o value -s TYPE ${NAME})" in
> ext[234]) ;;
> *) continue;;
> esac
>
> if [ "${scrub_all}" -eq 1 ] ||
> [ -n "$(lsblk -o MOUNTPOINT --noheadings ${NAME})" ]; then
> echo ${NAME}
> fi
> done | sort | uniq
> }
>
> This means that we always run e2scrub on the device name, which in
> some cases might result in some ugliness, e.g.
>
> systemctl start e2scrub@-dev-lambda-test\\x2d1k
>
> But I think I can live with that. (However, the fact that
> systemd-escape will create Unicode characters which themselves have to
> be escaped is, well, sad....)
>
> What do you see on your system when you benchmark the above? The fact
> that we only determine the mountpoints on ext[234] file systems should
> save some time. We are sheling out to blkid for each device but
> that's probably not a huge overhead.
>
> My before (v1.45.0 plus support for -n so we can have comparable
> times) and after times (with all of the changes):
>
> 0.16user 0.15system 0:00.83elapsed 38%CPU (0avgtext+0avgdata 13384maxresident)k
>
> 0.12user 0.11system 0:00.36elapsed 64%CPU (0avgtext+0avgdata 13420maxresident)k
For me this new function is the wors of all.
cold cache:
real 0m2.115s
user 0m0.040s
sys 0m0.154s
second time:
real 0m1.100s
user 0m0.037s
sys 0m0.122s
But that's because of blkid which is terribly slow for some reason.
Replacing it with lsblk I get much better results
cold cache:
real 0m0.383s
user 0m0.043s
sys 0m0.112s
second time:
real 0m0.153s
user 0m0.048s
sys 0m0.102s
-Lukas
>
> Your one-linder is a bit faster:
>
> 0.03user 0.04system 0:00.23elapsed 31%CPU (0avgtext+0avgdata 13316maxresident)k
>
> But if we need to determine thick versus thin LV's so we can
> potentially do thin snapshots, a bunch of these optimizations are
> going to go away anyway. And realistically, so long as we're fast in
> the "no LV's" and "LV's exist but there is no free space" cases, that
> should avoid most user complaints, since if we *do* trigger e2scrub,
> the cost of running ls_scan_targets will be in the noise.
>
> - Ted
