* [RFC] IB/srp: Remove create_workqueue
@ 2016-06-07 18:16 Bhaktipriya Shridhar
2016-06-07 18:32 ` Bart Van Assche
0 siblings, 1 reply; 7+ messages in thread
From: Bhaktipriya Shridhar @ 2016-06-07 18:16 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Tejun Heo, linux-rdma, linux-kernel
alloc_workqueue replaces deprecated create_workqueue().
A dedicated workqueue has been used since the workqueue srp_remove_wq with
workitem &target->remove_work, is a work queue for the SRP target removal.
WQ_MEM_RECLAIM has been set to ensure forward progress under memory
pressure.
Since there are only a fixed number of work items, explicit
concurrency limit is unnecessary here.
Is the workqueue being used on a memory reclaim path?
Does it require WQ_MEM_RECLAIM?
Thanks.
Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 646de17..1730d1f 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3685,7 +3685,7 @@ static int __init srp_init_module(void)
indirect_sg_entries = cmd_sg_entries;
}
- srp_remove_wq = create_workqueue("srp_remove");
+ srp_remove_wq = alloc_workqueue("srp_remove", WQ_MEM_RECLAIM, 0);
if (!srp_remove_wq) {
ret = -ENOMEM;
goto out;
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC] IB/srp: Remove create_workqueue
2016-06-07 18:16 [RFC] IB/srp: Remove create_workqueue Bhaktipriya Shridhar
@ 2016-06-07 18:32 ` Bart Van Assche
2016-06-07 19:21 ` Tejun Heo
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2016-06-07 18:32 UTC (permalink / raw)
To: Bhaktipriya Shridhar, Bart Van Assche, Doug Ledford, Sean Hefty,
Hal Rosenstock
Cc: Tejun Heo, linux-rdma, linux-kernel
On 06/07/2016 11:16 AM, Bhaktipriya Shridhar wrote:
> alloc_workqueue replaces deprecated create_workqueue().
>
> A dedicated workqueue has been used since the workqueue srp_remove_wq with
> workitem &target->remove_work, is a work queue for the SRP target removal.
> WQ_MEM_RECLAIM has been set to ensure forward progress under memory
> pressure.
> Since there are only a fixed number of work items, explicit
> concurrency limit is unnecessary here.
>
> Is the workqueue being used on a memory reclaim path?
> Does it require WQ_MEM_RECLAIM?
Hello Bhaktipriya,
srp_remove_wq is used for SRP target port removal work only. This work
is neither queued from inside a shrinker nor by the page writeback code
so I think it is safe to drop WQ_MEM_RECLAIM.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] IB/srp: Remove create_workqueue
2016-06-07 18:32 ` Bart Van Assche
@ 2016-06-07 19:21 ` Tejun Heo
2016-06-07 19:56 ` Bart Van Assche
0 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2016-06-07 19:21 UTC (permalink / raw)
To: Bart Van Assche
Cc: Bhaktipriya Shridhar, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel
On Tue, Jun 07, 2016 at 11:32:42AM -0700, Bart Van Assche wrote:
> On 06/07/2016 11:16 AM, Bhaktipriya Shridhar wrote:
> > alloc_workqueue replaces deprecated create_workqueue().
> >
> > A dedicated workqueue has been used since the workqueue srp_remove_wq with
> > workitem &target->remove_work, is a work queue for the SRP target removal.
> > WQ_MEM_RECLAIM has been set to ensure forward progress under memory
> > pressure.
> > Since there are only a fixed number of work items, explicit
> > concurrency limit is unnecessary here.
> >
> > Is the workqueue being used on a memory reclaim path?
> > Does it require WQ_MEM_RECLAIM?
>
> Hello Bhaktipriya,
>
> srp_remove_wq is used for SRP target port removal work only. This work is
> neither queued from inside a shrinker nor by the page writeback code so I
> think it is safe to drop WQ_MEM_RECLAIM.
It should be able to use system_wq then.
Thanks for the explanation!
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] IB/srp: Remove create_workqueue
2016-06-07 19:21 ` Tejun Heo
@ 2016-06-07 19:56 ` Bart Van Assche
2016-06-07 20:00 ` Bart Van Assche
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2016-06-07 19:56 UTC (permalink / raw)
To: Tejun Heo
Cc: Bhaktipriya Shridhar, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel
On 06/07/2016 12:21 PM, Tejun Heo wrote:
> On Tue, Jun 07, 2016 at 11:32:42AM -0700, Bart Van Assche wrote:
>> On 06/07/2016 11:16 AM, Bhaktipriya Shridhar wrote:
>>> alloc_workqueue replaces deprecated create_workqueue().
>>>
>>> A dedicated workqueue has been used since the workqueue srp_remove_wq with
>>> workitem &target->remove_work, is a work queue for the SRP target removal.
>>> WQ_MEM_RECLAIM has been set to ensure forward progress under memory
>>> pressure.
>>> Since there are only a fixed number of work items, explicit
>>> concurrency limit is unnecessary here.
>>>
>>> Is the workqueue being used on a memory reclaim path?
>>> Does it require WQ_MEM_RECLAIM?
>>
>> Hello Bhaktipriya,
>>
>> srp_remove_wq is used for SRP target port removal work only. This work is
>> neither queued from inside a shrinker nor by the page writeback code so I
>> think it is safe to drop WQ_MEM_RECLAIM.
>
> It should be able to use system_wq then.
No. I have tried that but that resulted in a deadlock.
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] IB/srp: Remove create_workqueue
2016-06-07 19:56 ` Bart Van Assche
@ 2016-06-07 20:00 ` Bart Van Assche
2016-06-20 18:59 ` Tejun Heo
0 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2016-06-07 20:00 UTC (permalink / raw)
To: Tejun Heo
Cc: Bhaktipriya Shridhar, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel
On 06/07/2016 12:56 PM, Bart Van Assche wrote:
> On 06/07/2016 12:21 PM, Tejun Heo wrote:
>> On Tue, Jun 07, 2016 at 11:32:42AM -0700, Bart Van Assche wrote:
>>> On 06/07/2016 11:16 AM, Bhaktipriya Shridhar wrote:
>>>> alloc_workqueue replaces deprecated create_workqueue().
>>>>
>>>> A dedicated workqueue has been used since the workqueue srp_remove_wq with
>>>> workitem &target->remove_work, is a work queue for the SRP target removal.
>>>> WQ_MEM_RECLAIM has been set to ensure forward progress under memory
>>>> pressure.
>>>> Since there are only a fixed number of work items, explicit
>>>> concurrency limit is unnecessary here.
>>>>
>>>> Is the workqueue being used on a memory reclaim path?
>>>> Does it require WQ_MEM_RECLAIM?
>>>
>>> Hello Bhaktipriya,
>>>
>>> srp_remove_wq is used for SRP target port removal work only. This work is
>>> neither queued from inside a shrinker nor by the page writeback code so I
>>> think it is safe to drop WQ_MEM_RECLAIM.
>>
>> It should be able to use system_wq then.
>
> No. I have tried that but that resulted in a deadlock.
See also commit bcc059103591 for the details.
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] IB/srp: Remove create_workqueue
2016-06-07 20:00 ` Bart Van Assche
@ 2016-06-20 18:59 ` Tejun Heo
2016-06-21 8:14 ` Bart Van Assche
0 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2016-06-20 18:59 UTC (permalink / raw)
To: Bart Van Assche
Cc: Bhaktipriya Shridhar, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel
Hello, Bart.
On Tue, Jun 07, 2016 at 01:00:13PM -0700, Bart Van Assche wrote:
> > > > srp_remove_wq is used for SRP target port removal work only. This work is
> > > > neither queued from inside a shrinker nor by the page writeback code so I
> > > > think it is safe to drop WQ_MEM_RECLAIM.
> > >
> > > It should be able to use system_wq then.
> >
> > No. I have tried that but that resulted in a deadlock.
>
> See also commit bcc059103591 for the details.
So, create_workqueue() limits concurrency to 1 per cpu and if you have
a dependency between two work items and they get scheduled on the same
cpu they can deadlock. system_wq doesn't have that restriction and
should be fine, AFAICS.
Thanks!
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] IB/srp: Remove create_workqueue
2016-06-20 18:59 ` Tejun Heo
@ 2016-06-21 8:14 ` Bart Van Assche
0 siblings, 0 replies; 7+ messages in thread
From: Bart Van Assche @ 2016-06-21 8:14 UTC (permalink / raw)
To: Tejun Heo
Cc: Bhaktipriya Shridhar, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel
On 06/20/2016 08:59 PM, Tejun Heo wrote:
> On Tue, Jun 07, 2016 at 01:00:13PM -0700, Bart Van Assche wrote:
>>>>> srp_remove_wq is used for SRP target port removal work only. This work is
>>>>> neither queued from inside a shrinker nor by the page writeback code so I
>>>>> think it is safe to drop WQ_MEM_RECLAIM.
>>>>
>>>> It should be able to use system_wq then.
>>>
>>> No. I have tried that but that resulted in a deadlock.
>>
>> See also commit bcc059103591 for the details.
>
> So, create_workqueue() limits concurrency to 1 per cpu and if you have
> a dependency between two work items and they get scheduled on the same
> cpu they can deadlock. system_wq doesn't have that restriction and
> should be fine, AFAICS.
Agreed, as long as WQ_DFL_ACTIVE is not reduced from its current value
(256) to a very low value (e.g. 1 or 2). This assumption should be
documented but I'm not sure what the best way is to document this...
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-21 8:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07 18:16 [RFC] IB/srp: Remove create_workqueue Bhaktipriya Shridhar
2016-06-07 18:32 ` Bart Van Assche
2016-06-07 19:21 ` Tejun Heo
2016-06-07 19:56 ` Bart Van Assche
2016-06-07 20:00 ` Bart Van Assche
2016-06-20 18:59 ` Tejun Heo
2016-06-21 8:14 ` Bart Van Assche
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).