Re: [PATCH for-7.1 0/9] spapr: add drc->index, remove spapr_drc_index()

[Daniel Henrique Barboza]

On 3/21/22 00:55, David Gibson wrote:
> On Fri, Mar 18, 2022 at 02:33:11PM -0300, Daniel Henrique Barboza wrote:
> > Hi,
> >
> > I decided to make this change after realizing that (1) spapr_drc_index()
> > always return the same index value for the DRC regardless of machine or
> > device state and (2) we call spapr_drc_index() a lot throughout the
> > spapr code.
>
> Hmm.. so, spapr_drc_index() wasn't ever intended as an abstraction
> point.  Rather, it's just there as a matter of data redundancy.  The
> index can be derived from the drc->id and the type.  Unless there's a
> compelling reason otherwise, it's usually a good idea to store data in
> just one form (if there's more it's an opportunity for bugs to let it
> get out of sync).


Hmm what if we store drc->index instead and derive drc->id from it? drc->index
is read from several places, while drc->id is used just in spapr_drc_name() to
write the DT (via spapr_dt_drc()).

I'll think more about it.


Thanks,


Daniel

> > This means that a new attribute to store the generated index in the DRC
> > object time will spare us from calling a function that always returns
> > the same value.
> >
> > No functional changes were made.
> >
> >   
> > Daniel Henrique Barboza (9):
> >    hw/ppc/spapr_drc.c: add drc->index
> >    hw/ppc/spapr_drc.c: redefine 'index' SpaprDRC property
> >    hw/ppc/spapr_drc.c: use drc->index in trace functions
> >    hw/ppc/spapr_drc.c: use drc->index
> >    hw/ppc/spapr.c: use drc->index
> >    hw/ppc/spapr_events.c: use drc->index
> >    hw/ppc/spapr_nvdimm.c: use drc->index
> >    hw/ppc/spapr_pci.c: use drc->index
> >    hw/ppc/spapr_drc.c: remove spapr_drc_index()
> >
> >   hw/ppc/spapr.c             | 18 ++++-----
> >   hw/ppc/spapr_drc.c         | 79 +++++++++++++++-----------------------
> >   hw/ppc/spapr_events.c      |  4 +-
> >   hw/ppc/spapr_nvdimm.c      | 10 ++---
> >   hw/ppc/spapr_pci.c         |  5 +--
> >   include/hw/ppc/spapr_drc.h |  2 +-
> >   6 files changed, 48 insertions(+), 70 deletions(-)