1. Patchew
  2. QEMU
  3. View series

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

Daniel Henrique Barboza posted 9 patches 2 years, 1 month ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220318173320.320541-1-danielhb413@gmail.com
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>
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(-)
[PATCH for-7.1 0/9] spapr: add drc->index, remove spapr_drc_index()
Posted by Daniel Henrique Barboza 2 years, 1 month ago 
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.

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(-)

-- 
2.35.1
Re: [PATCH for-7.1 0/9] spapr: add drc->index, remove spapr_drc_index()
Posted by David Gibson 2 years, 1 month ago 
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).

> 
> 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(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH for-7.1 0/9] spapr: add drc->index, remove spapr_drc_index()
Posted by Daniel Henrique Barboza 2 years, 1 month ago 

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(-)
>>
>
Re: [PATCH for-7.1 0/9] spapr: add drc->index, remove spapr_drc_index()
Posted by David Gibson 2 years, 1 month ago 
On Mon, Mar 21, 2022 at 04:58:47AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> 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()).

That could work.  It's still slightly redundant since the type part of
the index can be derived from the class, but I think it's not unreasonable.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.