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 748928 - Rework mem.c for FreeBSD
Rework mem.c for FreeBSD
Status: RESOLVED OBSOLETE
Product: libgtop
Classification: Core
Component: bsd
unspecified
Other FreeBSD
: Normal normal
: ---
Assigned To: Benoît Dejean
libgtop maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-05 07:49 UTC by Ting-Wei Lan
Modified: 2018-01-10 19:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
freebsd: rework system memory usage (3.95 KB, patch)
2015-05-05 07:49 UTC, Ting-Wei Lan
none Details | Review
freebsd: rework system memory usage - updated (3.58 KB, patch)
2015-06-27 16:59 UTC, Ting-Wei Lan
committed Details | Review
Add ZFS ARC and rework `user` (1.07 KB, patch)
2015-07-18 03:07 UTC, Benoît Dejean
none Details | Review
Add ZFS ARC and rework `user` (1.64 KB, patch)
2015-07-18 13:37 UTC, Benoît Dejean
none Details | Review
current master (93.76 KB, image/png)
2015-07-22 21:04 UTC, Benoît Dejean
  Details
patched (93.97 KB, image/png)
2015-07-22 21:04 UTC, Benoît Dejean
  Details

Description Ting-Wei Lan 2015-05-05 07:49:06 UTC
The system memory usage showed in gnome-system-monitor is usually much lower than the real usage on FreeBSD. Current implmentation in libgtop is:

  Used   = Wired
  Free   = Total - Wired
  Shared = 0
  Buffer = 0
  Cached = Cache
  User   = Wired - Cache
  Locked = Not Supported

I think it is better to change it to:

  Used   = Active
  Free   = Free
  Shared = 0
  Buffer = Buf
  Cached = Cache
  User   = Active + Wired
  Locked = Wired

The left-hand side means fields in the glibtop_mem struct and the right-hand
side means the value showed by `top' command. It seems that the relation between various values showed in `top' command is:

  Total = Active + Inact + Wired + Cache + Free

Buf is a part of Wired, so it is not listed it in the above relation.


I don't know whether there is any relation between fields in glibtop_mem struct because I cannot find documentation that describes meanings of them, so I just make the change based on their names.
Comment 1 Ting-Wei Lan 2015-05-05 07:49:41 UTC
Created attachment 302914 [details] [review]
freebsd: rework system memory usage
Comment 2 Robert Roth 2015-06-14 06:12:04 UTC
Review of attachment 302914 [details] [review]:

Patch looks clean and nice code-wise, and I assume that it works fin as you have tested it on FreeBSD. I would say it's ok to commit, but will ask for another opinion and commit after that.
Comment 3 Benoît Dejean 2015-06-20 19:43:13 UTC
Hello, I think this brings wrong values.
Have a look on my system (before / after the patch):
- total: 777MB / 777MB : unchanged
- used: 507MB / 81MB : that's wrong
- free: 270MB / 154MB : why not
- shared: 0MB / 0MB : unchanged
- buffer: 0MB / 64kB
- cached: 3MB / 7MB
- user: 504MB / 577MB: ok
- locked: 0MB / 496MB: hum...


Here's what I think:
- 'user' is meant to tell what is used by application, that is to say 'used' - { 'free' + 'buffer' + 'cached' }. So the current code is fine with me, I think it would be a mistake to change it.
- I like that total = used + free. The patch breaks it.
- The patch switches the used into locked. I'm not sure about the meaning of the locked member. Let's ignore it.

So for now, I would only keep 2 things in the patch:
- the factoring into functions to do the sysctbyname
- setting buffers from vfs.bufspace
Comment 4 Ting-Wei Lan 2015-06-27 15:03:47 UTC
(In reply to Benoît Dejean from comment #3)
> Here's what I think:
> - 'user' is meant to tell what is used by application, that is to say 'used'
> - { 'free' + 'buffer' + 'cached' }. So the current code is fine with me, I
> think it would be a mistake to change it.

I don't think using wired as used is correct. Wired is usually the memory used by the kernel or ballooned down by the hypervisor, not the memory directly used by applications.

> - I like that total = used + free. The patch breaks it.

So used = total - free should be easier.

> - The patch switches the used into locked. I'm not sure about the meaning of
> the locked member. Let's ignore it.
> 
> So for now, I would only keep 2 things in the patch:
> - the factoring into functions to do the sysctbyname
> - setting buffers from vfs.bufspace


Is this better?

  Used   = Total - Free
  Free   = Free
  Shared = 0
  Buffer = Buf
  Cached = Cache
  User   = Active + Wired
  Locked = 0
Comment 5 Ting-Wei Lan 2015-06-27 16:59:08 UTC
Created attachment 306207 [details] [review]
freebsd: rework system memory usage - updated
Comment 6 Ting-Wei Lan 2015-07-10 08:07:21 UTC
Can anyone review the updated patch? I hope this can be available in GNOME 3.18.
Comment 7 Benoît Dejean 2015-07-10 12:21:53 UTC
Review of attachment 306207 [details] [review]:

Are we sure that wired and active do not overlap ? I'll check on that later, but for now, this is good. And now our free matches the `top`'s free.
Comment 8 Ting-Wei Lan 2015-07-10 16:40:37 UTC
Attachment 306207 [details] pushed as a206cfc - freebsd: rework system memory usage


'Active + Inact + Wired + Cache + Free' is less than 'Total' on my machine, but the difference is less than 3%.

If I use vm.stats.vm.v_page_count as total and compare page counts instead of bytes, the difference is only 2 pages, so I think they really don't overlap.
Comment 9 Benoît Dejean 2015-07-18 03:07:05 UTC
I'm still not happy with that. I worked all night and the ZFS ARC fills my RAM and everything looks fuller and fuller with system-monitor, even if I haven't started more programs. And at random times, it would be flushed and my RAM usage would then go down.

Adding arcstats.size into Cached looks better, but then User is still not fine.

Which do you think is best ?
1) User = Active + Wired - Buffer - Cached
2) User = Used - Buffer - Cached # That's the 'old' version for Linux

I think 2) looks really better with system-monitor.
Please give it a try (you could also test with examples/free + top).
Comment 10 Benoît Dejean 2015-07-18 03:07:50 UTC
Created attachment 307650 [details] [review]
Add ZFS ARC and rework `user`
Comment 11 Ting-Wei Lan 2015-07-18 12:51:19 UTC
(In reply to Benoît Dejean from comment #9)
> I'm still not happy with that. I worked all night and the ZFS ARC fills my
> RAM and everything looks fuller and fuller with system-monitor, even if I
> haven't started more programs. And at random times, it would be flushed and
> my RAM usage would then go down.

I used to have a 10 GiB ZFS on my system when I still used FreeBSD 9.x. My ZFS ARC (5 GiB) became larger than half of my physical memory (8 GiB), and many programs were randomly killed because of out of memory.


> 
> Adding arcstats.size into Cached looks better, but then User is still not
> fine.

I no longer use ZFS on FreeBSD 10.x now. Was the situation changed so ZFS memory can be immediately freed when programs need more memory to run? If it is true then I think adding it to Cached is good.

kstat.zfs.misc.arcstats.size is not available when zfs.ko is not loaded, so attachment 307650 [details] [review] generates many warnings if users don't use ZFS.


> 
> Which do you think is best ?
> 1) User = Active + Wired - Buffer - Cached

This means 'Active + (Wired - Buf - ARC) - Cache' in top. As 'Cache' in top isn't part of Active or Wired, this makes wrong value.


> 2) User = Used - Buffer - Cached # That's the 'old' version for Linux

This means 'Active + Inact + (Wired - Buf - ARC)' in top. 'Inact' causes the value to become very large. I have 16 GiB physical memory on my system now, and Inact is 8 GiB, so User is larger than 10 GiB even if I only run GNOME desktop, Epiphany and Terminal.


> 
> I think 2) looks really better with system-monitor.
> Please give it a try (you could also test with examples/free + top).


If Buf and ARC can be immediately available for programs to use, I think it is good to exclude them from User. 'User = Used - Buffer - Cached - Inact'


If they need more time to free and can cause programs to be killed, keeping 'User = Active + Wired' should be better.
Comment 12 Benoît Dejean 2015-07-18 13:36:39 UTC
I'm running 10.1 and yes, the ARC goes up and down.
For example, it was 255MB then I just allocated a big chunk of memory and it decreased to 104M. It surely goes down when there's memory pressure but also on unknown events. That's why not accounting it as cache makes the memory usage in system-monitor go up & down without any reason. So yes, it seems immediately available for programs to use.

It's easy to fix the case when zfs.ko is not loaded.
Comment 13 Benoît Dejean 2015-07-18 13:37:29 UTC
Created attachment 307661 [details] [review]
Add ZFS ARC and rework `user`

How about this one ?
Comment 14 Ting-Wei Lan 2015-07-19 19:24:55 UTC
We have to decide whether Buf should be counted as User. As excluding it causes User value to become too low, I think we can keep it.


We counts Buf as User here:
buf->user = memactive + memwired;

But Buf is not included here:
buf->user = buf->used - (buf->buffer + buf->cached + meminactive);

If Buf is included, it should be:
buf->user = buf->used - (buf->cached + meminactive);
Comment 15 Ting-Wei Lan 2015-07-19 19:38:45 UTC
It seems it is better to remove the check and use:
buf->user = memactive + memwired - zfs_arc_size;

zfs_arc_size should be 0 when ZFS is not used.
Comment 16 Benoît Dejean 2015-07-22 21:04:16 UTC
Created attachment 307931 [details]
current master
Comment 17 Benoît Dejean 2015-07-22 21:04:35 UTC
Created attachment 307932 [details]
patched
Comment 18 Benoît Dejean 2015-07-22 21:10:04 UTC
Here are the system-monitor graphs:
- workload: "cd libgtop && gmake -j clean && gmake -j && gmake -j install"
- running app: startx+openbox, kdeconsole (gnome-terminal crash on 10.1), system-monitor, nothing else. Not even emacs.
- all FS are ZFS

I think that:
- the "current" memory usage is unrealistic considering how light my system is.
- the "patched" memory usage has a broader range/amplitude when compiling and I think this really represents the peak memory usage of my workload.
Comment 19 Ting-Wei Lan 2015-07-23 07:52:42 UTC
(In reply to Benoît Dejean from comment #18)
> Here are the system-monitor graphs:
> - workload: "cd libgtop && gmake -j clean && gmake -j && gmake -j install"
> - running app: startx+openbox, kdeconsole (gnome-terminal crash on 10.1),

Are you using a UTF-8 locale? gnome-terminal requires it.

> system-monitor, nothing else. Not even emacs.
> - all FS are ZFS
> 
> I think that:
> - the "current" memory usage is unrealistic considering how light my system
> is.
> - the "patched" memory usage has a broader range/amplitude when compiling
> and I think this really represents the peak memory usage of my workload.

Can we just replace this check

if (zfs_arc_size) {
	buf->cached += zfs_arc_size;
	buf->user = buf->used - (buf->buffer + buf->cached + meminactive);
}
else {
	buf->user = memactive + memwired;
}

with 'buf->user = memactive + memwired - zfs_arc_size' ?
Comment 20 Ting-Wei Lan 2015-07-23 07:53:33 UTC
Sorry, it should be

buf->cached += zfs_arc_size;
buf->user = memactive + memwired - zfs_arc_size;
Comment 21 Benoît Dejean 2015-07-23 18:42:52 UTC
I was just sharing about accounting zfs_arc_size makes the memory usage more dynamic and more realistic. I'm not sure to understand why you favor 'memactive + memwired' over 'used - (buffer + cached + inactive)'. I'm not sure which one is correct. My system is only ZFS so I have no idea how buffer/cached behave with ufs. But as I don't really see the difference (a few MB ...), let's do it your way.
Comment 22 Benoît Dejean 2015-07-23 19:15:05 UTC
I hope I haven't been rude. I respect your point of view but the difference between the 2 computations is not really meaningful to me. I'm a long time Linux user, so I'm used to something else. And I'm a bit cautious because last week I've realised that libgtop did not match Linux `free` command output: so I've made a `free` clone in example and tuned sysdeps/linux/mem.c until both matched.
Comment 23 Ting-Wei Lan 2015-07-24 08:12:23 UTC
(In reply to Benoît Dejean from comment #21)
> I was just sharing about accounting zfs_arc_size makes the memory usage more
> dynamic and more realistic. I'm not sure to understand why you favor
> 'memactive + memwired' over 'used - (buffer + cached + inactive)'. I'm not
> sure which one is correct. My system is only ZFS so I have no idea how
> buffer/cached behave with ufs. But as I don't really see the difference (a
> few MB ...), let's do it your way.

I think 'memactive + memwired - zfs_arc_size' is almost the same as 'used - (cache + zfs_arc_size + inactive)'. There should be really no difference. I am not proposing a different way to calculate the value. I prefer 'memactive + memwired - zfs_arc_size' simply because it is easier for other users and developers to understand what are included in the user value. I think we can remove the check simply because it is not needed.

(In reply to Benoît Dejean from comment #22)
> I hope I haven't been rude. I respect your point of view but the difference
> between the 2 computations is not really meaningful to me. I'm a long time
> Linux user, so I'm used to something else. And I'm a bit cautious because
> last week I've realised that libgtop did not match Linux `free` command
> output: so I've made a `free` clone in example and tuned sysdeps/linux/mem.c
> until both matched.

Yes, there is no meaningful difference in values produced by libgtop and gnome-system-monitor. The only difference is that memactive, memwired, zfs_arc_size are values that can be easily found in top, but used or total are not.
Comment 24 GNOME Infrastructure Team 2018-01-10 19:51:49 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/31.