On Tue, Feb 20, 2024 at 5:21 PM Vlastimil Babka <vbabka@xxxxxxx> wrote: > > On 2/20/24 09:49, zhang fangzheng wrote: > > On Mon, Feb 19, 2024 at 4:09 PM Vlastimil Babka <vbabka@xxxxxxx> wrote: > >> > >> On 2/19/24 07:23, zhang fangzheng wrote: > >> > On Mon, Feb 19, 2024 at 12:24 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > >> >> > >> >> On Mon, Feb 19, 2024 at 11:19:11AM +0800, Fangzheng Zhang wrote: > >> >> > +Note, <slabreclaim> comes from the collected results in the file > >> >> > +/sys/kernel/slab/$cache/reclaim_account. Next, we will mark /proc/slabinfo > >> >> > +as deprecated and recommend the use of either sysfs directly or > >> >> > +use of the "slabinfo" tool that we have been providing in linux/tools/mm. > >> >> > >> >> Wait, so you're going to all of the trouble of changing the format of > >> >> slabinfo (with the associated costs of updating every tool that currently > >> >> parses it), only to recommend that we stop using it and start using > >> >> tools/mm/slabinfo instead? > >> >> > >> > >> Hi, > >> > >> > The initial purpose was to obtain the type of each slab through > >> > a simple command 'cat proc/slabinfo'. So here, my intention is not to > >> > update all slabinfo-related tools for the time being, but to modify > >> > the version number of proc/slabinfo and further display the results > >> > of using the command. > >> > >> I'm not sure you understand the concern. There are existing consumers of > >> /proc/slabinfo, that might become broken by patch 1/2. We don't even know > >> them all, they might not be all opensource etc. So we can't even make sure > >> all of them are updated. What can happen after patch 1/2: > >> - they keep working and ignore the new column (good) > >> - they include a version check and notice a new unsupported version and > >> refuse to work > >> - confused by the new column they start throwing error, or report wrong > >> stats (that's worse) > >> > > I generally understand your concerns about modifying patch 1/2. > > > > But judging from my modifications, this worry does not seem to be valid. > > Because the “/proc/slabinfo” is not used in related slabinfo debugging tools > > (such as tools/mm/slabinfo), > > Hi, > > we are not concerned about slabinfo debugging tools that are part of kernel > source tree, but about those outside, including those created privately and > we don't even know they exist. > For your concerns, I think the supplementary introduction that new output results of slabinfo v2.2 in patch 2/2 will be necessary. This can help them optimize their tools more quickly to adapt to proc/slabinfo. Is this more friendly? > > but "/sys/kernel/slab/<slab_name>/" (in > > Documentation/mm/slub.rst) or "/ sys/kernel/debug/slab" (in > > tools/mm/slabinfo.c). > > > > Furthermore, the current modification only involves optimizing the output > > of proc/slabinfo, > > It's not "only", the output of /proc/slabinfo is what those tools consume, > so that's what concerns us the most. > > > and does not modify the struct slabinfo or struct kmem_cache. > > So there is no need to adapt other modifications. > > These on the other hand are internal details of the kernel which we can > modify as much we want > > >> >> How about we simply do nothing? > >> > >> Agreed wrt modifying /proc/slabinfo > >> > >> > The note here means what changes will occur after > >> > we modify the version number of proc/slabinfo to 2.2. > >> > As for the replacement of tools/mm/slabinfo (that inspired > >> > by Christoph’s suggestions), it will be implemented in the next version > >> > or even the later version. > >> > >> So what is your motivation for all this in the first place? You have some > >> monitoring tool that relies on /proc/slabinfo and want to distinguish > >> reclaimable caches? So you can change it to parse the /sys directories. Is > >> it more work? Yes, but you only have to do that once per boot, because > >> unlike the object/memory stats in /proc/slabinfo, the reclaimable flag will > >> not change for a cache. > >> > > The situation as you mentioned is very suitable for my current needs. > > My original intention is just to get an intuitive slab screen through a > > simple ‘cat proc/slabinfo’ command. As for the description "<slabreclaim> > > That would be nice, but again we must be careful about existing consumers of > /proc/slabinfo so we can't always have nice things. > > > comes from the collected results in the file > > /sys/kernel/slab/$cache/reclaim_account" > > may not be appropriate. Here I want to express that the column <slabreclaim> has > > the same effect as traversing "/sys/kernel/slab/$ cache/reclaim_account". > > > >> Would tools/mm/slabinfo almost work for you, but you're missing something? > >> Then send patches for that in the first place. Changing /proc/slabinfo (and > >> breaking other consumers) for a quick and easy fix with a different solution > >> planned for the future is simply not feasible. > >> > > Using the slabinfo tool to parse /sys/kernel/slab/$cache/reclaim_account > > is what I think about optimizing future tools during the discussion. > > It will not affect the current patch 1/2, and patch 2/2 is mainly to > > supplement the output examples of proc/slabinfo. > > > > If the community is willing to accept it, I will only modify > > patch 1/2 to implement it. > > > > Thanks very much! > > > >> HTH, > >> Vlastimil > >> > >> > Thanks! > >> >
