| Message ID | 1481160187-9652-7-git-send-email-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/07/2016 06:23 PM, Rasmus Villemoes wrote: > A struct ida ends up costing > 16 KB of runtime memory, which is quite > a lot for something which on my laptop as of this writing has handed > out 27 ids in its lifetime. So use the simpler and lighter-weight > struct tida. I'm worried that your example of your laptop isn't an all encompassing test case. How well does the simplified ida allocator work for tens of thousands of disks, at scan time? SCSI is notorious for setting up and tearing down a ton of queues at probe time. Unless we have more testing than 'it works on my laptop and saves 16k', I'm not super intereted in the patch.
On Wed, Dec 07, 2016 at 08:56:27PM -0700, Jens Axboe wrote: > On 12/07/2016 06:23 PM, Rasmus Villemoes wrote: > > A struct ida ends up costing > 16 KB of runtime memory, which is quite > > a lot for something which on my laptop as of this writing has handed > > out 27 ids in its lifetime. So use the simpler and lighter-weight > > struct tida. > > I'm worried that your example of your laptop isn't an all encompassing > test case. How well does the simplified ida allocator work for tens of > thousands of disks, at scan time? SCSI is notorious for setting up and > tearing down a ton of queues at probe time. > > Unless we have more testing than 'it works on my laptop and saves 16k', > I'm not super intereted in the patch. Rasmus, you can create 30k virtual scsi devices on your laptop to test if this really does work or saves any real time or memory. I'd recommend doing that if you want to get patches like this ever accepted. good luck! greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/block/blk-core.c b/block/blk-core.c index 14d7c0740dc0..6919a519d797 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -46,7 +46,7 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete); EXPORT_TRACEPOINT_SYMBOL_GPL(block_split); EXPORT_TRACEPOINT_SYMBOL_GPL(block_unplug); -DEFINE_IDA(blk_queue_ida); +DEFINE_TIDA(blk_queue_ida); /* * For the allocated request tables @@ -699,7 +699,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id) if (!q) return NULL; - q->id = ida_simple_get(&blk_queue_ida, 0, 0, gfp_mask); + q->id = tida_get(&blk_queue_ida, gfp_mask); if (q->id < 0) goto fail_q; @@ -771,7 +771,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id) fail_split: bioset_free(q->bio_split); fail_id: - ida_simple_remove(&blk_queue_ida, q->id); + tida_put(&blk_queue_ida, q->id); fail_q: kmem_cache_free(blk_requestq_cachep, q); return NULL; diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 9cc8d7c5439a..5a04dd6cb20d 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -652,7 +652,7 @@ static void blk_release_queue(struct kobject *kobj) if (q->bio_split) bioset_free(q->bio_split); - ida_simple_remove(&blk_queue_ida, q->id); + tida_put(&blk_queue_ida, q->id); call_rcu(&q->rcu_head, blk_free_queue_rcu); } diff --git a/block/blk.h b/block/blk.h index 74444c49078f..a0a606efed91 100644 --- a/block/blk.h +++ b/block/blk.h @@ -1,7 +1,7 @@ #ifndef BLK_INTERNAL_H #define BLK_INTERNAL_H -#include <linux/idr.h> +#include <linux/tida.h> #include <linux/blk-mq.h> #include "blk-mq.h" @@ -34,7 +34,7 @@ struct blk_flush_queue { extern struct kmem_cache *blk_requestq_cachep; extern struct kmem_cache *request_cachep; extern struct kobj_type blk_queue_ktype; -extern struct ida blk_queue_ida; +extern struct tida blk_queue_ida; static inline struct blk_flush_queue *blk_get_flush_queue( struct request_queue *q, struct blk_mq_ctx *ctx)
A struct ida ends up costing > 16 KB of runtime memory, which is quite a lot for something which on my laptop as of this writing has handed out 27 ids in its lifetime. So use the simpler and lighter-weight struct tida. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- block/blk-core.c | 6 +++--- block/blk-sysfs.c | 2 +- block/blk.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-)