* [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check.
@ 2010-10-25 6:37 KAMEZAWA Hiroyuki
2010-10-25 7:49 ` Wu Fengguang
2010-10-25 17:03 ` Mel Gorman
0 siblings, 2 replies; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-10-25 6:37 UTC (permalink / raw)
To: linux-mm@kvack.org
Cc: akpm@linux-foundation.org, Michal Hocko, fengguang.wu, Mel Gorman
I wonder this should be for stable tree...but want to hear opinions before.
==
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
page_order() is called by memory hotplug's user interface to check
the section is removable or not. (is_mem_section_removable())
It calls page_order() withoug holding zone->lock.
So, even if the caller does
if (PageBuddy(page))
ret = page_order(page) ...
The caller may hit BUG_ON().
For fixing this, there are 2 choices.
1. add zone->lock.
2. remove BUG_ON().
is_mem_section_removable() is used for some "advice" and doesn't need
to be 100% accurate. This is_removable() can be called via user program..
We don't want to take this important lock for long by user's request.
So, this patch removes BUG_ON().
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
mm/internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: mmotm-1024/mm/internal.h
===================================================================
--- mmotm-1024.orig/mm/internal.h
+++ mmotm-1024/mm/internal.h
@@ -62,7 +62,7 @@ extern bool is_free_buddy_page(struct pa
*/
static inline unsigned long page_order(struct page *page)
{
- VM_BUG_ON(!PageBuddy(page));
+ /* PageBuddy() must be checked by the caller */
return page_private(page);
}
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check.
2010-10-25 6:37 [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check KAMEZAWA Hiroyuki
@ 2010-10-25 7:49 ` Wu Fengguang
2010-10-25 13:10 ` Michal Hocko
2010-10-25 17:03 ` Mel Gorman
1 sibling, 1 reply; 6+ messages in thread
From: Wu Fengguang @ 2010-10-25 7:49 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-mm@kvack.org, akpm@linux-foundation.org, Michal Hocko,
Mel Gorman
On Mon, Oct 25, 2010 at 02:37:26PM +0800, KAMEZAWA Hiroyuki wrote:
> I wonder this should be for stable tree...but want to hear opinions before.
> ==
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
> page_order() is called by memory hotplug's user interface to check
> the section is removable or not. (is_mem_section_removable())
>
> It calls page_order() withoug holding zone->lock.
> So, even if the caller does
>
> if (PageBuddy(page))
> ret = page_order(page) ...
> The caller may hit BUG_ON().
>
> For fixing this, there are 2 choices.
> 1. add zone->lock.
> 2. remove BUG_ON().
One more alternative might be to introduce a private
maybe_page_order() for is_mem_section_removable(). Not a big deal.
> is_mem_section_removable() is used for some "advice" and doesn't need
> to be 100% accurate. This is_removable() can be called via user program..
> We don't want to take this important lock for long by user's request.
> So, this patch removes BUG_ON().
Acked-by: Wu Fengguang <fengguang.wu@intel.com>
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check.
2010-10-25 7:49 ` Wu Fengguang
@ 2010-10-25 13:10 ` Michal Hocko
2010-10-25 13:16 ` Wu Fengguang
2010-10-25 21:24 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Michal Hocko @ 2010-10-25 13:10 UTC (permalink / raw)
To: Wu Fengguang
Cc: KAMEZAWA Hiroyuki, linux-mm@kvack.org, akpm@linux-foundation.org,
Mel Gorman
On Mon 25-10-10 15:49:33, Wu Fengguang wrote:
> On Mon, Oct 25, 2010 at 02:37:26PM +0800, KAMEZAWA Hiroyuki wrote:
> > I wonder this should be for stable tree...but want to hear opinions before.
> > ==
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >
> > page_order() is called by memory hotplug's user interface to check
> > the section is removable or not. (is_mem_section_removable())
> >
> > It calls page_order() withoug holding zone->lock.
> > So, even if the caller does
> >
> > if (PageBuddy(page))
> > ret = page_order(page) ...
> > The caller may hit BUG_ON().
> >
> > For fixing this, there are 2 choices.
> > 1. add zone->lock.
> > 2. remove BUG_ON().
>
> One more alternative might be to introduce a private
> maybe_page_order() for is_mem_section_removable(). Not a big deal.
I guess this is not necessary as all page_order callers check PageBuddy
anyway AFAICS.
>
> > is_mem_section_removable() is used for some "advice" and doesn't need
> > to be 100% accurate. This is_removable() can be called via user program..
> > We don't want to take this important lock for long by user's request.
> > So, this patch removes BUG_ON().
>
> Acked-by: Wu Fengguang <fengguang.wu@intel.com>
Yes, the change looks good.
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check.
2010-10-25 13:10 ` Michal Hocko
@ 2010-10-25 13:16 ` Wu Fengguang
2010-10-25 21:24 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2010-10-25 13:16 UTC (permalink / raw)
To: Michal Hocko
Cc: KAMEZAWA Hiroyuki, linux-mm@kvack.org, akpm@linux-foundation.org,
Mel Gorman
> I guess this is not necessary as all page_order callers check PageBuddy
> anyway AFAICS.
Sure -- otherwise the VM_BUG_ON() will already go bang :-)
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check.
2010-10-25 13:10 ` Michal Hocko
2010-10-25 13:16 ` Wu Fengguang
@ 2010-10-25 21:24 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2010-10-25 21:24 UTC (permalink / raw)
To: Michal Hocko
Cc: Wu Fengguang, KAMEZAWA Hiroyuki, linux-mm@kvack.org, Mel Gorman
On Mon, 25 Oct 2010 15:10:25 +0200
Michal Hocko <mhocko@suse.cz> wrote:
> On Mon 25-10-10 15:49:33, Wu Fengguang wrote:
> > On Mon, Oct 25, 2010 at 02:37:26PM +0800, KAMEZAWA Hiroyuki wrote:
> > > I wonder this should be for stable tree...but want to hear opinions before.
> > > ==
> > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > >
> > > page_order() is called by memory hotplug's user interface to check
> > > the section is removable or not. (is_mem_section_removable())
> > >
> > > It calls page_order() withoug holding zone->lock.
> > > So, even if the caller does
> > >
> > > if (PageBuddy(page))
> > > ret = page_order(page) ...
> > > The caller may hit BUG_ON().
> > >
> > > For fixing this, there are 2 choices.
> > > 1. add zone->lock.
> > > 2. remove BUG_ON().
> >
> > One more alternative might be to introduce a private
> > maybe_page_order() for is_mem_section_removable(). Not a big deal.
>
> I guess this is not necessary as all page_order callers check PageBuddy
> anyway AFAICS.
And hold zone->lock, one hopes.
> >
> > > is_mem_section_removable() is used for some "advice" and doesn't need
> > > to be 100% accurate. This is_removable() can be called via user program..
> > > We don't want to take this important lock for long by user's request.
> > > So, this patch removes BUG_ON().
> >
> > Acked-by: Wu Fengguang <fengguang.wu@intel.com>
>
> Yes, the change looks good.
It removes a valid assertion because one of the callers is doing
something exceptional. That exceptional caller should implement the
exceptional code, rather than weakening useful code for other callers!
But I'll grab the patch anyway, since BUG_ON in an inlined function is
a pretty bad idea from a bloat POV.
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check.
2010-10-25 6:37 [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check KAMEZAWA Hiroyuki
2010-10-25 7:49 ` Wu Fengguang
@ 2010-10-25 17:03 ` Mel Gorman
1 sibling, 0 replies; 6+ messages in thread
From: Mel Gorman @ 2010-10-25 17:03 UTC (permalink / raw)
To: KAMEZAWA Hiroyuki
Cc: linux-mm@kvack.org, akpm@linux-foundation.org, Michal Hocko,
fengguang.wu
On Mon, Oct 25, 2010 at 03:37:26PM +0900, KAMEZAWA Hiroyuki wrote:
> I wonder this should be for stable tree...but want to hear opinions before.
Because it's VM_BUG_ON instead of BUG_ON, I don't think it's something
we are likely to see triggered on stable kernels. It is worth
introducing a VM_WARN_ON do you think to catch really bad callers, ones
where it is not advisory?
Whether such a helper was introduced or not though, this does fix a real
problem so;
Acked-by: Mel Gorman <mel@csn.ul.ie>
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-25 21:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-25 6:37 [BUGFIX][PATCH] fix is_mem_section_removable() page_order BUG_ON check KAMEZAWA Hiroyuki
2010-10-25 7:49 ` Wu Fengguang
2010-10-25 13:10 ` Michal Hocko
2010-10-25 13:16 ` Wu Fengguang
2010-10-25 21:24 ` Andrew Morton
2010-10-25 17:03 ` Mel Gorman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).