On 06/01/2017 02:30 AM, David Gibson wrote:
On Wed, May 31, 2017 at 11:25:41PM -0500, Michael Roth wrote:
Quoting Bharata B Rao (2017-05-31 23:06:46)
On Thu, Jun 01, 2017 at 11:52:14AM +1000, David Gibson wrote:
The code managing DRCs[0] has quite a few things that are more
complicated than they need to be. In particular the object
representing a DRC has a bunch of method pointers, despite the fact
that there are currently no subclasses, and even if there were the
method implementations would be unlikely to differ.
So you are getting rid of a few methods. How about other methods ?
Specially attach and detach which have incorporated all the logic needed
to handle logical and physical DRs into their implementations ?
I would avoid any methods that incorporate special-casing for
physical vs. logical DRCs, since that seems like a good logical
starting point for moving to 'physical'/'logical' DRC
sub-classes to help simplify the increasingly complicated
state-tracking.
Right, I'm looking at making subclasses for each of the DRC types.
Possibly with intermediate subclasses for physical vs. logical, we'll
see how it works out.
Back in the DRC migration patch series I talked with Mike about refactoring
the DRC code in such fashion (physical DRC and logical DRC). But first I
would
implement some kind of unit testing in this code to avoid breaking too much
stuff during this refactoring.
I am not sure about the effort to implementing unit test in the current
DRC code.
This series is simplifying the DRC code, making it more minimalist and
possibly
easier to be tested. In the end it would be a first step towards unit
testing.
However, there is the issue of backward compatibility. I fear this DRC
refactoring
of Logical/Physical DRC would be too drastic to maintain such compatibility
(assuming that it is not already broken). If this refactor goes live
only in 2.11 then
we will have a hard time to migrate from 2.11 to 2.10.
All that said, I believe we can live without unit testing for a little
longer and if
we're going for this Physical/DRC refactoring, we need to push it for
2.10. We can
think about unit test later with the refactored code. Feel free to send
to me any
unfinished/beta DRC refactoring code you might be working on and want
tested. I can help in the refactoring too, just let me know.
Daniel
I also don't think we should expose DRC internal fields to
outside callers (which attach/detach would involve).
Well.. just changing attach/detach to plain functions instead of
methods wouldn't break that.
This
series does that to some extent with the RTAS calls, but
since those are now moved to spapr_drc.c it makes more sense.
Right - the semantics of the RTAS calls are tied closely to the DRC
semantics, so I don't think there's any point considering the RTAS
calls to be "outside" the DRC code itself.
