| Message ID | 20150106164340.55e83f742d6f57c19e6500ff@linux-foundation.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 6 Jan 2015, Andrew Morton wrote: > On Fri, 26 Dec 2014 19:56:49 +0800 "Wang, Yalin" <Yalin.Wang@sonymobile.com> wrote: > > > This patch subtract sharedram from cached, > > sharedram can only be swap into swap partitions, > > they should be treated as swap pages, not as cached pages. > > > > ... > > > > --- a/fs/proc/meminfo.c > > +++ b/fs/proc/meminfo.c > > @@ -45,7 +45,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) > > committed = percpu_counter_read_positive(&vm_committed_as); > > > > cached = global_page_state(NR_FILE_PAGES) - > > - total_swapcache_pages() - i.bufferram; > > + total_swapcache_pages() - i.bufferram - i.sharedram; > > if (cached < 0) > > cached = 0; > > Documentation/filesystems/proc.txt says > > : Cached: in-memory cache for files read from the disk (the > : pagecache). Doesn't include SwapCached > > So yes, I guess it should not include shmem. > > And why not do this as well? > > > --- a/Documentation/filesystems/proc.txt~mm-change-meminfo-cached-calculation-fix > +++ a/Documentation/filesystems/proc.txt > @@ -811,7 +811,7 @@ MemAvailable: An estimate of how much me > Buffers: Relatively temporary storage for raw disk blocks > shouldn't get tremendously large (20MB or so) > Cached: in-memory cache for files read from the disk (the > - pagecache). Doesn't include SwapCached > + pagecache). Doesn't include SwapCached or Shmem. > SwapCached: Memory that once was swapped out, is swapped back in but > still also is in the swapfile (if memory is needed it > doesn't need to be swapped out AGAIN because it is already Whoa. Changes of this kind would have made good sense about 14 years ago. And there's plenty more which would benefit from having anon/shmem/file properly distinguished. But how can we make such a change now, breaking everything that has made its own sense of these counts? Hugh
On Tue, 6 Jan 2015 17:04:33 -0800 (PST) Hugh Dickins <hughd@google.com> wrote: > On Tue, 6 Jan 2015, Andrew Morton wrote: > > On Fri, 26 Dec 2014 19:56:49 +0800 "Wang, Yalin" <Yalin.Wang@sonymobile.com> wrote: > > > > > This patch subtract sharedram from cached, > > > sharedram can only be swap into swap partitions, > > > they should be treated as swap pages, not as cached pages. > > > > > > ... > > > > > > --- a/fs/proc/meminfo.c > > > +++ b/fs/proc/meminfo.c > > > @@ -45,7 +45,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) > > > committed = percpu_counter_read_positive(&vm_committed_as); > > > > > > cached = global_page_state(NR_FILE_PAGES) - > > > - total_swapcache_pages() - i.bufferram; > > > + total_swapcache_pages() - i.bufferram - i.sharedram; > > > if (cached < 0) > > > cached = 0; > > > > Documentation/filesystems/proc.txt says > > > > : Cached: in-memory cache for files read from the disk (the > > : pagecache). Doesn't include SwapCached > > > > So yes, I guess it should not include shmem. > > > > And why not do this as well? > > > > > > --- a/Documentation/filesystems/proc.txt~mm-change-meminfo-cached-calculation-fix > > +++ a/Documentation/filesystems/proc.txt > > @@ -811,7 +811,7 @@ MemAvailable: An estimate of how much me > > Buffers: Relatively temporary storage for raw disk blocks > > shouldn't get tremendously large (20MB or so) > > Cached: in-memory cache for files read from the disk (the > > - pagecache). Doesn't include SwapCached > > + pagecache). Doesn't include SwapCached or Shmem. > > SwapCached: Memory that once was swapped out, is swapped back in but > > still also is in the swapfile (if memory is needed it > > doesn't need to be swapped out AGAIN because it is already > > Whoa. Changes of this kind would have made good sense about 14 years ago. > And there's plenty more which would benefit from having anon/shmem/file > properly distinguished. But how can we make such a change now, > breaking everything that has made its own sense of these counts? That's what I was wondering, but I was having some trouble picking a situation where it mattered much. What's the problematic scenario here? Userspace that is taking Cached, saying "that was silly" and subtracting Shmem from it by hand? I suppose that as nobody knows we should err on the side of caution and leave this alone. But the situation is pretty sad - it would be nice to make the code agree with the documentation at least.
On Tue, 6 Jan 2015, Andrew Morton wrote: > On Tue, 6 Jan 2015 17:04:33 -0800 (PST) Hugh Dickins <hughd@google.com> wrote: > > On Tue, 6 Jan 2015, Andrew Morton wrote: > > > On Fri, 26 Dec 2014 19:56:49 +0800 "Wang, Yalin" <Yalin.Wang@sonymobile.com> wrote: > > > > > > > This patch subtract sharedram from cached, > > > > sharedram can only be swap into swap partitions, > > > > they should be treated as swap pages, not as cached pages. > > > > > > > > ... > > > > > > > > --- a/fs/proc/meminfo.c > > > > +++ b/fs/proc/meminfo.c > > > > @@ -45,7 +45,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) > > > > committed = percpu_counter_read_positive(&vm_committed_as); > > > > > > > > cached = global_page_state(NR_FILE_PAGES) - > > > > - total_swapcache_pages() - i.bufferram; > > > > + total_swapcache_pages() - i.bufferram - i.sharedram; > > > > if (cached < 0) > > > > cached = 0; > > > > > > Documentation/filesystems/proc.txt says > > > > > > : Cached: in-memory cache for files read from the disk (the > > > : pagecache). Doesn't include SwapCached > > > > > > So yes, I guess it should not include shmem. > > > > > > And why not do this as well? > > > > > > > > > --- a/Documentation/filesystems/proc.txt~mm-change-meminfo-cached-calculation-fix > > > +++ a/Documentation/filesystems/proc.txt > > > @@ -811,7 +811,7 @@ MemAvailable: An estimate of how much me > > > Buffers: Relatively temporary storage for raw disk blocks > > > shouldn't get tremendously large (20MB or so) > > > Cached: in-memory cache for files read from the disk (the > > > - pagecache). Doesn't include SwapCached > > > + pagecache). Doesn't include SwapCached or Shmem. > > > SwapCached: Memory that once was swapped out, is swapped back in but > > > still also is in the swapfile (if memory is needed it > > > doesn't need to be swapped out AGAIN because it is already > > > > Whoa. Changes of this kind would have made good sense about 14 years ago. > > And there's plenty more which would benefit from having anon/shmem/file > > properly distinguished. But how can we make such a change now, > > breaking everything that has made its own sense of these counts? > > That's what I was wondering, but I was having some trouble picking a > situation where it mattered much. If it doesn't matter, then we don't need to change it. > What's the problematic scenario > here? Userspace that is taking Cached, saying "that was silly" and > subtracting Shmem from it by hand? Someone a long time ago saw "that was silly", worked out it was because of Shmem, adjusted their scripts or whatever accordingly, and has run happily ever since. > > I suppose that as nobody knows we should err on the side of caution and > leave this alone. But the situation is pretty sad - it would be nice > to make the code agree with the documentation at least. By all means fix the documentation. And work on a /proc/meminfo.2015 which has sensibly differentiated counts (and probably omits that wonderful Linux 2.2-compatible "Buffers"). But there's more to do than I can think of. Cc'ing Jerome who has a particular interest in this (no, I haven't forgotten his patches, but nor have I had a moment to reconsider them). Hugh
On Wed, Jan 7, 2015 at 5:03 AM, Hugh Dickins <hughd@google.com> wrote: > On Tue, 6 Jan 2015, Andrew Morton wrote: >> On Tue, 6 Jan 2015 17:04:33 -0800 (PST) Hugh Dickins <hughd@google.com> wrote: >> > On Tue, 6 Jan 2015, Andrew Morton wrote: >> > > On Fri, 26 Dec 2014 19:56:49 +0800 "Wang, Yalin" <Yalin.Wang@sonymobile.com> wrote: >> > > >> > > > This patch subtract sharedram from cached, >> > > > sharedram can only be swap into swap partitions, >> > > > they should be treated as swap pages, not as cached pages. >> > > > >> > > > ... >> > > > >> > > > --- a/fs/proc/meminfo.c >> > > > +++ b/fs/proc/meminfo.c >> > > > @@ -45,7 +45,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v) >> > > > committed = percpu_counter_read_positive(&vm_committed_as); >> > > > >> > > > cached = global_page_state(NR_FILE_PAGES) - >> > > > - total_swapcache_pages() - i.bufferram; >> > > > + total_swapcache_pages() - i.bufferram - i.sharedram; >> > > > if (cached < 0) >> > > > cached = 0; >> > > >> > > Documentation/filesystems/proc.txt says >> > > >> > > : Cached: in-memory cache for files read from the disk (the >> > > : pagecache). Doesn't include SwapCached >> > > >> > > So yes, I guess it should not include shmem. >> > > >> > > And why not do this as well? >> > > >> > > >> > > --- a/Documentation/filesystems/proc.txt~mm-change-meminfo-cached-calculation-fix >> > > +++ a/Documentation/filesystems/proc.txt >> > > @@ -811,7 +811,7 @@ MemAvailable: An estimate of how much me >> > > Buffers: Relatively temporary storage for raw disk blocks >> > > shouldn't get tremendously large (20MB or so) >> > > Cached: in-memory cache for files read from the disk (the >> > > - pagecache). Doesn't include SwapCached >> > > + pagecache). Doesn't include SwapCached or Shmem. >> > > SwapCached: Memory that once was swapped out, is swapped back in but >> > > still also is in the swapfile (if memory is needed it >> > > doesn't need to be swapped out AGAIN because it is already >> > >> > Whoa. Changes of this kind would have made good sense about 14 years ago. >> > And there's plenty more which would benefit from having anon/shmem/file >> > properly distinguished. But how can we make such a change now, >> > breaking everything that has made its own sense of these counts? >> >> That's what I was wondering, but I was having some trouble picking a >> situation where it mattered much. > > If it doesn't matter, then we don't need to change it. > >> What's the problematic scenario >> here? Userspace that is taking Cached, saying "that was silly" and >> subtracting Shmem from it by hand? > > Someone a long time ago saw "that was silly", worked out it was because > of Shmem, adjusted their scripts or whatever accordingly, and has run > happily ever since. Totally agree. I know some of these guys. But that's here not for so long time, 'Shmem' has appeared only in 2.6.32. > >> >> I suppose that as nobody knows we should err on the side of caution and >> leave this alone. But the situation is pretty sad - it would be nice >> to make the code agree with the documentation at least. > > By all means fix the documentation. And work on a /proc/meminfo.2015 > which has sensibly differentiated counts (and probably omits that > wonderful Linux 2.2-compatible "Buffers"). 'Buffers' is actually very useful. Ext4 keeps almost all metadata in bdev page-cache. Meminfo has a bigger and much more confusing problem: there is no subset of fields which sums to all ram. Some paged allocations are showed nowhere. Probably it would be good to show that 'Untracked' memory as well, calculated as: Total - Free - Cached - Buffers - Slab - PageTables - KernelStack - AnonPages. (fix me if I'm wrong =) > > But there's more to do than I can think of. Cc'ing Jerome who has a > particular interest in this (no, I haven't forgotten his patches, > but nor have I had a moment to reconsider them). > > Hugh
--- a/Documentation/filesystems/proc.txt~mm-change-meminfo-cached-calculation-fix +++ a/Documentation/filesystems/proc.txt @@ -811,7 +811,7 @@ MemAvailable: An estimate of how much me Buffers: Relatively temporary storage for raw disk blocks shouldn't get tremendously large (20MB or so) Cached: in-memory cache for files read from the disk (the - pagecache). Doesn't include SwapCached + pagecache). Doesn't include SwapCached or Shmem. SwapCached: Memory that once was swapped out, is swapped back in but still also is in the swapfile (if memory is needed it doesn't need to be swapped out AGAIN because it is already