| Message ID | 20240821075311.1953-1-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] mm/page_poison: slightly optimize check_poison_mem() | expand |
On 21.08.24 09:53, Zhen Lei wrote: > When the debug information needs to be suppressed due to ratelimit, > it is unnecessary to determine the end of the corrupted memory. > > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- > mm/page_poison.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/mm/page_poison.c b/mm/page_poison.c > index 3e9037363cf9d85..23fa799214720f1 100644 > --- a/mm/page_poison.c > +++ b/mm/page_poison.c > @@ -55,14 +55,15 @@ static void check_poison_mem(struct page *page, unsigned char *mem, size_t bytes > if (!start) > return; > > + if (!__ratelimit(&ratelimit)) > + return; > + > for (end = mem + bytes - 1; end > start; end--) { > if (*end != PAGE_POISON) > break; > } > > - if (!__ratelimit(&ratelimit)) > - return; > - else if (start == end && single_bit_flip(*start, PAGE_POISON)) > + if (start == end && single_bit_flip(*start, PAGE_POISON)) > pr_err("pagealloc: single bit error\n"); > else > pr_err("pagealloc: memory corruption\n"); This way, you will be ratelimiting on every function call, possibly skipping PAGE_POISON checks even if there was no prior corruption detected?
On 2024/8/21 19:28, David Hildenbrand wrote: > On 21.08.24 09:53, Zhen Lei wrote: >> When the debug information needs to be suppressed due to ratelimit, >> it is unnecessary to determine the end of the corrupted memory. >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> --- >> mm/page_poison.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/mm/page_poison.c b/mm/page_poison.c >> index 3e9037363cf9d85..23fa799214720f1 100644 >> --- a/mm/page_poison.c >> +++ b/mm/page_poison.c >> @@ -55,14 +55,15 @@ static void check_poison_mem(struct page *page, unsigned char *mem, size_t bytes >> if (!start) >> return; >> + if (!__ratelimit(&ratelimit)) >> + return; >> + >> for (end = mem + bytes - 1; end > start; end--) { >> if (*end != PAGE_POISON) >> break; >> } >> - if (!__ratelimit(&ratelimit)) >> - return; >> - else if (start == end && single_bit_flip(*start, PAGE_POISON)) >> + if (start == end && single_bit_flip(*start, PAGE_POISON)) >> pr_err("pagealloc: single bit error\n"); >> else >> pr_err("pagealloc: memory corruption\n"); > > This way, you will be ratelimiting on every function call, possibly skipping PAGE_POISON checks even if there was no prior corruption detected? > No, the previous memchr_inv() does the PAGE_POISON check, determine the start of the corrupted memory. !start means no corruption. start = memchr_inv(mem, PAGE_POISON, bytes); if (!start) return; + if (!__ratelimit(&ratelimit)) + return; for (end = mem + bytes - 1; end > start; end--) { if (*end != PAGE_POISON) break; }
diff --git a/mm/page_poison.c b/mm/page_poison.c index 3e9037363cf9d85..23fa799214720f1 100644 --- a/mm/page_poison.c +++ b/mm/page_poison.c @@ -55,14 +55,15 @@ static void check_poison_mem(struct page *page, unsigned char *mem, size_t bytes if (!start) return; + if (!__ratelimit(&ratelimit)) + return; + for (end = mem + bytes - 1; end > start; end--) { if (*end != PAGE_POISON) break; } - if (!__ratelimit(&ratelimit)) - return; - else if (start == end && single_bit_flip(*start, PAGE_POISON)) + if (start == end && single_bit_flip(*start, PAGE_POISON)) pr_err("pagealloc: single bit error\n"); else pr_err("pagealloc: memory corruption\n");
When the debug information needs to be suppressed due to ratelimit, it is unnecessary to determine the end of the corrupted memory. Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- mm/page_poison.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)