Thread (10 messages) 10 messages, 3 authors, 2012-08-24

Re: Fixup the page of buddy_higher address's calculation

From: Gavin Shan <hidden>
Date: 2012-08-23 12:31:14

On Thu, Aug 23, 2012 at 06:21:06PM +0800, Li Haifeng wrote:
I am sorry for my mistake.

higher_buddy is corresponding with buddy_index, and higher page is
corresponding with combined_idx. That is right.

But, How we get the page address from index offset? The key answer is
what is the base value.
So calculating the address based page should be (page + (buddy_idx - page_idx)).

Maybe, a diagram is easier to understand.

|-------------------------|-------------|
page               combined   buddy

buddy's page address= pagea??s page address + (buddy - page)*sizeof(struct page)

Clear?
It sounds reasonable.
2012/8/23 Michal Hocko [off-list ref]:
quoted
On Thu 23-08-12 16:40:13, Li Haifeng wrote:
quoted
From d7cd78f9d71a5c9ddeed02724558096f0bb4508a Mon Sep 17 00:00:00 2001
From: Haifeng Li <redacted>
Date: Thu, 23 Aug 2012 16:27:19 +0800
Subject: [PATCH] Fixup the page of buddy_higher address's calculation
Some general questions:
Any word about the change? Is it really that obvious? Why do you think the
current state is incorrect? How did you find out?

And more specific below:
quoted
Signed-off-by: Haifeng Li <redacted>
---
 mm/page_alloc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ddbc17d..5588f68 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -579,7 +579,7 @@ static inline void __free_one_page(struct page *page,
                combined_idx = buddy_idx & page_idx;
                higher_page = page + (combined_idx - page_idx);
                buddy_idx = __find_buddy_index(combined_idx, order + 1);
-               higher_buddy = page + (buddy_idx - combined_idx);
+               higher_buddy = page + (buddy_idx - page_idx);
Haifeng, Not sure it would be better? At least, the expression
would be more explicitly meaningful than yours.

		    higher_buddy = higher_page + (buddy_idx - combined_idx);

Thanks,
Gavin
quoted
We are finding buddy index for combined_idx so why should we use
page_idx here?
quoted
                if (page_is_buddy(higher_page, higher_buddy, order + 1)) {
                        list_add_tail(&page->lru,
                                &zone->free_area[order].free_list[migratetype]);
--
1.7.5.4
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help