Re: [PATCH 41/41] Move CMT feature filtering to QEMU driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/16/2016 09:03 AM, Jiri Denemark wrote:
> On Tue, Aug 30, 2016 at 18:06:36 -0400, John Ferlan wrote:
>>
>>
>> On 08/12/2016 09:33 AM, Jiri Denemark wrote:
>>> It really doesn't belong to the generic CPU driver.
>>>
>>> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
>>> ---
>>>  src/cpu/cpu_x86.c            | 16 ++--------------
>>>  src/qemu/qemu_capabilities.c | 16 +++++++++++++++-
>>>  2 files changed, 17 insertions(+), 15 deletions(-)
>>>
>>
>> Hmm... interesting is this something that the online perf add more stats
>> will need to also adjust, see (8/8):
>>
>> http://www.redhat.com/archives/libvir-list/2016-August/msg00209.html
>>
>> It doesn't seem so, but since I recognized the acronyms I figured I'd
>> check...
> 
> The support for perf events is advertised by CMT CPU feature (and a few
> others), but otherwise the parts of our code dealing with CPU features
> and perf events are unrelated. There's no need to adjust anything in the
> perf events code.
> 
>> So here we are again at a summary - if I didn't comment on something
>> consider it an implicit ACK.
>>
>> There's a couple of reviews that are simple and ACK'able - I think
>> they're obvious.
>>
>> However, there's also a couple where I'm just looking for information. I
>> have no reason to not ACK, just wanted some clarity. I don't necessarily
>> need to see a whole new series.  I think it just the interaction noted
>> in patch 40, 35, and 26 (update and compare callbacks).
> 
> So after my replies to your comments, do you want me to resend any
> patches from the series? I think the best option is to resend them all
> and mark all unchanged patches so that reviewers do not need to look at
> them again, but it's going to be a series of 45 patches.

Not sure I look forward to the idea of looking at and trying to put 45
patches into short term memory. But I also think process wise it's what
is should be done. Then again I think there were certainly quite a few
at the beginning (up through perhaps 19) were OK.  It was at 20 where
that Format algorithm (sorting by yes, no, unknown) got me side tracked.
Then starting at 26 is where I kept shuttling back and forth between things.

Part of me says - let's push and move on in order to make progress. If
you wait for another few days it's that much longer to fix any weird
compiler or testing issues on other platforms that invariably creep up
when large series are pushed.

How's that for non-committal... I'm 85% on the push side though as I
believe you've dealt with my comments and questions.

John
> 
> Anyway, thanks for the review, I can imagine going through so many
> patches to the ugly CPU code was not easy. But I think the code is
> going to get better thanks to the patches :-)
> 
> Jirka
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]