Skip to content

Conversation

@pengxianghao21
Copy link

@pengxianghao21 pengxianghao21 commented Jul 28, 2022

Summary

What

  1. replaced TCB_FLAG_MEM_CHECK by TCB_FLAG_HEAPCHECK
  2. add heapcheck flag

Why

kmm_checkcorruption is used to check whether memory heap is normal.

  1. Comparing with flag name TCB_FLAG_MEM_CHECK, TCB_FLAG_HEAPCHECK should be more suitable in such condition.
  2. A dynamic flag control would efficiently debug using kmm_checkcorruption().

How

kmm_checkcorruption() with flexible flag control for developer to choose for each tcb.

1 for open heapcheck, 0 for close
echo 1 > /proc/xxx/heapcheck
echo 0 > /proc/xxx/heapcheck

Impact

Using of TCB_FLAG_MEM_CHECK may need to be replaced with TCB_FLAG_HEAPCHECK.

Testing

Vela CI

Copy link
Contributor

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

@pengxiang-hao -

The title and Summary, Impact, Testing do not document what this PR does. I makes is sound like you renamed and added a flag. Please document the changes herein and answer the questions Why it is needed and What does and How Please do this by editing the title and Summary, Impact, Testing above.

@xiaoxiang781216 xiaoxiang781216 requested a review from davids5 July 28, 2022 09:14
@davids5
Copy link
Contributor

davids5 commented Jul 28, 2022

@xiaoxiang781216 - I think we cross posted. There have been no changes since I reviewed the PR and asked for changes.

@davids5 davids5 requested a review from xiaoxiang781216 July 28, 2022 09:27
@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 - I think we cross posted. There have been no changes since I reviewed the PR and asked for changes.

I think "The title and Summary, Impact, Testing" is updated, but the code isn't. @pengxiang-hao please update the code too.

@davids5
Copy link
Contributor

davids5 commented Jul 28, 2022

@xiaoxiang781216 - I think we cross posted. There have been no changes since I reviewed the PR and asked for changes.

I think "The title and Summary, Impact, Testing" is updated, but the code isn't. @pengxiang-hao please update the code too.

GH logs the edits. I do not see any since I posted. Do you?

Signed-off-by: haopengxiang <haopengxiang@xiaomi.com>
1 for open heapcheck, 0 for close
echo 1 > /proc/xxx/heapcheck
echo 0 > /proc/xxx/heapcheck

Signed-off-by: haopengxiang <haopengxiang@xiaomi.com>
@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Jul 31, 2022

@davids5 please review the update again, the change look good to me now.

@xiaoxiang781216 xiaoxiang781216 merged commit 9bb5148 into apache:master Aug 2, 2022
@pengxianghao21 pengxianghao21 deleted the memcheck_flag branch August 4, 2022 03:39
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.

3 participants