Skip to content

Write perfmap and jitdump files to /tmp by default [6.0 port] #88486

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.

Sign up for GitHub

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

Jump to bottom
Merged
tommcdon merged 3 commits into from
Jul 29, 2023
Merged

Write perfmap and jitdump files to /tmp by default [6.0 port] #88486

tommcdon merged 3 commits into from
Jul 29, 2023

Conversation

@tommcdon
Copy link
Member

@tommcdon tommcdon commented Jul 6, 2023 •
edited
Loading

Description

Perfmaps and Jitdumps is a Linux kernel feature that allows .NET to symbolicate dynamically generated code with perf_events. The perfmap/jitdump files are processed by the the Linux perf tool to generate human readable callstacks for CPU traces, for example. The Linux implementation looks for the perfmap/jitdump files in /tmp (please see https://elixir.bootlin.com/linux/v6.4.3/source/tools/perf/util/symbol.c#L1838). The .NET implementation defaults to writing to /tmp but this can be overridden with the $TMPDIR environment variable. This leads to incorrect behavior in environments where $TMPDIR is in use. This code change alters the behavior and will use /tmp regardless of $TMPDIR settings. There is a minor fix included with this change that correctly sets perfmap location if DOTNET_PerfMapJitDumpPath is set.

This is a port of #88776 from .NET 8

Customer Impact

Internal partner teams have requested this change in .NET servicing

Regression?

Not a regression

Risk

Low, perfmaps+jitdumps is opt-in feature and we are only changing the default location where they are being written to

@ghost ghost added the area-VM-coreclr label Jul 6, 2023
@ghost ghost assigned tommcdon Jul 6, 2023
@carlossanlop
Copy link
Contributor

carlossanlop commented Jul 10, 2023

@tommcdon I see this is still a draft. Reminder - Tomorrow Monday 10th is Code Complete for the August Release. If you intend to get this fix included in that servicing release, please make sure to get a Tactics approval and merge the PR before 4pm, because that's when I close the branches to start merging staging into internal.

@tommcdon tommcdon force-pushed the dev/tommcdon/perfmaps_tmp_6 branch from 691ee26 to 1a29631 Compare July 10, 2023 15:11
@tommcdon
Copy link
Member Author

tommcdon commented Jul 12, 2023

@tommcdon I see this is still a draft. Reminder - Tomorrow Monday 10th is Code Complete for the August Release. If you intend to get this fix included in that servicing release, please make sure to get a Tactics approval and merge the PR before 4pm, because that's when I close the branches to start merging staging into internal.

Thanks @carlossanlop for the reminder. We didn't receive partner team signoff until after the snap, so we will be targeting the next servicing opportunity for .net 6.

@ghost
Copy link

ghost commented Jul 12, 2023

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: tommcdon
Assignees: tommcdon
Labels:

area-Diagnostics-coreclr, area-VM-coreclr
Milestone: -

@tommcdon tommcdon changed the title Dev/tommcdon/perfmaps tmp 6 Write perfmap and jitdump files to /tmp by default Jul 12, 2023
@tommcdon tommcdon added this to the 6.0.x milestone Jul 12, 2023
@tommcdon tommcdon changed the base branch from release/6.0 to release/6.0-staging July 12, 2023 22:49
@tommcdon tommcdon marked this pull request as ready for review July 12, 2023 22:50
@tommcdon tommcdon changed the title Write perfmap and jitdump files to /tmp by default Write perfmap and jitdump files to /tmp by default [6.0 port] Jul 13, 2023
@hoyosjs hoyosjs added the Servicing-consider Issue for next servicing release review label Jul 14, 2023
jeffschwMSFT
jeffschwMSFT approved these changes Jul 17, 2023
View reviewed changes
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved. we will take for consideration in 6.0.x

@rbhanda rbhanda modified the milestones: 6.0.x, 6.0.21 Jul 20, 2023
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jul 20, 2023
@tommcdon tommcdon merged commit 70bff51 into dotnet:release/6.0-staging Jul 29, 2023
@tommcdon tommcdon modified the milestones: 6.0.21, 6.0.x Aug 24, 2023
@leecow leecow modified the milestones: 6.0.x, 6.0.22 Aug 24, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@brianrob brianrob brianrob approved these changes

@jeffschwMSFT jeffschwMSFT jeffschwMSFT approved these changes

@hoyosjs hoyosjs hoyosjs approved these changes

+1 more reviewer

@davmason davmason davmason approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tommcdon tommcdon

Labels

area-Tracing-coreclr Servicing-approved Approved for servicing release

Projects

None yet

Milestone

6.0.22

Development

Successfully merging this pull request may close these issues.

8 participants

@tommcdon @carlossanlop @brianrob @jeffschwMSFT @davmason @hoyosjs @leecow @rbhanda