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

Re: [PATCH v2.1 14/12] xen: Switch to new TRACE() API

  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 24 Sep 2021 15:34:50 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=vm0NoYzt0htDoXnmvDOfpIHeSYoMkw+l5gL+SKv2DPs=; b=fMxzWPbRbgrrjPLRG0QPPc4LJ2tcbdLfT8vGQ8f0czuKrPQH+MUHRWG2irryzlN5HS649K9Gutm0DDfNvYUR6uHokQmIqLdnI0+w6ZHsijNFQGDaXPg2R8I0NlMwvwLUczGCdBbw48hPx5S3TFy7s+KI5kLpoFR5qu8Nouf/Bh95l1i0r/2rDps9uaJK8ojJo3MLcAVIQnzREph9xmCEMe4F8oh70cHczjiuR27p5J/OLpvAgkFalycXYcG5DNSE4cTe11Xzv24ykSIAr8xW2Hmh/8En1pnx2JOwBS0fJcbheSl51hFfrcFVQS5VPy6SHw2WAA20TCDv8jLLbLLObQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RfAqsoJlrXPNN2UTA67MkPT0lxWpaePFegM6BNJGFnYYdRJc1HcK6Jq42kRAKXaRPZ8vmP/5fdp6SKpur3+sPEK2wguz3Xsg/knBxNqseGYenLySz42GlYJWKdCFh5uAOwuOEP9NgKcYnWfmHexU4QSYPENxlHKMJWTN8z/d2g2CHmOC72cLYdEaaVqSLSyiUgk75XG7MT7A+40HkOnGAboYyGwqT3yKnZ3ICSRM2PhnEhfOCXctTYwYGJJfLYU1DzsCIoLkdS3/2FKMaI1ZWXmAU/YEwtyGEVMDq6omxeInNuy70k55E2JIOCGxe4UF0//XPFZA2e15wz2Ho4p/yQ==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 24 Sep 2021 13:34:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 20.09.2021 21:32, Andrew Cooper wrote:
> (Almost) no functional change.
> irq_move_cleanup_interrupt() changes two smp_processor_id() calls to the 'me'
> local variable which manifests as a minor code improvement.  All other
> differences in the compiled binary are to do with line numbers changing.
> Some conversion notes:
>  * HVMTRACE_LONG_[234]D() and TRACE_2_LONG_[234]D() were latently buggy.  They
>    blindly discard extra parameters, but luckily no users are impacted.  They
>    are also obfuscated wrappers, depending on exactly one or two parameters
>    being TRC_PAR_LONG() to compile successfully.
>  * HVMTRACE_LONG_1D() behaves unlike its named companions, and takes exactly
>    one 64bit parameter which it splits manually.  It's one user,
>    vmx_cr_access()'s LMSW path, is gets adjusted to use TRACE_PARAM64().
>  * TRACE_?D() and TRACE_2_LONG_*() change to TRACE_TIME() as cycles is always.
>  * HVMTRACE_ND() is opencoded for VMENTRY/VMEXIT records to include cycles.
>    These are converted to TRACE_TIME(), with the old modifier parameter
>    expressed as an OR at the callsite.  One callsite, svm_vmenter_helper() had
>    a nested tb_init_done check, which is dropped.  (The optimiser also spotted
>    this, which is why it doesn't manifest as a binary difference.)
>  * All HVMTRACE_?D() change to TRACE() as cycles is explicitly skipped.
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

> I'm in two minds as to whether to split this up by subsystem or not.  It is
> 95% x86, and isn't a massive patch.

Either way looks fine to me in this case; splitting might allow parts
to go in before you've managed to get acks from all relevant people.
If anything I might have preferred seeing e.g. all the HVM*() macros
getting replaced and dropped at the same time, rather than the
dropping (combined with others) getting split off.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.