On Thu, May 03, 2012 at 04:35:06PM +0800, Gavin Shan wrote:
> >From: Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx>
> >Subject: [patch 1/2] mm: bootmem: fix checking the bitmap when finally
> > freeing bootmem
> >
> >When bootmem releases an unaligned chunk of memory at the beginning of
> >a node to the page allocator, it iterates from that unaligned PFN but
> >checks an aligned word of the page bitmap. The checked bits do not
> >correspond to the PFNs and, as a result, reserved pages can be freed.
> >
> >Properly shift the bitmap word so that the lowest bit corresponds to
> >the starting PFN before entering the freeing loop.
> >
>
> Thanks for changing it correctly, Johannes ;-)
>
> >Signed-off-by: Gavin Shan <shangw@xxxxxxxxxxxxxxxxxx>
> >Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> >---
> > mm/bootmem.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/mm/bootmem.c b/mm/bootmem.c
> >index 0131170..67872fc 100644
> >--- a/mm/bootmem.c
> >+++ b/mm/bootmem.c
> >@@ -203,6 +203,7 @@ static unsigned long __init free_all_bootmem_core(bootmem_data_t *bdata)
> > } else {
> > unsigned long off = 0;
> >
> >+ vec >>= start & (BITS_PER_LONG - 1);
> > while (vec && off < BITS_PER_LONG) {
>
> I think it can be changed to "while (vec) {" since it's duplicate
> check. "vec" has no chance to have more bits than BITS_PER_LONG here.
Yes, I think it should be removed too, but as a separate patch. It's
an unrelated cleanup, better to keep it out of the bugfix change.
> Others look good. Need I change it accordingly and send it out
> again?
It doesn't really matter who sends the patches, your original
authorship is preserved (see the From: in the patch header). If you
don't have any objections, I'll send both patches to Andrew later.
Here is 2/2, btw:
---
From: Johannes Weiner <hannes@xxxxxxxxxxx>
Subject: [patch 2/2] mm: bootmem: remove redundant offset check when finally
freeing bootmem
When bootmem releases an unaligned BITS_PER_LONG pages chunk of memory
to the page allocator, it checks the bitmap if there are still
unreserved pages in the chunk (set bits), but also if the offset in
the chunk indicates BITS_PER_LONG loop iterations already.
But since the consulted bitmap is only a one-word-excerpt of the full
per-node bitmap, there can not be more than BITS_PER_LONG bits set in
it. The additional offset check is unnecessary.
Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
---
mm/bootmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/bootmem.c b/mm/bootmem.c
index 67872fc..053ac3f 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -204,7 +204,7 @@ static unsigned long __init free_all_bootmem_core(bootmem_data_t *bdata)
unsigned long off = 0;
vec >>= start & (BITS_PER_LONG - 1);
- while (vec && off < BITS_PER_LONG) {
+ while (vec) {
if (vec & 1) {
page = pfn_to_page(start + off);
__free_pages_bootmem(page, 0);
--
1.7.10
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>