* [PATCH v5 0/9] infiniband: Remove semaphores
@ 2016-11-21 6:08 ` Binoy Jayan
0 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
Hi,
These are a set of patches [v5] which removes semaphores from infiniband.
These are part of a bigger effort to eliminate all semaphores from the
linux kernel.
v4 --> v5
---------
IB/isert: Replace semaphore sem with completion
- Modified changelog to support use of completion
IB/mlx5: Simplify completion into a wait_event
- Avoid this patch.
As umr_context is on the stack, and we are waiting
for it to be fully done, it really should be a completion.
v3 -> v4:
---------
IB/mlx5: Added patch - Replace semaphore umr_common:sem with wait_event
IB/mlx5: Fixed a bug pointed out by Leon Romanovsky
v2 -> v3:
---------
IB/mlx5: Move '&umr_context' into helper fn
IB/mthca: Restructure mthca_cmd.c to manage free_head
IB/hns: Restructure hns_roce_cmd.c to manage free_head
IB/core: Convert completion to wait_event
IB/mlx5: Simplify completion into a wait_event
v1 -> v2:
---------
IB/hns : Use wait_event instead of open coding counting semaphores
IB/mthca : Use wait_event instead of open coding counting semaphores
IB/mthca : Remove mutex_[un]lock from *_cmd_use_events/*_cmd_use_polling
IB/mlx5 : Cleanup, add helper mlx5_ib_post_send_wait
v1
---------
IB/core: iwpm_nlmsg_request: Replace semaphore with completion
IB/core: Replace semaphore sm_sem with completion
IB/hns: Replace semaphore poll_sem with mutex
IB/mthca: Replace semaphore poll_sem with mutex
IB/isert: Replace semaphore sem with completion
IB/hns: Replace counting semaphore event_sem with wait condition
IB/mthca: Replace counting semaphore event_sem with wait condition
IB/mlx5: Replace counting semaphore sem with wait condition
Thanks,
Binoy
Binoy Jayan (9):
IB/core: iwpm_nlmsg_request: Replace semaphore with completion
IB/core: Replace semaphore sm_sem with an atomic wait
IB/hns: Replace semaphore poll_sem with mutex
IB/mthca: Replace semaphore poll_sem with mutex
IB/isert: Replace semaphore sem with completion
IB/hns: Replace counting semaphore event_sem with wait_event
IB/mthca: Replace counting semaphore event_sem with wait_event
IB/mlx5: Add helper mlx5_ib_post_send_wait
IB/mlx5: Replace semaphore umr_common:sem with wait_event
drivers/infiniband/core/iwpm_msg.c | 8 +-
drivers/infiniband/core/iwpm_util.c | 7 +-
drivers/infiniband/core/iwpm_util.h | 3 +-
drivers/infiniband/core/user_mad.c | 20 +++--
drivers/infiniband/hw/hns/hns_roce_cmd.c | 57 +++++++++-----
drivers/infiniband/hw/hns/hns_roce_device.h | 5 +-
drivers/infiniband/hw/mlx5/main.c | 6 +-
drivers/infiniband/hw/mlx5/mlx5_ib.h | 7 +-
drivers/infiniband/hw/mlx5/mr.c | 117 ++++++++--------------------
drivers/infiniband/hw/mthca/mthca_cmd.c | 57 ++++++++------
drivers/infiniband/hw/mthca/mthca_cmd.h | 1 +
drivers/infiniband/hw/mthca/mthca_dev.h | 5 +-
drivers/infiniband/ulp/isert/ib_isert.c | 6 +-
drivers/infiniband/ulp/isert/ib_isert.h | 3 +-
14 files changed, 147 insertions(+), 155 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 1/9] IB/core: iwpm_nlmsg_request: Replace semaphore with completion
2016-11-21 6:08 ` Binoy Jayan
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
Semaphore sem in iwpm_nlmsg_request is used as completion, so
convert it to a struct completion type. Semaphores are going
away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/core/iwpm_msg.c | 8 ++++----
drivers/infiniband/core/iwpm_util.c | 7 +++----
drivers/infiniband/core/iwpm_util.h | 3 ++-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/core/iwpm_msg.c b/drivers/infiniband/core/iwpm_msg.c
index 1c41b95..761358f 100644
--- a/drivers/infiniband/core/iwpm_msg.c
+++ b/drivers/infiniband/core/iwpm_msg.c
@@ -394,7 +394,7 @@ int iwpm_register_pid_cb(struct sk_buff *skb, struct netlink_callback *cb)
/* always for found nlmsg_request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_register_pid_cb);
@@ -463,7 +463,7 @@ int iwpm_add_mapping_cb(struct sk_buff *skb, struct netlink_callback *cb)
/* always for found request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_add_mapping_cb);
@@ -555,7 +555,7 @@ int iwpm_add_and_query_mapping_cb(struct sk_buff *skb,
/* always for found request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_add_and_query_mapping_cb);
@@ -749,7 +749,7 @@ int iwpm_mapping_error_cb(struct sk_buff *skb, struct netlink_callback *cb)
/* always for found request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_mapping_error_cb);
diff --git a/drivers/infiniband/core/iwpm_util.c b/drivers/infiniband/core/iwpm_util.c
index ade71e7..08ddd2e 100644
--- a/drivers/infiniband/core/iwpm_util.c
+++ b/drivers/infiniband/core/iwpm_util.c
@@ -323,8 +323,7 @@ struct iwpm_nlmsg_request *iwpm_get_nlmsg_request(__u32 nlmsg_seq,
nlmsg_request->nl_client = nl_client;
nlmsg_request->request_done = 0;
nlmsg_request->err_code = 0;
- sema_init(&nlmsg_request->sem, 1);
- down(&nlmsg_request->sem);
+ init_completion(&nlmsg_request->comp);
return nlmsg_request;
}
@@ -368,8 +367,8 @@ int iwpm_wait_complete_req(struct iwpm_nlmsg_request *nlmsg_request)
{
int ret;
- ret = down_timeout(&nlmsg_request->sem, IWPM_NL_TIMEOUT);
- if (ret) {
+ ret = wait_for_completion_timeout(&nlmsg_request->comp, IWPM_NL_TIMEOUT);
+ if (!ret) {
ret = -EINVAL;
pr_info("%s: Timeout %d sec for netlink request (seq = %u)\n",
__func__, (IWPM_NL_TIMEOUT/HZ), nlmsg_request->nlmsg_seq);
diff --git a/drivers/infiniband/core/iwpm_util.h b/drivers/infiniband/core/iwpm_util.h
index af1fc14..ea6c299 100644
--- a/drivers/infiniband/core/iwpm_util.h
+++ b/drivers/infiniband/core/iwpm_util.h
@@ -43,6 +43,7 @@
#include <linux/delay.h>
#include <linux/workqueue.h>
#include <linux/mutex.h>
+#include <linux/completion.h>
#include <linux/jhash.h>
#include <linux/kref.h>
#include <net/netlink.h>
@@ -69,7 +70,7 @@ struct iwpm_nlmsg_request {
u8 nl_client;
u8 request_done;
u16 err_code;
- struct semaphore sem;
+ struct completion comp;
struct kref kref;
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 2/9] IB/core: Replace semaphore sm_sem with an atomic wait
2016-11-21 6:08 ` Binoy Jayan
(?)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
[not found] ` <CA+55aFxGjaqduhRCyk0mVxEA7aqQ-omdG8SBreZ=x5cW2ovngQ@mail.gmail.com>
-1 siblings, 1 reply; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
The semaphore 'sm_sem' is used for an exclusive ownership of the device
so model the same as an atomic variable with an associated wait_event.
Semaphores are going away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/core/user_mad.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 415a318..6101c0a 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -67,6 +67,8 @@ enum {
IB_UMAD_MINOR_BASE = 0
};
+#define UMAD_F_CLAIM 0x01
+
/*
* Our lifetime rules for these structs are the following:
* device special file is opened, we take a reference on the
@@ -87,7 +89,8 @@ struct ib_umad_port {
struct cdev sm_cdev;
struct device *sm_dev;
- struct semaphore sm_sem;
+ wait_queue_head_t wq;
+ unsigned long flags;
struct mutex file_mutex;
struct list_head file_list;
@@ -1030,12 +1033,14 @@ static int ib_umad_sm_open(struct inode *inode, struct file *filp)
port = container_of(inode->i_cdev, struct ib_umad_port, sm_cdev);
if (filp->f_flags & O_NONBLOCK) {
- if (down_trylock(&port->sm_sem)) {
+ if (test_and_set_bit(UMAD_F_CLAIM, &port->flags)) {
ret = -EAGAIN;
goto fail;
}
} else {
- if (down_interruptible(&port->sm_sem)) {
+ if (wait_event_interruptible(port->wq,
+ !test_and_set_bit(UMAD_F_CLAIM,
+ &port->flags))) {
ret = -ERESTARTSYS;
goto fail;
}
@@ -1060,7 +1065,8 @@ static int ib_umad_sm_open(struct inode *inode, struct file *filp)
ib_modify_port(port->ib_dev, port->port_num, 0, &props);
err_up_sem:
- up(&port->sm_sem);
+ clear_bit(UMAD_F_CLAIM, &port->flags);
+ wake_up(&port->wq);
fail:
return ret;
@@ -1079,7 +1085,8 @@ static int ib_umad_sm_close(struct inode *inode, struct file *filp)
ret = ib_modify_port(port->ib_dev, port->port_num, 0, &props);
mutex_unlock(&port->file_mutex);
- up(&port->sm_sem);
+ clear_bit(UMAD_F_CLAIM, &port->flags);
+ wake_up(&port->wq);
kobject_put(&port->umad_dev->kobj);
@@ -1177,7 +1184,8 @@ static int ib_umad_init_port(struct ib_device *device, int port_num,
port->ib_dev = device;
port->port_num = port_num;
- sema_init(&port->sm_sem, 1);
+ init_waitqueue_head(&port->wq);
+ __clear_bit(UMAD_F_CLAIM, &port->flags);
mutex_init(&port->file_mutex);
INIT_LIST_HEAD(&port->file_list);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
[parent not found: <1479708496-9828-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* [PATCH v5 3/9] IB/hns: Replace semaphore poll_sem with mutex
2016-11-21 6:08 ` Binoy Jayan
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
target-devel-u79uwXL29TY76Z2rM5mHXA, Binoy Jayan
The semaphore 'poll_sem' is a simple mutex, so it should be written as one.
Semaphores are going away in the future. So replace it with a mutex. Also,
remove mutex_[un]lock from mthca_cmd_use_events and mthca_cmd_use_polling
respectively.
Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/hw/hns/hns_roce_cmd.c | 11 ++++-------
drivers/infiniband/hw/hns/hns_roce_device.h | 3 ++-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 2a0b6c0..51a0675 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -119,7 +119,7 @@ static int hns_roce_cmd_mbox_post_hw(struct hns_roce_dev *hr_dev, u64 in_param,
return ret;
}
-/* this should be called with "poll_sem" */
+/* this should be called with "poll_mutex" */
static int __hns_roce_cmd_mbox_poll(struct hns_roce_dev *hr_dev, u64 in_param,
u64 out_param, unsigned long in_modifier,
u8 op_modifier, u16 op,
@@ -167,10 +167,10 @@ static int hns_roce_cmd_mbox_poll(struct hns_roce_dev *hr_dev, u64 in_param,
{
int ret;
- down(&hr_dev->cmd.poll_sem);
+ mutex_lock(&hr_dev->cmd.poll_mutex);
ret = __hns_roce_cmd_mbox_poll(hr_dev, in_param, out_param, in_modifier,
op_modifier, op, timeout);
- up(&hr_dev->cmd.poll_sem);
+ mutex_unlock(&hr_dev->cmd.poll_mutex);
return ret;
}
@@ -275,7 +275,7 @@ int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
struct device *dev = &hr_dev->pdev->dev;
mutex_init(&hr_dev->cmd.hcr_mutex);
- sema_init(&hr_dev->cmd.poll_sem, 1);
+ mutex_init(&hr_dev->cmd.poll_mutex);
hr_dev->cmd.use_events = 0;
hr_dev->cmd.toggle = 1;
hr_dev->cmd.max_cmds = CMD_MAX_NUM;
@@ -319,8 +319,6 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
hr_cmd->token_mask = CMD_TOKEN_MASK;
hr_cmd->use_events = 1;
- down(&hr_cmd->poll_sem);
-
return 0;
}
@@ -335,7 +333,6 @@ void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)
down(&hr_cmd->event_sem);
kfree(hr_cmd->context);
- up(&hr_cmd->poll_sem);
}
struct hns_roce_cmd_mailbox
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 3417315..2afe075 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -34,6 +34,7 @@
#define _HNS_ROCE_DEVICE_H
#include <rdma/ib_verbs.h>
+#include <linux/mutex.h>
#define DRV_NAME "hns_roce"
@@ -358,7 +359,7 @@ struct hns_roce_cmdq {
struct dma_pool *pool;
u8 __iomem *hcr;
struct mutex hcr_mutex;
- struct semaphore poll_sem;
+ struct mutex poll_mutex;
/*
* Event mode: cmd register mutex protection,
* ensure to not exceed max_cmds and user use limit region
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 3/9] IB/hns: Replace semaphore poll_sem with mutex
@ 2016-11-21 6:08 ` Binoy Jayan
0 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
The semaphore 'poll_sem' is a simple mutex, so it should be written as one.
Semaphores are going away in the future. So replace it with a mutex. Also,
remove mutex_[un]lock from mthca_cmd_use_events and mthca_cmd_use_polling
respectively.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/hns/hns_roce_cmd.c | 11 ++++-------
drivers/infiniband/hw/hns/hns_roce_device.h | 3 ++-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 2a0b6c0..51a0675 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -119,7 +119,7 @@ static int hns_roce_cmd_mbox_post_hw(struct hns_roce_dev *hr_dev, u64 in_param,
return ret;
}
-/* this should be called with "poll_sem" */
+/* this should be called with "poll_mutex" */
static int __hns_roce_cmd_mbox_poll(struct hns_roce_dev *hr_dev, u64 in_param,
u64 out_param, unsigned long in_modifier,
u8 op_modifier, u16 op,
@@ -167,10 +167,10 @@ static int hns_roce_cmd_mbox_poll(struct hns_roce_dev *hr_dev, u64 in_param,
{
int ret;
- down(&hr_dev->cmd.poll_sem);
+ mutex_lock(&hr_dev->cmd.poll_mutex);
ret = __hns_roce_cmd_mbox_poll(hr_dev, in_param, out_param, in_modifier,
op_modifier, op, timeout);
- up(&hr_dev->cmd.poll_sem);
+ mutex_unlock(&hr_dev->cmd.poll_mutex);
return ret;
}
@@ -275,7 +275,7 @@ int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
struct device *dev = &hr_dev->pdev->dev;
mutex_init(&hr_dev->cmd.hcr_mutex);
- sema_init(&hr_dev->cmd.poll_sem, 1);
+ mutex_init(&hr_dev->cmd.poll_mutex);
hr_dev->cmd.use_events = 0;
hr_dev->cmd.toggle = 1;
hr_dev->cmd.max_cmds = CMD_MAX_NUM;
@@ -319,8 +319,6 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
hr_cmd->token_mask = CMD_TOKEN_MASK;
hr_cmd->use_events = 1;
- down(&hr_cmd->poll_sem);
-
return 0;
}
@@ -335,7 +333,6 @@ void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)
down(&hr_cmd->event_sem);
kfree(hr_cmd->context);
- up(&hr_cmd->poll_sem);
}
struct hns_roce_cmd_mailbox
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 3417315..2afe075 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -34,6 +34,7 @@
#define _HNS_ROCE_DEVICE_H
#include <rdma/ib_verbs.h>
+#include <linux/mutex.h>
#define DRV_NAME "hns_roce"
@@ -358,7 +359,7 @@ struct hns_roce_cmdq {
struct dma_pool *pool;
u8 __iomem *hcr;
struct mutex hcr_mutex;
- struct semaphore poll_sem;
+ struct mutex poll_mutex;
/*
* Event mode: cmd register mutex protection,
* ensure to not exceed max_cmds and user use limit region
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 4/9] IB/mthca: Replace semaphore poll_sem with mutex
2016-11-21 6:08 ` Binoy Jayan
` (3 preceding siblings ...)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
The semaphore 'poll_sem' is a simple mutex, so it should be written as one.
Semaphores are going away in the future. So replace it with a mutex. Also,
remove mutex_[un]lock from mthca_cmd_use_events and mthca_cmd_use_polling
respectively.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mthca/mthca_cmd.c | 10 +++-------
drivers/infiniband/hw/mthca/mthca_cmd.h | 1 +
drivers/infiniband/hw/mthca/mthca_dev.h | 2 +-
3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index c7f49bb..49c6e19 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -347,7 +347,7 @@ static int mthca_cmd_poll(struct mthca_dev *dev,
unsigned long end;
u8 status;
- down(&dev->cmd.poll_sem);
+ mutex_lock(&dev->cmd.poll_mutex);
err = mthca_cmd_post(dev, in_param,
out_param ? *out_param : 0,
@@ -382,7 +382,7 @@ static int mthca_cmd_poll(struct mthca_dev *dev,
}
out:
- up(&dev->cmd.poll_sem);
+ mutex_unlock(&dev->cmd.poll_mutex);
return err;
}
@@ -520,7 +520,7 @@ static int mthca_cmd_imm(struct mthca_dev *dev,
int mthca_cmd_init(struct mthca_dev *dev)
{
mutex_init(&dev->cmd.hcr_mutex);
- sema_init(&dev->cmd.poll_sem, 1);
+ mutex_init(&dev->cmd.poll_mutex);
dev->cmd.flags = 0;
dev->hcr = ioremap(pci_resource_start(dev->pdev, 0) + MTHCA_HCR_BASE,
@@ -582,8 +582,6 @@ int mthca_cmd_use_events(struct mthca_dev *dev)
dev->cmd.flags |= MTHCA_CMD_USE_EVENTS;
- down(&dev->cmd.poll_sem);
-
return 0;
}
@@ -600,8 +598,6 @@ void mthca_cmd_use_polling(struct mthca_dev *dev)
down(&dev->cmd.event_sem);
kfree(dev->cmd.context);
-
- up(&dev->cmd.poll_sem);
}
struct mthca_mailbox *mthca_alloc_mailbox(struct mthca_dev *dev,
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.h b/drivers/infiniband/hw/mthca/mthca_cmd.h
index d2e5b19..a7f197e 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.h
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.h
@@ -35,6 +35,7 @@
#ifndef MTHCA_CMD_H
#define MTHCA_CMD_H
+#include <linux/mutex.h>
#include <rdma/ib_verbs.h>
#define MTHCA_MAILBOX_SIZE 4096
diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h
index 4393a02..87ab964 100644
--- a/drivers/infiniband/hw/mthca/mthca_dev.h
+++ b/drivers/infiniband/hw/mthca/mthca_dev.h
@@ -120,7 +120,7 @@ enum {
struct mthca_cmd {
struct pci_pool *pool;
struct mutex hcr_mutex;
- struct semaphore poll_sem;
+ struct mutex poll_mutex;
struct semaphore event_sem;
int max_cmds;
spinlock_t context_lock;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
2016-11-21 6:08 ` Binoy Jayan
` (4 preceding siblings ...)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
2016-11-21 7:36 ` Sagi Grimberg
-1 siblings, 1 reply; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
The semaphore 'sem' in isert_device is used as completion, but in a
counting fashion as isert_connected_handler could be called multiple times
during which it allows for that number of waiters (isert_accept_np) to
continue without blocking, each consuming one node out from the list
isert_np-pending in the same order in which they were enqueued (FIFO). So,
convert it to struct completion. Semaphores are going away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/ulp/isert/ib_isert.c | 6 +++---
drivers/infiniband/ulp/isert/ib_isert.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 6dd43f6..de80f56 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -619,7 +619,7 @@
mutex_unlock(&isert_np->mutex);
isert_info("np %p: Allow accept_np to continue\n", isert_np);
- up(&isert_np->sem);
+ complete(&isert_np->comp);
}
static void
@@ -2311,7 +2311,7 @@ struct rdma_cm_id *
isert_err("Unable to allocate struct isert_np\n");
return -ENOMEM;
}
- sema_init(&isert_np->sem, 0);
+ init_completion(&isert_np->comp);
mutex_init(&isert_np->mutex);
INIT_LIST_HEAD(&isert_np->accepted);
INIT_LIST_HEAD(&isert_np->pending);
@@ -2427,7 +2427,7 @@ struct rdma_cm_id *
int ret;
accept_wait:
- ret = down_interruptible(&isert_np->sem);
+ ret = wait_for_completion_interruptible(&isert_np->comp);
if (ret)
return -ENODEV;
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index c02ada5..a1277c0 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -3,6 +3,7 @@
#include <linux/in6.h>
#include <rdma/ib_verbs.h>
#include <rdma/rdma_cm.h>
+#include <linux/completion.h>
#include <rdma/rw.h>
#include <scsi/iser.h>
@@ -190,7 +191,7 @@ struct isert_device {
struct isert_np {
struct iscsi_np *np;
- struct semaphore sem;
+ struct completion comp;
struct rdma_cm_id *cm_id;
struct mutex mutex;
struct list_head accepted;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
2016-11-21 6:08 ` [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion Binoy Jayan
@ 2016-11-21 7:36 ` Sagi Grimberg
2016-11-21 10:22 ` Arnd Bergmann
0 siblings, 1 reply; 27+ messages in thread
From: Sagi Grimberg @ 2016-11-21 7:36 UTC (permalink / raw)
To: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel
> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> index 6dd43f6..de80f56 100644
> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> @@ -619,7 +619,7 @@
> mutex_unlock(&isert_np->mutex);
>
> isert_info("np %p: Allow accept_np to continue\n", isert_np);
> - up(&isert_np->sem);
> + complete(&isert_np->comp);
> }
>
> static void
> @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
> isert_err("Unable to allocate struct isert_np\n");
> return -ENOMEM;
> }
> - sema_init(&isert_np->sem, 0);
> + init_completion(&isert_np->comp);
This is still racy, a connect event can complete just before we
init the completion and *will* get lost...
This code started off with a waitqueue which exposes the same
problem, see:
531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
So, still NAK from me...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
2016-11-21 7:36 ` Sagi Grimberg
@ 2016-11-21 10:22 ` Arnd Bergmann
0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2016-11-21 10:22 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier),
Faisal Latif, Mustafa Ismail, Mark Brown, linux-rdma
On Monday, November 21, 2016 9:36:22 AM CET Sagi Grimberg wrote:
> > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> > index 6dd43f6..de80f56 100644
> > --- a/drivers/infiniband/ulp/isert/ib_isert.c
> > +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> > @@ -619,7 +619,7 @@
> > mutex_unlock(&isert_np->mutex);
> >
> > isert_info("np %p: Allow accept_np to continue\n", isert_np);
> > - up(&isert_np->sem);
> > + complete(&isert_np->comp);
> > }
> >
> > static void
> > @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
> > isert_err("Unable to allocate struct isert_np\n");
> > return -ENOMEM;
> > }
> > - sema_init(&isert_np->sem, 0);
> > + init_completion(&isert_np->comp);
>
> This is still racy, a connect event can complete just before we
> init the completion and *will* get lost...
>
> This code started off with a waitqueue which exposes the same
> problem, see:
> 531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
>
> So, still NAK from me...
I don't see what you mean here. The code using a waitqueue had no
counter but the completion does. I had suggested that Binoy add
a comment into the code about this, as it is a rarely used
property of completions, but it does seem entirely valid to me.
Arnd
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
@ 2016-11-21 10:22 ` Arnd Bergmann
0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2016-11-21 10:22 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier),
Faisal Latif, Mustafa Ismail, Mark Brown, linux-rdma,
linux-kernel, target-devel
On Monday, November 21, 2016 9:36:22 AM CET Sagi Grimberg wrote:
> > diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> > index 6dd43f6..de80f56 100644
> > --- a/drivers/infiniband/ulp/isert/ib_isert.c
> > +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> > @@ -619,7 +619,7 @@
> > mutex_unlock(&isert_np->mutex);
> >
> > isert_info("np %p: Allow accept_np to continue\n", isert_np);
> > - up(&isert_np->sem);
> > + complete(&isert_np->comp);
> > }
> >
> > static void
> > @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
> > isert_err("Unable to allocate struct isert_np\n");
> > return -ENOMEM;
> > }
> > - sema_init(&isert_np->sem, 0);
> > + init_completion(&isert_np->comp);
>
> This is still racy, a connect event can complete just before we
> init the completion and *will* get lost...
>
> This code started off with a waitqueue which exposes the same
> problem, see:
> 531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
>
> So, still NAK from me...
I don't see what you mean here. The code using a waitqueue had no
counter but the completion does. I had suggested that Binoy add
a comment into the code about this, as it is a rarely used
property of completions, but it does seem entirely valid to me.
Arnd
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
2016-11-21 10:22 ` Arnd Bergmann
@ 2016-11-21 12:33 ` Sagi Grimberg
-1 siblings, 0 replies; 27+ messages in thread
From: Sagi Grimberg @ 2016-11-21 12:33 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier),
Faisal Latif, Mustafa Ismail, Mark Brown, linux-rdma
>>> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
>>> index 6dd43f6..de80f56 100644
>>> --- a/drivers/infiniband/ulp/isert/ib_isert.c
>>> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
>>> @@ -619,7 +619,7 @@
>>> mutex_unlock(&isert_np->mutex);
>>>
>>> isert_info("np %p: Allow accept_np to continue\n", isert_np);
>>> - up(&isert_np->sem);
>>> + complete(&isert_np->comp);
>>> }
>>>
>>> static void
>>> @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
>>> isert_err("Unable to allocate struct isert_np\n");
>>> return -ENOMEM;
>>> }
>>> - sema_init(&isert_np->sem, 0);
>>> + init_completion(&isert_np->comp);
>>
>> This is still racy, a connect event can complete just before we
>> init the completion and *will* get lost...
>>
>> This code started off with a waitqueue which exposes the same
>> problem, see:
>> 531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
>>
>> So, still NAK from me...
>
> I don't see what you mean here. The code using a waitqueue had no
> counter but the completion does.
The problem here is that init_completion sets the counter to zero
and between this thread wakes up and until it initializes comp->done
we might have another connect event completing it and I fail to
see how this is handled correctly.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
@ 2016-11-21 12:33 ` Sagi Grimberg
0 siblings, 0 replies; 27+ messages in thread
From: Sagi Grimberg @ 2016-11-21 12:33 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier),
Faisal Latif, Mustafa Ismail, Mark Brown, linux-rdma,
linux-kernel, target-devel
>>> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
>>> index 6dd43f6..de80f56 100644
>>> --- a/drivers/infiniband/ulp/isert/ib_isert.c
>>> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
>>> @@ -619,7 +619,7 @@
>>> mutex_unlock(&isert_np->mutex);
>>>
>>> isert_info("np %p: Allow accept_np to continue\n", isert_np);
>>> - up(&isert_np->sem);
>>> + complete(&isert_np->comp);
>>> }
>>>
>>> static void
>>> @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
>>> isert_err("Unable to allocate struct isert_np\n");
>>> return -ENOMEM;
>>> }
>>> - sema_init(&isert_np->sem, 0);
>>> + init_completion(&isert_np->comp);
>>
>> This is still racy, a connect event can complete just before we
>> init the completion and *will* get lost...
>>
>> This code started off with a waitqueue which exposes the same
>> problem, see:
>> 531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
>>
>> So, still NAK from me...
>
> I don't see what you mean here. The code using a waitqueue had no
> counter but the completion does.
The problem here is that init_completion sets the counter to zero
and between this thread wakes up and until it initializes comp->done
we might have another connect event completing it and I fail to
see how this is handled correctly.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
2016-11-21 12:33 ` Sagi Grimberg
@ 2016-11-21 14:50 ` Arnd Bergmann
-1 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2016-11-21 14:50 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier),
Faisal Latif, Mustafa Ismail, Mark Brown, linux-rdma
On Monday, November 21, 2016 2:33:02 PM CET Sagi Grimberg wrote:
> >>> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> >>> index 6dd43f6..de80f56 100644
> >>> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> >>> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> >>> @@ -619,7 +619,7 @@
> >>> mutex_unlock(&isert_np->mutex);
> >>>
> >>> isert_info("np %p: Allow accept_np to continue\n", isert_np);
> >>> - up(&isert_np->sem);
> >>> + complete(&isert_np->comp);
> >>> }
> >>>
> >>> static void
> >>> @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
> >>> isert_err("Unable to allocate struct isert_np\n");
> >>> return -ENOMEM;
> >>> }
> >>> - sema_init(&isert_np->sem, 0);
> >>> + init_completion(&isert_np->comp);
> >>
> >> This is still racy, a connect event can complete just before we
> >> init the completion and *will* get lost...
> >>
> >> This code started off with a waitqueue which exposes the same
> >> problem, see:
> >> 531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
> >>
> >> So, still NAK from me...
> >
> > I don't see what you mean here. The code using a waitqueue had no
> > counter but the completion does.
>
> The problem here is that init_completion sets the counter to zero
> and between this thread wakes up and until it initializes comp->done
> we might have another connect event completing it and I fail to
> see how this is handled correctly.
But the sema_init()/init_completion() is done right after the
structure is allocated, so nothing should have a pointer to it
at this time.
Later when we do the down()/wait_for_completion(), that will
just decrement the counter rather than initialize, and converting
from one to the other doesn't change anything here.
Arnd
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5 5/9] IB/isert: Replace semaphore sem with completion
@ 2016-11-21 14:50 ` Arnd Bergmann
0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2016-11-21 14:50 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier),
Faisal Latif, Mustafa Ismail, Mark Brown, linux-rdma,
linux-kernel, target-devel
On Monday, November 21, 2016 2:33:02 PM CET Sagi Grimberg wrote:
> >>> diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
> >>> index 6dd43f6..de80f56 100644
> >>> --- a/drivers/infiniband/ulp/isert/ib_isert.c
> >>> +++ b/drivers/infiniband/ulp/isert/ib_isert.c
> >>> @@ -619,7 +619,7 @@
> >>> mutex_unlock(&isert_np->mutex);
> >>>
> >>> isert_info("np %p: Allow accept_np to continue\n", isert_np);
> >>> - up(&isert_np->sem);
> >>> + complete(&isert_np->comp);
> >>> }
> >>>
> >>> static void
> >>> @@ -2311,7 +2311,7 @@ struct rdma_cm_id *
> >>> isert_err("Unable to allocate struct isert_np\n");
> >>> return -ENOMEM;
> >>> }
> >>> - sema_init(&isert_np->sem, 0);
> >>> + init_completion(&isert_np->comp);
> >>
> >> This is still racy, a connect event can complete just before we
> >> init the completion and *will* get lost...
> >>
> >> This code started off with a waitqueue which exposes the same
> >> problem, see:
> >> 531b7bf4bd79 Target/iser: Fix iscsit_accept_np and rdma_cm racy flow
> >>
> >> So, still NAK from me...
> >
> > I don't see what you mean here. The code using a waitqueue had no
> > counter but the completion does.
>
> The problem here is that init_completion sets the counter to zero
> and between this thread wakes up and until it initializes comp->done
> we might have another connect event completing it and I fail to
> see how this is handled correctly.
But the sema_init()/init_completion() is done right after the
structure is allocated, so nothing should have a pointer to it
at this time.
Later when we do the down()/wait_for_completion(), that will
just decrement the counter rather than initialize, and converting
from one to the other doesn't change anything here.
Arnd
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 6/9] IB/hns: Replace counting semaphore event_sem with wait_event
2016-11-21 6:08 ` Binoy Jayan
` (5 preceding siblings ...)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
Counting semaphores are going away in the future, so replace the semaphore
mthca_cmd::event_sem with a conditional wait_event.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/hns/hns_roce_cmd.c | 46 ++++++++++++++++++++---------
drivers/infiniband/hw/hns/hns_roce_device.h | 2 +-
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 51a0675..12ef3d8 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -189,6 +189,34 @@ void hns_roce_cmd_event(struct hns_roce_dev *hr_dev, u16 token, u8 status,
complete(&context->done);
}
+static inline struct hns_roce_cmd_context *
+hns_roce_try_get_context(struct hns_roce_cmdq *cmd)
+{
+ struct hns_roce_cmd_context *context = NULL;
+
+ spin_lock(&cmd->context_lock);
+
+ if (cmd->free_head < 0)
+ goto out;
+
+ context = &cmd->context[cmd->free_head];
+ context->token += cmd->token_mask + 1;
+ cmd->free_head = context->next;
+out:
+ spin_unlock(&cmd->context_lock);
+ return context;
+}
+
+/* wait for and acquire a free context */
+static inline struct hns_roce_cmd_context *
+hns_roce_get_free_context(struct hns_roce_cmdq *cmd)
+{
+ struct hns_roce_cmd_context *context;
+
+ wait_event(cmd->wq, (context = hns_roce_try_get_context(cmd)));
+ return context;
+}
+
/* this should be called with "use_events" */
static int __hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
u64 out_param, unsigned long in_modifier,
@@ -200,13 +228,7 @@ static int __hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
struct hns_roce_cmd_context *context;
int ret = 0;
- spin_lock(&cmd->context_lock);
- WARN_ON(cmd->free_head < 0);
- context = &cmd->context[cmd->free_head];
- context->token += cmd->token_mask + 1;
- cmd->free_head = context->next;
- spin_unlock(&cmd->context_lock);
-
+ context = hns_roce_get_free_context(cmd);
init_completion(&context->done);
ret = hns_roce_cmd_mbox_post_hw(hr_dev, in_param, out_param,
@@ -238,6 +260,7 @@ static int __hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
context->next = cmd->free_head;
cmd->free_head = context - cmd->context;
spin_unlock(&cmd->context_lock);
+ wake_up(&cmd->wq);
return ret;
}
@@ -248,10 +271,8 @@ static int hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
{
int ret = 0;
- down(&hr_dev->cmd.event_sem);
ret = __hns_roce_cmd_mbox_wait(hr_dev, in_param, out_param,
in_modifier, op_modifier, op, timeout);
- up(&hr_dev->cmd.event_sem);
return ret;
}
@@ -313,7 +334,7 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
hr_cmd->context[hr_cmd->max_cmds - 1].next = -1;
hr_cmd->free_head = 0;
- sema_init(&hr_cmd->event_sem, hr_cmd->max_cmds);
+ init_waitqueue_head(&hr_cmd->wq);
spin_lock_init(&hr_cmd->context_lock);
hr_cmd->token_mask = CMD_TOKEN_MASK;
@@ -325,12 +346,9 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)
{
struct hns_roce_cmdq *hr_cmd = &hr_dev->cmd;
- int i;
hr_cmd->use_events = 0;
-
- for (i = 0; i < hr_cmd->max_cmds; ++i)
- down(&hr_cmd->event_sem);
+ hr_cmd->free_head = -1;
kfree(hr_cmd->context);
}
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 2afe075..ac95f52 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -364,7 +364,7 @@ struct hns_roce_cmdq {
* Event mode: cmd register mutex protection,
* ensure to not exceed max_cmds and user use limit region
*/
- struct semaphore event_sem;
+ wait_queue_head_t wq;
int max_cmds;
spinlock_t context_lock;
int free_head;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 7/9] IB/mthca: Replace counting semaphore event_sem with wait_event
2016-11-21 6:08 ` Binoy Jayan
` (6 preceding siblings ...)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
Counting semaphores are going away in the future, so replace the semaphore
mthca_cmd::event_sem with a conditional wait_event.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mthca/mthca_cmd.c | 47 ++++++++++++++++++++++-----------
drivers/infiniband/hw/mthca/mthca_dev.h | 3 ++-
2 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index 49c6e19..d6a048a 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -405,6 +405,34 @@ void mthca_cmd_event(struct mthca_dev *dev,
complete(&context->done);
}
+static inline struct mthca_cmd_context *
+mthca_try_get_context(struct mthca_cmd *cmd)
+{
+ struct mthca_cmd_context *context = NULL;
+
+ spin_lock(&cmd->context_lock);
+
+ if (cmd->free_head < 0)
+ goto out;
+
+ context = &cmd->context[cmd->free_head];
+ context->token += cmd->token_mask + 1;
+ cmd->free_head = context->next;
+out:
+ spin_unlock(&cmd->context_lock);
+ return context;
+}
+
+/* wait for and acquire a free context */
+static inline struct mthca_cmd_context *
+mthca_get_free_context(struct mthca_cmd *cmd)
+{
+ struct mthca_cmd_context *context;
+
+ wait_event(cmd->wq, (context = mthca_try_get_context(cmd)));
+ return context;
+}
+
static int mthca_cmd_wait(struct mthca_dev *dev,
u64 in_param,
u64 *out_param,
@@ -417,15 +445,7 @@ static int mthca_cmd_wait(struct mthca_dev *dev,
int err = 0;
struct mthca_cmd_context *context;
- down(&dev->cmd.event_sem);
-
- spin_lock(&dev->cmd.context_lock);
- BUG_ON(dev->cmd.free_head < 0);
- context = &dev->cmd.context[dev->cmd.free_head];
- context->token += dev->cmd.token_mask + 1;
- dev->cmd.free_head = context->next;
- spin_unlock(&dev->cmd.context_lock);
-
+ context = mthca_get_free_context(&dev->cmd);
init_completion(&context->done);
err = mthca_cmd_post(dev, in_param,
@@ -458,8 +478,8 @@ static int mthca_cmd_wait(struct mthca_dev *dev,
context->next = dev->cmd.free_head;
dev->cmd.free_head = context - dev->cmd.context;
spin_unlock(&dev->cmd.context_lock);
+ wake_up(&dev->cmd.wq);
- up(&dev->cmd.event_sem);
return err;
}
@@ -571,7 +591,7 @@ int mthca_cmd_use_events(struct mthca_dev *dev)
dev->cmd.context[dev->cmd.max_cmds - 1].next = -1;
dev->cmd.free_head = 0;
- sema_init(&dev->cmd.event_sem, dev->cmd.max_cmds);
+ init_waitqueue_head(&dev->cmd.wq);
spin_lock_init(&dev->cmd.context_lock);
for (dev->cmd.token_mask = 1;
@@ -590,12 +610,9 @@ int mthca_cmd_use_events(struct mthca_dev *dev)
*/
void mthca_cmd_use_polling(struct mthca_dev *dev)
{
- int i;
-
dev->cmd.flags &= ~MTHCA_CMD_USE_EVENTS;
- for (i = 0; i < dev->cmd.max_cmds; ++i)
- down(&dev->cmd.event_sem);
+ dev->cmd.free_head = -1;
kfree(dev->cmd.context);
}
diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h
index 87ab964..2fc86db 100644
--- a/drivers/infiniband/hw/mthca/mthca_dev.h
+++ b/drivers/infiniband/hw/mthca/mthca_dev.h
@@ -46,6 +46,7 @@
#include <linux/list.h>
#include <linux/semaphore.h>
+#include <rdma/ib_sa.h>
#include "mthca_provider.h"
#include "mthca_doorbell.h"
@@ -121,7 +122,7 @@ struct mthca_cmd {
struct pci_pool *pool;
struct mutex hcr_mutex;
struct mutex poll_mutex;
- struct semaphore event_sem;
+ wait_queue_head_t wq;
int max_cmds;
spinlock_t context_lock;
int free_head;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 8/9] IB/mlx5: Add helper mlx5_ib_post_send_wait
2016-11-21 6:08 ` Binoy Jayan
` (7 preceding siblings ...)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
Clean up the following common code (to post a list of work requests to the
send queue of the specified QP) at various places and add a helper function
'mlx5_ib_post_send_wait' to implement the same.
- Initialize 'mlx5_ib_umr_context' on stack
- Assign "mlx5_umr_wr:wr:wr_cqe to umr_context.cqe
- Acquire the semaphore
- call ib_post_send with a single ib_send_wr
- wait_for_completion()
- Check for umr_context.status
- Release the semaphore
As semaphores are going away in the future, moving all of these into the
shared helper leaves only a single function using the semaphore, which
can then be rewritten to use something else.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mlx5/mr.c | 115 +++++++++++-----------------------------
1 file changed, 32 insertions(+), 83 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index d4ad672..1593856 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -856,16 +856,40 @@ static inline void mlx5_ib_init_umr_context(struct mlx5_ib_umr_context *context)
init_completion(&context->done);
}
+static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev,
+ struct mlx5_umr_wr *umrwr)
+{
+ struct umr_common *umrc = &dev->umrc;
+ struct ib_send_wr *bad;
+ int err;
+ struct mlx5_ib_umr_context umr_context;
+
+ mlx5_ib_init_umr_context(&umr_context);
+ umrwr->wr.wr_cqe = &umr_context.cqe;
+
+ down(&umrc->sem);
+ err = ib_post_send(umrc->qp, &umrwr->wr, &bad);
+ if (err) {
+ mlx5_ib_warn(dev, "UMR post send failed, err %d\n", err);
+ } else {
+ wait_for_completion(&umr_context.done);
+ if (umr_context.status != IB_WC_SUCCESS) {
+ mlx5_ib_warn(dev, "reg umr failed (%u)\n",
+ umr_context.status);
+ err = -EFAULT;
+ }
+ }
+ up(&umrc->sem);
+ return err;
+}
+
static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
u64 virt_addr, u64 len, int npages,
int page_shift, int order, int access_flags)
{
struct mlx5_ib_dev *dev = to_mdev(pd->device);
struct device *ddev = dev->ib_dev.dma_device;
- struct umr_common *umrc = &dev->umrc;
- struct mlx5_ib_umr_context umr_context;
struct mlx5_umr_wr umrwr = {};
- struct ib_send_wr *bad;
struct mlx5_ib_mr *mr;
struct ib_sge sg;
int size;
@@ -894,24 +918,12 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
if (err)
goto free_mr;
- mlx5_ib_init_umr_context(&umr_context);
-
- umrwr.wr.wr_cqe = &umr_context.cqe;
prep_umr_reg_wqe(pd, &umrwr.wr, &sg, dma, npages, mr->mmkey.key,
page_shift, virt_addr, len, access_flags);
- down(&umrc->sem);
- err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
- if (err) {
- mlx5_ib_warn(dev, "post send failed, err %d\n", err);
+ err = mlx5_ib_post_send_wait(dev, &umrwr);
+ if (err && err != -EFAULT)
goto unmap_dma;
- } else {
- wait_for_completion(&umr_context.done);
- if (umr_context.status != IB_WC_SUCCESS) {
- mlx5_ib_warn(dev, "reg umr failed\n");
- err = -EFAULT;
- }
- }
mr->mmkey.iova = virt_addr;
mr->mmkey.size = len;
@@ -920,7 +932,6 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
mr->live = 1;
unmap_dma:
- up(&umrc->sem);
dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
kfree(mr_pas);
@@ -940,13 +951,10 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
{
struct mlx5_ib_dev *dev = mr->dev;
struct device *ddev = dev->ib_dev.dma_device;
- struct umr_common *umrc = &dev->umrc;
- struct mlx5_ib_umr_context umr_context;
struct ib_umem *umem = mr->umem;
int size;
__be64 *pas;
dma_addr_t dma;
- struct ib_send_wr *bad;
struct mlx5_umr_wr wr;
struct ib_sge sg;
int err = 0;
@@ -1011,10 +1019,7 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
dma_sync_single_for_device(ddev, dma, size, DMA_TO_DEVICE);
- mlx5_ib_init_umr_context(&umr_context);
-
memset(&wr, 0, sizeof(wr));
- wr.wr.wr_cqe = &umr_context.cqe;
sg.addr = dma;
sg.length = ALIGN(npages * sizeof(u64),
@@ -1031,19 +1036,7 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
wr.mkey = mr->mmkey.key;
wr.target.offset = start_page_index;
- down(&umrc->sem);
- err = ib_post_send(umrc->qp, &wr.wr, &bad);
- if (err) {
- mlx5_ib_err(dev, "UMR post send failed, err %d\n", err);
- } else {
- wait_for_completion(&umr_context.done);
- if (umr_context.status != IB_WC_SUCCESS) {
- mlx5_ib_err(dev, "UMR completion failed, code %d\n",
- umr_context.status);
- err = -EFAULT;
- }
- }
- up(&umrc->sem);
+ err = mlx5_ib_post_send_wait(dev, &wr);
}
dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
@@ -1210,39 +1203,14 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
static int unreg_umr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)
{
struct mlx5_core_dev *mdev = dev->mdev;
- struct umr_common *umrc = &dev->umrc;
- struct mlx5_ib_umr_context umr_context;
struct mlx5_umr_wr umrwr = {};
- struct ib_send_wr *bad;
- int err;
if (mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)
return 0;
- mlx5_ib_init_umr_context(&umr_context);
-
- umrwr.wr.wr_cqe = &umr_context.cqe;
prep_umr_unreg_wqe(dev, &umrwr.wr, mr->mmkey.key);
- down(&umrc->sem);
- err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
- if (err) {
- up(&umrc->sem);
- mlx5_ib_dbg(dev, "err %d\n", err);
- goto error;
- } else {
- wait_for_completion(&umr_context.done);
- up(&umrc->sem);
- }
- if (umr_context.status != IB_WC_SUCCESS) {
- mlx5_ib_warn(dev, "unreg umr failed\n");
- err = -EFAULT;
- goto error;
- }
- return 0;
-
-error:
- return err;
+ return mlx5_ib_post_send_wait(dev, &umrwr);
}
static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
@@ -1251,19 +1219,13 @@ static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
{
struct mlx5_ib_dev *dev = to_mdev(pd->device);
struct device *ddev = dev->ib_dev.dma_device;
- struct mlx5_ib_umr_context umr_context;
- struct ib_send_wr *bad;
struct mlx5_umr_wr umrwr = {};
struct ib_sge sg;
- struct umr_common *umrc = &dev->umrc;
dma_addr_t dma = 0;
__be64 *mr_pas = NULL;
int size;
int err;
- mlx5_ib_init_umr_context(&umr_context);
-
- umrwr.wr.wr_cqe = &umr_context.cqe;
umrwr.wr.send_flags = MLX5_IB_SEND_UMR_FAIL_IF_FREE;
if (flags & IB_MR_REREG_TRANS) {
@@ -1291,21 +1253,8 @@ static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
}
/* post send request to UMR QP */
- down(&umrc->sem);
- err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
+ err = mlx5_ib_post_send_wait(dev, &umrwr);
- if (err) {
- mlx5_ib_warn(dev, "post send failed, err %d\n", err);
- } else {
- wait_for_completion(&umr_context.done);
- if (umr_context.status != IB_WC_SUCCESS) {
- mlx5_ib_warn(dev, "reg umr failed (%u)\n",
- umr_context.status);
- err = -EFAULT;
- }
- }
-
- up(&umrc->sem);
if (flags & IB_MR_REREG_TRANS) {
dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
kfree(mr_pas);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 9/9] IB/mlx5: Replace semaphore umr_common:sem with wait_event
2016-11-21 6:08 ` Binoy Jayan
` (8 preceding siblings ...)
(?)
@ 2016-11-21 6:08 ` Binoy Jayan
-1 siblings, 0 replies; 27+ messages in thread
From: Binoy Jayan @ 2016-11-21 6:08 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Sagi Grimberg, Hal Rosenstock,
Leon Romanovsky, Sagi Grimberg, Bart Van Assche,
Nicholas Bellinger, Jenny Derzhavetz, Ira Weiny, Steve Wise,
Mark Bloch, Tatyana E Nikolova, Matan Barak, Lijun Ou,
Wei Hu(Xavier)
Cc: Faisal Latif, Mustafa Ismail, Mark Brown, Arnd Bergmann,
linux-rdma, linux-kernel, target-devel, Binoy Jayan
Remove semaphore umr_common:sem used to limit concurrent access to umr qp
and introduce an atomic value 'users' to keep track of the same. Use a
wait_event to block when the limit is reached.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mlx5/main.c | 6 +-----
drivers/infiniband/hw/mlx5/mlx5_ib.h | 7 ++++++-
drivers/infiniband/hw/mlx5/mr.c | 6 ++++--
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 63036c7..9de716c 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -2437,10 +2437,6 @@ static void destroy_umrc_res(struct mlx5_ib_dev *dev)
ib_dealloc_pd(dev->umrc.pd);
}
-enum {
- MAX_UMR_WR = 128,
-};
-
static int create_umr_res(struct mlx5_ib_dev *dev)
{
struct ib_qp_init_attr *init_attr = NULL;
@@ -2520,7 +2516,7 @@ static int create_umr_res(struct mlx5_ib_dev *dev)
dev->umrc.cq = cq;
dev->umrc.pd = pd;
- sema_init(&dev->umrc.sem, MAX_UMR_WR);
+ init_waitqueue_head(&dev->umrc.wq);
ret = mlx5_mr_cache_init(dev);
if (ret) {
mlx5_ib_warn(dev, "mr cache init failed %d\n", ret);
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index dcdcd19..de31b5f 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -533,7 +533,12 @@ struct umr_common {
struct ib_qp *qp;
/* control access to UMR QP
*/
- struct semaphore sem;
+ wait_queue_head_t wq;
+ atomic_t users;
+};
+
+enum {
+ MAX_UMR_WR = 128,
};
enum {
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 1593856..dfaf6f6 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -867,7 +867,8 @@ static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev,
mlx5_ib_init_umr_context(&umr_context);
umrwr->wr.wr_cqe = &umr_context.cqe;
- down(&umrc->sem);
+ /* limit number of concurrent ib_post_send() on qp */
+ wait_event(umrc->wq, atomic_add_unless(&umrc->users, 1, MAX_UMR_WR));
err = ib_post_send(umrc->qp, &umrwr->wr, &bad);
if (err) {
mlx5_ib_warn(dev, "UMR post send failed, err %d\n", err);
@@ -879,7 +880,8 @@ static inline int mlx5_ib_post_send_wait(struct mlx5_ib_dev *dev,
err = -EFAULT;
}
}
- up(&umrc->sem);
+ atomic_dec(&umrc->users);
+ wake_up(&umrc->wq);
return err;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 27+ messages in thread