* [PATCH for-next] iw_cxgb4: Fix possible circular dependency locking warning
@ 2017-11-09 12:17 Potnuri Bharat Teja
[not found] ` <1510229853-25875-1-git-send-email-bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Potnuri Bharat Teja @ 2017-11-09 12:17 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, bharat-ut6Up61K2wZBDgjK7y7TUQ,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW
Locking sequence of iw_cxgb4 and RoCE drivers in ib_register_device() is
slightly different and this leads to possible circular dependency locking
warning when both the devices are brought up.
Here is the locking sequence upto ib_register_device():
iw_cxgb4: rtnl_mutex(net stack) --> uld_mutex --> device_mutex
RoCE drivers: device_mutex --> rtnl_mutex
Here is the possibility of cross locking:
CPU #0 (iw_cxgb4) CPU #1 (RoCE drivers)
-> on interface up cxgb4_up()
executed with rtnl_mutex held
-> hold uld_mutex and try
registering ib device
-> In ib_register_device() hold
device_mutex
-> hold device mutex in
ib_register_device
-> try acquiring rtnl_mutex in
ib_enum_roce_netdev()
Current patch schedules the ib_register_device() functionality of
iw_cxgb4 to a workqueue to prevent the possible cross-locking.
Also rename the labels in c4iw_reister_device().
Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/hw/cxgb4/device.c | 28 +++++++++++++---------------
drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 10 +++++++++-
drivers/infiniband/hw/cxgb4/provider.c | 26 +++++++++++++++++---------
3 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 33bfddf3b064..af77d128d242 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -64,14 +64,9 @@ module_param(c4iw_wr_log_size_order, int, 0444);
MODULE_PARM_DESC(c4iw_wr_log_size_order,
"Number of entries (log2) in the work request timing log.");
-struct uld_ctx {
- struct list_head entry;
- struct cxgb4_lld_info lldi;
- struct c4iw_dev *dev;
-};
-
static LIST_HEAD(uld_ctx_list);
static DEFINE_MUTEX(dev_mutex);
+struct workqueue_struct *reg_workq;
#define DB_FC_RESUME_SIZE 64
#define DB_FC_RESUME_DELAY 1
@@ -912,7 +907,7 @@ static void c4iw_rdev_close(struct c4iw_rdev *rdev)
c4iw_destroy_resource(&rdev->resource);
}
-static void c4iw_dealloc(struct uld_ctx *ctx)
+void c4iw_dealloc(struct uld_ctx *ctx)
{
c4iw_rdev_close(&ctx->dev->rdev);
WARN_ON_ONCE(!idr_is_empty(&ctx->dev->cqidr));
@@ -1208,8 +1203,6 @@ static int c4iw_uld_state_change(void *handle, enum cxgb4_state new_state)
case CXGB4_STATE_UP:
pr_info("%s: Up\n", pci_name(ctx->lldi.pdev));
if (!ctx->dev) {
- int ret;
-
ctx->dev = c4iw_alloc(&ctx->lldi);
if (IS_ERR(ctx->dev)) {
pr_err("%s: initialization failed: %ld\n",
@@ -1218,12 +1211,9 @@ static int c4iw_uld_state_change(void *handle, enum cxgb4_state new_state)
ctx->dev = NULL;
break;
}
- ret = c4iw_register_device(ctx->dev);
- if (ret) {
- pr_err("%s: RDMA registration failed: %d\n",
- pci_name(ctx->lldi.pdev), ret);
- c4iw_dealloc(ctx);
- }
+
+ INIT_WORK(&ctx->reg_work, c4iw_register_device);
+ queue_work(reg_workq, &ctx->reg_work);
}
break;
case CXGB4_STATE_DOWN:
@@ -1551,6 +1541,12 @@ static int __init c4iw_init_module(void)
if (!c4iw_debugfs_root)
pr_warn("could not create debugfs entry, continuing\n");
+ reg_workq = create_singlethread_workqueue("Register_iWARP_device");
+ if (!reg_workq) {
+ pr_err("Failed creating workqueue to register iwarp device\n");
+ return -ENOMEM;
+ }
+
cxgb4_register_uld(CXGB4_ULD_RDMA, &c4iw_uld_info);
return 0;
@@ -1567,6 +1563,8 @@ static void __exit c4iw_exit_module(void)
kfree(ctx);
}
mutex_unlock(&dev_mutex);
+ flush_workqueue(reg_workq);
+ destroy_workqueue(reg_workq);
cxgb4_unregister_uld(CXGB4_ULD_RDMA);
c4iw_cm_term();
debugfs_remove_recursive(c4iw_debugfs_root);
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 788ddb6aa57b..42500cbb9669 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -322,6 +322,13 @@ struct c4iw_dev {
wait_queue_head_t wait;
};
+struct uld_ctx {
+ struct list_head entry;
+ struct cxgb4_lld_info lldi;
+ struct c4iw_dev *dev;
+ struct work_struct reg_work;
+};
+
static inline struct c4iw_dev *to_c4iw_dev(struct ib_device *ibdev)
{
return container_of(ibdev, struct c4iw_dev, ibdev);
@@ -989,7 +996,7 @@ void c4iw_rqtpool_destroy(struct c4iw_rdev *rdev);
void c4iw_ocqp_pool_destroy(struct c4iw_rdev *rdev);
void c4iw_destroy_resource(struct c4iw_resource *rscp);
int c4iw_destroy_ctrl_qp(struct c4iw_rdev *rdev);
-int c4iw_register_device(struct c4iw_dev *dev);
+void c4iw_register_device(struct work_struct *work);
void c4iw_unregister_device(struct c4iw_dev *dev);
int __init c4iw_cm_init(void);
void c4iw_cm_term(void);
@@ -1015,6 +1022,7 @@ struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd,
int c4iw_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
unsigned int *sg_offset);
int c4iw_dealloc_mw(struct ib_mw *mw);
+void c4iw_dealloc(struct uld_ctx *ctx);
struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
struct ib_udata *udata);
struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start,
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index fb99a05562a3..3387776915a2 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -530,10 +530,12 @@ static void get_dev_fw_str(struct ib_device *dev, char *str)
FW_HDR_FW_VER_BUILD_G(c4iw_dev->rdev.lldi.fw_vers));
}
-int c4iw_register_device(struct c4iw_dev *dev)
+void c4iw_register_device(struct work_struct *work)
{
int ret;
int i;
+ struct uld_ctx *ctx = container_of(work, struct uld_ctx, reg_work);
+ struct c4iw_dev *dev = ctx->dev;
pr_debug("c4iw_dev %p\n", dev);
BUG_ON(!dev->rdev.lldi.ports[0]);
@@ -609,8 +611,10 @@ int c4iw_register_device(struct c4iw_dev *dev)
dev->ibdev.get_dev_fw_str = get_dev_fw_str;
dev->ibdev.iwcm = kmalloc(sizeof(struct iw_cm_verbs), GFP_KERNEL);
- if (!dev->ibdev.iwcm)
- return -ENOMEM;
+ if (!dev->ibdev.iwcm) {
+ ret = -ENOMEM;
+ goto err_dealloc_ctx;
+ }
dev->ibdev.iwcm->connect = c4iw_connect;
dev->ibdev.iwcm->accept = c4iw_accept_cr;
@@ -625,20 +629,24 @@ int c4iw_register_device(struct c4iw_dev *dev)
ret = ib_register_device(&dev->ibdev, NULL);
if (ret)
- goto bail1;
+ goto err_kfree_iwcm;
for (i = 0; i < ARRAY_SIZE(c4iw_class_attributes); ++i) {
ret = device_create_file(&dev->ibdev.dev,
c4iw_class_attributes[i]);
if (ret)
- goto bail2;
+ goto err_unregister_device;
}
- return 0;
-bail2:
+ return;
+err_unregister_device:
ib_unregister_device(&dev->ibdev);
-bail1:
+err_kfree_iwcm:
kfree(dev->ibdev.iwcm);
- return ret;
+err_dealloc_ctx:
+ pr_err("%s - Failed registering iwarp device: %d\n",
+ pci_name(ctx->lldi.pdev), ret);
+ c4iw_dealloc(ctx);
+ return;
}
void c4iw_unregister_device(struct c4iw_dev *dev)
--
2.5.3
--
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] 5+ messages in thread
* RE: [PATCH for-next] iw_cxgb4: Fix possible circular dependency locking warning
[not found] ` <1510229853-25875-1-git-send-email-bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
@ 2017-11-09 14:44 ` Steve Wise
2017-11-13 21:59 ` Doug Ledford
0 siblings, 1 reply; 5+ messages in thread
From: Steve Wise @ 2017-11-09 14:44 UTC (permalink / raw)
To: 'Potnuri Bharat Teja', dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
>
> Locking sequence of iw_cxgb4 and RoCE drivers in ib_register_device() is
> slightly different and this leads to possible circular dependency locking
> warning when both the devices are brought up.
>
> Here is the locking sequence upto ib_register_device():
> iw_cxgb4: rtnl_mutex(net stack) --> uld_mutex --> device_mutex
> RoCE drivers: device_mutex --> rtnl_mutex
>
> Here is the possibility of cross locking:
>
> CPU #0 (iw_cxgb4) CPU #1 (RoCE drivers)
>
> -> on interface up cxgb4_up()
> executed with rtnl_mutex held
> -> hold uld_mutex and try
> registering ib device
> -> In ib_register_device() hold
> device_mutex
> -> hold device mutex in
> ib_register_device
> -> try acquiring rtnl_mutex in
> ib_enum_roce_netdev()
>
> Current patch schedules the ib_register_device() functionality of
> iw_cxgb4 to a workqueue to prevent the possible cross-locking.
> Also rename the labels in c4iw_reister_device().
>
> Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
--
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] 5+ messages in thread
* Re: [PATCH for-next] iw_cxgb4: Fix possible circular dependency locking warning
2017-11-09 14:44 ` Steve Wise
@ 2017-11-13 21:59 ` Doug Ledford
0 siblings, 0 replies; 5+ messages in thread
From: Doug Ledford @ 2017-11-13 21:59 UTC (permalink / raw)
To: Steve Wise, 'Potnuri Bharat Teja'
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]
On Thu, 2017-11-09 at 08:44 -0600, Steve Wise wrote:
> >
> > Locking sequence of iw_cxgb4 and RoCE drivers in ib_register_device() is
> > slightly different and this leads to possible circular dependency locking
> > warning when both the devices are brought up.
> >
> > Here is the locking sequence upto ib_register_device():
> > iw_cxgb4: rtnl_mutex(net stack) --> uld_mutex --> device_mutex
> > RoCE drivers: device_mutex --> rtnl_mutex
> >
> > Here is the possibility of cross locking:
> >
> > CPU #0 (iw_cxgb4) CPU #1 (RoCE drivers)
> >
> > -> on interface up cxgb4_up()
> > executed with rtnl_mutex held
> > -> hold uld_mutex and try
> > registering ib device
> > -> In ib_register_device() hold
> > device_mutex
> > -> hold device mutex in
> > ib_register_device
> > -> try acquiring rtnl_mutex in
> > ib_enum_roce_netdev()
> >
> > Current patch schedules the ib_register_device() functionality of
> > iw_cxgb4 to a workqueue to prevent the possible cross-locking.
> > Also rename the labels in c4iw_reister_device().
> >
> > Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
>
> Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
>
>
Thanks, applied.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-next] iw_cxgb4: Fix possible circular dependency locking warning
[not found] ` <1510226844-4165-1-git-send-email-chelsiocudbg-KJX8L1YACloTKYOLMXNBRxrm3jAUxWOA@public.gmane.org>
@ 2017-11-09 11:36 ` Potnuri Bharat Teja
0 siblings, 0 replies; 5+ messages in thread
From: Potnuri Bharat Teja @ 2017-11-09 11:36 UTC (permalink / raw)
To: Chelsio Cudbg
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, SWise OGC
Please ignore this.
I sent it with wrong user name.
On Thursday, November 11/09/17, 2017 at 16:57:24 +0530, Chelsio Cudbg wrote:
> From: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
>
> Locking sequence of iw_cxgb4 and RoCE drivers in ib_register_device() is
> slightly different and this leads to possible circular dependency locking
> warning when both the devices are brought up.
>
> Here is the locking sequence upto ib_register_device():
> iw_cxgb4: rtnl_mutex(net stack) --> uld_mutex --> device_mutex
> RoCE drivers: device_mutex --> rtnl_mutex
>
> Here is the possibility of cross locking:
>
> CPU #0 (iw_cxgb4) CPU #1 (RoCE drivers)
>
> -> on interface up cxgb4_up()
> executed with rtnl_mutex held
> -> hold uld_mutex and try
> registering ib device
> -> In ib_register_device() hold
> device_mutex
> -> hold device mutex in
> ib_register_device
> -> try acquiring rtnl_mutex in
> ib_enum_roce_netdev()
>
> Current patch schedules the ib_register_device() functionality of
> iw_cxgb4 to a workqueue to prevent the possible cross-locking.
> Also rename the labels in c4iw_reister_device().
>
> Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
> ---
> drivers/infiniband/hw/cxgb4/device.c | 28 +++++++++++++---------------
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 10 +++++++++-
> drivers/infiniband/hw/cxgb4/provider.c | 26 +++++++++++++++++---------
> 3 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> index 33bfddf3b064..af77d128d242 100644
> --- a/drivers/infiniband/hw/cxgb4/device.c
> +++ b/drivers/infiniband/hw/cxgb4/device.c
> @@ -64,14 +64,9 @@ module_param(c4iw_wr_log_size_order, int, 0444);
> MODULE_PARM_DESC(c4iw_wr_log_size_order,
> "Number of entries (log2) in the work request timing log.");
>
> -struct uld_ctx {
> - struct list_head entry;
> - struct cxgb4_lld_info lldi;
> - struct c4iw_dev *dev;
> -};
> -
> static LIST_HEAD(uld_ctx_list);
> static DEFINE_MUTEX(dev_mutex);
> +struct workqueue_struct *reg_workq;
>
> #define DB_FC_RESUME_SIZE 64
> #define DB_FC_RESUME_DELAY 1
> @@ -912,7 +907,7 @@ static void c4iw_rdev_close(struct c4iw_rdev *rdev)
> c4iw_destroy_resource(&rdev->resource);
> }
>
> -static void c4iw_dealloc(struct uld_ctx *ctx)
> +void c4iw_dealloc(struct uld_ctx *ctx)
> {
> c4iw_rdev_close(&ctx->dev->rdev);
> WARN_ON_ONCE(!idr_is_empty(&ctx->dev->cqidr));
> @@ -1208,8 +1203,6 @@ static int c4iw_uld_state_change(void *handle, enum cxgb4_state new_state)
> case CXGB4_STATE_UP:
> pr_info("%s: Up\n", pci_name(ctx->lldi.pdev));
> if (!ctx->dev) {
> - int ret;
> -
> ctx->dev = c4iw_alloc(&ctx->lldi);
> if (IS_ERR(ctx->dev)) {
> pr_err("%s: initialization failed: %ld\n",
> @@ -1218,12 +1211,9 @@ static int c4iw_uld_state_change(void *handle, enum cxgb4_state new_state)
> ctx->dev = NULL;
> break;
> }
> - ret = c4iw_register_device(ctx->dev);
> - if (ret) {
> - pr_err("%s: RDMA registration failed: %d\n",
> - pci_name(ctx->lldi.pdev), ret);
> - c4iw_dealloc(ctx);
> - }
> +
> + INIT_WORK(&ctx->reg_work, c4iw_register_device);
> + queue_work(reg_workq, &ctx->reg_work);
> }
> break;
> case CXGB4_STATE_DOWN:
> @@ -1551,6 +1541,12 @@ static int __init c4iw_init_module(void)
> if (!c4iw_debugfs_root)
> pr_warn("could not create debugfs entry, continuing\n");
>
> + reg_workq = create_singlethread_workqueue("Register_iWARP_device");
> + if (!reg_workq) {
> + pr_err("Failed creating workqueue to register iwarp device\n");
> + return -ENOMEM;
> + }
> +
> cxgb4_register_uld(CXGB4_ULD_RDMA, &c4iw_uld_info);
>
> return 0;
> @@ -1567,6 +1563,8 @@ static void __exit c4iw_exit_module(void)
> kfree(ctx);
> }
> mutex_unlock(&dev_mutex);
> + flush_workqueue(reg_workq);
> + destroy_workqueue(reg_workq);
> cxgb4_unregister_uld(CXGB4_ULD_RDMA);
> c4iw_cm_term();
> debugfs_remove_recursive(c4iw_debugfs_root);
> diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> index 788ddb6aa57b..42500cbb9669 100644
> --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> @@ -322,6 +322,13 @@ struct c4iw_dev {
> wait_queue_head_t wait;
> };
>
> +struct uld_ctx {
> + struct list_head entry;
> + struct cxgb4_lld_info lldi;
> + struct c4iw_dev *dev;
> + struct work_struct reg_work;
> +};
> +
> static inline struct c4iw_dev *to_c4iw_dev(struct ib_device *ibdev)
> {
> return container_of(ibdev, struct c4iw_dev, ibdev);
> @@ -989,7 +996,7 @@ void c4iw_rqtpool_destroy(struct c4iw_rdev *rdev);
> void c4iw_ocqp_pool_destroy(struct c4iw_rdev *rdev);
> void c4iw_destroy_resource(struct c4iw_resource *rscp);
> int c4iw_destroy_ctrl_qp(struct c4iw_rdev *rdev);
> -int c4iw_register_device(struct c4iw_dev *dev);
> +void c4iw_register_device(struct work_struct *work);
> void c4iw_unregister_device(struct c4iw_dev *dev);
> int __init c4iw_cm_init(void);
> void c4iw_cm_term(void);
> @@ -1015,6 +1022,7 @@ struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd,
> int c4iw_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
> unsigned int *sg_offset);
> int c4iw_dealloc_mw(struct ib_mw *mw);
> +void c4iw_dealloc(struct uld_ctx *ctx);
> struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
> struct ib_udata *udata);
> struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start,
> diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
> index fb99a05562a3..575875a7e7f8 100644
> --- a/drivers/infiniband/hw/cxgb4/provider.c
> +++ b/drivers/infiniband/hw/cxgb4/provider.c
> @@ -530,10 +530,12 @@ static void get_dev_fw_str(struct ib_device *dev, char *str)
> FW_HDR_FW_VER_BUILD_G(c4iw_dev->rdev.lldi.fw_vers));
> }
>
> -int c4iw_register_device(struct c4iw_dev *dev)
> +void c4iw_register_device(struct work_struct *work)
> {
> int ret;
> int i;
> + struct uld_ctx *ctx = container_of(work, struct uld_ctx, reg_work);
> + struct c4iw_dev *dev = ctx->dev;
>
> pr_debug("c4iw_dev %p\n", dev);
> BUG_ON(!dev->rdev.lldi.ports[0]);
> @@ -609,8 +611,10 @@ int c4iw_register_device(struct c4iw_dev *dev)
> dev->ibdev.get_dev_fw_str = get_dev_fw_str;
>
> dev->ibdev.iwcm = kmalloc(sizeof(struct iw_cm_verbs), GFP_KERNEL);
> - if (!dev->ibdev.iwcm)
> - return -ENOMEM;
> + if (!dev->ibdev.iwcm) {
> + ret = -ENOMEM;
> + goto err_dealloc_ctx;
> + }
>
> dev->ibdev.iwcm->connect = c4iw_connect;
> dev->ibdev.iwcm->accept = c4iw_accept_cr;
> @@ -625,20 +629,24 @@ int c4iw_register_device(struct c4iw_dev *dev)
>
> ret = ib_register_device(&dev->ibdev, NULL);
> if (ret)
> - goto bail1;
> + goto err_unregister_device;
>
> for (i = 0; i < ARRAY_SIZE(c4iw_class_attributes); ++i) {
> ret = device_create_file(&dev->ibdev.dev,
> c4iw_class_attributes[i]);
> if (ret)
> - goto bail2;
> + goto err_kfree_iwcm;
> }
> - return 0;
> -bail2:
> + return;
> +err_kfree_iwcm:
> ib_unregister_device(&dev->ibdev);
> -bail1:
> +err_unregister_device:
> kfree(dev->ibdev.iwcm);
> - return ret;
> +err_dealloc_ctx:
> + pr_err("%s - Failed registering iwarp device: %d\n",
> + pci_name(ctx->lldi.pdev), ret);
> + c4iw_dealloc(ctx);
> + return;
> }
>
> void c4iw_unregister_device(struct c4iw_dev *dev)
> --
> 2.5.3
>
--
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] 5+ messages in thread
* [PATCH for-next] iw_cxgb4: Fix possible circular dependency locking warning
@ 2017-11-09 11:27 Chelsio Cudbg
[not found] ` <1510226844-4165-1-git-send-email-chelsiocudbg-KJX8L1YACloTKYOLMXNBRxrm3jAUxWOA@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Chelsio Cudbg @ 2017-11-09 11:27 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, bharat-ut6Up61K2wZBDgjK7y7TUQ,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW
From: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
Locking sequence of iw_cxgb4 and RoCE drivers in ib_register_device() is
slightly different and this leads to possible circular dependency locking
warning when both the devices are brought up.
Here is the locking sequence upto ib_register_device():
iw_cxgb4: rtnl_mutex(net stack) --> uld_mutex --> device_mutex
RoCE drivers: device_mutex --> rtnl_mutex
Here is the possibility of cross locking:
CPU #0 (iw_cxgb4) CPU #1 (RoCE drivers)
-> on interface up cxgb4_up()
executed with rtnl_mutex held
-> hold uld_mutex and try
registering ib device
-> In ib_register_device() hold
device_mutex
-> hold device mutex in
ib_register_device
-> try acquiring rtnl_mutex in
ib_enum_roce_netdev()
Current patch schedules the ib_register_device() functionality of
iw_cxgb4 to a workqueue to prevent the possible cross-locking.
Also rename the labels in c4iw_reister_device().
Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/hw/cxgb4/device.c | 28 +++++++++++++---------------
drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 10 +++++++++-
drivers/infiniband/hw/cxgb4/provider.c | 26 +++++++++++++++++---------
3 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 33bfddf3b064..af77d128d242 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -64,14 +64,9 @@ module_param(c4iw_wr_log_size_order, int, 0444);
MODULE_PARM_DESC(c4iw_wr_log_size_order,
"Number of entries (log2) in the work request timing log.");
-struct uld_ctx {
- struct list_head entry;
- struct cxgb4_lld_info lldi;
- struct c4iw_dev *dev;
-};
-
static LIST_HEAD(uld_ctx_list);
static DEFINE_MUTEX(dev_mutex);
+struct workqueue_struct *reg_workq;
#define DB_FC_RESUME_SIZE 64
#define DB_FC_RESUME_DELAY 1
@@ -912,7 +907,7 @@ static void c4iw_rdev_close(struct c4iw_rdev *rdev)
c4iw_destroy_resource(&rdev->resource);
}
-static void c4iw_dealloc(struct uld_ctx *ctx)
+void c4iw_dealloc(struct uld_ctx *ctx)
{
c4iw_rdev_close(&ctx->dev->rdev);
WARN_ON_ONCE(!idr_is_empty(&ctx->dev->cqidr));
@@ -1208,8 +1203,6 @@ static int c4iw_uld_state_change(void *handle, enum cxgb4_state new_state)
case CXGB4_STATE_UP:
pr_info("%s: Up\n", pci_name(ctx->lldi.pdev));
if (!ctx->dev) {
- int ret;
-
ctx->dev = c4iw_alloc(&ctx->lldi);
if (IS_ERR(ctx->dev)) {
pr_err("%s: initialization failed: %ld\n",
@@ -1218,12 +1211,9 @@ static int c4iw_uld_state_change(void *handle, enum cxgb4_state new_state)
ctx->dev = NULL;
break;
}
- ret = c4iw_register_device(ctx->dev);
- if (ret) {
- pr_err("%s: RDMA registration failed: %d\n",
- pci_name(ctx->lldi.pdev), ret);
- c4iw_dealloc(ctx);
- }
+
+ INIT_WORK(&ctx->reg_work, c4iw_register_device);
+ queue_work(reg_workq, &ctx->reg_work);
}
break;
case CXGB4_STATE_DOWN:
@@ -1551,6 +1541,12 @@ static int __init c4iw_init_module(void)
if (!c4iw_debugfs_root)
pr_warn("could not create debugfs entry, continuing\n");
+ reg_workq = create_singlethread_workqueue("Register_iWARP_device");
+ if (!reg_workq) {
+ pr_err("Failed creating workqueue to register iwarp device\n");
+ return -ENOMEM;
+ }
+
cxgb4_register_uld(CXGB4_ULD_RDMA, &c4iw_uld_info);
return 0;
@@ -1567,6 +1563,8 @@ static void __exit c4iw_exit_module(void)
kfree(ctx);
}
mutex_unlock(&dev_mutex);
+ flush_workqueue(reg_workq);
+ destroy_workqueue(reg_workq);
cxgb4_unregister_uld(CXGB4_ULD_RDMA);
c4iw_cm_term();
debugfs_remove_recursive(c4iw_debugfs_root);
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 788ddb6aa57b..42500cbb9669 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -322,6 +322,13 @@ struct c4iw_dev {
wait_queue_head_t wait;
};
+struct uld_ctx {
+ struct list_head entry;
+ struct cxgb4_lld_info lldi;
+ struct c4iw_dev *dev;
+ struct work_struct reg_work;
+};
+
static inline struct c4iw_dev *to_c4iw_dev(struct ib_device *ibdev)
{
return container_of(ibdev, struct c4iw_dev, ibdev);
@@ -989,7 +996,7 @@ void c4iw_rqtpool_destroy(struct c4iw_rdev *rdev);
void c4iw_ocqp_pool_destroy(struct c4iw_rdev *rdev);
void c4iw_destroy_resource(struct c4iw_resource *rscp);
int c4iw_destroy_ctrl_qp(struct c4iw_rdev *rdev);
-int c4iw_register_device(struct c4iw_dev *dev);
+void c4iw_register_device(struct work_struct *work);
void c4iw_unregister_device(struct c4iw_dev *dev);
int __init c4iw_cm_init(void);
void c4iw_cm_term(void);
@@ -1015,6 +1022,7 @@ struct ib_mr *c4iw_alloc_mr(struct ib_pd *pd,
int c4iw_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg, int sg_nents,
unsigned int *sg_offset);
int c4iw_dealloc_mw(struct ib_mw *mw);
+void c4iw_dealloc(struct uld_ctx *ctx);
struct ib_mw *c4iw_alloc_mw(struct ib_pd *pd, enum ib_mw_type type,
struct ib_udata *udata);
struct ib_mr *c4iw_reg_user_mr(struct ib_pd *pd, u64 start,
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index fb99a05562a3..575875a7e7f8 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -530,10 +530,12 @@ static void get_dev_fw_str(struct ib_device *dev, char *str)
FW_HDR_FW_VER_BUILD_G(c4iw_dev->rdev.lldi.fw_vers));
}
-int c4iw_register_device(struct c4iw_dev *dev)
+void c4iw_register_device(struct work_struct *work)
{
int ret;
int i;
+ struct uld_ctx *ctx = container_of(work, struct uld_ctx, reg_work);
+ struct c4iw_dev *dev = ctx->dev;
pr_debug("c4iw_dev %p\n", dev);
BUG_ON(!dev->rdev.lldi.ports[0]);
@@ -609,8 +611,10 @@ int c4iw_register_device(struct c4iw_dev *dev)
dev->ibdev.get_dev_fw_str = get_dev_fw_str;
dev->ibdev.iwcm = kmalloc(sizeof(struct iw_cm_verbs), GFP_KERNEL);
- if (!dev->ibdev.iwcm)
- return -ENOMEM;
+ if (!dev->ibdev.iwcm) {
+ ret = -ENOMEM;
+ goto err_dealloc_ctx;
+ }
dev->ibdev.iwcm->connect = c4iw_connect;
dev->ibdev.iwcm->accept = c4iw_accept_cr;
@@ -625,20 +629,24 @@ int c4iw_register_device(struct c4iw_dev *dev)
ret = ib_register_device(&dev->ibdev, NULL);
if (ret)
- goto bail1;
+ goto err_unregister_device;
for (i = 0; i < ARRAY_SIZE(c4iw_class_attributes); ++i) {
ret = device_create_file(&dev->ibdev.dev,
c4iw_class_attributes[i]);
if (ret)
- goto bail2;
+ goto err_kfree_iwcm;
}
- return 0;
-bail2:
+ return;
+err_kfree_iwcm:
ib_unregister_device(&dev->ibdev);
-bail1:
+err_unregister_device:
kfree(dev->ibdev.iwcm);
- return ret;
+err_dealloc_ctx:
+ pr_err("%s - Failed registering iwarp device: %d\n",
+ pci_name(ctx->lldi.pdev), ret);
+ c4iw_dealloc(ctx);
+ return;
}
void c4iw_unregister_device(struct c4iw_dev *dev)
--
2.5.3
--
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] 5+ messages in thread
end of thread, other threads:[~2017-11-13 21:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09 12:17 [PATCH for-next] iw_cxgb4: Fix possible circular dependency locking warning Potnuri Bharat Teja
[not found] ` <1510229853-25875-1-git-send-email-bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
2017-11-09 14:44 ` Steve Wise
2017-11-13 21:59 ` Doug Ledford
-- strict thread matches above, loose matches on Subject: below --
2017-11-09 11:27 Chelsio Cudbg
[not found] ` <1510226844-4165-1-git-send-email-chelsiocudbg-KJX8L1YACloTKYOLMXNBRxrm3jAUxWOA@public.gmane.org>
2017-11-09 11:36 ` Potnuri Bharat Teja
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.