Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
Hugh Dickins wrote: > On Thu, 20 Dec 2007, Andrew Morton wrote: >> On Thu, 20 Dec 2007 21:54:15 +0530 Balbir Singh <[EMAIL PROTECTED]> wrote: >>> I was going to say the same thing, page_get_page_cgroup() does not hold >>> any references. May be _get_ in the name is confusing. >> It is a bit unconventional. page_cgroup()? > > Seems ideal to me > (though there is some argument for page_page_cgroup(), weird as it is). > > Hugh Sounds good to me as well, except that it sounds like a constructor :-) I'll try and make the changes and submit a patch. I am in the middle of losing control_type now (based on Hugh's comments) -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 20 Dec 2007, Andrew Morton wrote: > On Thu, 20 Dec 2007 21:54:15 +0530 Balbir Singh <[EMAIL PROTECTED]> wrote: > > I was going to say the same thing, page_get_page_cgroup() does not hold > > any references. May be _get_ in the name is confusing. > > It is a bit unconventional. page_cgroup()? Seems ideal to me (though there is some argument for page_page_cgroup(), weird as it is). Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 20 Dec 2007 21:54:15 +0530 Balbir Singh <[EMAIL PROTECTED]> wrote: > >> struct page_cgroup *page_get_page_cgroup(struct page *page) > >> { > >>return (struct page_cgroup *) > >>(page->page_cgroup & ~PAGE_CGROUP_LOCK); > >> } > >> > >> I guess the issue is that often a "get" function has a complementary > >> "put" function, but this isn't one of them. Would page_page_cgroup > >> be a better name, perhaps? I don't know. > > > > Ah, yes, I mistakenly assumed it was a reference get. In that case I > > stand corrected and do not have any objections. > > > > I was going to say the same thing, page_get_page_cgroup() does not hold > any references. May be _get_ in the name is confusing. It is a bit unconventional. page_cgroup()? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 2007-12-20 at 21:54 +0530, Balbir Singh wrote: > > I was going to say the same thing, page_get_page_cgroup() does not hold > any references. May be _get_ in the name is confusing. OK, you three had the entire conversation outing me before I even fot to respond! :) Yeah, I thought it was a refcount acquisition. Balbir, it's fine the way it stands (except for the slightly confusing name). -- Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
Peter Zijlstra wrote: > On Thu, 2007-12-20 at 14:16 +, Hugh Dickins wrote: >> On Thu, 20 Dec 2007, Peter Zijlstra wrote: >>> On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: On Wed, 19 Dec 2007, Dave Hansen wrote: >> - page_assign_page_cgroup(page, NULL); >> + VM_BUG_ON(page_get_page_cgroup(page)); > Hi Balbir, > > You generally want to do these like: > > foo = page_assign_page_cgroup(page, NULL); > VM_BUG_ON(foo); > > Some embedded people have been known to optimize kernel size like this: > > #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. >>> There was a lengthy discussion here: >>> http://lkml.org/lkml/2007/12/14/131 >>> >>> on the merit of debug statements with side effects. >> Of course, but what's the relevance? >> >>> But looking at our definition: >>> >>> #ifdef CONFIG_DEBUG_VM >>> #define VM_BUG_ON(cond) BUG_ON(cond) >>> #else >>> #define VM_BUG_ON(condition) do { } while(0) >>> #endif >>> >>> disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that >>> it will no longer acquire the reference. >> But what reference? >> >> struct page_cgroup *page_get_page_cgroup(struct page *page) >> { >> return (struct page_cgroup *) >> (page->page_cgroup & ~PAGE_CGROUP_LOCK); >> } >> >> I guess the issue is that often a "get" function has a complementary >> "put" function, but this isn't one of them. Would page_page_cgroup >> be a better name, perhaps? I don't know. > > Ah, yes, I mistakenly assumed it was a reference get. In that case I > stand corrected and do not have any objections. > I was going to say the same thing, page_get_page_cgroup() does not hold any references. May be _get_ in the name is confusing. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 2007-12-20 at 14:16 +, Hugh Dickins wrote: > On Thu, 20 Dec 2007, Peter Zijlstra wrote: > > On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: > > > On Wed, 19 Dec 2007, Dave Hansen wrote: > > > > > - page_assign_page_cgroup(page, NULL); > > > > > + VM_BUG_ON(page_get_page_cgroup(page)); > > > > > > > > Hi Balbir, > > > > > > > > You generally want to do these like: > > > > > > > > foo = page_assign_page_cgroup(page, NULL); > > > > VM_BUG_ON(foo); > > > > > > > > Some embedded people have been known to optimize kernel size like this: > > > > > > > > #define VM_BUG_ON(x) do{}while(0) > > > > > > Balbir's patch looks fine to me: I don't get your point there, Dave. > > > > There was a lengthy discussion here: > > http://lkml.org/lkml/2007/12/14/131 > > > > on the merit of debug statements with side effects. > > Of course, but what's the relevance? > > > But looking at our definition: > > > > #ifdef CONFIG_DEBUG_VM > > #define VM_BUG_ON(cond) BUG_ON(cond) > > #else > > #define VM_BUG_ON(condition) do { } while(0) > > #endif > > > > disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that > > it will no longer acquire the reference. > > But what reference? > > struct page_cgroup *page_get_page_cgroup(struct page *page) > { > return (struct page_cgroup *) > (page->page_cgroup & ~PAGE_CGROUP_LOCK); > } > > I guess the issue is that often a "get" function has a complementary > "put" function, but this isn't one of them. Would page_page_cgroup > be a better name, perhaps? I don't know. Ah, yes, I mistakenly assumed it was a reference get. In that case I stand corrected and do not have any objections. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 20 Dec 2007, Peter Zijlstra wrote: > On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: > > On Wed, 19 Dec 2007, Dave Hansen wrote: > > > > - page_assign_page_cgroup(page, NULL); > > > > + VM_BUG_ON(page_get_page_cgroup(page)); > > > > > > Hi Balbir, > > > > > > You generally want to do these like: > > > > > > foo = page_assign_page_cgroup(page, NULL); > > > VM_BUG_ON(foo); > > > > > > Some embedded people have been known to optimize kernel size like this: > > > > > > #define VM_BUG_ON(x) do{}while(0) > > > > Balbir's patch looks fine to me: I don't get your point there, Dave. > > There was a lengthy discussion here: > http://lkml.org/lkml/2007/12/14/131 > > on the merit of debug statements with side effects. Of course, but what's the relevance? > But looking at our definition: > > #ifdef CONFIG_DEBUG_VM > #define VM_BUG_ON(cond) BUG_ON(cond) > #else > #define VM_BUG_ON(condition) do { } while(0) > #endif > > disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that > it will no longer acquire the reference. But what reference? struct page_cgroup *page_get_page_cgroup(struct page *page) { return (struct page_cgroup *) (page->page_cgroup & ~PAGE_CGROUP_LOCK); } I guess the issue is that often a "get" function has a complementary "put" function, but this isn't one of them. Would page_page_cgroup be a better name, perhaps? I don't know. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: > On Wed, 19 Dec 2007, Dave Hansen wrote: > > > --- > > > linux-2.6.24-rc5/mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page > > > 2007-12-19 11:31:46.0 +0530 > > > +++ linux-2.6.24-rc5-balbir/mm/page_alloc.c 2007-12-19 > > > 11:33:45.0 +0530 > > > @@ -995,7 +995,7 @@ static void fastcall free_hot_cold_page( > > > > > > if (!PageHighMem(page)) > > > debug_check_no_locks_freed(page_address(page), PAGE_SIZE); > > > - page_assign_page_cgroup(page, NULL); > > > + VM_BUG_ON(page_get_page_cgroup(page)); > > > arch_free_page(page, 0); > > > kernel_map_pages(page, 1, 0); > > > > Hi Balbir, > > > > You generally want to do these like: > > > > foo = page_assign_page_cgroup(page, NULL); > > VM_BUG_ON(foo); > > > > Some embedded people have been known to optimize kernel size like this: > > > > #define VM_BUG_ON(x) do{}while(0) > > Balbir's patch looks fine to me: I don't get your point there, Dave. There was a lengthy discussion here: http://lkml.org/lkml/2007/12/14/131 on the merit of debug statements with side effects. But looking at our definition: #ifdef CONFIG_DEBUG_VM #define VM_BUG_ON(cond) BUG_ON(cond) #else #define VM_BUG_ON(condition) do { } while(0) #endif disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that it will no longer acquire the reference. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Wed, 19 Dec 2007, Dave Hansen wrote: > > --- > > linux-2.6.24-rc5/mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page > > 2007-12-19 11:31:46.0 +0530 > > +++ linux-2.6.24-rc5-balbir/mm/page_alloc.c 2007-12-19 > > 11:33:45.0 +0530 > > @@ -995,7 +995,7 @@ static void fastcall free_hot_cold_page( > > > > if (!PageHighMem(page)) > > debug_check_no_locks_freed(page_address(page), PAGE_SIZE); > > - page_assign_page_cgroup(page, NULL); > > + VM_BUG_ON(page_get_page_cgroup(page)); > > arch_free_page(page, 0); > > kernel_map_pages(page, 1, 0); > > Hi Balbir, > > You generally want to do these like: > > foo = page_assign_page_cgroup(page, NULL); > VM_BUG_ON(foo); > > Some embedded people have been known to optimize kernel size like this: > > #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Wed, 19 Dec 2007, Dave Hansen wrote: --- linux-2.6.24-rc5/mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page 2007-12-19 11:31:46.0 +0530 +++ linux-2.6.24-rc5-balbir/mm/page_alloc.c 2007-12-19 11:33:45.0 +0530 @@ -995,7 +995,7 @@ static void fastcall free_hot_cold_page( if (!PageHighMem(page)) debug_check_no_locks_freed(page_address(page), PAGE_SIZE); - page_assign_page_cgroup(page, NULL); + VM_BUG_ON(page_get_page_cgroup(page)); arch_free_page(page, 0); kernel_map_pages(page, 1, 0); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: On Wed, 19 Dec 2007, Dave Hansen wrote: --- linux-2.6.24-rc5/mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page 2007-12-19 11:31:46.0 +0530 +++ linux-2.6.24-rc5-balbir/mm/page_alloc.c 2007-12-19 11:33:45.0 +0530 @@ -995,7 +995,7 @@ static void fastcall free_hot_cold_page( if (!PageHighMem(page)) debug_check_no_locks_freed(page_address(page), PAGE_SIZE); - page_assign_page_cgroup(page, NULL); + VM_BUG_ON(page_get_page_cgroup(page)); arch_free_page(page, 0); kernel_map_pages(page, 1, 0); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. There was a lengthy discussion here: http://lkml.org/lkml/2007/12/14/131 on the merit of debug statements with side effects. But looking at our definition: #ifdef CONFIG_DEBUG_VM #define VM_BUG_ON(cond) BUG_ON(cond) #else #define VM_BUG_ON(condition) do { } while(0) #endif disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that it will no longer acquire the reference. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 20 Dec 2007, Peter Zijlstra wrote: On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: On Wed, 19 Dec 2007, Dave Hansen wrote: - page_assign_page_cgroup(page, NULL); + VM_BUG_ON(page_get_page_cgroup(page)); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. There was a lengthy discussion here: http://lkml.org/lkml/2007/12/14/131 on the merit of debug statements with side effects. Of course, but what's the relevance? But looking at our definition: #ifdef CONFIG_DEBUG_VM #define VM_BUG_ON(cond) BUG_ON(cond) #else #define VM_BUG_ON(condition) do { } while(0) #endif disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that it will no longer acquire the reference. But what reference? struct page_cgroup *page_get_page_cgroup(struct page *page) { return (struct page_cgroup *) (page-page_cgroup ~PAGE_CGROUP_LOCK); } I guess the issue is that often a get function has a complementary put function, but this isn't one of them. Would page_page_cgroup be a better name, perhaps? I don't know. Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 2007-12-20 at 14:16 +, Hugh Dickins wrote: On Thu, 20 Dec 2007, Peter Zijlstra wrote: On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: On Wed, 19 Dec 2007, Dave Hansen wrote: - page_assign_page_cgroup(page, NULL); + VM_BUG_ON(page_get_page_cgroup(page)); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. There was a lengthy discussion here: http://lkml.org/lkml/2007/12/14/131 on the merit of debug statements with side effects. Of course, but what's the relevance? But looking at our definition: #ifdef CONFIG_DEBUG_VM #define VM_BUG_ON(cond) BUG_ON(cond) #else #define VM_BUG_ON(condition) do { } while(0) #endif disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that it will no longer acquire the reference. But what reference? struct page_cgroup *page_get_page_cgroup(struct page *page) { return (struct page_cgroup *) (page-page_cgroup ~PAGE_CGROUP_LOCK); } I guess the issue is that often a get function has a complementary put function, but this isn't one of them. Would page_page_cgroup be a better name, perhaps? I don't know. Ah, yes, I mistakenly assumed it was a reference get. In that case I stand corrected and do not have any objections. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
Peter Zijlstra wrote: On Thu, 2007-12-20 at 14:16 +, Hugh Dickins wrote: On Thu, 20 Dec 2007, Peter Zijlstra wrote: On Thu, 2007-12-20 at 13:14 +, Hugh Dickins wrote: On Wed, 19 Dec 2007, Dave Hansen wrote: - page_assign_page_cgroup(page, NULL); + VM_BUG_ON(page_get_page_cgroup(page)); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) Balbir's patch looks fine to me: I don't get your point there, Dave. There was a lengthy discussion here: http://lkml.org/lkml/2007/12/14/131 on the merit of debug statements with side effects. Of course, but what's the relevance? But looking at our definition: #ifdef CONFIG_DEBUG_VM #define VM_BUG_ON(cond) BUG_ON(cond) #else #define VM_BUG_ON(condition) do { } while(0) #endif disabling CONFIG_DEBUG_VM breaks the code as proposed by Balbir in that it will no longer acquire the reference. But what reference? struct page_cgroup *page_get_page_cgroup(struct page *page) { return (struct page_cgroup *) (page-page_cgroup ~PAGE_CGROUP_LOCK); } I guess the issue is that often a get function has a complementary put function, but this isn't one of them. Would page_page_cgroup be a better name, perhaps? I don't know. Ah, yes, I mistakenly assumed it was a reference get. In that case I stand corrected and do not have any objections. I was going to say the same thing, page_get_page_cgroup() does not hold any references. May be _get_ in the name is confusing. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 2007-12-20 at 21:54 +0530, Balbir Singh wrote: I was going to say the same thing, page_get_page_cgroup() does not hold any references. May be _get_ in the name is confusing. OK, you three had the entire conversation outing me before I even fot to respond! :) Yeah, I thought it was a refcount acquisition. Balbir, it's fine the way it stands (except for the slightly confusing name). -- Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 20 Dec 2007 21:54:15 +0530 Balbir Singh [EMAIL PROTECTED] wrote: struct page_cgroup *page_get_page_cgroup(struct page *page) { return (struct page_cgroup *) (page-page_cgroup ~PAGE_CGROUP_LOCK); } I guess the issue is that often a get function has a complementary put function, but this isn't one of them. Would page_page_cgroup be a better name, perhaps? I don't know. Ah, yes, I mistakenly assumed it was a reference get. In that case I stand corrected and do not have any objections. I was going to say the same thing, page_get_page_cgroup() does not hold any references. May be _get_ in the name is confusing. It is a bit unconventional. page_cgroup()? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Thu, 20 Dec 2007, Andrew Morton wrote: On Thu, 20 Dec 2007 21:54:15 +0530 Balbir Singh [EMAIL PROTECTED] wrote: I was going to say the same thing, page_get_page_cgroup() does not hold any references. May be _get_ in the name is confusing. It is a bit unconventional. page_cgroup()? Seems ideal to me (though there is some argument for page_page_cgroup(), weird as it is). Hugh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
Hugh Dickins wrote: On Thu, 20 Dec 2007, Andrew Morton wrote: On Thu, 20 Dec 2007 21:54:15 +0530 Balbir Singh [EMAIL PROTECTED] wrote: I was going to say the same thing, page_get_page_cgroup() does not hold any references. May be _get_ in the name is confusing. It is a bit unconventional. page_cgroup()? Seems ideal to me (though there is some argument for page_page_cgroup(), weird as it is). Hugh Sounds good to me as well, except that it sounds like a constructor :-) I'll try and make the changes and submit a patch. I am in the middle of losing control_type now (based on Hugh's comments) -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Wed, 2007-12-19 at 11:48 +0530, Balbir Singh wrote: > diff -puN > mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page > mm/page_alloc.c > --- > linux-2.6.24-rc5/mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page > 2007-12-19 11:31:46.0 +0530 > +++ linux-2.6.24-rc5-balbir/mm/page_alloc.c 2007-12-19 11:33:45.0 > +0530 > @@ -995,7 +995,7 @@ static void fastcall free_hot_cold_page( > > if (!PageHighMem(page)) > debug_check_no_locks_freed(page_address(page), PAGE_SIZE); > - page_assign_page_cgroup(page, NULL); > + VM_BUG_ON(page_get_page_cgroup(page)); > arch_free_page(page, 0); > kernel_map_pages(page, 1, 0); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) -- Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Move page_assign_page_cgroup to VM_BUG_ON in free_hot_cold_page
On Wed, 2007-12-19 at 11:48 +0530, Balbir Singh wrote: diff -puN mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page mm/page_alloc.c --- linux-2.6.24-rc5/mm/page_alloc.c~memory-controller-move-to-bug-on-in-free_hot_cold_page 2007-12-19 11:31:46.0 +0530 +++ linux-2.6.24-rc5-balbir/mm/page_alloc.c 2007-12-19 11:33:45.0 +0530 @@ -995,7 +995,7 @@ static void fastcall free_hot_cold_page( if (!PageHighMem(page)) debug_check_no_locks_freed(page_address(page), PAGE_SIZE); - page_assign_page_cgroup(page, NULL); + VM_BUG_ON(page_get_page_cgroup(page)); arch_free_page(page, 0); kernel_map_pages(page, 1, 0); Hi Balbir, You generally want to do these like: foo = page_assign_page_cgroup(page, NULL); VM_BUG_ON(foo); Some embedded people have been known to optimize kernel size like this: #define VM_BUG_ON(x) do{}while(0) -- Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/