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

Re: [RFC PATCH 1/2] Add ETW support



On 02/02/2024 07:39, Owen Smith wrote:
ARRAYSIZE vs sizeof - looks like I missed something here.
EtwExit vs EtwExit2 - specifically to reduce having to insert EtwExit(.. STATUS_SUCCESS) in functions that do not return a NTSTATUS


Ok, the naming is ugly though so let's have EtwExit() be the generic one that takes the NTSTATUS. How about EtwSuccess() if you really want a short-hand?

  Paul

Owen

On Thu, Feb 1, 2024 at 5:41 PM Paul Durrant <xadimgnik@xxxxxxxxx <mailto:xadimgnik@xxxxxxxxx>> wrote:

    On 12/01/2024 12:59, Owen Smith wrote:
     > Defines ETW schema with EtwEnter and EtwExit events, which can be
    used to track
     > code-paths and produce flamegraphs of executions.
     > Only uses EtwEnter and EtwExit in the driver.c file of XenBus,
    but more events
     > will be added in subsequent patches.
     >
     > Signed-off-by: Owen Smith <owen.smith@xxxxxxxxx
    <mailto:owen.smith@xxxxxxxxx>>
     > ---
     >   src/common/dbg_print.h       | 60
    +++++++++++++++++++++++++++++++++++
     >   src/xenbus/driver.c          | 11 +++++++
     >   src/xenbus/xenbus_etw.xml    | 61
    ++++++++++++++++++++++++++++++++++++
     >   vs2019/xenbus/xenbus.vcxproj | 13 +++++++-
     >   vs2022/xenbus/xenbus.vcxproj | 13 +++++++-
     >   5 files changed, 156 insertions(+), 2 deletions(-)
     >   create mode 100644 src/xenbus/xenbus_etw.xml
     >
     > diff --git a/src/common/dbg_print.h b/src/common/dbg_print.h
     > index a0bd727..98c22cf 100644
     > --- a/src/common/dbg_print.h
     > +++ b/src/common/dbg_print.h
     > @@ -42,6 +42,66 @@
     >
     >   #pragma warning(disable:4127)   // conditional expression is
    constant
     >
     > +#ifdef ETW_HEADER
     > +#include ETW_HEADER
     > +
     > +static __inline VOID
     > +__EtwEnter(
     > +    IN  const CHAR      *Module,
     > +    IN  const CHAR      *Function,
     > +    IN  ULONG           Line
     > +    )
     > +{
     > +    CHAR                _Module[16];
     > +    CHAR                _Function[32];
     > +
     > +    if (!EventEnabledEvtEnter())
     > +        return;
     > +
     > +    RtlZeroMemory(_Module, sizeof(_Module));
     > +    RtlZeroMemory(_Function, sizeof(_Function));
     > +
     > +    strncpy(_Module, Module, ARRAYSIZE(_Module) -1);

    Missing space. I can fix.

     > +    strncpy(_Function, Function, ARRAYSIZE(_Function) - 1);
     > +
     > +    EventWriteEvtEnter(NULL, _Module, _Function, Line);
     > +}
     > +
     > +static __inline VOID
     > +__EtwExit(
     > +    IN  const CHAR      *Module,
     > +    IN  const CHAR      *Function,
     > +    IN  ULONG           Line,
     > +    IN  NTSTATUS        Status
     > +    )
     > +{
     > +    CHAR                _Module[16];
     > +    CHAR                _Function[32];
     > +
     > +    if (!EventEnabledEvtExit())
     > +        return;
     > +
     > +    RtlZeroMemory(_Module, sizeof(_Module));
     > +    RtlZeroMemory(_Function, sizeof(_Function));
     > +
     > +    strncpy(_Module, Module, sizeof(_Module));

    Why sizeof() here but ARRAYSIZE above?

     > +    strncpy(_Function, Function, sizeof(_Function));
     > +
     > +    EventWriteEvtExit(NULL, _Module, _Function, Line,
    (ULONG)Status);
     > +}
     > +
     > +#define EtwEnter()          __EtwEnter(__MODULE__, __FUNCTION__,
    __LINE__)
     > +#define EtwExit()           __EtwExit(__MODULE__, __FUNCTION__,
    __LINE__, STATUS_SUCCESS)
     > +#define EtwExit2(status)    __EtwExit(__MODULE__, __FUNCTION__,
    __LINE__, status)

    Why this rather than requiring an explicit status for EtwExit()?

        Paul

     > +
     > +#else
     > +
     > +#define EtwEnter()          (VOID)0
     > +#define EtwExit()           (VOID)0
     > +#define EtwExit2(status)    (VOID)status
     > +
     > +#endif
     > +
     >   static __inline VOID
     >   __Error(
     >       IN  const CHAR  *Prefix,
     > diff --git a/src/xenbus/driver.c b/src/xenbus/driver.c
     > index 522acef..4443559 100644
     > --- a/src/xenbus/driver.c
     > +++ b/src/xenbus/driver.c
     > @@ -708,6 +708,8 @@ DriverUnload(
     >
     >       __DriverSetDriverObject(NULL);
     >
     > +    EventUnregisterXenBus_Driver();
     > +
     >       ASSERT(IsZeroMemory(&Driver, sizeof (XENBUS_DRIVER)));
     >
     >       Trace("<====\n");
     > @@ -726,6 +728,7 @@ DriverAddDevice(
     >
     >       ASSERT3P(DriverObject, ==, __DriverGetDriverObject());
     >
     > +    EtwEnter();
     >       Trace("====>\n");
     >
     >       __DriverAcquireMutex();
     > @@ -737,12 +740,14 @@ DriverAddDevice(
     >       __DriverReleaseMutex();
     >
     >       Trace("<====\n");
     > +    EtwExit();
     >
     >       return STATUS_SUCCESS;
     >
     >   fail1:
     >       __DriverReleaseMutex();
     >
     > +    EtwExit2(status);
     >       return status;
     >   }
     >
     > @@ -757,6 +762,8 @@ DriverDispatch(
     >       PXENBUS_DX          Dx;
     >       NTSTATUS            status;
     >
     > +    EtwEnter();
     > +
     >       Dx = (PXENBUS_DX)DeviceObject->DeviceExtension;
     >       ASSERT3P(Dx->DeviceObject, ==, DeviceObject);
     >
     > @@ -800,6 +807,7 @@ DriverDispatch(
     >       }
     >
     >   done:
     > +    EtwExit2(status);
     >       return status;
     >   }
     >
     > @@ -819,6 +827,7 @@ DriverEntry(
     >
     >       ASSERT3P(__DriverGetDriverObject(), ==, NULL);
     >
     > +    EventRegisterXenBus_Driver();
     >       ExInitializeDriverRuntime(DrvRtPoolNxOptIn);
     >       WdmlibProcgrpInitialize();
     >
     > @@ -915,6 +924,8 @@ fail1:
     >
     >       __DriverSetDriverObject(NULL);
     >
     > +    EventUnregisterXenBus_Driver();
     > +
     >       ASSERT(IsZeroMemory(&Driver, sizeof (XENBUS_DRIVER)));
     >
     >       return status;
     > diff --git a/src/xenbus/xenbus_etw.xml b/src/xenbus/xenbus_etw.xml
     > new file mode 100644
     > index 0000000..c73dfe6
     > --- /dev/null
     > +++ b/src/xenbus/xenbus_etw.xml
     > @@ -0,0 +1,61 @@
     > +<?xml version='1.0' encoding='utf-8' standalone='yes'?>
     > +<instrumentationManifest
     > +    xmlns="http://schemas.microsoft.com/win/2004/08/events
    <http://schemas.microsoft.com/win/2004/08/events>"
> + xmlns:win="http://manifests.microsoft.com/win/2004/08/windows/events
    <http://manifests.microsoft.com/win/2004/08/windows/events>"
     > +    xmlns:xs="http://www.w3.org/2001/XMLSchema
    <http://www.w3.org/2001/XMLSchema>"
     > +    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance
    <http://www.w3.org/2001/XMLSchema-instance>"
> + xsi:schemaLocation="http://schemas.microsoft.com/win/2004/08/events
    <http://schemas.microsoft.com/win/2004/08/events> eventman.xsd" >
     > +  <instrumentation>
     > +    <events>
     > +      <provider
     > +          guid="{e580595b-a8a6-41b7-a9c1-1954d2138ffc}"
> + messageFileName="%SystemDrive%\windows\system32\drivers\xenbus.sys"
     > +          name="XenBus_Driver"
> + resourceFileName="%SystemDrive%\windows\system32\drivers\xenbus.sys"
     > +          symbol="DriverControlGuid" >
     > +        <channels>
     > +          <channel chid="XENBUS" name="XenBus" type="Analytic"
    enabled="true" />
     > +        </channels>
     > +        <templates>
     > +          <template tid="tid_enter">
     > +            <data name="Module" inType="win:Int8" count="16"
    outType="xs:string" />     <!-- 16 chars -->
     > +            <data name="Function" inType="win:Int8" count="32"
    outType="xs:string" />   <!-- 32 chars -->
     > +            <data name="Line" inType="win:UInt32"
    outType="xs:unsignedInt" />           <!-- line number -->
     > +          </template>
     > +          <template tid="tid_exit">
     > +            <data name="Module" inType="win:Int8" count="16"
    outType="xs:string" />     <!-- 16 chars -->
     > +            <data name="Function" inType="win:Int8" count="32"
    outType="xs:string" />   <!-- 32 chars -->
     > +            <data name="Line" inType="win:UInt32"
    outType="xs:unsignedInt" />           <!-- line number -->
     > +            <data name="Status" inType="win:UInt32"
    outType="xs:unsignedInt" />         <!-- NTSTATUS -->
     > +          </template>
     > +        </templates>
     > +        <events>
     > +          <event
     > +            channel="XENBUS"
     > +            level="win:Informational"
     > +            message="$(string.EventTraceInfo.Enter)"
     > +            opcode="win:Info"
     > +            symbol="EvtEnter"
     > +            template="tid_enter"
     > +            value="10" />
     > +          <event
     > +            channel="XENBUS"
     > +            level="win:Informational"
     > +            message="$(string.EventTraceInfo.Exit)"
     > +            opcode="win:Info"
     > +            symbol="EvtExit"
     > +            template="tid_exit"
     > +            value="11" />
     > +        </events>
     > +      </provider>
     > +    </events>
     > +  </instrumentation>
     > +  <localization
    xmlns="http://schemas.microsoft.com/win/2004/08/events
    <http://schemas.microsoft.com/win/2004/08/events>">
     > +    <resources culture="en-US">
     > +      <stringTable>
     > +        <string id="EventTraceInfo.Enter" value="[Enter]" />
     > +        <string id="EventTraceInfo.Exit"  value="[Exit ]" />
     > +      </stringTable>
     > +    </resources>
     > +  </localization>
     > +</instrumentationManifest>
     > diff --git a/vs2019/xenbus/xenbus.vcxproj
    b/vs2019/xenbus/xenbus.vcxproj
     > index aa88980..e191527 100644
     > --- a/vs2019/xenbus/xenbus.vcxproj
     > +++ b/vs2019/xenbus/xenbus.vcxproj
     > @@ -20,7 +20,7 @@
     >     <ItemDefinitionGroup>
     >       <ClCompile>
     >         <AdditionalOptions>/ZH:SHA_256
    %(AdditionalOptions)</AdditionalOptions>
> - <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;%(PreprocessorDefinitions)</PreprocessorDefinitions> > + <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;ETW_HEADER="xenbus_etw.h";%(PreprocessorDefinitions)</PreprocessorDefinitions>
     >         <IntrinsicFunctions>true</IntrinsicFunctions>
>  <AdditionalIncludeDirectories>$(WindowsSdkDir)\include\km;..\..\include;..\..\include\xen;..\..\src\common;</AdditionalIncludeDirectories>
     >         <WarningLevel>EnableAllWarnings</WarningLevel>
     > @@ -38,6 +38,13 @@
     >         <MapExports>true</MapExports>
     >         <AdditionalOptions>/INTEGRITYCHECK
    %(AdditionalOptions)</AdditionalOptions>
     >       </Link>
     > +    <MessageCompile>
     > +      <HeaderFilePath>..\..\include</HeaderFilePath>
     > +      <GeneratedFilesBaseName>%(Filename)</GeneratedFilesBaseName>
     > +      <RCFilePath>..\..\src\xenbus</RCFilePath>
> + <GenerateKernelModeLoggingMacros>true</GenerateKernelModeLoggingMacros>
     > +      <UseBaseNameOfInput>true</UseBaseNameOfInput>
     > +    </MessageCompile>
     >       <DriverSign>
     >         <FileDigestAlgorithm>sha256</FileDigestAlgorithm>
     >       </DriverSign>
     > @@ -68,6 +75,7 @@
     >     <ItemGroup>
     >       <FilesToPackage Include="$(TargetPath)" />
     >       <FilesToPackage Include="$(OutDir)$(TargetName).pdb" />
     > +    <FilesToPackage Include="..\..\src\xenbus\xenbus_etw.xml" />
     >     </ItemGroup>
     >     <ItemGroup>
     >       <ClCompile Include="..\..\src\common\registry.c" />
     > @@ -97,5 +105,8 @@
     >     <ItemGroup>
     >       <ResourceCompile Include="..\..\src\xenbus\xenbus.rc" />
     >     </ItemGroup>
     > +  <ItemGroup>
     > +    <MessageCompile Include="..\..\src\xenbus\xenbus_etw.xml" />
     > +  </ItemGroup>
     >     <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
     >   </Project>
     > diff --git a/vs2022/xenbus/xenbus.vcxproj
    b/vs2022/xenbus/xenbus.vcxproj
     > index ce0526f..1322d15 100644
     > --- a/vs2022/xenbus/xenbus.vcxproj
     > +++ b/vs2022/xenbus/xenbus.vcxproj
     > @@ -20,7 +20,7 @@
     >     <ItemDefinitionGroup>
     >       <ClCompile>
     >         <AdditionalOptions>/ZH:SHA_256
    %(AdditionalOptions)</AdditionalOptions>
> - <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;%(PreprocessorDefinitions)</PreprocessorDefinitions> > + <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;ETW_HEADER="xenbus_etw.h";%(PreprocessorDefinitions)</PreprocessorDefinitions>
     >         <IntrinsicFunctions>true</IntrinsicFunctions>
>  <AdditionalIncludeDirectories>$(WindowsSdkDir)\include\km;..\..\include;..\..\include\xen;..\..\src\common;</AdditionalIncludeDirectories>
     >         <WarningLevel>EnableAllWarnings</WarningLevel>
     > @@ -38,6 +38,13 @@
     >         <MapExports>true</MapExports>
     >         <AdditionalOptions>/INTEGRITYCHECK
    %(AdditionalOptions)</AdditionalOptions>
     >       </Link>
     > +    <MessageCompile>
     > +      <HeaderFilePath>..\..\include</HeaderFilePath>
     > +      <GeneratedFilesBaseName>%(Filename)</GeneratedFilesBaseName>
     > +      <RCFilePath>..\..\src\xenbus</RCFilePath>
> + <GenerateKernelModeLoggingMacros>true</GenerateKernelModeLoggingMacros>
     > +      <UseBaseNameOfInput>true</UseBaseNameOfInput>
     > +    </MessageCompile>
     >       <DriverSign>
     >         <FileDigestAlgorithm>sha256</FileDigestAlgorithm>
     >       </DriverSign>
     > @@ -60,6 +67,7 @@
     >     <ItemGroup>
     >       <FilesToPackage Include="$(TargetPath)" />
     >       <FilesToPackage Include="$(OutDir)$(TargetName).pdb" />
     > +    <FilesToPackage Include="..\..\src\xenbus\xenbus_etw.xml" />
     >     </ItemGroup>
     >     <ItemGroup>
     >       <ClCompile Include="..\..\src\common\registry.c" />
     > @@ -89,5 +97,8 @@
     >     <ItemGroup>
     >       <ResourceCompile Include="..\..\src\xenbus\xenbus.rc" />
     >     </ItemGroup>
     > +  <ItemGroup>
     > +    <MessageCompile Include="..\..\src\xenbus\xenbus_etw.xml" />
     > +  </ItemGroup>
     >     <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
     >   </Project>






 


Rackspace

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