[PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Ezequiel Garcia]

This field was being used to store size allocation so it could be
retrieved by ksize(). However, it is a bad practice to not mark a page
as a slab page and then use fields for special purposes.
There is no need to store the allocated size and
ksize() can simply return PAGE_SIZE << compound_order(page).

Cc: Pekka Enberg <penberg@kernel.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Glauber Costa <glommer@parallels.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 mm/slob.c |   23 ++++++++++-------------
 1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/mm/slob.c b/mm/slob.c
index 686e98b..987da93 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -28,9 +28,8 @@
  * from kmalloc are prepended with a 4-byte header with the kmalloc size.
  * If kmalloc is asked for objects of PAGE_SIZE or larger, it calls
  * alloc_pages() directly, allocating compound pages so the page order
- * does not have to be separately tracked, and also stores the exact
- * allocation size in page->private so that it can be used to accurately
- * provide ksize(). These objects are detected in kfree() because slob_page()
+ * does not have to be separately tracked.
+ * These objects are detected in kfree() because PageSlab()
  * is false for them.
  *
  * SLAB is emulated on top of SLOB by simply calling constructors and
@@ -450,7 +449,6 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 				   size, size + align, gfp, node);
 	} else {
 		unsigned int order = get_order(size);
-		struct page *page;
 
 		if (likely(order))
 			gfp |= __GFP_COMP;
@@ -458,9 +456,6 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 		if (!ret)
 			return NULL;
 
-		page = virt_to_page(ret);
-		page->private = size;
-
 		trace_kmalloc_node(_RET_IP_, ret,
 				   size, PAGE_SIZE << order, gfp, node);
 	}
@@ -494,18 +489,20 @@ EXPORT_SYMBOL(kfree);
 size_t ksize(const void *block)
 {
 	struct page *sp;
+	int align;
+	unsigned int *m;
 
 	BUG_ON(!block);
 	if (unlikely(block == ZERO_SIZE_PTR))
 		return 0;
 
 	sp = virt_to_page(block);
-	if (PageSlab(sp)) {
-		int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
-		unsigned int *m = (unsigned int *)(block - align);
-		return SLOB_UNITS(*m) * SLOB_UNIT;
-	} else
-		return sp->private;
+	if (unlikely(!PageSlab(sp)))
+		return PAGE_SIZE << compound_order(sp);
+
+	align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
+	m = (unsigned int *)(block - align);
+	return SLOB_UNITS(*m) * SLOB_UNIT;
 }
 EXPORT_SYMBOL(ksize);
 
-- 
1.7.8.6

--
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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Christoph Lameter]

On Tue, 14 Aug 2012, Ezequiel Garcia wrote:

> This field was being used to store size allocation so it could be
> retrieved by ksize(). However, it is a bad practice to not mark a page
> as a slab page and then use fields for special purposes.
> There is no need to store the allocated size and
> ksize() can simply return PAGE_SIZE << compound_order(page).

Acked-by: Christoph Lameter <cl@linux.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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Ezequiel Garcia]

Hi Christoph,

On Tue, Aug 14, 2012 at 5:53 PM, Christoph Lameter <cl@linux.com> wrote:
> On Tue, 14 Aug 2012, Ezequiel Garcia wrote:
>
>> This field was being used to store size allocation so it could be
>> retrieved by ksize(). However, it is a bad practice to not mark a page
>> as a slab page and then use fields for special purposes.
>> There is no need to store the allocated size and
>> ksize() can simply return PAGE_SIZE << compound_order(page).
>
> Acked-by: Christoph Lameter <cl@linux.com>
>

Who's the slob maintainer? Currently MAINTAINERS file
mentions slob's author Matt Mackal, but I didn't notice his presence
in this ML.

Thanks,
Ezequiel.

--
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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Pekka Enberg]

On Wed, Aug 15, 2012 at 2:38 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> Who's the slob maintainer? Currently MAINTAINERS file
> mentions slob's author Matt Mackal, but I didn't notice his presence
> in this ML.

I'm handling patches for all slab allocators.

--
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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Ezequiel Garcia]

Hi Pekka,

On Wed, Aug 15, 2012 at 8:42 AM, Pekka Enberg <penberg@kernel.org> wrote:
> On Wed, Aug 15, 2012 at 2:38 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>> Who's the slob maintainer? Currently MAINTAINERS file
>> mentions slob's author Matt Mackal, but I didn't notice his presence
>> in this ML.
>
> I'm handling patches for all slab allocators.

Ah, great. I sent these three based on your branch:

git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux.git slab/next

I hope this is ok.

Regards and thank you,
Ezequiel.

--
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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Christoph Lameter]

On Wed, 15 Aug 2012, Ezequiel Garcia wrote:

> Hi Christoph,
>
> On Tue, Aug 14, 2012 at 5:53 PM, Christoph Lameter <cl@linux.com> wrote:
> > On Tue, 14 Aug 2012, Ezequiel Garcia wrote:
> >
> >> This field was being used to store size allocation so it could be
> >> retrieved by ksize(). However, it is a bad practice to not mark a page
> >> as a slab page and then use fields for special purposes.
> >> There is no need to store the allocated size and
> >> ksize() can simply return PAGE_SIZE << compound_order(page).
> >
> > Acked-by: Christoph Lameter <cl@linux.com>
> >
>
> Who's the slob maintainer? Currently MAINTAINERS file
> mentions slob's author Matt Mackal, but I didn't notice his presence
> in this ML.

Well I have not heard from him recently. Matt, Pekka and I are the
"official" (whatever that means...) maintainers of the slab allocators
which includes slob. See the MAINTAINERS file.



--
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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Pekka Enberg]

On Tue, Aug 14, 2012 at 11:03 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> This field was being used to store size allocation so it could be
> retrieved by ksize(). However, it is a bad practice to not mark a page
> as a slab page and then use fields for special purposes.
> There is no need to store the allocated size and
> ksize() can simply return PAGE_SIZE << compound_order(page).
>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Glauber Costa <glommer@parallels.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>

Looks good to me. Matt?

> ---
>  mm/slob.c |   23 ++++++++++-------------
>  1 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/mm/slob.c b/mm/slob.c
> index 686e98b..987da93 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -28,9 +28,8 @@
>   * from kmalloc are prepended with a 4-byte header with the kmalloc size.
>   * If kmalloc is asked for objects of PAGE_SIZE or larger, it calls
>   * alloc_pages() directly, allocating compound pages so the page order
> - * does not have to be separately tracked, and also stores the exact
> - * allocation size in page->private so that it can be used to accurately
> - * provide ksize(). These objects are detected in kfree() because slob_page()
> + * does not have to be separately tracked.
> + * These objects are detected in kfree() because PageSlab()
>   * is false for them.
>   *
>   * SLAB is emulated on top of SLOB by simply calling constructors and
> @@ -450,7 +449,6 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
>                                    size, size + align, gfp, node);
>         } else {
>                 unsigned int order = get_order(size);
> -               struct page *page;
>
>                 if (likely(order))
>                         gfp |= __GFP_COMP;
> @@ -458,9 +456,6 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
>                 if (!ret)
>                         return NULL;
>
> -               page = virt_to_page(ret);
> -               page->private = size;
> -
>                 trace_kmalloc_node(_RET_IP_, ret,
>                                    size, PAGE_SIZE << order, gfp, node);
>         }
> @@ -494,18 +489,20 @@ EXPORT_SYMBOL(kfree);
>  size_t ksize(const void *block)
>  {
>         struct page *sp;
> +       int align;
> +       unsigned int *m;
>
>         BUG_ON(!block);
>         if (unlikely(block == ZERO_SIZE_PTR))
>                 return 0;
>
>         sp = virt_to_page(block);
> -       if (PageSlab(sp)) {
> -               int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> -               unsigned int *m = (unsigned int *)(block - align);
> -               return SLOB_UNITS(*m) * SLOB_UNIT;
> -       } else
> -               return sp->private;
> +       if (unlikely(!PageSlab(sp)))
> +               return PAGE_SIZE << compound_order(sp);
> +
> +       align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> +       m = (unsigned int *)(block - align);
> +       return SLOB_UNITS(*m) * SLOB_UNIT;
>  }
>  EXPORT_SYMBOL(ksize);
>
> --
> 1.7.8.6
>
> --
> 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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Ezequiel Garcia]

Hi Pekka,

On Wed, Aug 15, 2012 at 8:43 AM, Pekka Enberg <penberg@kernel.org> wrote:
> On Tue, Aug 14, 2012 at 11:03 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
>> This field was being used to store size allocation so it could be
>> retrieved by ksize(). However, it is a bad practice to not mark a page
>> as a slab page and then use fields for special purposes.
>> There is no need to store the allocated size and
>> ksize() can simply return PAGE_SIZE << compound_order(page).
>>
>> Cc: Pekka Enberg <penberg@kernel.org>
>> Cc: Christoph Lameter <cl@linux.com>
>> Cc: Glauber Costa <glommer@parallels.com>
>> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
>
> Looks good to me. Matt?
>

Will you carry this (and the other 3 patches I sent for mm/) on your tree?
Or do I need to send them to someone else?

Thanks,
Ezequiel.

--
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>

Re: [PATCH] mm, slob: Drop usage of page->private for storing page-sized allocations

[Pekka Enberg]

Hi Ezequiel,

On Tue, Aug 14, 2012 at 11:03 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> This field was being used to store size allocation so it could be
> retrieved by ksize(). However, it is a bad practice to not mark a page
> as a slab page and then use fields for special purposes.
> There is no need to store the allocated size and
> ksize() can simply return PAGE_SIZE << compound_order(page).
>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Glauber Costa <glommer@parallels.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>

I'm getting rejects for this. Care to resend against latest slab/next branch?

--
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>