2011-02-18 01:46:43

by Namhyung Kim

Subject: [PATCH] mm: fix dubious code in __count_immobile_pages()

When pfn_valid_within() failed 'iter' was incremented twice.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
---
mm/page_alloc.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e8b02771ccea..bf83d1c1d648 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5380,10 +5380,9 @@ __count_immobile_pages(struct zone *zone, struct page *page, int count)
for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) {
unsigned long check = pfn + iter;

- if (!pfn_valid_within(check)) {
- iter++;
+ if (!pfn_valid_within(check))
continue;
- }
+
page = pfn_to_page(check);
if (!page_count(page)) {
if (PageBuddy(page))
--
1.7.4

2011-02-18 02:26:28

by Minchan Kim

Subject: Re: [PATCH] mm: fix dubious code in __count_immobile_pages()

On Fri, Feb 18, 2011 at 10:46 AM, Namhyung Kim <[email protected]> wrote:
> When pfn_valid_within() failed 'iter' was incremented twice.
>
> Signed-off-by: Namhyung Kim <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
Reviewed-by: Minchan Kim <[email protected]>

Nitpick.

I am not sure it's best. I don't like below trick.
iter += (1 << page_order(page)) - 1;

So we can change for loop with while as removing -1 trick of PageBuddy.
But if you don't like it, I don't mind it. :)

Thanks!

--
Kind regards,
Minchan Kim

2011-02-18 03:00:37

by Namhyung Kim

Subject: Re: [PATCH] mm: fix dubious code in __count_immobile_pages()

2011-02-18 (금), 11:26 +0900, Minchan Kim:
> On Fri, Feb 18, 2011 at 10:46 AM, Namhyung Kim <[email protected]> wrote:
> > When pfn_valid_within() failed 'iter' was incremented twice.
> >
> > Signed-off-by: Namhyung Kim <[email protected]>
> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> Reviewed-by: Minchan Kim <[email protected]>
>
> Nitpick.
>
> I am not sure it's best. I don't like below trick.
> iter += (1 << page_order(page)) - 1;
>
> So we can change for loop with while as removing -1 trick of PageBuddy.
> But if you don't like it, I don't mind it. :)
>
> Thanks!
>

Hi Minchan,

Either is fine to me. But I think current code would be shorter.


--
Regards,
Namhyung Kim

2011-02-21 10:04:16

by Kamezawa Hiroyuki

Subject: Re: [PATCH] mm: fix dubious code in __count_immobile_pages()

On Fri, 18 Feb 2011 10:46:26 +0900
Namhyung Kim <[email protected]> wrote:

> When pfn_valid_within() failed 'iter' was incremented twice.
>
> Signed-off-by: Namhyung Kim <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>

Thank you.
Reviewed-by: KAMEZAWA Hiroyuki <[email protected]>


> ---
> mm/page_alloc.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e8b02771ccea..bf83d1c1d648 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5380,10 +5380,9 @@ __count_immobile_pages(struct zone *zone, struct page *page, int count)
> for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) {
> unsigned long check = pfn + iter;
>
> - if (!pfn_valid_within(check)) {
> - iter++;
> + if (!pfn_valid_within(check))
> continue;
> - }
> +
> page = pfn_to_page(check);
> if (!page_count(page)) {
> if (PageBuddy(page))
> --
> 1.7.4
>
>