linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Any reason to use put_page in slub.c?
@ 2012-07-27 12:19 Glauber Costa
  2012-07-27 15:55 ` Christoph Lameter
  0 siblings, 1 reply; 15+ messages in thread
From: Glauber Costa @ 2012-07-27 12:19 UTC (permalink / raw)
  To: linux-mm; +Cc: Pekka Enberg, David Rientjes, Christoph Lameter, Andrew Morton

Hi,

I've recently came across a bug in my kmemcg slab implementation, where memory
wasn't being unaccounted every time I expected it to be.  (bugs found by myself
are becoming a lot lot rarer, for the record)

I tracked it down to be due to the fact that we are now unaccounting at the
page allocator by calling __free_accounted_pages instead of normal
__free_pages.

However, higher order kmalloc allocations in the slub doesn't do that.  They
call put_page() instead, and I missed the conversion spot when converting
__free_pages() to __free_accounted_pages().

Now, although of course I can come up with put_accounted_page(), this is a bit
more awkward: first, it is in everybody's interest in keeping changes to the
page allocator to a minimum; also, put_page will not necessarily free the page,
so the semantics can get a bit complicated.

Since we are not doing any kind of page sharing with those pages in the slub -
and are already doing compound checks ourselves, I was wondering why couldn't
we just use __free_pages() instead. I see no reason not to. Replacing it with
__free_page() seems to work - my patched kernel is up and running, and doing
fine.

But I am still wondering if there is anything I am overlooking.

Do you guys think the following patch is safe?

---
 mm/slub.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index a136a75..a8fffeb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3399,7 +3399,7 @@ void kfree(const void *x)
 	if (unlikely(!PageSlab(page))) {
 		BUG_ON(!PageCompound(page));
 		kmemleak_free(x);
-		put_page(page);
+		__free_pages(page);
 		return;
 	}
 	slab_free(page->slab, page, object, _RET_IP_);
-- 
1.7.10.4

--
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] 15+ messages in thread

end of thread, other threads:[~2012-08-02  8:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-27 12:19 Any reason to use put_page in slub.c? Glauber Costa
2012-07-27 15:55 ` Christoph Lameter
2012-07-30  7:53   ` Glauber Costa
2012-07-30 19:23     ` Christoph Lameter
2012-07-31  8:25       ` Glauber Costa
2012-07-31 14:09         ` Christoph Lameter
2012-07-31 14:09           ` Glauber Costa
2012-07-31 14:17             ` Christoph Lameter
2012-07-31 14:18               ` Glauber Costa
2012-07-31 14:31                 ` Christoph Lameter
2012-07-31 14:52                   ` James Bottomley
2012-08-01 12:42                     ` Glauber Costa
2012-08-01 18:10                       ` Christoph Lameter
2012-08-02  7:55                         ` Glauber Costa
2012-08-02  8:07                         ` James Bottomley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox