Messages in this thread | | | Subject | Re: [PATCH 1/3] Add a new field to struct shrinker | From | Tony Jones <> | Date | Tue, 26 Jul 2016 09:40:57 -0700 |
| |
On 07/20/2016 07:54 AM, Michal Hocko wrote: > On Wed 20-07-16 20:11:09, Janani Ravichandran wrote: >> >>> On Jul 11, 2016, at 8:03 PM, Michal Hocko <mhocko@kernel.org> wrote: >>> >>> On Mon 11-07-16 10:12:51, Rik van Riel wrote: >>>> >>>> What mechanism do you have in mind for obtaining the name, >>>> Michal? >>> >>> Not sure whether tracing infrastructure allows printk like %ps. If not >>> then it doesn't sound too hard to add. >> >> It does allow %ps. Currently what is being printed is the function symbol >> of the callback using %pF. I’d like to know why %pF is used instead of >> %ps in this case. > > From a quick look into the code %pF should be doing the same thing as > %ps in the end. Some architectures just need some magic to get a proper > address of the function. > >> Michal, just to make sure I understand you correctly, do you mean that we >> could infer the names of the shrinkers by looking at the names of their callbacks? > > Yes, %ps can then be used for the name of the shrinker structure > (assuming it is available).
The "shrinker structure" (struct shrinker) isn't a good candidate (as it's often embedded as thus no symbol name can be resolved) but the callback seems to work fine in my testing.
I made an earlier suggestion to Janani that it was helpful to have the superblock shrinker name constructed to include the fstype. This level of specificity would be lost if just the callback is used. I talked briefly to Michal and his view is that more specific tracepoints can be added for this case. This is certainly an option as the super_cache_scan callback can access the superblock and thus the file_system_type via containing record. It's just more work to later reconcile the output of two tracepoints.
I talked briefly to Mel and we both think being able to have this level (of fstype) specificity would be useful and it would be lost just using the callback. Another option which would avoid the static overhead of the names would be to add a new shrinker_name() callback. If NULL, the caller can just perform the default, in this case lookup the symbol for the callback, if !NULL it would provide additional string information which the caller could use. The per-sb shrinker could implement it and return the fstype. It's obviously still a +1 word growth of the struct shrinker but it avoids the text overhead of the constructed names.
Opinions?
Tony
| |