Removing the percpu_ida code nets over 400 lines of removal. It's not as spectacular as deleting an entire architecture, but it's still a worthy reduction in lines of code. Untested due to lack of hardware and not understanding how to set up a target platform. Changes from v1: - Fixed bugs pointed out by Jens in iscsit_wait_for_tag() - Abstracted out tag freeing as requested by Bart - Made iscsit_wait_for_tag static as pointed out by 0day Matthew Wilcox (3): target: Abstract tag freeing Convert target drivers to use sbitmap Remove percpu_ida drivers/scsi/qla2xxx/qla_target.c | 14 +- drivers/target/iscsi/iscsi_target_util.c | 35 ++- drivers/target/sbp/sbp_target.c | 7 +- drivers/target/target_core_transport.c | 5 +- drivers/target/tcm_fc/tfc_cmd.c | 10 +- drivers/usb/gadget/function/f_tcm.c | 7 +- drivers/vhost/scsi.c | 8 +- drivers/xen/xen-scsiback.c | 9 +- include/linux/percpu_ida.h | 83 ----- include/target/iscsi/iscsi_target_core.h | 1 + include/target/target_core_base.h | 10 +- lib/Makefile | 2 +- lib/percpu_ida.c | 370 ----------------------- 13 files changed, 73 insertions(+), 488 deletions(-) delete mode 100644 include/linux/percpu_ida.h delete mode 100644 lib/percpu_ida.c -- 2.17.1
Introduce target_free_tag() and convert all drivers to use it. Signed-off-by: Matthew Wilcox <willy at infradead.org> --- drivers/scsi/qla2xxx/qla_target.c | 4 ++-- drivers/target/iscsi/iscsi_target_util.c | 2 +- drivers/target/sbp/sbp_target.c | 2 +- drivers/target/tcm_fc/tfc_cmd.c | 4 ++-- drivers/usb/gadget/function/f_tcm.c | 2 +- drivers/vhost/scsi.c | 2 +- drivers/xen/xen-scsiback.c | 4 +--- include/target/target_core_base.h | 5 +++++ 8 files changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index b85c833099ff..05290966e630 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -3783,7 +3783,7 @@ void qlt_free_cmd(struct qla_tgt_cmd *cmd) return; } cmd->jiffies_at_free = get_jiffies_64(); - percpu_ida_free(&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); + target_free_tag(sess->se_sess, &cmd->se_cmd); } EXPORT_SYMBOL(qlt_free_cmd); @@ -4146,7 +4146,7 @@ static void __qlt_do_work(struct qla_tgt_cmd *cmd) qlt_send_term_exchange(qpair, NULL, &cmd->atio, 1, 0); qlt_decr_num_pend_cmds(vha); - percpu_ida_free(&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); + target_free_tag(sess->se_sess, &cmd->se_cmd); spin_unlock_irqrestore(qpair->qp_lock_ptr, flags); spin_lock_irqsave(&ha->tgt.sess_lock, flags); diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 4435bf374d2d..7e98697cfb8e 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -711,7 +711,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); + target_free_tag(sess->se_sess, se_cmd); } EXPORT_SYMBOL(iscsit_release_cmd); diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c index fb1003921d85..679ae29d25ab 100644 --- a/drivers/target/sbp/sbp_target.c +++ b/drivers/target/sbp/sbp_target.c @@ -1460,7 +1460,7 @@ static void sbp_free_request(struct sbp_target_request *req) kfree(req->pg_tbl); kfree(req->cmd_buf); - percpu_ida_free(&se_sess->sess_tag_pool, se_cmd->map_tag); + target_free_tag(se_sess, se_cmd); } static void sbp_mgt_agent_process(struct work_struct *work) diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index ec372860106f..13e4efbe1ce7 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -92,7 +92,7 @@ static void ft_free_cmd(struct ft_cmd *cmd) if (fr_seq(fp)) fc_seq_release(fr_seq(fp)); fc_frame_free(fp); - percpu_ida_free(&sess->se_sess->sess_tag_pool, cmd->se_cmd.map_tag); + target_free_tag(sess->se_sess, &cmd->se_cmd); ft_sess_put(sess); /* undo get from lookup at recv */ } @@ -461,7 +461,7 @@ static void ft_recv_cmd(struct ft_sess *sess, struct fc_frame *fp) cmd->sess = sess; cmd->seq = fc_seq_assign(lport, fp); if (!cmd->seq) { - percpu_ida_free(&se_sess->sess_tag_pool, tag); + target_free_tag(se_sess, &cmd->se_cmd); goto busy; } cmd->req_frame = fp; /* hold frame during cmd */ diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c index d78dbb73bde8..9f670d9224b9 100644 --- a/drivers/usb/gadget/function/f_tcm.c +++ b/drivers/usb/gadget/function/f_tcm.c @@ -1288,7 +1288,7 @@ static void usbg_release_cmd(struct se_cmd *se_cmd) struct se_session *se_sess = se_cmd->se_sess; kfree(cmd->data_buf); - percpu_ida_free(&se_sess->sess_tag_pool, se_cmd->map_tag); + target_free_tag(se_sess, se_cmd); } static u32 usbg_sess_get_index(struct se_session *se_sess) diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 7ad57094d736..70d35e696533 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -324,7 +324,7 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd) } vhost_scsi_put_inflight(tv_cmd->inflight); - percpu_ida_free(&se_sess->sess_tag_pool, se_cmd->map_tag); + target_free_tag(se_sess, se_cmd); } static u32 vhost_scsi_sess_get_index(struct se_session *se_sess) diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index 7bc88fd43cfc..ec6635258ed8 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -1377,9 +1377,7 @@ static int scsiback_check_stop_free(struct se_cmd *se_cmd) static void scsiback_release_cmd(struct se_cmd *se_cmd) { - struct se_session *se_sess = se_cmd->se_sess; - - percpu_ida_free(&se_sess->sess_tag_pool, se_cmd->map_tag); + target_free_tag(se_cmd->se_sess, se_cmd); } static u32 scsiback_sess_get_index(struct se_session *se_sess) diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 922a39f45abc..260c2f3e9460 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -934,4 +934,9 @@ static inline void atomic_dec_mb(atomic_t *v) smp_mb__after_atomic(); } +static inline void target_free_tag(struct se_session *sess, struct se_cmd *cmd) +{ + percpu_ida_free(&sess->sess_tag_pool, cmd->map_tag); +} + #endif /* TARGET_CORE_BASE_H */ -- 2.17.1
The sbitmap and the percpu_ida perform essentially the same task, allocating tags for commands. The sbitmap outperforms the percpu_ida as documented here: https://lkml.org/lkml/2014/4/22/553 The sbitmap interface is a little harder to use, but being able to remove the percpu_ida code and getting better performance justifies the additional complexity. Signed-off-by: Matthew Wilcox <willy at infradead.org> Acked-by: Felipe Balbi <felipe.balbi at linux.intel.com> # f_tcm --- drivers/scsi/qla2xxx/qla_target.c | 10 ++++--- drivers/target/iscsi/iscsi_target_util.c | 33 +++++++++++++++++++++--- drivers/target/sbp/sbp_target.c | 5 ++-- drivers/target/target_core_transport.c | 5 ++-- drivers/target/tcm_fc/tfc_cmd.c | 6 ++--- drivers/usb/gadget/function/f_tcm.c | 5 ++-- drivers/vhost/scsi.c | 6 ++--- drivers/xen/xen-scsiback.c | 5 ++-- include/target/iscsi/iscsi_target_core.h | 1 + include/target/target_core_base.h | 7 ++--- 10 files changed, 59 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 05290966e630..a1725a054749 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -4277,9 +4277,9 @@ static struct qla_tgt_cmd *qlt_get_tag(scsi_qla_host_t *vha, { struct se_session *se_sess = sess->se_sess; struct qla_tgt_cmd *cmd; - int tag; + int tag, cpu; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) return NULL; @@ -4292,6 +4292,7 @@ static struct qla_tgt_cmd *qlt_get_tag(scsi_qla_host_t *vha, qlt_incr_num_pend_cmds(vha); cmd->vha = vha; cmd->se_cmd.map_tag = tag; + cmd->se_cmd.map_cpu = cpu; cmd->sess = sess; cmd->loop_id = sess->loop_id; cmd->conf_compl_supported = sess->conf_compl_supported; @@ -5294,7 +5295,7 @@ qlt_alloc_qfull_cmd(struct scsi_qla_host *vha, struct fc_port *sess; struct se_session *se_sess; struct qla_tgt_cmd *cmd; - int tag; + int tag, cpu; unsigned long flags; if (unlikely(tgt->tgt_stop)) { @@ -5326,7 +5327,7 @@ qlt_alloc_qfull_cmd(struct scsi_qla_host *vha, se_sess = sess->se_sess; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) return; @@ -5357,6 +5358,7 @@ qlt_alloc_qfull_cmd(struct scsi_qla_host *vha, cmd->reset_count = ha->base_qpair->chip_reset; cmd->q_full = 1; cmd->qpair = ha->base_qpair; + cmd->se_cmd.map_cpu = cpu; if (qfull) { cmd->q_full = 1; diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 7e98697cfb8e..8cfcf9033507 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -17,7 +17,7 @@ ******************************************************************************/ #include <linux/list.h> -#include <linux/percpu_ida.h> +#include <linux/sched/signal.h> #include <net/ipv6.h> /* ipv6_addr_equal() */ #include <scsi/scsi_tcq.h> #include <scsi/iscsi_proto.h> @@ -147,6 +147,30 @@ void iscsit_free_r2ts_from_list(struct iscsi_cmd *cmd) spin_unlock_bh(&cmd->r2t_lock); } +static int iscsit_wait_for_tag(struct se_session *se_sess, int state, int *cpup) +{ + int tag = -1; + DEFINE_WAIT(wait); + struct sbq_wait_state *ws; + + if (state == TASK_RUNNING) + return tag; + + ws = &se_sess->sess_tag_pool.ws[0]; + for (;;) { + prepare_to_wait_exclusive(&ws->wait, &wait, state); + if (signal_pending_state(state, current)) + break; + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, cpup); + if (tag >= 0) + break; + schedule(); + } + + finish_wait(&ws->wait, &wait); + return tag; +} + /* * May be called from software interrupt (timer) context for allocating * iSCSI NopINs. @@ -155,9 +179,11 @@ struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *conn, int state) { struct iscsi_cmd *cmd; struct se_session *se_sess = conn->sess->se_sess; - int size, tag; + int size, tag, cpu; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, state); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); + if (tag < 0) + tag = iscsit_wait_for_tag(se_sess, state, &cpu); if (tag < 0) return NULL; @@ -166,6 +192,7 @@ struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *conn, int state) memset(cmd, 0, size); cmd->se_cmd.map_tag = tag; + cmd->se_cmd.map_cpu = cpu; cmd->conn = conn; cmd->data_direction = DMA_NONE; INIT_LIST_HEAD(&cmd->i_conn_node); diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c index 679ae29d25ab..42b21f2ac8b0 100644 --- a/drivers/target/sbp/sbp_target.c +++ b/drivers/target/sbp/sbp_target.c @@ -926,15 +926,16 @@ static struct sbp_target_request *sbp_mgt_get_req(struct sbp_session *sess, { struct se_session *se_sess = sess->se_sess; struct sbp_target_request *req; - int tag; + int tag, cpu; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) return ERR_PTR(-ENOMEM); req = &((struct sbp_target_request *)se_sess->sess_cmd_map)[tag]; memset(req, 0, sizeof(*req)); req->se_cmd.map_tag = tag; + req->se_cmd.map_cpu = cpu; req->se_cmd.tag = next_orb; return req; diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index f0e8f0f4ccb4..18c53c5cdd3d 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -260,7 +260,8 @@ int transport_alloc_session_tags(struct se_session *se_sess, } } - rc = percpu_ida_init(&se_sess->sess_tag_pool, tag_num); + rc = sbitmap_queue_init_node(&se_sess->sess_tag_pool, tag_num, -1, + false, GFP_KERNEL, NUMA_NO_NODE); if (rc < 0) { pr_err("Unable to init se_sess->sess_tag_pool," " tag_num: %u\n", tag_num); @@ -547,7 +548,7 @@ void transport_free_session(struct se_session *se_sess) target_put_nacl(se_nacl); } if (se_sess->sess_cmd_map) { - percpu_ida_destroy(&se_sess->sess_tag_pool); + sbitmap_queue_free(&se_sess->sess_tag_pool); kvfree(se_sess->sess_cmd_map); } kmem_cache_free(se_sess_cache, se_sess); diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c index 13e4efbe1ce7..a183d4da7db2 100644 --- a/drivers/target/tcm_fc/tfc_cmd.c +++ b/drivers/target/tcm_fc/tfc_cmd.c @@ -28,7 +28,6 @@ #include <linux/configfs.h> #include <linux/ctype.h> #include <linux/hash.h> -#include <linux/percpu_ida.h> #include <asm/unaligned.h> #include <scsi/scsi_tcq.h> #include <scsi/libfc.h> @@ -448,9 +447,9 @@ static void ft_recv_cmd(struct ft_sess *sess, struct fc_frame *fp) struct ft_cmd *cmd; struct fc_lport *lport = sess->tport->lport; struct se_session *se_sess = sess->se_sess; - int tag; + int tag, cpu; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) goto busy; @@ -458,6 +457,7 @@ static void ft_recv_cmd(struct ft_sess *sess, struct fc_frame *fp) memset(cmd, 0, sizeof(struct ft_cmd)); cmd->se_cmd.map_tag = tag; + cmd->se_cmd.map_cpu = cpu; cmd->sess = sess; cmd->seq = fc_seq_assign(lport, fp); if (!cmd->seq) { diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c index 9f670d9224b9..5003e857dce7 100644 --- a/drivers/usb/gadget/function/f_tcm.c +++ b/drivers/usb/gadget/function/f_tcm.c @@ -1071,15 +1071,16 @@ static struct usbg_cmd *usbg_get_cmd(struct f_uas *fu, { struct se_session *se_sess = tv_nexus->tvn_se_sess; struct usbg_cmd *cmd; - int tag; + int tag, cpu; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) return ERR_PTR(-ENOMEM); cmd = &((struct usbg_cmd *)se_sess->sess_cmd_map)[tag]; memset(cmd, 0, sizeof(*cmd)); cmd->se_cmd.map_tag = tag; + cmd->se_cmd.map_cpu = cpu; cmd->se_cmd.tag = cmd->tag = scsi_tag; cmd->fu = fu; diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index 70d35e696533..c9c5d6b291cc 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -46,7 +46,6 @@ #include <linux/virtio_scsi.h> #include <linux/llist.h> #include <linux/bitmap.h> -#include <linux/percpu_ida.h> #include "vhost.h" @@ -567,7 +566,7 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, struct se_session *se_sess; struct scatterlist *sg, *prot_sg; struct page **pages; - int tag; + int tag, cpu; tv_nexus = tpg->tpg_nexus; if (!tv_nexus) { @@ -576,7 +575,7 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, } se_sess = tv_nexus->tvn_se_sess; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) { pr_err("Unable to obtain tag for vhost_scsi_cmd\n"); return ERR_PTR(-ENOMEM); @@ -591,6 +590,7 @@ vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg, cmd->tvc_prot_sgl = prot_sg; cmd->tvc_upages = pages; cmd->tvc_se_cmd.map_tag = tag; + cmd->tvc_se_cmd.map_cpu = cpu; cmd->tvc_tag = scsi_tag; cmd->tvc_lun = lun; cmd->tvc_task_attr = task_attr; diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c index ec6635258ed8..764dd9aa0131 100644 --- a/drivers/xen/xen-scsiback.c +++ b/drivers/xen/xen-scsiback.c @@ -654,9 +654,9 @@ static struct vscsibk_pend *scsiback_get_pend_req(struct vscsiif_back_ring *ring struct scsiback_nexus *nexus = tpg->tpg_nexus; struct se_session *se_sess = nexus->tvn_se_sess; struct vscsibk_pend *req; - int tag, i; + int tag, cpu, i; - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, TASK_RUNNING); + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu); if (tag < 0) { pr_err("Unable to obtain tag for vscsiif_request\n"); return ERR_PTR(-ENOMEM); @@ -665,6 +665,7 @@ static struct vscsibk_pend *scsiback_get_pend_req(struct vscsiif_back_ring *ring req = &((struct vscsibk_pend *)se_sess->sess_cmd_map)[tag]; memset(req, 0, sizeof(*req)); req->se_cmd.map_tag = tag; + req->se_cmd.map_cpu = cpu; for (i = 0; i < VSCSI_MAX_GRANTS; i++) req->grant_handles[i] = SCSIBACK_INVALID_HANDLE; diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h index cf5f3fff1f1a..f2e6abea8490 100644 --- a/include/target/iscsi/iscsi_target_core.h +++ b/include/target/iscsi/iscsi_target_core.h @@ -4,6 +4,7 @@ #include <linux/dma-direction.h> /* enum dma_data_direction */ #include <linux/list.h> /* struct list_head */ +#include <linux/sched.h> #include <linux/socket.h> /* struct sockaddr_storage */ #include <linux/types.h> /* u8 */ #include <scsi/iscsi_proto.h> /* itt_t */ diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 260c2f3e9460..448f291125c2 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -4,7 +4,7 @@ #include <linux/configfs.h> /* struct config_group */ #include <linux/dma-direction.h> /* enum dma_data_direction */ -#include <linux/percpu_ida.h> /* struct percpu_ida */ +#include <linux/sbitmap.h> #include <linux/percpu-refcount.h> #include <linux/semaphore.h> /* struct semaphore */ #include <linux/completion.h> @@ -455,6 +455,7 @@ struct se_cmd { int sam_task_attr; /* Used for se_sess->sess_tag_pool */ unsigned int map_tag; + int map_cpu; /* Transport protocol dependent state, see transport_state_table */ enum transport_state_table t_state; /* See se_cmd_flags_table */ @@ -608,7 +609,7 @@ struct se_session { struct list_head sess_wait_list; spinlock_t sess_cmd_lock; void *sess_cmd_map; - struct percpu_ida sess_tag_pool; + struct sbitmap_queue sess_tag_pool; }; struct se_device; @@ -936,7 +937,7 @@ static inline void atomic_dec_mb(atomic_t *v) static inline void target_free_tag(struct se_session *sess, struct se_cmd *cmd) { - percpu_ida_free(&sess->sess_tag_pool, cmd->map_tag); + sbitmap_queue_clear(&sess->sess_tag_pool, cmd->map_tag, cmd->map_cpu); } #endif /* TARGET_CORE_BASE_H */ -- 2.17.1
With its one user gone, remove the library code. Signed-off-by: Matthew Wilcox <willy at infradead.org> --- include/linux/percpu_ida.h | 83 --------- lib/Makefile | 2 +- lib/percpu_ida.c | 370 ------------------------------------- 3 files changed, 1 insertion(+), 454 deletions(-) delete mode 100644 include/linux/percpu_ida.h delete mode 100644 lib/percpu_ida.c diff --git a/include/linux/percpu_ida.h b/include/linux/percpu_ida.h deleted file mode 100644 index 07d78e4653bc..000000000000 --- a/include/linux/percpu_ida.h +++ /dev/null @@ -1,83 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef __PERCPU_IDA_H__ -#define __PERCPU_IDA_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_ida_cpu; - -struct percpu_ida { - /* - * number of tags available to be allocated, as passed to - * percpu_ida_init() - */ - unsigned nr_tags; - unsigned percpu_max_size; - unsigned percpu_batch_size; - - struct percpu_ida_cpu __percpu *tag_cpu; - - /* - * Bitmap of cpus that (may) have tags on their percpu freelists: - * steal_tags() uses this to decide when to steal tags, and which cpus - * to try stealing from. - * - * It's ok for a freelist to be empty when its bit is set - steal_tags() - * will just keep looking - but the bitmap _must_ be set whenever a - * percpu freelist does have tags. - */ - cpumask_t cpus_have_tags; - - struct { - spinlock_t lock; - /* - * When we go to steal tags from another cpu (see steal_tags()), - * we want to pick a cpu at random. Cycling through them every - * time we steal is a bit easier and more or less equivalent: - */ - unsigned cpu_last_stolen; - - /* For sleeping on allocation failure */ - wait_queue_head_t wait; - - /* - * Global freelist - it's a stack where nr_free points to the - * top - */ - unsigned nr_free; - unsigned *freelist; - } ____cacheline_aligned_in_smp; -}; - -/* - * Number of tags we move between the percpu freelist and the global freelist at - * a time - */ -#define IDA_DEFAULT_PCPU_BATCH_MOVE 32U -/* Max size of percpu freelist, */ -#define IDA_DEFAULT_PCPU_SIZE ((IDA_DEFAULT_PCPU_BATCH_MOVE * 3) / 2) - -int percpu_ida_alloc(struct percpu_ida *pool, int state); -void percpu_ida_free(struct percpu_ida *pool, unsigned tag); - -void percpu_ida_destroy(struct percpu_ida *pool); -int __percpu_ida_init(struct percpu_ida *pool, unsigned long nr_tags, - unsigned long max_size, unsigned long batch_size); -static inline int percpu_ida_init(struct percpu_ida *pool, unsigned long nr_tags) -{ - return __percpu_ida_init(pool, nr_tags, IDA_DEFAULT_PCPU_SIZE, - IDA_DEFAULT_PCPU_BATCH_MOVE); -} - -typedef int (*percpu_ida_cb)(unsigned, void *); -int percpu_ida_for_each_free(struct percpu_ida *pool, percpu_ida_cb fn, - void *data); - -unsigned percpu_ida_free_tags(struct percpu_ida *pool, int cpu); -#endif /* __PERCPU_IDA_H__ */ diff --git a/lib/Makefile b/lib/Makefile index 84c6dcb31fbb..f4722a7fa62c 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -40,7 +40,7 @@ obj-y += bcd.o div64.o sort.o parser.o debug_locks.o random32.o \ bust_spinlocks.o kasprintf.o bitmap.o scatterlist.o \ gcd.o lcm.o list_sort.o uuid.o flex_array.o iov_iter.o clz_ctz.o \ bsearch.o find_bit.o llist.o memweight.o kfifo.o \ - percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o \ + percpu-refcount.o rhashtable.o reciprocal_div.o \ once.o refcount.o usercopy.o errseq.o bucket_locks.o obj-$(CONFIG_STRING_SELFTEST) += test_string.o obj-y += string_helpers.o diff --git a/lib/percpu_ida.c b/lib/percpu_ida.c deleted file mode 100644 index 9bbd9c5d375a..000000000000 --- a/lib/percpu_ida.c +++ /dev/null @@ -1,370 +0,0 @@ -/* - * Percpu IDA library - * - * Copyright (C) 2013 Datera, Inc. Kent Overstreet - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2, or (at - * your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - */ - -#include <linux/mm.h> -#include <linux/bitmap.h> -#include <linux/bitops.h> -#include <linux/bug.h> -#include <linux/err.h> -#include <linux/export.h> -#include <linux/init.h> -#include <linux/kernel.h> -#include <linux/percpu.h> -#include <linux/sched/signal.h> -#include <linux/string.h> -#include <linux/spinlock.h> -#include <linux/percpu_ida.h> - -struct percpu_ida_cpu { - /* - * Even though this is percpu, we need a lock for tag stealing by remote - * CPUs: - */ - spinlock_t lock; - - /* nr_free/freelist form a stack of free IDs */ - unsigned nr_free; - unsigned freelist[]; -}; - -static inline void move_tags(unsigned *dst, unsigned *dst_nr, - unsigned *src, unsigned *src_nr, - unsigned nr) -{ - *src_nr -= nr; - memcpy(dst + *dst_nr, src + *src_nr, sizeof(unsigned) * nr); - *dst_nr += nr; -} - -/* - * Try to steal tags from a remote cpu's percpu freelist. - * - * We first check how many percpu freelists have tags - * - * Then we iterate through the cpus until we find some tags - we don't attempt - * to find the "best" cpu to steal from, to keep cacheline bouncing to a - * minimum. - */ -static inline void steal_tags(struct percpu_ida *pool, - struct percpu_ida_cpu *tags) -{ - unsigned cpus_have_tags, cpu = pool->cpu_last_stolen; - struct percpu_ida_cpu *remote; - - for (cpus_have_tags = cpumask_weight(&pool->cpus_have_tags); - cpus_have_tags; cpus_have_tags--) { - cpu = cpumask_next(cpu, &pool->cpus_have_tags); - - if (cpu >= nr_cpu_ids) { - cpu = cpumask_first(&pool->cpus_have_tags); - if (cpu >= nr_cpu_ids) - BUG(); - } - - pool->cpu_last_stolen = cpu; - remote = per_cpu_ptr(pool->tag_cpu, cpu); - - cpumask_clear_cpu(cpu, &pool->cpus_have_tags); - - if (remote == tags) - continue; - - spin_lock(&remote->lock); - - if (remote->nr_free) { - memcpy(tags->freelist, - remote->freelist, - sizeof(unsigned) * remote->nr_free); - - tags->nr_free = remote->nr_free; - remote->nr_free = 0; - } - - spin_unlock(&remote->lock); - - if (tags->nr_free) - break; - } -} - -/* - * Pop up to IDA_PCPU_BATCH_MOVE IDs off the global freelist, and push them onto - * our percpu freelist: - */ -static inline void alloc_global_tags(struct percpu_ida *pool, - struct percpu_ida_cpu *tags) -{ - move_tags(tags->freelist, &tags->nr_free, - pool->freelist, &pool->nr_free, - min(pool->nr_free, pool->percpu_batch_size)); -} - -/** - * percpu_ida_alloc - allocate a tag - * @pool: pool to allocate from - * @state: task state for prepare_to_wait - * - * Returns a tag - an integer in the range [0..nr_tags) (passed to - * tag_pool_init()), or otherwise -ENOSPC on allocation failure. - * - * Safe to be called from interrupt context (assuming it isn't passed - * TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE, of course). - * - * @gfp indicates whether or not to wait until a free id is available (it's not - * used for internal memory allocations); thus if passed __GFP_RECLAIM we may sleep - * however long it takes until another thread frees an id (same semantics as a - * mempool). - * - * Will not fail if passed TASK_UNINTERRUPTIBLE | TASK_INTERRUPTIBLE. - */ -int percpu_ida_alloc(struct percpu_ida *pool, int state) -{ - DEFINE_WAIT(wait); - struct percpu_ida_cpu *tags; - unsigned long flags; - int tag = -ENOSPC; - - tags = raw_cpu_ptr(pool->tag_cpu); - spin_lock_irqsave(&tags->lock, flags); - - /* Fastpath */ - if (likely(tags->nr_free >= 0)) { - tag = tags->freelist[--tags->nr_free]; - spin_unlock_irqrestore(&tags->lock, flags); - return tag; - } - spin_unlock_irqrestore(&tags->lock, flags); - - while (1) { - spin_lock_irqsave(&pool->lock, flags); - tags = this_cpu_ptr(pool->tag_cpu); - - /* - * prepare_to_wait() must come before steal_tags(), in case - * percpu_ida_free() on another cpu flips a bit in - * cpus_have_tags - * - * global lock held and irqs disabled, don't need percpu lock - */ - if (state != TASK_RUNNING) - prepare_to_wait(&pool->wait, &wait, state); - - if (!tags->nr_free) - alloc_global_tags(pool, tags); - if (!tags->nr_free) - steal_tags(pool, tags); - - if (tags->nr_free) { - tag = tags->freelist[--tags->nr_free]; - if (tags->nr_free) - cpumask_set_cpu(smp_processor_id(), - &pool->cpus_have_tags); - } - - spin_unlock_irqrestore(&pool->lock, flags); - - if (tag >= 0 || state == TASK_RUNNING) - break; - - if (signal_pending_state(state, current)) { - tag = -ERESTARTSYS; - break; - } - - schedule(); - } - if (state != TASK_RUNNING) - finish_wait(&pool->wait, &wait); - - return tag; -} -EXPORT_SYMBOL_GPL(percpu_ida_alloc); - -/** - * percpu_ida_free - free a tag - * @pool: pool @tag was allocated from - * @tag: a tag previously allocated with percpu_ida_alloc() - * - * Safe to be called from interrupt context. - */ -void percpu_ida_free(struct percpu_ida *pool, unsigned tag) -{ - struct percpu_ida_cpu *tags; - unsigned long flags; - unsigned nr_free; - - BUG_ON(tag >= pool->nr_tags); - - tags = raw_cpu_ptr(pool->tag_cpu); - - spin_lock_irqsave(&tags->lock, flags); - tags->freelist[tags->nr_free++] = tag; - - nr_free = tags->nr_free; - - if (nr_free == 1) { - cpumask_set_cpu(smp_processor_id(), - &pool->cpus_have_tags); - wake_up(&pool->wait); - } - spin_unlock_irqrestore(&tags->lock, flags); - - if (nr_free == pool->percpu_max_size) { - spin_lock_irqsave(&pool->lock, flags); - spin_lock(&tags->lock); - - if (tags->nr_free == pool->percpu_max_size) { - move_tags(pool->freelist, &pool->nr_free, - tags->freelist, &tags->nr_free, - pool->percpu_batch_size); - - wake_up(&pool->wait); - } - spin_unlock(&tags->lock); - spin_unlock_irqrestore(&pool->lock, flags); - } -} -EXPORT_SYMBOL_GPL(percpu_ida_free); - -/** - * percpu_ida_destroy - release a tag pool's resources - * @pool: pool to free - * - * Frees the resources allocated by percpu_ida_init(). - */ -void percpu_ida_destroy(struct percpu_ida *pool) -{ - free_percpu(pool->tag_cpu); - free_pages((unsigned long) pool->freelist, - get_order(pool->nr_tags * sizeof(unsigned))); -} -EXPORT_SYMBOL_GPL(percpu_ida_destroy); - -/** - * percpu_ida_init - initialize a percpu tag pool - * @pool: pool to initialize - * @nr_tags: number of tags that will be available for allocation - * - * Initializes @pool so that it can be used to allocate tags - integers in the - * range [0, nr_tags). Typically, they'll be used by driver code to refer to a - * preallocated array of tag structures. - * - * Allocation is percpu, but sharding is limited by nr_tags - for best - * performance, the workload should not span more cpus than nr_tags / 128. - */ -int __percpu_ida_init(struct percpu_ida *pool, unsigned long nr_tags, - unsigned long max_size, unsigned long batch_size) -{ - unsigned i, cpu, order; - - memset(pool, 0, sizeof(*pool)); - - init_waitqueue_head(&pool->wait); - spin_lock_init(&pool->lock); - pool->nr_tags = nr_tags; - pool->percpu_max_size = max_size; - pool->percpu_batch_size = batch_size; - - /* Guard against overflow */ - if (nr_tags > (unsigned) INT_MAX + 1) { - pr_err("percpu_ida_init(): nr_tags too large\n"); - return -EINVAL; - } - - order = get_order(nr_tags * sizeof(unsigned)); - pool->freelist = (void *) __get_free_pages(GFP_KERNEL, order); - if (!pool->freelist) - return -ENOMEM; - - for (i = 0; i < nr_tags; i++) - pool->freelist[i] = i; - - pool->nr_free = nr_tags; - - pool->tag_cpu = __alloc_percpu(sizeof(struct percpu_ida_cpu) + - pool->percpu_max_size * sizeof(unsigned), - sizeof(unsigned)); - if (!pool->tag_cpu) - goto err; - - for_each_possible_cpu(cpu) - spin_lock_init(&per_cpu_ptr(pool->tag_cpu, cpu)->lock); - - return 0; -err: - percpu_ida_destroy(pool); - return -ENOMEM; -} -EXPORT_SYMBOL_GPL(__percpu_ida_init); - -/** - * percpu_ida_for_each_free - iterate free ids of a pool - * @pool: pool to iterate - * @fn: interate callback function - * @data: parameter for @fn - * - * Note, this doesn't guarantee to iterate all free ids restrictly. Some free - * ids might be missed, some might be iterated duplicated, and some might - * be iterated and not free soon. - */ -int percpu_ida_for_each_free(struct percpu_ida *pool, percpu_ida_cb fn, - void *data) -{ - unsigned long flags; - struct percpu_ida_cpu *remote; - unsigned cpu, i, err = 0; - - for_each_possible_cpu(cpu) { - remote = per_cpu_ptr(pool->tag_cpu, cpu); - spin_lock_irqsave(&remote->lock, flags); - for (i = 0; i < remote->nr_free; i++) { - err = fn(remote->freelist[i], data); - if (err) - break; - } - spin_unlock_irqrestore(&remote->lock, flags); - if (err) - goto out; - } - - spin_lock_irqsave(&pool->lock, flags); - for (i = 0; i < pool->nr_free; i++) { - err = fn(pool->freelist[i], data); - if (err) - break; - } - spin_unlock_irqrestore(&pool->lock, flags); -out: - return err; -} -EXPORT_SYMBOL_GPL(percpu_ida_for_each_free); - -/** - * percpu_ida_free_tags - return free tags number of a specific cpu or global pool - * @pool: pool related - * @cpu: specific cpu or global pool if @cpu == nr_cpu_ids - * - * Note: this just returns a snapshot of free tags number. - */ -unsigned percpu_ida_free_tags(struct percpu_ida *pool, int cpu) -{ - struct percpu_ida_cpu *remote; - if (cpu == nr_cpu_ids) - return pool->nr_free; - remote = per_cpu_ptr(pool->tag_cpu, cpu); - return remote->nr_free; -} -EXPORT_SYMBOL_GPL(percpu_ida_free_tags); -- 2.17.1
On Tue, 2018-06-12 at 12:05 -0700, Matthew Wilcox wrote:> Removing the percpu_ida code nets over 400 lines of removal. It's not > as spectacular as deleting an entire architecture, but it's still a > worthy reduction in lines of code. > > Untested due to lack of hardware and not understanding how to set up a > target platform. > > Changes from v1: > - Fixed bugs pointed out by Jens in iscsit_wait_for_tag() > - Abstracted out tag freeing as requested by Bart > - Made iscsit_wait_for_tag static as pointed out by 0dayFor the whole series: Reviewed-by: Bart Van Assche <bart.vanassche at wdc.com> Thanks, Bart.
On 6/12/18 1:05 PM, Matthew Wilcox wrote:> Removing the percpu_ida code nets over 400 lines of removal. It's not > as spectacular as deleting an entire architecture, but it's still a > worthy reduction in lines of code. > > Untested due to lack of hardware and not understanding how to set up a > target platform. > > Changes from v1: > - Fixed bugs pointed out by Jens in iscsit_wait_for_tag() > - Abstracted out tag freeing as requested by Bart > - Made iscsit_wait_for_tag static as pointed out by 0dayReviewed-by: Jens Axboe <axboe at kernel.dk> -- Jens Axboe
Matthew,> Removing the percpu_ida code nets over 400 lines of removal. It's not > as spectacular as deleting an entire architecture, but it's still a > worthy reduction in lines of code.Since most of the changes are in scsi or target, should I take this series through my tree? -- Martin K. Petersen Oracle Linux Engineering
On Thu, Jun 14, 2018 at 10:06:58PM -0400, Martin K. Petersen wrote:> > Matthew, > > > Removing the percpu_ida code nets over 400 lines of removal. It's not > > as spectacular as deleting an entire architecture, but it's still a > > worthy reduction in lines of code. > > Since most of the changes are in scsi or target, should I take this > series through my tree?I'd welcome that. Nick seems to be inactive as target maintainer; his tree on kernel.org hasn't seen any updates in five months. Thanks!
Possibly Parallel Threads
- [PATCH 0/3] Use sbitmap instead of percpu_ida
- [PATCH 0/2] Use sbitmap instead of percpu_ida
- [PATCH-v3 0/4] target/vhost-scsi: Add per-cpu ida tag pre-allocation for v3.12
- [PATCH-v3 0/4] target/vhost-scsi: Add per-cpu ida tag pre-allocation for v3.12
- [PATCH-v2 0/3] target/vhost-scsi: Add per-cpu ida tag pre-allocation for v3.12