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]

 



+tj@xxxxxxxxxx
+hch@xxxxxxxxxxxxx
+sagi@

Hey Tejun, Christoph, and Sagi,

We seem to be (still) unclear on when and why work queues should be
created with WQ_MEM_RECLAIM.  Care to comment on this patch, as well as
the earlier discusson in [3] that led to removing WQ_MEM_RECLAIM on all
the RDMA workqueues?

more below:

On 1/31/2019 2:56 PM, Parav Pandit wrote:
>
>> -----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.

Funny, I reported [3] which led to removing WQ_MEM_RECLAIM on all the
rdma/core wqs [1] and [2].  If that was the correct thing to do, then
perhaps the nvme host workqueue, nvme_wq from nvme/host/core.c, needs to
remove WQ_MEM_RECLAIM:

...

int __init nvme_core_init(void)
{
        int result = -ENOMEM;

        nvme_wq = alloc_workqueue("nvme-wq",
                        WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_SYSFS, 0);
        if (!nvme_wq)
                goto out;
...




[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