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

Gcov #107

Merged
merged 2 commits into from
Mar 25, 2021
Merged

Gcov #107

merged 2 commits into from
Mar 25, 2021

Conversation

nickdesaulniers
Copy link
Member

No description provided.

@nathanchance
Copy link
Member

This can be done later but I assume it is probably worth expanding this in terms of trees and architectures (mainline and arm64 maybe, given that the report came from Qualcomm?)

Copy link
Member

@nathanchance nathanchance left a comment

Choose a reason for hiding this comment

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

LGTM. Is this going to be green without your patch?

@nickdesaulniers
Copy link
Member Author

This can be done later but I assume it is probably worth expanding this in terms of trees and architectures (mainline and arm64 maybe, given that the report came from Qualcomm?)

This bug wasn't arch specific. There's diminishing returns for additional coverage IMO, though we do have the capacity. :)

LGTM. Is this going to be green without your patch?

The build will be, but the boot wont be. So let's wait to merge until https://lore.kernel.org/lkml/20210312192139.2503087-1-ndesaulniers@google.com/T/#u is in -next?

@nathanchance
Copy link
Member

This can be done later but I assume it is probably worth expanding this in terms of trees and architectures (mainline and arm64 maybe, given that the report came from Qualcomm?)

This bug wasn't arch specific. There's diminishing returns for additional coverage IMO, though we do have the capacity. :)

Fair enough :)

LGTM. Is this going to be green without your patch?

The build will be, but the boot wont be. So let's wait to merge until https://lore.kernel.org/lkml/20210312192139.2503087-1-ndesaulniers@google.com/T/#u is in -next?

Yes, I think that is smart. I will test and review in a little bit.

@nathanchance
Copy link
Member

On its way to Linus, then we could turn this on for mainline and next?

https://lore.kernel.org/r/20210325043744.olufBBZcn%25akpm@linux-foundation.org/

@nickdesaulniers
Copy link
Member Author

On its way to Linus, then we could turn this on for mainline and next?

This is a config that I suspect will have diminishing returns for the additional test coverage. I may eat my words someday, but I'll add the CI coverage if so.

@nickdesaulniers nickdesaulniers merged commit 994097a into main Mar 25, 2021
@nickdesaulniers nickdesaulniers deleted the gcov branch March 25, 2021 17:02
@@ -185,6 +186,7 @@ builds:
- {<< : *x86_64_allmod_lto, << : *next, << : *llvm_full, boot: false, llvm_version: *llvm_tot}
- {<< : *x86_64_allno, << : *next, << : *llvm_full, boot: false, llvm_version: *llvm_tot}
- {<< : *x86_64_allyes, << : *next, << : *llvm_full, boot: false, llvm_version: *llvm_tot}
- {<< : *x86_64_gcov, << : *next, << : *llvm_full, boot: true, llvm_version: *llvm_tot}
Copy link
Member Author

Choose a reason for hiding this comment

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

d'oh, when rebasing, I fixed the indentation of the first hunk but not this. Oh well.

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

Successfully merging this pull request may close these issues.

None yet

2 participants