Re: [PATCH 1/3] Add a new field to struct shrinker

[Tony Jones]

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@xxxxxxxxxx> 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