Re: [PATCH 0/2] nvdimm deadcoding

["Dr. David Alan Gilbert" <linux@treblig.org>]

* Alison Schofield (alison.schofield@intel.com) wrote:
> On Thu, Feb 20, 2025 at 12:45:36AM +0000, linux@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > 
> > Hi,
> >   A couple of nvdimm dead coding patches; they just
> > remove entirely unused functions.
> 
> I see you've been sending patches for dead code removal
> for several months. What tool are you using for discovery?

Hi Alison,
  Thanks for noticing.

  I'm using my own very very hacky scripts; notes below.
The problem is they generate lots of false-positives
so I've got this big output file from one run of the scripts
and I then have to go through finding the useful ones.
I start with an all-yes-config build on x86, then the
script dumps all the relocations using readelf and finds
symbols that are defined but don't have any apparent
use.
(Actually not quite all-yes-config, but close)

Then for each of those symbols I grep the whole tree
for the symbol and get myself a huge debug file.
That means that for each symbol I'm hopefully left where
it's defined and declared, and if there are actually any
uses that weren't built in my world they show up.

Then I gently work through these, look them up with git log -G
to find when they were added/stopped being used.

On one side there are false positives (stuff only in other
builds, so that's why I grep for the symbol name, also
symbols that are defined but only used locally in a .o
file)
There are lots of false negatives as well.
But then I have to second guess from the git output
  a) If it was recently added don't bother, someone is
probably about to use it.
  b) If it's actually a bug and it *should* be called.
  c) If it's a trivial one liner I don't bother
  d) If it looks like it's really a wrapper of every
firmware call for that device I leave it.
  e) Skip anything __init etc marked, or look magic
(bpf etc)

It's tricky because they're all over, so you fall
into the preferences of each maitainers oddities of
what they care about.

My debug file is alphabetically searched; I'm just near
the end of the n's - a while to go!

I'm hoping once I get to the end, then it'll be a bit
cleaner and I can tidy the scripts up and watch for
new entries in each release.

For reference for the nvdimm symbols my output file looks
like:
---- nd_attach_ndns[^A-Z_a-z] ----
drivers/nvdimm/claim.c:44:bool __nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach,
drivers/nvdimm/claim.c:59:bool nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach,
drivers/nvdimm/claim.c:65:  claimed = __nd_attach_ndns(dev, attach, _ndns);
drivers/nvdimm/claim.c:216: if (!__nd_attach_ndns(dev, ndns, _ndns)) { 
drivers/nvdimm/nd-core.h:139:bool nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach,
drivers/nvdimm/nd-core.h:141:bool __nd_attach_ndns(struct device *dev, struct nd_namespace_common *attach,
drivers/nvdimm/btt_devs.c:211:  if (ndns && !__nd_attach_ndns(&nd_btt->dev, ndns, &nd_btt->ndns)) {
drivers/nvdimm/pfn_devs.c:311:  if (ndns && !__nd_attach_ndns(&nd_pfn->dev, ndns, &nd_pfn->ndns)) {
---- nd_region_conflict[^A-Z_a-z] ----
drivers/nvdimm/nd-core.h:130:int nd_region_conflict(struct nd_region *nd_region, resource_size_t start,
drivers/nvdimm/region_devs.c:1260:int nd_region_conflict(struct nd_region *nd_region, resource_size_t start,


Scripts
  (Which I've not run for a few months, I'm still working through
the first main run)

I start with:
  find . -name \*.o -exec ~/sym/dosyms {} \;
of which dosyms is:
--------
  echo $1
  DIR=$(dirname $1)
  NEWN=$DIR/$(basename -s .o $1).x

  readelf -W -s -r $1 | awk -f ~/sym/relocs.awk |sort|uniq > $NEWN
--------
so that gets me a load of .x files, which I run through
  awk -f ~/sym/collate.awk $(find . -name \*.x) > col
of which collate.awk is:
--------
  { if (($1=="u") || ($1=="U")) {
      use[$2]=use[$2] "," FILENAME
      usecount[$2]++
    } else {
      def[$2]=def[$2] ",:" $1 ":" FILENAME
      defcount[$2]++
    }
  }
  END {
    for (s in def) {
      if (usecount[s] == 0) {
        printf("%s:%d: %s from %s\n", s, usecount[s], use[s], def[s]) 
      }
    }
  }
--------
Then the following magic:
  cut -d' ' -f1 col | sed -e 's/:.*/[^A-Z_a-z]/' | while read SYM; do echo $SYM; ag -s --ignore '*.x' --ignore col --ignore col2 "$SYM" < /dev/null; done > search.out

which gives the output shown above.
(ag is a fast parallel grep, 'the-silver-searcher' )

> Thanks,
> Alison

Have fun,

Dave
> 
> > 
> > Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
> > 
> > 
> > Dr. David Alan Gilbert (2):
> >   libnvdimm: Remove unused nd_region_conflict
> >   libnvdimm: Remove unused nd_attach_ndns
> > 
> >  drivers/nvdimm/claim.c       | 11 ----------
> >  drivers/nvdimm/nd-core.h     |  4 ----
> >  drivers/nvdimm/region_devs.c | 41 ------------------------------------
> >  3 files changed, 56 deletions(-)
> > 
> > -- 
> > 2.48.1
> > 
> > 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/