* [PATCH 1/4] rbd: allocate image requests with a slab allocator
2013-05-01 21:34 [PATCH 0/4] rbd: use slab caches for frequently allocated structures Alex Elder
@ 2013-05-01 21:35 ` Alex Elder
2013-05-02 16:19 ` Josh Durgin
2013-05-01 21:35 ` [PATCH 2/4] rbd: allocate name separate from obj_request Alex Elder
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2013-05-01 21:35 UTC (permalink / raw)
To: ceph-devel
Create a slab cache to manage rbd_img_request allocation. Nothing
too fancy at this point--we'll still initialize everything at
allocation time (no constructor)
This is part of:
http://tracker.ceph.com/issues/3926
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index a6e5fe3..005c397 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -40,6 +40,7 @@
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/blkdev.h>
+#include <linux/slab.h>
#include "rbd_types.h"
@@ -344,6 +345,8 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
static LIST_HEAD(rbd_client_list); /* clients */
static DEFINE_SPINLOCK(rbd_client_list_lock);
+static struct kmem_cache *rbd_img_request_cache;
+
static int rbd_img_request_submit(struct rbd_img_request *img_request);
static void rbd_dev_device_release(struct device *dev);
@@ -1821,7 +1824,7 @@ static struct rbd_img_request *rbd_img_request_create(
{
struct rbd_img_request *img_request;
- img_request = kmalloc(sizeof (*img_request), GFP_ATOMIC);
+ img_request = kmem_cache_alloc(rbd_img_request_cache, GFP_KERNEL);
if (!img_request)
return NULL;
@@ -1884,7 +1887,7 @@ static void rbd_img_request_destroy(struct kref *kref)
if (img_request_child_test(img_request))
rbd_obj_request_put(img_request->obj_request);
- kfree(img_request);
+ kmem_cache_free(rbd_img_request_cache, img_request);
}
static bool rbd_img_obj_end_request(struct rbd_obj_request *obj_request)
@@ -4992,6 +4995,26 @@ static void rbd_sysfs_cleanup(void)
device_unregister(&rbd_root_dev);
}
+static int rbd_slab_init(void)
+{
+ rbd_assert(!rbd_img_request_cache);
+ rbd_img_request_cache = kmem_cache_create("rbd_img_request",
+ sizeof (struct rbd_img_request),
+ __alignof__(struct rbd_img_request),
+ 0, NULL);
+ if (rbd_img_request_cache)
+ return 0;
+
+ return -ENOMEM;
+}
+
+static void rbd_slab_exit(void)
+{
+ rbd_assert(rbd_img_request_cache);
+ kmem_cache_destroy(rbd_img_request_cache);
+ rbd_img_request_cache = NULL;
+}
+
static int __init rbd_init(void)
{
int rc;
@@ -5001,16 +5024,22 @@ static int __init rbd_init(void)
return -EINVAL;
}
- rc = rbd_sysfs_init();
+ rc = rbd_slab_init();
if (rc)
return rc;
- pr_info("loaded " RBD_DRV_NAME_LONG "\n");
- return 0;
+ rc = rbd_sysfs_init();
+ if (rc)
+ rbd_slab_exit();
+ else
+ pr_info("loaded " RBD_DRV_NAME_LONG "\n");
+
+ return rc;
}
static void __exit rbd_exit(void)
{
rbd_sysfs_cleanup();
+ rbd_slab_exit();
}
module_init(rbd_init);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] rbd: allocate image requests with a slab allocator
2013-05-01 21:35 ` [PATCH 1/4] rbd: allocate image requests with a slab allocator Alex Elder
@ 2013-05-02 16:19 ` Josh Durgin
2013-05-02 16:28 ` Alex Elder
0 siblings, 1 reply; 11+ messages in thread
From: Josh Durgin @ 2013-05-02 16:19 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
It looks like the GFP flags used throughout request creation
could be cleanup up (there's a mix of ATOMIC, KERNEL, and NOIO).
I just noticed since this changes a GFP_ATOMIC to GFP_KERNEL.
Making them consistent can be a later patch though.
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 05/01/2013 02:35 PM, Alex Elder wrote:
> Create a slab cache to manage rbd_img_request allocation. Nothing
> too fancy at this point--we'll still initialize everything at
> allocation time (no constructor)
>
> This is part of:
> http://tracker.ceph.com/issues/3926
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 39 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index a6e5fe3..005c397 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -40,6 +40,7 @@
> #include <linux/module.h>
> #include <linux/fs.h>
> #include <linux/blkdev.h>
> +#include <linux/slab.h>
>
> #include "rbd_types.h"
>
> @@ -344,6 +345,8 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
> static LIST_HEAD(rbd_client_list); /* clients */
> static DEFINE_SPINLOCK(rbd_client_list_lock);
>
> +static struct kmem_cache *rbd_img_request_cache;
> +
> static int rbd_img_request_submit(struct rbd_img_request *img_request);
>
> static void rbd_dev_device_release(struct device *dev);
> @@ -1821,7 +1824,7 @@ static struct rbd_img_request *rbd_img_request_create(
> {
> struct rbd_img_request *img_request;
>
> - img_request = kmalloc(sizeof (*img_request), GFP_ATOMIC);
> + img_request = kmem_cache_alloc(rbd_img_request_cache, GFP_KERNEL);
> if (!img_request)
> return NULL;
>
> @@ -1884,7 +1887,7 @@ static void rbd_img_request_destroy(struct kref *kref)
> if (img_request_child_test(img_request))
> rbd_obj_request_put(img_request->obj_request);
>
> - kfree(img_request);
> + kmem_cache_free(rbd_img_request_cache, img_request);
> }
>
> static bool rbd_img_obj_end_request(struct rbd_obj_request *obj_request)
> @@ -4992,6 +4995,26 @@ static void rbd_sysfs_cleanup(void)
> device_unregister(&rbd_root_dev);
> }
>
> +static int rbd_slab_init(void)
> +{
> + rbd_assert(!rbd_img_request_cache);
> + rbd_img_request_cache = kmem_cache_create("rbd_img_request",
> + sizeof (struct rbd_img_request),
> + __alignof__(struct rbd_img_request),
> + 0, NULL);
> + if (rbd_img_request_cache)
> + return 0;
> +
> + return -ENOMEM;
> +}
> +
> +static void rbd_slab_exit(void)
> +{
> + rbd_assert(rbd_img_request_cache);
> + kmem_cache_destroy(rbd_img_request_cache);
> + rbd_img_request_cache = NULL;
> +}
> +
> static int __init rbd_init(void)
> {
> int rc;
> @@ -5001,16 +5024,22 @@ static int __init rbd_init(void)
>
> return -EINVAL;
> }
> - rc = rbd_sysfs_init();
> + rc = rbd_slab_init();
> if (rc)
> return rc;
> - pr_info("loaded " RBD_DRV_NAME_LONG "\n");
> - return 0;
> + rc = rbd_sysfs_init();
> + if (rc)
> + rbd_slab_exit();
> + else
> + pr_info("loaded " RBD_DRV_NAME_LONG "\n");
> +
> + return rc;
> }
>
> static void __exit rbd_exit(void)
> {
> rbd_sysfs_cleanup();
> + rbd_slab_exit();
> }
>
> module_init(rbd_init);
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] rbd: allocate image requests with a slab allocator
2013-05-02 16:19 ` Josh Durgin
@ 2013-05-02 16:28 ` Alex Elder
0 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2013-05-02 16:28 UTC (permalink / raw)
To: Josh Durgin; +Cc: ceph-devel
On 05/02/2013 11:19 AM, Josh Durgin wrote:
> It looks like the GFP flags used throughout request creation
> could be cleanup up (there's a mix of ATOMIC, KERNEL, and NOIO).
> I just noticed since this changes a GFP_ATOMIC to GFP_KERNEL.
> Making them consistent can be a later patch though.
Well that was unintentional--a copy-paste with haste error.
But yes, I've already got a bug open on that.
http://tracker.ceph.com/issues/4233
> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
>
> On 05/01/2013 02:35 PM, Alex Elder wrote:
>> Create a slab cache to manage rbd_img_request allocation. Nothing
>> too fancy at this point--we'll still initialize everything at
>> allocation time (no constructor)
>>
>> This is part of:
>> http://tracker.ceph.com/issues/3926
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>> drivers/block/rbd.c | 39 ++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index a6e5fe3..005c397 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -40,6 +40,7 @@
>> #include <linux/module.h>
>> #include <linux/fs.h>
>> #include <linux/blkdev.h>
>> +#include <linux/slab.h>
>>
>> #include "rbd_types.h"
>>
>> @@ -344,6 +345,8 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
>> static LIST_HEAD(rbd_client_list); /* clients */
>> static DEFINE_SPINLOCK(rbd_client_list_lock);
>>
>> +static struct kmem_cache *rbd_img_request_cache;
>> +
>> static int rbd_img_request_submit(struct rbd_img_request *img_request);
>>
>> static void rbd_dev_device_release(struct device *dev);
>> @@ -1821,7 +1824,7 @@ static struct rbd_img_request
>> *rbd_img_request_create(
>> {
>> struct rbd_img_request *img_request;
>>
>> - img_request = kmalloc(sizeof (*img_request), GFP_ATOMIC);
>> + img_request = kmem_cache_alloc(rbd_img_request_cache, GFP_KERNEL);
>> if (!img_request)
>> return NULL;
>>
>> @@ -1884,7 +1887,7 @@ static void rbd_img_request_destroy(struct kref
>> *kref)
>> if (img_request_child_test(img_request))
>> rbd_obj_request_put(img_request->obj_request);
>>
>> - kfree(img_request);
>> + kmem_cache_free(rbd_img_request_cache, img_request);
>> }
>>
>> static bool rbd_img_obj_end_request(struct rbd_obj_request
>> *obj_request)
>> @@ -4992,6 +4995,26 @@ static void rbd_sysfs_cleanup(void)
>> device_unregister(&rbd_root_dev);
>> }
>>
>> +static int rbd_slab_init(void)
>> +{
>> + rbd_assert(!rbd_img_request_cache);
>> + rbd_img_request_cache = kmem_cache_create("rbd_img_request",
>> + sizeof (struct rbd_img_request),
>> + __alignof__(struct rbd_img_request),
>> + 0, NULL);
>> + if (rbd_img_request_cache)
>> + return 0;
>> +
>> + return -ENOMEM;
>> +}
>> +
>> +static void rbd_slab_exit(void)
>> +{
>> + rbd_assert(rbd_img_request_cache);
>> + kmem_cache_destroy(rbd_img_request_cache);
>> + rbd_img_request_cache = NULL;
>> +}
>> +
>> static int __init rbd_init(void)
>> {
>> int rc;
>> @@ -5001,16 +5024,22 @@ static int __init rbd_init(void)
>>
>> return -EINVAL;
>> }
>> - rc = rbd_sysfs_init();
>> + rc = rbd_slab_init();
>> if (rc)
>> return rc;
>> - pr_info("loaded " RBD_DRV_NAME_LONG "\n");
>> - return 0;
>> + rc = rbd_sysfs_init();
>> + if (rc)
>> + rbd_slab_exit();
>> + else
>> + pr_info("loaded " RBD_DRV_NAME_LONG "\n");
>> +
>> + return rc;
>> }
>>
>> static void __exit rbd_exit(void)
>> {
>> rbd_sysfs_cleanup();
>> + rbd_slab_exit();
>> }
>>
>> module_init(rbd_init);
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] rbd: allocate name separate from obj_request
2013-05-01 21:34 [PATCH 0/4] rbd: use slab caches for frequently allocated structures Alex Elder
2013-05-01 21:35 ` [PATCH 1/4] rbd: allocate image requests with a slab allocator Alex Elder
@ 2013-05-01 21:35 ` Alex Elder
2013-05-02 16:20 ` Josh Durgin
2013-05-01 21:36 ` [PATCH 3/4] rbd: allocate object requests with a slab allocator Alex Elder
2013-05-01 21:36 ` [PATCH 4/4] rbd: allocate image object names " Alex Elder
3 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2013-05-01 21:35 UTC (permalink / raw)
To: ceph-devel
The next patch will define a slab allocator for a object requests.
To use that we'll need to allocate the name of an object separate
from the request structure itself.
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 005c397..da9f41d 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -1758,11 +1758,16 @@ static struct rbd_obj_request
*rbd_obj_request_create(const char *object_name,
rbd_assert(obj_request_type_valid(type));
size = strlen(object_name) + 1;
- obj_request = kzalloc(sizeof (*obj_request) + size, GFP_KERNEL);
- if (!obj_request)
+ name = kmalloc(size, GFP_KERNEL);
+ if (!name)
+ return NULL;
+
+ obj_request = kzalloc(sizeof (*obj_request), GFP_KERNEL);
+ if (!obj_request) {
+ kfree(name);
return NULL;
+ }
- name = (char *)(obj_request + 1);
obj_request->object_name = memcpy(name, object_name, size);
obj_request->offset = offset;
obj_request->length = length;
@@ -1808,6 +1813,7 @@ static void rbd_obj_request_destroy(struct kref *kref)
break;
}
+ kfree(obj_request->object_name);
kfree(obj_request);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] rbd: allocate name separate from obj_request
2013-05-01 21:35 ` [PATCH 2/4] rbd: allocate name separate from obj_request Alex Elder
@ 2013-05-02 16:20 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2013-05-02 16:20 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 05/01/2013 02:35 PM, Alex Elder wrote:
> The next patch will define a slab allocator for a object requests.
> To use that we'll need to allocate the name of an object separate
> from the request structure itself.
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 005c397..da9f41d 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -1758,11 +1758,16 @@ static struct rbd_obj_request
> *rbd_obj_request_create(const char *object_name,
> rbd_assert(obj_request_type_valid(type));
>
> size = strlen(object_name) + 1;
> - obj_request = kzalloc(sizeof (*obj_request) + size, GFP_KERNEL);
> - if (!obj_request)
> + name = kmalloc(size, GFP_KERNEL);
> + if (!name)
> + return NULL;
> +
> + obj_request = kzalloc(sizeof (*obj_request), GFP_KERNEL);
> + if (!obj_request) {
> + kfree(name);
> return NULL;
> + }
>
> - name = (char *)(obj_request + 1);
> obj_request->object_name = memcpy(name, object_name, size);
> obj_request->offset = offset;
> obj_request->length = length;
> @@ -1808,6 +1813,7 @@ static void rbd_obj_request_destroy(struct kref *kref)
> break;
> }
>
> + kfree(obj_request->object_name);
> kfree(obj_request);
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] rbd: allocate object requests with a slab allocator
2013-05-01 21:34 [PATCH 0/4] rbd: use slab caches for frequently allocated structures Alex Elder
2013-05-01 21:35 ` [PATCH 1/4] rbd: allocate image requests with a slab allocator Alex Elder
2013-05-01 21:35 ` [PATCH 2/4] rbd: allocate name separate from obj_request Alex Elder
@ 2013-05-01 21:36 ` Alex Elder
2013-05-02 16:21 ` Josh Durgin
2013-05-01 21:36 ` [PATCH 4/4] rbd: allocate image object names " Alex Elder
3 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2013-05-01 21:36 UTC (permalink / raw)
To: ceph-devel
Create a slab cache to manage rbd_obj_request allocation. We aren't
using a constructor, and we'll zero-fill object request structures
when they're allocated.
This is part of:
http://tracker.ceph.com/issues/3926
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index da9f41d..28a5ea3 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -346,6 +346,7 @@ static LIST_HEAD(rbd_client_list); /* clients */
static DEFINE_SPINLOCK(rbd_client_list_lock);
static struct kmem_cache *rbd_img_request_cache;
+static struct kmem_cache *rbd_obj_request_cache;
static int rbd_img_request_submit(struct rbd_img_request *img_request);
@@ -1762,7 +1763,7 @@ static struct rbd_obj_request
*rbd_obj_request_create(const char *object_name,
if (!name)
return NULL;
- obj_request = kzalloc(sizeof (*obj_request), GFP_KERNEL);
+ obj_request = kmem_cache_zalloc(rbd_obj_request_cache, GFP_KERNEL);
if (!obj_request) {
kfree(name);
return NULL;
@@ -1814,7 +1815,8 @@ static void rbd_obj_request_destroy(struct kref *kref)
}
kfree(obj_request->object_name);
- kfree(obj_request);
+ obj_request->object_name = NULL;
+ kmem_cache_free(rbd_obj_request_cache, obj_request);
}
/*
@@ -5008,14 +5010,29 @@ static int rbd_slab_init(void)
sizeof (struct rbd_img_request),
__alignof__(struct rbd_img_request),
0, NULL);
- if (rbd_img_request_cache)
+ if (!rbd_img_request_cache)
+ return -ENOMEM;
+
+ rbd_assert(!rbd_obj_request_cache);
+ rbd_obj_request_cache = kmem_cache_create("rbd_obj_request",
+ sizeof (struct rbd_obj_request),
+ __alignof__(struct rbd_obj_request),
+ 0, NULL);
+ if (rbd_obj_request_cache)
return 0;
+ kmem_cache_destroy(rbd_img_request_cache);
+ rbd_img_request_cache = NULL;
+
return -ENOMEM;
}
static void rbd_slab_exit(void)
{
+ rbd_assert(rbd_obj_request_cache);
+ kmem_cache_destroy(rbd_obj_request_cache);
+ rbd_obj_request_cache = NULL;
+
rbd_assert(rbd_img_request_cache);
kmem_cache_destroy(rbd_img_request_cache);
rbd_img_request_cache = NULL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] rbd: allocate object requests with a slab allocator
2013-05-01 21:36 ` [PATCH 3/4] rbd: allocate object requests with a slab allocator Alex Elder
@ 2013-05-02 16:21 ` Josh Durgin
0 siblings, 0 replies; 11+ messages in thread
From: Josh Durgin @ 2013-05-02 16:21 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
On 05/01/2013 02:36 PM, Alex Elder wrote:
> Create a slab cache to manage rbd_obj_request allocation. We aren't
> using a constructor, and we'll zero-fill object request structures
> when they're allocated.
>
> This is part of:
> http://tracker.ceph.com/issues/3926
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index da9f41d..28a5ea3 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -346,6 +346,7 @@ static LIST_HEAD(rbd_client_list); /* clients */
> static DEFINE_SPINLOCK(rbd_client_list_lock);
>
> static struct kmem_cache *rbd_img_request_cache;
> +static struct kmem_cache *rbd_obj_request_cache;
>
> static int rbd_img_request_submit(struct rbd_img_request *img_request);
>
> @@ -1762,7 +1763,7 @@ static struct rbd_obj_request
> *rbd_obj_request_create(const char *object_name,
> if (!name)
> return NULL;
>
> - obj_request = kzalloc(sizeof (*obj_request), GFP_KERNEL);
> + obj_request = kmem_cache_zalloc(rbd_obj_request_cache, GFP_KERNEL);
> if (!obj_request) {
> kfree(name);
> return NULL;
> @@ -1814,7 +1815,8 @@ static void rbd_obj_request_destroy(struct kref *kref)
> }
>
> kfree(obj_request->object_name);
> - kfree(obj_request);
> + obj_request->object_name = NULL;
> + kmem_cache_free(rbd_obj_request_cache, obj_request);
> }
>
> /*
> @@ -5008,14 +5010,29 @@ static int rbd_slab_init(void)
> sizeof (struct rbd_img_request),
> __alignof__(struct rbd_img_request),
> 0, NULL);
> - if (rbd_img_request_cache)
> + if (!rbd_img_request_cache)
> + return -ENOMEM;
> +
> + rbd_assert(!rbd_obj_request_cache);
> + rbd_obj_request_cache = kmem_cache_create("rbd_obj_request",
> + sizeof (struct rbd_obj_request),
> + __alignof__(struct rbd_obj_request),
> + 0, NULL);
> + if (rbd_obj_request_cache)
> return 0;
>
> + kmem_cache_destroy(rbd_img_request_cache);
> + rbd_img_request_cache = NULL;
> +
> return -ENOMEM;
> }
>
> static void rbd_slab_exit(void)
> {
> + rbd_assert(rbd_obj_request_cache);
> + kmem_cache_destroy(rbd_obj_request_cache);
> + rbd_obj_request_cache = NULL;
> +
> rbd_assert(rbd_img_request_cache);
> kmem_cache_destroy(rbd_img_request_cache);
> rbd_img_request_cache = NULL;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] rbd: allocate image object names with a slab allocator
2013-05-01 21:34 [PATCH 0/4] rbd: use slab caches for frequently allocated structures Alex Elder
` (2 preceding siblings ...)
2013-05-01 21:36 ` [PATCH 3/4] rbd: allocate object requests with a slab allocator Alex Elder
@ 2013-05-01 21:36 ` Alex Elder
2013-05-02 16:24 ` Josh Durgin
3 siblings, 1 reply; 11+ messages in thread
From: Alex Elder @ 2013-05-01 21:36 UTC (permalink / raw)
To: ceph-devel
The names of objects used for image object requests are always fixed
size. So create a slab cache to manage them. Define a new function
rbd_segment_name_free() to match rbd_segment_name() (which is what
supplies the dynamically-allocated name buffer).
This is part of:
http://tracker.ceph.com/issues/3926
Signed-off-by: Alex Elder <elder@inktank.com>
---
drivers/block/rbd.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 28a5ea3..8d9aeef 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -345,8 +345,11 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
static LIST_HEAD(rbd_client_list); /* clients */
static DEFINE_SPINLOCK(rbd_client_list_lock);
+/* Slab caches for frequently-allocated structures */
+
static struct kmem_cache *rbd_img_request_cache;
static struct kmem_cache *rbd_obj_request_cache;
+static struct kmem_cache *rbd_segment_name_cache;
static int rbd_img_request_submit(struct rbd_img_request *img_request);
@@ -985,7 +988,7 @@ static const char *rbd_segment_name(struct
rbd_device *rbd_dev, u64 offset)
u64 segment;
int ret;
- name = kmalloc(MAX_OBJ_NAME_SIZE + 1, GFP_NOIO);
+ name = kmem_cache_alloc(rbd_segment_name_cache, GFP_NOIO);
if (!name)
return NULL;
segment = offset >> rbd_dev->header.obj_order;
@@ -1001,6 +1004,13 @@ static const char *rbd_segment_name(struct
rbd_device *rbd_dev, u64 offset)
return name;
}
+static void rbd_segment_name_free(const char *name)
+{
+ /* The explicit cast here is needed to drop the const qualifier */
+
+ kmem_cache_free(rbd_segment_name_cache, (void *)name);
+}
+
static u64 rbd_segment_offset(struct rbd_device *rbd_dev, u64 offset)
{
u64 segment_size = (u64) 1 << rbd_dev->header.obj_order;
@@ -2033,7 +2043,8 @@ static int rbd_img_request_fill(struct
rbd_img_request *img_request,
length = rbd_segment_length(rbd_dev, img_offset, resid);
obj_request = rbd_obj_request_create(object_name,
offset, length, type);
- kfree(object_name); /* object request has its own copy */
+ /* object request has its own copy of the object name */
+ rbd_segment_name_free(object_name);
if (!obj_request)
goto out_unwind;
@@ -5018,17 +5029,32 @@ static int rbd_slab_init(void)
sizeof (struct rbd_obj_request),
__alignof__(struct rbd_obj_request),
0, NULL);
- if (rbd_obj_request_cache)
- return 0;
+ if (!rbd_obj_request_cache)
+ goto out_err;
+ rbd_assert(!rbd_segment_name_cache);
+ rbd_segment_name_cache = kmem_cache_create("rbd_segment_name",
+ MAX_OBJ_NAME_SIZE + 1, 1, 0, NULL);
+ if (rbd_segment_name_cache)
+ return 0;
+out_err:
kmem_cache_destroy(rbd_img_request_cache);
rbd_img_request_cache = NULL;
+ if (rbd_img_request_cache) {
+ kmem_cache_destroy(rbd_obj_request_cache);
+ rbd_img_request_cache = NULL;
+ }
+
return -ENOMEM;
}
static void rbd_slab_exit(void)
{
+ rbd_assert(rbd_segment_name_cache);
+ kmem_cache_destroy(rbd_segment_name_cache);
+ rbd_segment_name_cache = NULL;
+
rbd_assert(rbd_obj_request_cache);
kmem_cache_destroy(rbd_obj_request_cache);
rbd_obj_request_cache = NULL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] rbd: allocate image object names with a slab allocator
2013-05-01 21:36 ` [PATCH 4/4] rbd: allocate image object names " Alex Elder
@ 2013-05-02 16:24 ` Josh Durgin
2013-05-02 16:30 ` Alex Elder
0 siblings, 1 reply; 11+ messages in thread
From: Josh Durgin @ 2013-05-02 16:24 UTC (permalink / raw)
To: Alex Elder; +Cc: ceph-devel
On 05/01/2013 02:36 PM, Alex Elder wrote:
> The names of objects used for image object requests are always fixed
> size. So create a slab cache to manage them. Define a new function
> rbd_segment_name_free() to match rbd_segment_name() (which is what
> supplies the dynamically-allocated name buffer).
>
> This is part of:
> http://tracker.ceph.com/issues/3926
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> drivers/block/rbd.c | 34 ++++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index 28a5ea3..8d9aeef 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -345,8 +345,11 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
> static LIST_HEAD(rbd_client_list); /* clients */
> static DEFINE_SPINLOCK(rbd_client_list_lock);
>
> +/* Slab caches for frequently-allocated structures */
> +
> static struct kmem_cache *rbd_img_request_cache;
> static struct kmem_cache *rbd_obj_request_cache;
> +static struct kmem_cache *rbd_segment_name_cache;
>
> static int rbd_img_request_submit(struct rbd_img_request *img_request);
>
> @@ -985,7 +988,7 @@ static const char *rbd_segment_name(struct
> rbd_device *rbd_dev, u64 offset)
> u64 segment;
> int ret;
>
> - name = kmalloc(MAX_OBJ_NAME_SIZE + 1, GFP_NOIO);
> + name = kmem_cache_alloc(rbd_segment_name_cache, GFP_NOIO);
> if (!name)
> return NULL;
> segment = offset >> rbd_dev->header.obj_order;
> @@ -1001,6 +1004,13 @@ static const char *rbd_segment_name(struct
> rbd_device *rbd_dev, u64 offset)
> return name;
> }
>
> +static void rbd_segment_name_free(const char *name)
> +{
> + /* The explicit cast here is needed to drop the const qualifier */
> +
> + kmem_cache_free(rbd_segment_name_cache, (void *)name);
> +}
> +
> static u64 rbd_segment_offset(struct rbd_device *rbd_dev, u64 offset)
> {
> u64 segment_size = (u64) 1 << rbd_dev->header.obj_order;
> @@ -2033,7 +2043,8 @@ static int rbd_img_request_fill(struct
> rbd_img_request *img_request,
> length = rbd_segment_length(rbd_dev, img_offset, resid);
> obj_request = rbd_obj_request_create(object_name,
> offset, length, type);
> - kfree(object_name); /* object request has its own copy */
> + /* object request has its own copy of the object name */
> + rbd_segment_name_free(object_name);
> if (!obj_request)
> goto out_unwind;
>
> @@ -5018,17 +5029,32 @@ static int rbd_slab_init(void)
> sizeof (struct rbd_obj_request),
> __alignof__(struct rbd_obj_request),
> 0, NULL);
> - if (rbd_obj_request_cache)
> - return 0;
> + if (!rbd_obj_request_cache)
> + goto out_err;
>
> + rbd_assert(!rbd_segment_name_cache);
> + rbd_segment_name_cache = kmem_cache_create("rbd_segment_name",
> + MAX_OBJ_NAME_SIZE + 1, 1, 0, NULL);
> + if (rbd_segment_name_cache)
> + return 0;
> +out_err:
> kmem_cache_destroy(rbd_img_request_cache);
> rbd_img_request_cache = NULL;
>
> + if (rbd_img_request_cache) {
> + kmem_cache_destroy(rbd_obj_request_cache);
> + rbd_img_request_cache = NULL;
Should be rbd_obj_request_cache here and in the condition two lines up.
With that fixed: Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
> + }
> +
> return -ENOMEM;
> }
>
> static void rbd_slab_exit(void)
> {
> + rbd_assert(rbd_segment_name_cache);
> + kmem_cache_destroy(rbd_segment_name_cache);
> + rbd_segment_name_cache = NULL;
> +
> rbd_assert(rbd_obj_request_cache);
> kmem_cache_destroy(rbd_obj_request_cache);
> rbd_obj_request_cache = NULL;
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] rbd: allocate image object names with a slab allocator
2013-05-02 16:24 ` Josh Durgin
@ 2013-05-02 16:30 ` Alex Elder
0 siblings, 0 replies; 11+ messages in thread
From: Alex Elder @ 2013-05-02 16:30 UTC (permalink / raw)
To: Josh Durgin; +Cc: ceph-devel
On 05/02/2013 11:24 AM, Josh Durgin wrote:
> On 05/01/2013 02:36 PM, Alex Elder wrote:
>> The names of objects used for image object requests are always fixed
>> size. So create a slab cache to manage them. Define a new function
>> rbd_segment_name_free() to match rbd_segment_name() (which is what
>> supplies the dynamically-allocated name buffer).
>>
>> This is part of:
>> http://tracker.ceph.com/issues/3926
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>> drivers/block/rbd.c | 34 ++++++++++++++++++++++++++++++----
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 28a5ea3..8d9aeef 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -345,8 +345,11 @@ static DEFINE_SPINLOCK(rbd_dev_list_lock);
>> static LIST_HEAD(rbd_client_list); /* clients */
>> static DEFINE_SPINLOCK(rbd_client_list_lock);
>>
>> +/* Slab caches for frequently-allocated structures */
>> +
>> static struct kmem_cache *rbd_img_request_cache;
>> static struct kmem_cache *rbd_obj_request_cache;
>> +static struct kmem_cache *rbd_segment_name_cache;
>>
>> static int rbd_img_request_submit(struct rbd_img_request *img_request);
>>
>> @@ -985,7 +988,7 @@ static const char *rbd_segment_name(struct
>> rbd_device *rbd_dev, u64 offset)
>> u64 segment;
>> int ret;
>>
>> - name = kmalloc(MAX_OBJ_NAME_SIZE + 1, GFP_NOIO);
>> + name = kmem_cache_alloc(rbd_segment_name_cache, GFP_NOIO);
>> if (!name)
>> return NULL;
>> segment = offset >> rbd_dev->header.obj_order;
>> @@ -1001,6 +1004,13 @@ static const char *rbd_segment_name(struct
>> rbd_device *rbd_dev, u64 offset)
>> return name;
>> }
>>
>> +static void rbd_segment_name_free(const char *name)
>> +{
>> + /* The explicit cast here is needed to drop the const qualifier */
>> +
>> + kmem_cache_free(rbd_segment_name_cache, (void *)name);
>> +}
>> +
>> static u64 rbd_segment_offset(struct rbd_device *rbd_dev, u64 offset)
>> {
>> u64 segment_size = (u64) 1 << rbd_dev->header.obj_order;
>> @@ -2033,7 +2043,8 @@ static int rbd_img_request_fill(struct
>> rbd_img_request *img_request,
>> length = rbd_segment_length(rbd_dev, img_offset, resid);
>> obj_request = rbd_obj_request_create(object_name,
>> offset, length, type);
>> - kfree(object_name); /* object request has its own copy */
>> + /* object request has its own copy of the object name */
>> + rbd_segment_name_free(object_name);
>> if (!obj_request)
>> goto out_unwind;
>>
>> @@ -5018,17 +5029,32 @@ static int rbd_slab_init(void)
>> sizeof (struct rbd_obj_request),
>> __alignof__(struct rbd_obj_request),
>> 0, NULL);
>> - if (rbd_obj_request_cache)
>> - return 0;
>> + if (!rbd_obj_request_cache)
>> + goto out_err;
>>
>> + rbd_assert(!rbd_segment_name_cache);
>> + rbd_segment_name_cache = kmem_cache_create("rbd_segment_name",
>> + MAX_OBJ_NAME_SIZE + 1, 1, 0, NULL);
>> + if (rbd_segment_name_cache)
>> + return 0;
>> +out_err:
>> kmem_cache_destroy(rbd_img_request_cache);
>> rbd_img_request_cache = NULL;
>>
>> + if (rbd_img_request_cache) {
>> + kmem_cache_destroy(rbd_obj_request_cache);
>> + rbd_img_request_cache = NULL;
>
> Should be rbd_obj_request_cache here and in the condition two lines up.
> With that fixed: Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Got it, I'll fix that, and I'll also fix that GFP
mixup in the previous one. Thanks for the review.
-Alex
>> + }
>> +
>> return -ENOMEM;
>> }
>>
>> static void rbd_slab_exit(void)
>> {
>> + rbd_assert(rbd_segment_name_cache);
>> + kmem_cache_destroy(rbd_segment_name_cache);
>> + rbd_segment_name_cache = NULL;
>> +
>> rbd_assert(rbd_obj_request_cache);
>> kmem_cache_destroy(rbd_obj_request_cache);
>> rbd_obj_request_cache = NULL;
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread