Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pcm-latency write-ddr latency is wrong? #565

Open
ZouTaooo opened this issue Jul 5, 2023 · 4 comments
Open

pcm-latency write-ddr latency is wrong? #565

ZouTaooo opened this issue Jul 5, 2023 · 4 comments

Comments

@ZouTaooo
Copy link

ZouTaooo commented Jul 5, 2023

In Ice Lake micro architecture, the relevent event is below (come from some intel manual.):

event evsel umask desc
RPQ_INSERTS.PCH0 0x10 bxxxxxxx1 Read Pending Queue Allocations
RPQ_INSERTS.PCH1 0x10 bxxxxxx1x Read Pending Queue Allocations
RPQ_OCCUPANCY_PCH0 0x80 0x00 Read Pending Queue Occupancy
RPQ_OCCUPANCY_PCH1 0x81 0x00 Read Pending Queue Occupancy
WPQ_INSERTS.PCH0 0x20 bxxxxxxx1 Write Pending Queue Allocations
WPQ_INSERTS.PCH1 0x20 bxxxxxx1x Write Pending Queue Allocations
WPQ_OCCUPANCY_PCH0 0x82 0x00 Write Pending Queue Occupancy
WPQ_OCCUPANCY_PCH1 0x83 0x00 Write Pending Queue Occupancy

I found the event of DRAM WPQ Occupany is incorrect. It should be 0x82. And the umask of rpq-occupancy should be zero, the umask of wpq-insert should be one.

The source code is below, in cpucounters.cpp.

Or I am wrong?
Please give a answer. Thank you!

PCM::ErrorCode PCM::programServerUncoreLatencyMetrics(bool enable_pmm)
{
    uint32 DDRConfig[4] = {0,0,0,0};

    if (enable_pmm == false)
    {   //DDR is false
        if (ICX == cpu_model || SPR == cpu_model)
	{
            DDRConfig[0] = MC_CH_PCI_PMON_CTL_EVENT(0x80) + MC_CH_PCI_PMON_CTL_UMASK(1);  // DRAM RPQ occupancy
            DDRConfig[1] = MC_CH_PCI_PMON_CTL_EVENT(0x10) + MC_CH_PCI_PMON_CTL_UMASK(1);  // DRAM RPQ Insert
            DDRConfig[2] = MC_CH_PCI_PMON_CTL_EVENT(0x81) + MC_CH_PCI_PMON_CTL_UMASK(0);  // DRAM WPQ Occupancy
            DDRConfig[3] = MC_CH_PCI_PMON_CTL_EVENT(0x20) + MC_CH_PCI_PMON_CTL_UMASK(0);  // DRAM WPQ Insert

	} else {

            DDRConfig[0] = MC_CH_PCI_PMON_CTL_EVENT(0x80) + MC_CH_PCI_PMON_CTL_UMASK(0);  // DRAM RPQ occupancy
            DDRConfig[1] = MC_CH_PCI_PMON_CTL_EVENT(0x10) + MC_CH_PCI_PMON_CTL_UMASK(0);  // DRAM RPQ Insert
            DDRConfig[2] = MC_CH_PCI_PMON_CTL_EVENT(0x81) + MC_CH_PCI_PMON_CTL_UMASK(0);  // DRAM WPQ Occupancy
            DDRConfig[3] = MC_CH_PCI_PMON_CTL_EVENT(0x20) + MC_CH_PCI_PMON_CTL_UMASK(0);  // DRAM WPQ Insert
	}
    } else {
        DDRConfig[0] = MC_CH_PCI_PMON_CTL_EVENT(0xe0) + MC_CH_PCI_PMON_CTL_UMASK(1);  // PMM RDQ occupancy
        DDRConfig[1] = MC_CH_PCI_PMON_CTL_EVENT(0xe3) + MC_CH_PCI_PMON_CTL_UMASK(0);  // PMM RDQ Insert
        DDRConfig[2] = MC_CH_PCI_PMON_CTL_EVENT(0xe4) + MC_CH_PCI_PMON_CTL_UMASK(1);  // PMM WPQ Occupancy
        DDRConfig[3] = MC_CH_PCI_PMON_CTL_EVENT(0xe7) + MC_CH_PCI_PMON_CTL_UMASK(0);  // PMM WPQ Insert
    }

    if (DDRLatencyMetricsAvailable())
    {
        for (size_t i = 0; i < (size_t)serverUncorePMUs.size(); ++i)
        {
            serverUncorePMUs[i]->programIMC(DDRConfig);
        }
    }
    return PCM::Success;
}
@rdementi
Copy link
Contributor

thanks for reporting this. We will take a look with the developer

@ZouTaooo
Copy link
Author

ZouTaooo commented Aug 1, 2023

Thanks.
Looking forward to your reply.

@sravisun
Copy link

sravisun commented Aug 9, 2023

Hi @ZouTaooo,

Thank you reporting this bug.
-Currently the code only reports out DDR read latency and not write latency. Though we can make the necessary event change to update the WPQ occupancy and inserts, it will not be reported out.
-The RPQ inserts needs to be updated as pointed out above. Ran with both the scenarios (inserts with umask 0 and 1), both the scenarios reports the same number.
Though we need to update the values in code, there is no direct impact on the latency reported.

Thank you

@ZouTaooo
Copy link
Author

I hava two quesitons:

  1. No matter impact,is my idea right or wrong?Because I want to get write latency.
  2. Why not report the write latency?I think it‘s a important metric like read latency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants