[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 1/2] Add ETW support
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
Owen
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>
> ---
> 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"
> + xmlns:win="http://manifests.microsoft.com/win/2004/08/windows/events"
> + xmlns:xs="http://www.w3.org/2001/XMLSchema"
> + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
> + xsi:schemaLocation="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">
> + <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>
|