After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 160441 - Add disk I/O stats for LVM/DM when
Add disk I/O stats for LVM/DM when
Status: RESOLVED OBSOLETE
Product: libgtop
Classification: Core
Component: linux
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: libgtop maintainers
libgtop maintainers
: 109920 325823 586853 (view as bug list)
Depends on:
Blocks: 157442 633613
 
 
Reported: 2004-12-04 18:49 UTC by Marius Gedminas
Modified: 2018-01-10 19:42 UTC
See Also:
GNOME target: ---
GNOME version: 2.7/2.8


Attachments
fsusage.c patch (4.62 KB, patch)
2009-07-21 04:13 UTC, Mike
none Details | Review
fsusage.c patch for gnome-2.24 (4.69 KB, patch)
2009-08-15 14:17 UTC, victor
none Details | Review
Variation of Victor's patch for libgtop2-28.4 (4.65 KB, patch)
2012-09-22 13:27 UTC, K.J. Valencik
none Details | Review
New variation of patch tested with GNOME 3.18 and Linux 4.2 kernel (6.06 KB, patch)
2015-12-15 06:52 UTC, Tristan Van Berkom
none Details | Review

Description Marius Gedminas 2004-12-04 18:49:39 UTC
Please add disk I/O throughput statistics to libgtop.  My primary reason for
asking is to have a disk I/O graph in the multiload applet (see bug #157442).

On Linux systems, the data is available in /proc/stat (Linux 2.4) or
/proc/diskstats (Linux 2.6).
Comment 1 Benoît Dejean 2004-12-04 18:58:37 UTC
it's already available in 2.8, but the implementation has been done only for
linux2.6 and i'm not very happy with it.

glibtop_fsusage has a .read and .write members.
i'm not closing since it's hasn't been really tested
Comment 2 Benoît Dejean 2004-12-04 19:23:30 UTC
/me stupid
looks like i forgot to commit the documentation update :/

but it's pretty simple
http://cvs.gnome.org/viewcvs/libgtop/include/glibtop/fsusage.h?rev=1.22&view=markup
Comment 3 Benoît Dejean 2004-12-05 15:59:45 UTC
are you happy with it ? may i close ?
Comment 4 Marius Gedminas 2004-12-09 19:53:32 UTC
Actually, I'm not sure per-filesystem stats are what I'd like to have.  I'm more
interested in stats for the whole disk.  I do not think I could get that just by
summing all reads/writes for all filesystems -- things like network drives,
ramdisks, bind mounts and swap files/partitions will probably cause much confusion.

Other than that, I'd be happier if gnome-applets folks answered your question
about happiness -- I filed this wishlist bug report at their request.
Comment 5 Benoît Dejean 2004-12-09 20:20:01 UTC
well, there's nothing about 'drive' in libgtop. we're talking about 'disk' so i
don't understand why it would be hard to sum up stats. you can get the mountlist
with glibtop_get_mountlist, then foreach entry, call glibtop_fs_usage.
Comment 6 Benoît Dejean 2004-12-20 11:03:47 UTC
*** Bug 109920 has been marked as a duplicate of this bug. ***
Comment 7 Marius Gedminas 2005-01-15 19:19:50 UTC
The multiload applet now has disk throughput stats, so I guess this bug can be
closed.

To answer your question: I wasn't sure if glibtop_get_mountlist returned mount
points for filesystems that are not stored on disk -- e.g. network drives (mount
-t smbfs //server/share /mnt/foo) or ramdisks (mount -t tmpfs none /tmp), and if
it did, whether glibtop_fs_usage would return I/O statistics for those
filesystems.  Also, I didn't know if bind mounts (mount --bind /path1 /path2)
would cause the same filesystem be returned twice by glibtop_fs_usage (they do
cause the same block device to appear twice in /proc/mounts) thus causing the
disk stats to be two times higher than actual.

I did some experimenting with the new disk I/O applet, and it does not show
anything for writes on network filesystems or tmpfs mounts.  Good!

My only gripe is that when the system runs out of memory and starts swapping
madly, the disk I/O applet thinks the disk is mostly idle.  Which is
understandable, given that swap partitions are not filesystems.  I guess I can
live with this, so feel free to close this bug.
Comment 8 Benoît Dejean 2005-01-15 23:44:49 UTC
1) it's os-dependant. On in Linux, it returns network stuff. But i don't know
how to retrieve disk usage for them (don't know if it has a meaning).
<quote>Good!</quote> : OK, everything is fine

2) bind mounts are ignored when all_fs = False

3) you're right. swap is not a fs. Have a look at the new CPU iowait, on my
laptop, i have no HDD led, and it's very useful to detect disk usage (including
swaping)
Comment 9 Benoît Dejean 2005-01-18 20:49:55 UTC
I've updated Linux implementation. Works much better, but still fails with DM/LVM
Comment 10 Benoît Dejean 2006-01-06 16:05:07 UTC
*** Bug 325823 has been marked as a duplicate of this bug. ***
Comment 11 Benoît Dejean 2007-03-13 22:37:56 UTC
Got stats for DM in r2566
Comment 12 Matthias Clasen 2008-11-11 21:36:29 UTC
Doesn't work here, or maybe it got recently broken by kernel changes.

It gets a device name of /dev/mapper/VolGroup00-LogVol02

Then it strips /dev, and ends up trying to read

/sys/block/mapper/VolGroup00-LogVol02/stat

which doesn't exist. It probably needs to read something like

/sys/block/dm-2/stat

but I have no idea how the mapping from /dev/mapper... to /sys/block/dm- works
Comment 13 Benoît Dejean 2009-07-01 22:32:05 UTC
*** Bug 586853 has been marked as a duplicate of this bug. ***
Comment 14 Mike 2009-07-17 04:39:07 UTC
It looks like the intelligent code for figuring out which partitions are available, get_partition(), was replaced by the short sighted is_partition() function.  The latter merely assumes that it can strip everything past the first number in a device named /dev/hda1 to arrive at the parent device /dev/hda.  This, however, breaks horribly with devices named /dev/md0 or /dev/ram3, because there is no parent device named "md" or "ram".  It merely happens to work for /dev/cdrom0, which is presumably why this scheme was developed (see the commit log message).

See the diff here:
http://git.gnome.org/cgit/libgtop/commit/?h=gnome-2-24&id=7f779e078dfe859e8e5ffbe889ed80458219ec38

The correct way to get to the device statistics, however is to avoid this horrible string parsing nightmare:
1) stat the mountpoint to get the device major and minor numbers.

2a) get the disk stats in /sys/dev/block/<major>:<minor>/stat

Or, in older kernels:

2b) walk the list of block devices looking for your <major>:<minor> in /sys/block/*/dev, and then get the stats in /sys/block/<device>/stat

This should replace most of the code in sysdeps/linux/fsusage.c
Comment 15 Mike 2009-07-17 04:46:15 UTC
This technique works on my machine with LVM.
Comment 16 Mike 2009-07-21 04:13:15 UTC
Created attachment 138876 [details] [review]
fsusage.c patch

Here's my first attempt at a patch which fetches file system IO statistics by following device major/minor numbers.
Comment 17 Fermulator 2009-07-25 15:15:10 UTC
Thanks Mike:

Can you please describe how exactly one can test your patch?  I have an idea of what to do, but it'd be good if you could provide instructions so that I know I'm doing it right and reporting against the right code changes.

Thanks!
Comment 18 Benoît Dejean 2009-07-25 16:33:59 UTC
I did not know about the major/minor subsystem in /sys/dev. It's a good generalization (no diff between device and partition).

But crawling the whole /sys/dev seems very expensive. Do you have an example about a device not found by M/m but found by name ?
Comment 19 Mike 2009-07-28 00:59:58 UTC
How to test the patch:

0) Set up a LVM or Device Mapper based filesystem.  I'm going to be vague about this step because it's a complex process and there is a lot of documentation already covering it.

1) Run the "examples/df" test program.

With an unpatched version of libgtop, you will notice that the last two columns, "Read" and "Write" have a value of 0 for all LVM and Device Mapper file systems.

With a patched version, you will notice that these columns return values which change as you modify the file system.  You can confirm that these values are accurate by looking the at the contents of /sys/block/XXX/stat (Where XXX is the block device for the file system).

Comment 20 Mike 2009-07-28 01:27:57 UTC
How the patch works:

The patch looks hideous because of the backwards compatibility code.  It appears that the /sys/dev/block/M:m/ code was added in kernel 2.6.27.

The first check I do is to see if /sys/dev/block/M:m/stat exists, and if it does, we return immediately.  This will work for all kernels from 2.6.27 onwards.

In theory, only kernels older than 2.6.27 will have to enter the remaining lines of code in get_sys_path().  This code is ugly, but it works.  It crawls /sys/block, ignoring the names and looking in the "dev" files until it finds a matching M:n.  The code tries to be a little clever by not trying partitions unless the Major number matches.  There are many ways to further optimize this code, not the least of which would be to cache the results of our crawls, but I wanted to provide a clear patch which demonstrated the theory of operation.

Another option would be to try to fall back to old method of doing string compares and only resort to crawling when that didn't work.  Once again, caching would help.

As far as I can tell, the kernel lists all block device M:m in this directory.  I won't pretend that I understand the kernel well enough to know if it is possible for a block device to exist in /sys/block and not in /sys/dev/block, but my guess is that all block devices show up in both places and if they don't it is a kernel bug.  Here is a link to the change which adds this feature: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=e105b8bfc769b0545b6f0f395179d1e43cbee822
Comment 21 victor 2009-08-15 14:17:37 UTC
Created attachment 140841 [details] [review]
fsusage.c patch for gnome-2.24
Comment 22 victor 2009-08-15 14:19:26 UTC
this issue is also related to libgtop-2.24 and libgtop-2.26

i solved the problem using data from /proc/diskstats (attached). seems it does exactly same things as the patch provided above but uses less system calls

i believe this approach is more reliable as documented in Documentation/sysfs-rules.txt
Comment 23 Gilles Dartiguelongue 2009-09-10 22:04:20 UTC
@maintainers, it'd be nice to have a word on these patch ? I'd like to integrate one of them in gentoo following [1] but both look a bit heavy on the changes and don't apply out of the box on 2.26.

[1] https://bugs.gentoo.org/show_bug.cgi?id=279952
Comment 24 Nick Jenkins 2010-07-22 01:28:20 UTC
Any feedback on the patches? Would be great to resolve this as the underlying problem appears to have caused these downstream bug reports: 
https://bugzilla.redhat.com/show_bug.cgi?id=469668
https://bugzilla.redhat.com/show_bug.cgi?id=608948
https://bugs.launchpad.net/gnome-system-monitor/+bug/126618
(plus the gentoo bug linked to above).
Comment 25 Mikko Rapeli 2011-11-25 13:45:09 UTC
Debian bug on the same issue:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=597994

Patch from comment https://bugzilla.gnome.org/show_bug.cgi?id=160441#c16
works well on Debian squeeze libgtop2 version 2.28.1.
Comment 26 K.J. Valencik 2012-09-22 13:27:35 UTC
Created attachment 224975 [details] [review]
Variation of Victor's patch for libgtop2-28.4

The first patch did not work for me in libgtop2-28.4 (Fedora 17) on fake raids, only LVMs. Here is an updated version of Victor's patch, which did work for me, for the later version of libgtop.
Comment 27 Robert Roth 2013-08-24 17:08:20 UTC
@K.J. Valencik: Could you please provide the patch generated wit git format-patch? Neither bugzilla(reviewin it doesn't work), nor git wants to handle it (git apply doesn't work)
Comment 28 Tristan Van Berkom 2015-12-15 06:52:35 UTC
Created attachment 317403 [details] [review]
New variation of patch tested with GNOME 3.18 and Linux 4.2 kernel

So, I finally installed GNOME Desktop, fun... and I like the system monitor extension, very nice.

Problem is that the disk I/O does not work with LUKS encrypted FS with LVM partitioning.

Found this bug, patch variations did not work for me, fixed bugs, made patch pretty... now the system monitor extension actually works... YAY !
Comment 29 James 2015-12-15 07:44:55 UTC
(In reply to Tristan Van Berkom from comment #28)
> Created attachment 317403 [details] [review] [review]
> New variation of patch tested with GNOME 3.18 and Linux 4.2 kernel

Oh awesome. I'm not a very good c programmer, so I can't comment, but thank you!
If someone can test with an luks+btrfs system, that would be awesome too :)
Comment 30 Tristan Van Berkom 2017-09-23 07:07:10 UTC
Dear maintainers,

Is this patch obsoleted by the things implemented in bug 768304 ?

I wrote this because disk I/O usage was totally broken with LUKS/LVM and have been happily using the patch locally for the last couple of years, but I'm told that libgtop no longer has this problem anymore.

Is this true ?

If so, please close this bug as obsolete, and I wont keep it on my radar anymore :)
Comment 31 GNOME Infrastructure Team 2018-01-10 19:42:45 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/libgtop/issues/3.