Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1145723 > unrolled thread

Bug#1035486: dpkg: consider making FS_IOC_FIEMAP an option that cna be turned off

Back to article view | Back to linux.debian.bugs.dist

Contents

  Bug#1035486: dpkg: consider making FS_IOC_FIEMAP an option that cna be turned off Marc Lehmann <debian-reportbug@plan9.de> - 2023-05-04 06:30 +0200
    Bug#1035486: dpkg: consider making FS_IOC_FIEMAP an option that cna be turned off Guillem Jover <guillem@debian.org> - 2023-05-22 02:00 +0200
      Bug#1035486: dpkg: consider making FS_IOC_FIEMAP an option that cna be turned off Guillem Jover <guillem@debian.org> - 2023-06-09 04:30 +0200

#1145723 — Bug#1035486: dpkg: consider making FS_IOC_FIEMAP an option that cna be turned off

Package: dpkg
Version: 1.20.12
Severity: wishlist

Dear Maintainer,

on many of my systems using nvme drives or even sata ssds, the
FS_IOC_FIEMAP scan that dpkg does is not only superfluous, but with a
large number of packages installed, absolutely dominates the installation
time (based on profiling) - essentially, dpkg does nothing else during the
preparing/unpacxking/selecting steps.

and while being a very useful optimisation for spinning rust drives, when
installing many small packages, the scan is apparently done for every
package unpack, when, again, on modern systems, everything is in the cache
aftzer the first pass.

could you consider making this scan optional (i.e. off-switchable via an
option)? this way, users can disable it in dpkg.cfg on those systems where
it matters, while still retaining it by default (I do not think trying to
autodetect ssds is a good idea here, and a switch should be trivial to
implement).

thanks a lot for your consideration!

PS: it might also be a good idea to reconsider the whole scan - fiemap
often creates just as many seeks as a stat or an open, especially on more
complicated filesystems. In fact, sorting by inode numbers (which are
essentially free info) is often a good approximation for locality on many
filesystems and might be a good replacement strategy.

-- Package-specific info:
System tainted due to merged-usr-via-aliased-dirs.

-- System Information:
Debian Release: 11.7
  APT prefers stable-updates
  APT policy: (990, 'stable-updates'), (990, 'stable-security'), (990, 'stable'), (500, 'unstable-debug'), (500, 'testing-debug'), (500, 'stable-debug'), (500, 'unstable'), (500, 'testing'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, x32

Kernel: Linux 6.1.27-schmorp (SMP w/24 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_USER, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_IE.UTF-8, LC_CTYPE=en_IE.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages dpkg depends on:
ii  libbz2-1.0   1.0.8-4
ii  libc6        2.31-13+deb11u6
ii  liblzma5     5.2.5-2.1~deb11u1
ii  libselinux1  3.1-3
ii  tar          1.34+dfsg-1
ii  zlib1g       1:1.2.11.dfsg-2+deb11u2

dpkg recommends no packages.

Versions of packages dpkg suggests:
ii  apt            2.2.4
pn  debsig-verify  <none>

-- Configuration Files:
/etc/dpkg/dpkg.cfg changed [not included]

-- no debconf information

[toc] | [next] | [standalone]

#1147503

[Multipart message — attachments visible in raw view] — view raw

Hi!

On Thu, 2023-05-04 at 06:16:52 +0200, Marc Lehmann wrote:
> Package: dpkg
> Version: 1.20.12
> Severity: wishlist

> on many of my systems using nvme drives or even sata ssds, the
> FS_IOC_FIEMAP scan that dpkg does is not only superfluous, but with a
> large number of packages installed, absolutely dominates the installation
> time (based on profiling) - essentially, dpkg does nothing else during the
> preparing/unpacxking/selecting steps.
> 
> and while being a very useful optimisation for spinning rust drives, when
> installing many small packages, the scan is apparently done for every
> package unpack, when, again, on modern systems, everything is in the cache
> aftzer the first pass.

Hmm, right, whenever we update and write the files list file on disk
we invalidate the in-core lists and force a reload for that package,
so the scan might indeed be performed for at least each unpacked
package when processing each subsequent one.

The other potential cause for the entire database being loaded for
each package you might also be seeing is because apt tends to split
essential package installation into individual runs to reduce
potential breakage (AFAIR).

> could you consider making this scan optional (i.e. off-switchable via an
> option)? this way, users can disable it in dpkg.cfg on those systems where
> it matters, while still retaining it by default (I do not think trying to
> autodetect ssds is a good idea here, and a switch should be trivial to
> implement).

Yes, AFAIR at the time when implementing this, autodetection was
considered, but was quickly discarded as unfeasible/unportable and
error-prone, and we opted for performing this unconditionally.

I've now locally implemented this via a new force option enabled by
default, that I'm tentatively calling --force-mechanical-load, but I'll
ponder about the name because it's the first thing that came to mind,
and I'm not fully convinced. If you happen to have proposals I'm all
ears. :)

But looking further, the scan is not ignoring packages that already
have a valid loaded files list file (although it should already be
caching the physical offset if it was found and then skip the scan),
so I'll skip those, which might also help by substantially reducing
the work being done on both HDD and NVME/SSD.

I might also look into avoiding the scan completely if there are only
few files list files to reload (instead of say the entire lot), but
I'm not sure how much that will help once/if it is only done for the
few packages that need a reload (if the current caching is not effective
for some reason, then I'd assume it might not help a lot?). And thinking
about it perhaps even the force option is not really necessary after
the scan fix anyway? (But it would still help in the apt splitting case
mentioned above though.)

I'm attaching the change to skip unnecessary optimization scan, in
case you want to give it a go, although I've only compile tested it
for now.

> PS: it might also be a good idea to reconsider the whole scan - fiemap
> often creates just as many seeks as a stat or an open, especially on more
> complicated filesystems. In fact, sorting by inode numbers (which are
> essentially free info) is often a good approximation for locality on many
> filesystems and might be a good replacement strategy.

Perhaps, I would need to check the context from when it was added as
I cannot recall whether this option was considered and discarded,
and otherwise it would need to be benchmarked on a bunch of different
filesystems and types of disks to see whether there would be
significant performance degradation/regression.

Thanks,
Guillem

[toc] | [prev] | [next] | [standalone]

#1149082

Hi!

[ Leaving most mail context for Colin (in CC). ]

On Mon, 2023-05-22 at 01:47:27 +0200, Guillem Jover wrote:
> On Thu, 2023-05-04 at 06:16:52 +0200, Marc Lehmann wrote:
> > Package: dpkg
> > Version: 1.20.12
> > Severity: wishlist
> 
> > on many of my systems using nvme drives or even sata ssds, the
> > FS_IOC_FIEMAP scan that dpkg does is not only superfluous, but with a
> > large number of packages installed, absolutely dominates the installation
> > time (based on profiling) - essentially, dpkg does nothing else during the
> > preparing/unpacxking/selecting steps.
> > 
> > and while being a very useful optimisation for spinning rust drives, when
> > installing many small packages, the scan is apparently done for every
> > package unpack, when, again, on modern systems, everything is in the cache
> > aftzer the first pass.
> 
> Hmm, right, whenever we update and write the files list file on disk
> we invalidate the in-core lists and force a reload for that package,
> so the scan might indeed be performed for at least each unpacked
> package when processing each subsequent one.
> 
> The other potential cause for the entire database being loaded for
> each package you might also be seeing is because apt tends to split
> essential package installation into individual runs to reduce
> potential breakage (AFAIR).

I guess the other one is whether the filesystem in use does not
support fiemap, in which case the ioctl will fail, and the current
caching logic would retrigger the ioctls for each iteration. The
patch I attached previously should fix this case, so I'm going to
include it no matter what on my next push.

> > could you consider making this scan optional (i.e. off-switchable via an
> > option)? this way, users can disable it in dpkg.cfg on those systems where
> > it matters, while still retaining it by default (I do not think trying to
> > autodetect ssds is a good idea here, and a switch should be trivial to
> > implement).
> 
> Yes, AFAIR at the time when implementing this, autodetection was
> considered, but was quickly discarded as unfeasible/unportable and
> error-prone, and we opted for performing this unconditionally.
> 
> I've now locally implemented this via a new force option enabled by
> default, that I'm tentatively calling --force-mechanical-load, but I'll
> ponder about the name because it's the first thing that came to mind,
> and I'm not fully convinced. If you happen to have proposals I'm all
> ears. :)
> 
> But looking further, the scan is not ignoring packages that already
> have a valid loaded files list file (although it should already be
> caching the physical offset if it was found and then skip the scan),
> so I'll skip those, which might also help by substantially reducing
> the work being done on both HDD and NVME/SSD.
> 
> I might also look into avoiding the scan completely if there are only
> few files list files to reload (instead of say the entire lot), but
> I'm not sure how much that will help once/if it is only done for the
> few packages that need a reload (if the current caching is not effective
> for some reason, then I'd assume it might not help a lot?). And thinking
> about it perhaps even the force option is not really necessary after
> the scan fix anyway? (But it would still help in the apt splitting case
> mentioned above though.)
> 
> I'm attaching the change to skip unnecessary optimization scan, in
> case you want to give it a go, although I've only compile tested it
> for now.
> 
> > PS: it might also be a good idea to reconsider the whole scan - fiemap
> > often creates just as many seeks as a stat or an open, especially on more
> > complicated filesystems. In fact, sorting by inode numbers (which are
> > essentially free info) is often a good approximation for locality on many
> > filesystems and might be a good replacement strategy.
> 
> Perhaps, I would need to check the context from when it was added as
> I cannot recall whether this option was considered and discarded,
> and otherwise it would need to be benchmarked on a bunch of different
> filesystems and types of disks to see whether there would be
> significant performance degradation/regression.

While testing this briefly, I noticed that on my system the main
program emitting these ioctls was actually man-db from its trigger,
which has code based on the dpkg logic. So that would also need
additional changes to support this, perhaps the trigger could honor
the DPKG_FORCE envvar and pass some new option to man-db? (Colin?)

Thanks,
Guillem

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web