percpu_tags: Prototype implementation
Subsystems (7)
cover percpu_tags: Prototype implementation
Hello Gentleman,
This is a development of "percpu_ida" library. I named it "percpu_tags" to simplify review, since most of "percpu_ida" code has gone and a diff would not be informative.
While the concept of per-cpu arrays is is preserved, the implementation is heavily reworked. The main objective is to improve the "percpu_ida" locking scheme.
Here is the list of major differrences between "percpu_ida" and "percpu_tags":
* The global freelist has gone. As result, CPUs do not compete for the global lock.
* Long-running operatons (scanning thru a cpumask) are executed with local interrupts enabled;
* percpu_ida::percpu_max_size limit is eliminated. Instead, the limit is determined dynamically. Depending from how many CPUs are requesting tags each CPU gets a fair share of the tag space;
* A tag is attempted to return to the CPU it was allocated on. As result, it is expected the locality of data associated with the tag benefits;
The code is largely raw and untested. The reason I am posting is the prototype implementation performs 2-3 times faster than "percpu_ida", so I would like to ensure if it worth going further with this approach or is there a no-go.
The performance test is not decent, though. I used "fio" random read against a "null_blk" device sitting on top of "percpu_tags", which is not exactly how "percpu_ida" is used. This is another reason I am posting - an advice on how to properly test is very appreciated.
The source code could be found at
https://github.com/a-gordeev/linux.git percpu_tags-v0
Thanks!
Alexander Gordeev (2):
percpu_tags: Prototype implementation
percpu_tags: Use percpu_tags instead of percpu_ida
drivers/scsi/qla2xxx/qla_target.c | 6 +- drivers/target/iscsi/iscsi_target_util.c | 6 +- drivers/target/target_core_transport.c | 4 +- drivers/target/tcm_fc/tfc_cmd.c | 8 +- drivers/vhost/scsi.c | 6 +- include/linux/percpu_tags.h | 37 ++ include/target/target_core_base.h | 4 +- lib/Makefile | 2 +- lib/percpu_tags.c | 556 ++++++++++++++++++++++++++++++ 9 files changed, 611 insertions(+), 18 deletions(-) create mode 100644 include/linux/percpu_tags.h create mode 100644 lib/percpu_tags.c
Signature · 1 line
1.7.7.6
RFC PATCH 1/2 percpu_tags: Prototype implementation
This is a development of "percpu_ida" library. The major differrences between "percpu_ida" and "percpu_tags" are:
* The global freelist has gone. As result, CPUs do not compete for the global lock.
* Long-running operatons (scanning thru a cpumask) are executed with local interrupts enabled;
* percpu_ida::percpu_max_size limit is eliminated. Instead, the limit is determined dynamically. Depending from how many CPUs are requesting tags each CPU gets a fair share of the tag space;
* A tag is attempted to return to the CPU it was allocated on. As result, it is expected the locality of data associated with the tag benefits;
--- include/linux/percpu_tags.h | 37 +++ lib/Makefile | 2 +- lib/percpu_tags.c | 556 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 594 insertions(+), 1 deletions(-) create mode 100644 include/linux/percpu_tags.h create mode 100644 lib/percpu_tags.c
include/linux/percpu_tags.h new file +37 -0
@@ -0,0 +1,37 @@ +#ifndef __PERCPU_TAGS_H__ +#define __PERCPU_TAGS_H__ + +#include <linux/types.h> +#include <linux/bitops.h> +#include <linux/init.h> +#include <linux/sched.h> +#include <linux/spinlock_types.h> +#include <linux/wait.h> +#include <linux/cpumask.h> + +struct percpu_cache ; + +struct percpu_tags { +int nr_tags; +struct percpu_cache __percpu *cache; + +int *tag_cpu_map; + +cpumask_t alloc_tags; +cpumask_t free_tags; +cpumask_t wait_tags; +}; + +int percpu_tags_alloc (struct percpu_tags *pt,int state); +void percpu_tags_free (struct percpu_tags *pt,int tag); + +int percpu_tags_init (struct percpu_tags *pt,int nr_tags); +void percpu_tags_destroy (struct percpu_tags *pt); + +int percpu_tags_for_each_free (struct percpu_tags *pt, +int (*fn)(unsigned ,void *),void *data); +int percpu_tags_free_tags (struct percpu_tags *pt,int cpu); + +#define PERCPU_TAGS_BATCH_MAX 4 + +#endif /* __PERCPU_TAGS_H__ */
lib/Makefile +1 -1
@@ -26,7 +26,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \ bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \ gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \ bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \ - percpu-refcount.o percpu_ida.o hash.o + percpu-refcount.o percpu_ida.o percpu_tags.o hash.o obj-y += string_helpers.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o obj-y += kstrtox.o
lib/percpu_tags.c new file +556 -0
556 lines not on this page · raw message
Signature · 1 line
1.7.7.6
RFC PATCH 2/2 percpu_tags: Use percpu_tags instead of percpu_ida
In reply to Alexander GordeevAll users of "percpu_ida" are blindly converted to "percpu_tags" users. No testing has been conducted.
--- drivers/scsi/qla2xxx/qla_target.c | 6 +++--- drivers/target/iscsi/iscsi_target_util.c | 6 +++--- drivers/target/target_core_transport.c | 4 ++-- drivers/target/tcm_fc/tfc_cmd.c | 8 ++++---- drivers/vhost/scsi.c | 6 +++--- include/target/target_core_base.h | 4 ++-- 6 files changed, 17 insertions(+), 17 deletions(-)
drivers/scsi/qla2xxx/qla_target.c +3 -3
@@ -2729,7 +2729,7 @@ void qlt_free_cmd(struct qla_tgt_cmd *cmd)WARN_ON (1 );return ; } -percpu_ida_free (&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); +percpu_tags_free (&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); }EXPORT_SYMBOL (qlt_free_cmd);
@@ -3153,7 +3153,7 @@ out_term:*/ spin_lock_irqsave (&ha->hardware_lock, flags);qlt_send_term_exchange (vha,NULL , &cmd->atio,1 ); -percpu_ida_free (&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); +percpu_tags_free (&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); ha->tgt.tgt_ops->put_sess (sess);spin_unlock_irqrestore (&ha->hardware_lock, flags); }
@@ -3173,7 +3173,7 @@ static struct qla_tgt_cmd *qlt_get_tag(scsi_qla_host_t *vha,struct qla_tgt_cmd *cmd;int tag; - tag =percpu_ida_alloc (&se_sess->sess_tag_pool,TASK_RUNNING ); + tag =percpu_tags_alloc (&se_sess->sess_tag_pool,TASK_RUNNING );if (tag <0 )return NULL ;
drivers/target/iscsi/iscsi_target_util.c +3 -3
@@ -17,7 +17,7 @@******************************************************************************/ #include <linux/list.h> -#include <linux/percpu_ida.h> +#include <linux/percpu_tags.h> #include <scsi/scsi_tcq.h> #include <scsi/iscsi_proto.h> #include <target/target_core_base.h>
@@ -158,7 +158,7 @@ struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *conn, int state)struct se_session *se_sess = conn->sess->se_sess;int size, tag; - tag =percpu_ida_alloc (&se_sess->sess_tag_pool, state); + tag =percpu_tags_alloc (&se_sess->sess_tag_pool, state);if (tag <0 )return NULL ;
@@ -701,7 +701,7 @@ void iscsit_release_cmd(struct iscsi_cmd *cmd)kfree (cmd->iov_data);kfree (cmd->text_in_ptr); -percpu_ida_free (&sess->se_sess->sess_tag_pool, se_cmd->map_tag); +percpu_tags_free (&sess->se_sess->sess_tag_pool, se_cmd->map_tag); }EXPORT_SYMBOL (iscsit_release_cmd);
drivers/target/target_core_transport.c +2 -2
@@ -272,7 +272,7 @@ int transport_alloc_session_tags(struct se_session *se_sess, } } - rc =percpu_ida_init (&se_sess->sess_tag_pool, tag_num); + rc =percpu_tags_init (&se_sess->sess_tag_pool, tag_num);if (rc <0 ) {pr_err ("Unable to init se_sess->sess_tag_pool," " tag_num: %u\n" , tag_num);
@@ -445,7 +445,7 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);void transport_free_session (struct se_session *se_sess) {if (se_sess->sess_cmd_map) { -percpu_ida_destroy (&se_sess->sess_tag_pool); +percpu_tags_destroy (&se_sess->sess_tag_pool);if (is_vmalloc_addr (se_sess->sess_cmd_map))vfree (se_sess->sess_cmd_map);else
drivers/target/tcm_fc/tfc_cmd.c +4 -4
@@ -28,7 +28,7 @@#include <linux/configfs.h> #include <linux/ctype.h> #include <linux/hash.h> -#include <linux/percpu_ida.h> +#include <linux/percpu_tags.h> #include <asm/unaligned.h> #include <scsi/scsi.h> #include <scsi/scsi_host.h>
@@ -100,7 +100,7 @@ static void ft_free_cmd(struct ft_cmd *cmd)if (fr_seq (fp)) lport->tt.seq_release (fr_seq (fp));fc_frame_free (fp); -percpu_ida_free (&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); +percpu_tags_free (&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag);ft_sess_put (sess);/* undo get from lookup at recv */ }
@@ -456,7 +456,7 @@ static void ft_recv_cmd(struct ft_sess *sess, struct fc_frame *fp)struct se_session *se_sess = sess->se_sess;int tag; - tag =percpu_ida_alloc (&se_sess->sess_tag_pool,TASK_RUNNING ); + tag =percpu_tags_alloc (&se_sess->sess_tag_pool,TASK_RUNNING );if (tag <0 ) goto busy;
@@ -467,7 +467,7 @@ static void ft_recv_cmd(struct ft_sess *sess, struct fc_frame *fp) cmd->sess = sess; cmd->seq = lport->tt.seq_assign (lport, fp);if (!cmd->seq) { -percpu_ida_free (&se_sess->sess_tag_pool, tag); +percpu_tags_free (&se_sess->sess_tag_pool, tag); goto busy; } cmd->req_frame = fp;/* hold frame during cmd */
drivers/vhost/scsi.c +3 -3
@@ -48,7 +48,7 @@#include <linux/virtio_scsi.h> #include <linux/llist.h> #include <linux/bitmap.h> -#include <linux/percpu_ida.h> +#include <linux/percpu_tags.h> #include "vhost.h"
@@ -472,7 +472,7 @@ static void tcm_vhost_release_cmd(struct se_cmd *se_cmd) }tcm_vhost_put_inflight (tv_cmd->inflight); -percpu_ida_free (&se_sess->sess_tag_pool, se_cmd->map_tag); +percpu_tags_free (&se_sess->sess_tag_pool, se_cmd->map_tag); }static int tcm_vhost_shutdown_session (struct se_session *se_sess)
@@ -739,7 +739,7 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct tcm_vhost_tpg *tpg, } se_sess = tv_nexus->tvn_se_sess; - tag =percpu_ida_alloc (&se_sess->sess_tag_pool,TASK_RUNNING ); + tag =percpu_tags_alloc (&se_sess->sess_tag_pool,TASK_RUNNING );if (tag <0 ) {pr_err ("Unable to obtain tag for tcm_vhost_cmd\n" );return ERR_PTR (-ENOMEM );
include/target/target_core_base.h +2 -2
@@ -5,7 +5,7 @@#include <linux/configfs.h> #include <linux/dma-mapping.h> #include <linux/blkdev.h> -#include <linux/percpu_ida.h> +#include <linux/percpu_tags.h> #include <scsi/scsi_cmnd.h> #include <net/sock.h> #include <net/tcp.h>
@@ -620,7 +620,7 @@ struct se_session {spinlock_t sess_cmd_lock;struct kref sess_kref;void *sess_cmd_map; -struct percpu_ida sess_tag_pool; +struct percpu_tags sess_tag_pool; };struct se_device ;
Signature · 1 line
1.7.7.6
Quoted 5 lines · Alexander Gordeev
The performance test is not decent, though. I used "fio" random read against a "
null_blk" device sitting on top of "percpu_tags", which is not exactly how "percpu_ida" is used. This is another reason I am posting - an advice on how to properly test is very appreciated.
Hi Nicholas et al,
I expect the best possible performance test for percpu_ida/percpu_tags would be to stress drivers/vhost/scsi.c vhost_scsi_get_tag() function.
I tried to make such test by attaching ramdisk to a virtual machine (similar to https://lkml.org/lkml/2012/8/10/347) but ultimately failed to configure the necessary environment - the stock qemu does not have -vhost-scsi parameter.
Could you please advice how to make this configuration exposed to guests?
o- / ..................................................................... [...]
o- backstores .......................................................... [...]
| o- block .............................................. [Storage Objects: 0]
| o- fileio ............................................. [Storage Objects: 0]
| o- pscsi .............................................. [Storage Objects: 0]
| o- ramdisk ............................................ [Storage Objects: 1]
| o- rda .............................................. [(1.0GiB) activated]
o- iscsi ........................................................ [Targets: 0]
o- loopback ..................................................... [Targets: 0]
o- vhost ........................................................ [Targets: 1]
o- naa.5001405b171ee405 .......................................... [TPGs: 1]
o- tpg1 .............................. [naa.5001405983a5b1a4, no-gen-acls]
o- acls ...................................................... [ACLs: 0]
o- luns ...................................................... [LUNs: 1]
o- lun0 ................................................ [ramdisk/rda]Thanks!
(Responding again without gmail, as the last email hit a failure when responding to the lists..)
Quoted 21 lines · Alexander Gordeev
Quoted 5 lines · Alexander Gordeev
The performance test is not decent, though. I used "fio" random read against a "
null_blk" device sitting on top of "percpu_tags", which is not exactly how "percpu_ida" is used. This is another reason I am posting - an advice on how to properly test is very appreciated.Hi Nicholas et al,
I expect the best possible performance test for percpu_ida/percpu_tags would be to stress
drivers/vhost/scsi.cvhost_scsi_get_tag()function.I tried to make such test by attaching ramdisk to a virtual machine (similar to https://lkml.org/lkml/2012/8/10/347) but ultimately failed to configure the necessary environment - the stock qemu does not have -vhost-scsi parameter.
Could you please advice how to make this configuration exposed to guests?
o- / ..................................................................... [...]
o- backstores .......................................................... [...]
Quoted 14 lines
| o- block .............................................. [Storage Objects: 0]
| o- fileio ............................................. [Storage Objects: 0]
| o- pscsi .............................................. [Storage Objects: 0]
| o- ramdisk ............................................ [Storage Objects: 1]
| o- rda .............................................. [(1.0GiB) activated]
o- iscsi ........................................................ [Targets: 0]
o- loopback ..................................................... [Targets: 0]
o- vhost ........................................................ [Targets: 1]
o- naa.5001405b171ee405 .......................................... [TPGs: 1]
o- tpg1 .............................. [naa.5001405983a5b1a4, no-gen-acls]
o- acls ...................................................... [ACLs: 0]
o- luns ...................................................... [LUNs: 1]
o- lun0 ................................................ [ramdisk/rda]So qemu expects '-device vhost-scsi-pci' with the following syntax:
-device vhost-scsi-pci,wwpn=naa.5001405b171ee405,num_queues=1,cmd_per_lun=64
For best results I'd recommend setting the IRQ affinity for each of the virtio*_request MSI-X vectors to a dedicated vCPU in KVM guest.
Also, I've been using the scsi-mq prototype for small block I/O performance testing in order to push vhost-scsi and avoid the legacy scsi_request_fn() bottleneck(s) with virtio-scsi, and now that hch's scsi-mq work (CC'ed) has been merged upstream in v3.17-rc0, it would be a good time for a scsi-mq + virtio-scsi + vhost-scsi performance checkpoint. ;)
--nab
Quoted 47 lines · Nicholas A. Bellinger
(Responding again without gmail, as the last email hit a failure when responding to the lists..)
Quoted 21 lines · Alexander Gordeev
Quoted 5 lines · Alexander Gordeev
The performance test is not decent, though. I used "fio" random read against a "
null_blk" device sitting on top of "percpu_tags", which is not exactly how "percpu_ida" is used. This is another reason I am posting - an advice on how to properly test is very appreciated.Hi Nicholas et al,
I expect the best possible performance test for percpu_ida/percpu_tags would be to stress
drivers/vhost/scsi.cvhost_scsi_get_tag()function.I tried to make such test by attaching ramdisk to a virtual machine (similar to https://lkml.org/lkml/2012/8/10/347) but ultimately failed to configure the necessary environment - the stock qemu does not have -vhost-scsi parameter.
Could you please advice how to make this configuration exposed to guests?
o- / ..................................................................... [...]
o- backstores .......................................................... [...]Quoted 14 lines
| o- block .............................................. [Storage Objects: 0] | o- fileio ............................................. [Storage Objects: 0] | o- pscsi .............................................. [Storage Objects: 0] | o- ramdisk ............................................ [Storage Objects: 1] | o- rda .............................................. [(1.0GiB) activated] o- iscsi ........................................................ [Targets: 0] o- loopback ..................................................... [Targets: 0] o- vhost ........................................................ [Targets: 1] o- naa.5001405b171ee405 .......................................... [TPGs: 1] o- tpg1 .............................. [naa.5001405983a5b1a4, no-gen-acls] o- acls ...................................................... [ACLs: 0] o- luns ...................................................... [LUNs: 1] o- lun0 ................................................ [ramdisk/rda]So qemu expects '-device vhost-scsi-pci' with the following syntax:
-device vhost-scsi-pci,wwpn=naa.5001405b171ee405,num_queues=1,cmd_per_lun=64For best results I'd recommend setting the IRQ affinity for each of the virtio*_request MSI-X vectors to a dedicated vCPU in KVM guest.
Quoted 7 lines
Also, I've been using the scsi-mq prototype for small block I/O performance testing in order to push vhost-scsi and avoid the legacy
scsi_request_fn()bottleneck(s) with virtio-scsi, and now that hch's scsi-mq work (CC'ed) has been merged upstream in v3.17-rc0, it would be a good time for a scsi-mq + virtio-scsi + vhost-scsi performance checkpoint. ;)
Oh yeah, for performance testing you'll want to use the rd_nullio=1 flag when creating RAMDISK backends, to avoid the normal fast-path memcpys between ramdisk + per I/O descriptor memory.
--nab