RE: [PATCH rdma-next] RDMA/addr: create addr_wq with WQ_MEM_RECLAIM flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> owner@xxxxxxxxxxxxxxx> On Behalf Of Steve Wise
> Sent: Thursday, January 31, 2019 1:31 PM
> To: dledford@xxxxxxxxxx; Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx
> Subject: [PATCH rdma-next] RDMA/addr: create addr_wq with
> WQ_MEM_RECLAIM flag
> 
> While running NVMe/oF wire unplug tests, we hit this warning in
> kernel/workqueue.c:check_flush_dependency():
> 
> WARN_ONCE(worker && ((worker->current_pwq->wq->flags &
> 		      (WQ_MEM_RECLAIM | __WQ_LEGACY)) ==
> WQ_MEM_RECLAIM),
> 	  "workqueue: WQ_MEM_RECLAIM %s:%pf is flushing
> !WQ_MEM_RECLAIM %s:%pf",
> 	  worker->current_pwq->wq->name, worker->current_func,
> 	  target_wq->name, target_func);
> 
> Which I think means we're flushing a workq that doesn't have
> WQ_MEM_RECLAIM set, from workqueue context that does have it set.
> 
> Looking at rdma_addr_cancel() which is doing the flushing, it flushes the
> addr_wq which doesn't have MEM_RECLAIM set.  Yet rdma_addr_cancel() is
> being called by the nvme host connection timeout/reconnect workqueue
> thread that does have WQ_MEM_RECLAIM set.
> 
> So set WQ_MEM_RECLAIM on the addr_req workqueue.
>

Please add below [1] fixes by tag.
I removed this flag based on commit [2] of Sagi and discussion[3].
Which I think [2] and [3] are incorrect.

Memory reclaim path could have been triggered by processes or kernel and I think it is ok to allocate a memory in a work item trigger as part of reclaim path or otherwise.
Memory allocation in a wq has nothing to do with memory reclaim. I wish if Tejun or Christoph confirm this is right or correct our understanding.

Based these past discussions, it appears to me that WQ_MEM_RECLAIM limitation and its use is not clear.

Commit [2] also requires a fix.
This [3] is the past discussion thread when flag was dropped incorrectly as starting point.

If [3] is right, there are several wq which need a fix and proposed patch doesn't make sense too.

[1] Fixes: 39baf10310e6 ("IB/core: Fix use workqueue without WQ_MEM_RECLAIM")
[2] cb93e597779e ("cm: Don't allocate ib_cm workqueue with WQ_MEM_RECLAIM")
[3] https://www.spinics.net/lists/linux-rdma/msg53314.html

Reviewed-by: Parav Pandit <parav@xxxxxxxxxxxx> 
This was reported internally too a while back. It was in my TODO list, for a while now, thanks for the fix.
Lets get the clarity of WQ_MEM_RECLAIM once for all.
I would be happy to submit a documentation update patch once its clear.

> 
> Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/infiniband/core/addr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
> index 0dce94e3c495..1d88ee3ac8c7 100644
> --- a/drivers/infiniband/core/addr.c
> +++ b/drivers/infiniband/core/addr.c
> @@ -868,7 +868,7 @@ static int netevent_callback(struct notifier_block
> *self, unsigned long event,
> 
>  int addr_init(void)
>  {
> -	addr_wq = alloc_ordered_workqueue("ib_addr", 0);
> +	addr_wq = alloc_ordered_workqueue("ib_addr",
> WQ_MEM_RECLAIM);
>  	if (!addr_wq)
>  		return -ENOMEM;
> 
> --
> 1.8.3.1





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux