Re: [PATCH 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: > Well, as a reference, the user-space equivalent is defined in SUSv3 as: > > "The realloc() function shall change the size of the memory object > pointed to by ptr to the size specified by size." The realloc functions intent is to leave the object in place if possible. Otherwise one can simply alloc a new object and free the old one. - 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 1/3] slab: introduce krealloc
On 2/21/07, Christoph Lameter <[EMAIL PROTECTED]> wrote: Why not? Its a realloc call and these are the classic semantics of realloc. Otherwise realloc will always move the memory. Well, as a reference, the user-space equivalent is defined in SUSv3 as: "The realloc() function shall change the size of the memory object pointed to by ptr to the size specified by size." I think it is reasonable to expect krealloc() to not waste too much space if I, say, reallocate a 128 byte buffer to 32 bytes. On 2/21/07, Christoph Lameter <[EMAIL PROTECTED]> wrote: Check that both sizes fall into the same general cache. Do the following at the beginning of the function Not available in the slob allocator AFAIK but yeah, I'll add this optimization to the slab version. Thanks Christoph. Pekka - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: > Christoph Lameter wrote: > > 2. Check if the size specified is larger than the next smallest general > > cache and only copy if we would really would allocate from a different > > cache. > > Yeah, I was thinking about this too but decided against it (for now) as I am > mostly concerned with removing the slab abuses from unionfs. Also, it is not > immediately obvious we want to do this for all cases of krealloc so I'd prefer Why not? Its a realloc call and these are the classic semantics of realloc. Otherwise realloc will always move the memory. > to keep the API for a while and decide to optimize or not optimize later. Note > that we would only get rid of one of the kfree callers, the other one doesn't > want to do krealloc(), it never reuses the old values. Check that both sizes fall into the same general cache. Do the following at the beginning of the function struct kmem_cache *cachep = page_get_slab(virt_to_page(object)); if (new_size && cachep == kmem_find_general_cachep(new_size, cachep->gfpflags)) /* * Old and new object size us the same general slab so we do not * have to do anything */ return object; - 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 1/3] slab: introduce krealloc
Hi Christoph, Christoph Lameter wrote: 1. Just do not allow shrinking via realloc. Probably no big loss and best performance. Not a big loss if you can afford the wasted memory. But, I don't think we should do this, there's no way for the caller to know that we will hold on to the memory indefinitely. Christoph Lameter wrote: 2. Check if the size specified is larger than the next smallest general cache and only copy if we would really would allocate from a different cache. Yeah, I was thinking about this too but decided against it (for now) as I am mostly concerned with removing the slab abuses from unionfs. Also, it is not immediately obvious we want to do this for all cases of krealloc so I'd prefer to keep the API for a while and decide to optimize or not optimize later. Note that we would only get rid of one of the kfree callers, the other one doesn't want to do krealloc(), it never reuses the old values. Pekka - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: > Christoph Lameter wrote: > > Well could you check ksize for the old object first and if ksize <= new size > > then just skip the copy? I think this may allow you to get rid of the ksize > > callers. > > And not reallocate at all, right? I thought about that but then you wouldn't > be able to use realloc() to make the buffer smaller. I think there are two ways to address that: 1. Just do not allow shrinking via realloc. Probably no big loss and best performance. 2. Check if the size specified is larger than the next smallest general cache and only copy if we would really would allocate from a different cache. - 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 1/3] slab: introduce krealloc
Christoph Lameter wrote: Well could you check ksize for the old object first and if ksize <= new size then just skip the copy? I think this may allow you to get rid of the ksize callers. And not reallocate at all, right? I thought about that but then you wouldn't be able to use realloc() to make the buffer smaller. Pekka - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka J Enberg wrote:
> +void *krealloc(const void *p, size_t new_size, gfp_t flags)
> +{
> + void *ret;
> +
> + if (unlikely(!p))
> + return kmalloc_track_caller(new_size, flags);
> +
> + if (unlikely(!new_size)) {
> + kfree(p);
> + return NULL;
> + }
> +
> + ret = kmalloc_track_caller(new_size, flags);
> + if (!ret)
> + return NULL;
> +
> + memcpy(ret, p, min(new_size, ksize(p)));
> + kfree(p);
> + return ret;
> +}
> +EXPORT_SYMBOL(krealloc);
Well could you check ksize for the old object first and if ksize <= new
size then just skip the copy? I think this may allow you
to get rid of the ksize callers.
-
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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: > On 2/21/07, Arjan van de Ven <[EMAIL PROTECTED]> wrote: > > please mark this one __must_check.. not storing realloc() return values > > is one of the more nasty types of bugs... but gcc can help us greatly > > here ;) > > So I guess we want the same thing for the other allocator functions > (__kmalloc et al) as well? I think so. - 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 1/3] slab: introduce krealloc
On Wed, 2007-02-21 at 11:33 +0200, Pekka Enberg wrote: > On 2/21/07, Arjan van de Ven <[EMAIL PROTECTED]> wrote: > > please mark this one __must_check.. not storing realloc() return values > > is one of the more nasty types of bugs... but gcc can help us greatly > > here ;) > > So I guess we want the same thing for the other allocator functions > (__kmalloc et al) as well? kmalloc doesn't normally get forgotten. realloc() otoh is a very classic and far more frequent mistake -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - 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 1/3] slab: introduce krealloc
On 2/21/07, Arjan van de Ven <[EMAIL PROTECTED]> wrote: please mark this one __must_check.. not storing realloc() return values is one of the more nasty types of bugs... but gcc can help us greatly here ;) So I guess we want the same thing for the other allocator functions (__kmalloc et al) as well? - 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 1/3] slab: introduce krealloc
On Wed, 2007-02-21 at 10:06 +0200, Pekka J Enberg wrote: > From: Pekka Enberg <[EMAIL PROTECTED]> > > Introduce krealloc() for reallocating memory while preserving contents. please mark this one __must_check.. not storing realloc() return values is one of the more nasty types of bugs... but gcc can help us greatly here ;) - 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/
[PATCH 1/3] slab: introduce krealloc
From: Pekka Enberg <[EMAIL PROTECTED]>
Introduce krealloc() for reallocating memory while preserving contents.
Cc: Christoph Lameter <[EMAIL PROTECTED]>
Signed-off-by: Pekka Enberg <[EMAIL PROTECTED]>
---
include/linux/slab.h |1 +
mm/util.c| 34 ++
2 files changed, 35 insertions(+)
Index: 2.6/include/linux/slab.h
===
--- 2.6.orig/include/linux/slab.h 2007-02-21 09:05:08.0 +0200
+++ 2.6/include/linux/slab.h2007-02-21 09:05:47.0 +0200
@@ -72,6 +72,7 @@
*/
void *__kmalloc(size_t, gfp_t);
void *__kzalloc(size_t, gfp_t);
+void *krealloc(const void *, size_t, gfp_t);
void kfree(const void *);
unsigned int ksize(const void *);
Index: 2.6/mm/util.c
===
--- 2.6.orig/mm/util.c 2007-02-21 09:05:49.0 +0200
+++ 2.6/mm/util.c 2007-02-21 09:34:32.0 +0200
@@ -18,6 +18,40 @@
}
EXPORT_SYMBOL(__kzalloc);
+/**
+ * krealloc - reallocate memory. The contents will remain unchanged.
+ *
+ * @p: object to reallocate memory for.
+ * @new_size: how many bytes of memory are required.
+ * @flags: the type of memory to allocate.
+ *
+ * The contents of the object pointed to are preserved up to the
+ * lesser of the new and old sizes. If @p is %NULL, krealloc()
+ * behaves exactly like kmalloc(). If @size is 0 and @p is not a
+ * %NULL pointer, the object pointed to is freed.
+ */
+void *krealloc(const void *p, size_t new_size, gfp_t flags)
+{
+ void *ret;
+
+ if (unlikely(!p))
+ return kmalloc_track_caller(new_size, flags);
+
+ if (unlikely(!new_size)) {
+ kfree(p);
+ return NULL;
+ }
+
+ ret = kmalloc_track_caller(new_size, flags);
+ if (!ret)
+ return NULL;
+
+ memcpy(ret, p, min(new_size, ksize(p)));
+ kfree(p);
+ return ret;
+}
+EXPORT_SYMBOL(krealloc);
+
/*
* kstrdup - allocate space for and copy an existing string
*
-
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/
[PATCH 1/3] slab: introduce krealloc
From: Pekka Enberg [EMAIL PROTECTED]
Introduce krealloc() for reallocating memory while preserving contents.
Cc: Christoph Lameter [EMAIL PROTECTED]
Signed-off-by: Pekka Enberg [EMAIL PROTECTED]
---
include/linux/slab.h |1 +
mm/util.c| 34 ++
2 files changed, 35 insertions(+)
Index: 2.6/include/linux/slab.h
===
--- 2.6.orig/include/linux/slab.h 2007-02-21 09:05:08.0 +0200
+++ 2.6/include/linux/slab.h2007-02-21 09:05:47.0 +0200
@@ -72,6 +72,7 @@
*/
void *__kmalloc(size_t, gfp_t);
void *__kzalloc(size_t, gfp_t);
+void *krealloc(const void *, size_t, gfp_t);
void kfree(const void *);
unsigned int ksize(const void *);
Index: 2.6/mm/util.c
===
--- 2.6.orig/mm/util.c 2007-02-21 09:05:49.0 +0200
+++ 2.6/mm/util.c 2007-02-21 09:34:32.0 +0200
@@ -18,6 +18,40 @@
}
EXPORT_SYMBOL(__kzalloc);
+/**
+ * krealloc - reallocate memory. The contents will remain unchanged.
+ *
+ * @p: object to reallocate memory for.
+ * @new_size: how many bytes of memory are required.
+ * @flags: the type of memory to allocate.
+ *
+ * The contents of the object pointed to are preserved up to the
+ * lesser of the new and old sizes. If @p is %NULL, krealloc()
+ * behaves exactly like kmalloc(). If @size is 0 and @p is not a
+ * %NULL pointer, the object pointed to is freed.
+ */
+void *krealloc(const void *p, size_t new_size, gfp_t flags)
+{
+ void *ret;
+
+ if (unlikely(!p))
+ return kmalloc_track_caller(new_size, flags);
+
+ if (unlikely(!new_size)) {
+ kfree(p);
+ return NULL;
+ }
+
+ ret = kmalloc_track_caller(new_size, flags);
+ if (!ret)
+ return NULL;
+
+ memcpy(ret, p, min(new_size, ksize(p)));
+ kfree(p);
+ return ret;
+}
+EXPORT_SYMBOL(krealloc);
+
/*
* kstrdup - allocate space for and copy an existing string
*
-
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 1/3] slab: introduce krealloc
On Wed, 2007-02-21 at 10:06 +0200, Pekka J Enberg wrote: From: Pekka Enberg [EMAIL PROTECTED] Introduce krealloc() for reallocating memory while preserving contents. please mark this one __must_check.. not storing realloc() return values is one of the more nasty types of bugs... but gcc can help us greatly here ;) - 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 1/3] slab: introduce krealloc
On 2/21/07, Arjan van de Ven [EMAIL PROTECTED] wrote: please mark this one __must_check.. not storing realloc() return values is one of the more nasty types of bugs... but gcc can help us greatly here ;) So I guess we want the same thing for the other allocator functions (__kmalloc et al) as well? - 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 1/3] slab: introduce krealloc
On Wed, 2007-02-21 at 11:33 +0200, Pekka Enberg wrote: On 2/21/07, Arjan van de Ven [EMAIL PROTECTED] wrote: please mark this one __must_check.. not storing realloc() return values is one of the more nasty types of bugs... but gcc can help us greatly here ;) So I guess we want the same thing for the other allocator functions (__kmalloc et al) as well? kmalloc doesn't normally get forgotten. realloc() otoh is a very classic and far more frequent mistake -- if you want to mail me at work (you don't), use arjan (at) linux.intel.com Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: On 2/21/07, Arjan van de Ven [EMAIL PROTECTED] wrote: please mark this one __must_check.. not storing realloc() return values is one of the more nasty types of bugs... but gcc can help us greatly here ;) So I guess we want the same thing for the other allocator functions (__kmalloc et al) as well? I think so. - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka J Enberg wrote:
+void *krealloc(const void *p, size_t new_size, gfp_t flags)
+{
+ void *ret;
+
+ if (unlikely(!p))
+ return kmalloc_track_caller(new_size, flags);
+
+ if (unlikely(!new_size)) {
+ kfree(p);
+ return NULL;
+ }
+
+ ret = kmalloc_track_caller(new_size, flags);
+ if (!ret)
+ return NULL;
+
+ memcpy(ret, p, min(new_size, ksize(p)));
+ kfree(p);
+ return ret;
+}
+EXPORT_SYMBOL(krealloc);
Well could you check ksize for the old object first and if ksize = new
size then just skip the copy? I think this may allow you
to get rid of the ksize callers.
-
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 1/3] slab: introduce krealloc
Christoph Lameter wrote: Well could you check ksize for the old object first and if ksize = new size then just skip the copy? I think this may allow you to get rid of the ksize callers. And not reallocate at all, right? I thought about that but then you wouldn't be able to use realloc() to make the buffer smaller. Pekka - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: Christoph Lameter wrote: Well could you check ksize for the old object first and if ksize = new size then just skip the copy? I think this may allow you to get rid of the ksize callers. And not reallocate at all, right? I thought about that but then you wouldn't be able to use realloc() to make the buffer smaller. I think there are two ways to address that: 1. Just do not allow shrinking via realloc. Probably no big loss and best performance. 2. Check if the size specified is larger than the next smallest general cache and only copy if we would really would allocate from a different cache. - 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 1/3] slab: introduce krealloc
Hi Christoph, Christoph Lameter wrote: 1. Just do not allow shrinking via realloc. Probably no big loss and best performance. Not a big loss if you can afford the wasted memory. But, I don't think we should do this, there's no way for the caller to know that we will hold on to the memory indefinitely. Christoph Lameter wrote: 2. Check if the size specified is larger than the next smallest general cache and only copy if we would really would allocate from a different cache. Yeah, I was thinking about this too but decided against it (for now) as I am mostly concerned with removing the slab abuses from unionfs. Also, it is not immediately obvious we want to do this for all cases of krealloc so I'd prefer to keep the API for a while and decide to optimize or not optimize later. Note that we would only get rid of one of the kfree callers, the other one doesn't want to do krealloc(), it never reuses the old values. Pekka - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: Christoph Lameter wrote: 2. Check if the size specified is larger than the next smallest general cache and only copy if we would really would allocate from a different cache. Yeah, I was thinking about this too but decided against it (for now) as I am mostly concerned with removing the slab abuses from unionfs. Also, it is not immediately obvious we want to do this for all cases of krealloc so I'd prefer Why not? Its a realloc call and these are the classic semantics of realloc. Otherwise realloc will always move the memory. to keep the API for a while and decide to optimize or not optimize later. Note that we would only get rid of one of the kfree callers, the other one doesn't want to do krealloc(), it never reuses the old values. Check that both sizes fall into the same general cache. Do the following at the beginning of the function struct kmem_cache *cachep = page_get_slab(virt_to_page(object)); if (new_size cachep == kmem_find_general_cachep(new_size, cachep-gfpflags)) /* * Old and new object size us the same general slab so we do not * have to do anything */ return object; - 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 1/3] slab: introduce krealloc
On 2/21/07, Christoph Lameter [EMAIL PROTECTED] wrote: Why not? Its a realloc call and these are the classic semantics of realloc. Otherwise realloc will always move the memory. Well, as a reference, the user-space equivalent is defined in SUSv3 as: The realloc() function shall change the size of the memory object pointed to by ptr to the size specified by size. I think it is reasonable to expect krealloc() to not waste too much space if I, say, reallocate a 128 byte buffer to 32 bytes. On 2/21/07, Christoph Lameter [EMAIL PROTECTED] wrote: Check that both sizes fall into the same general cache. Do the following at the beginning of the function Not available in the slob allocator AFAIK but yeah, I'll add this optimization to the slab version. Thanks Christoph. Pekka - 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 1/3] slab: introduce krealloc
On Wed, 21 Feb 2007, Pekka Enberg wrote: Well, as a reference, the user-space equivalent is defined in SUSv3 as: The realloc() function shall change the size of the memory object pointed to by ptr to the size specified by size. The realloc functions intent is to leave the object in place if possible. Otherwise one can simply alloc a new object and free the old one. - 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/
