[PATCH] netfilter: conntrack: use power efficient workqueue

[PATCH] netfilter: conntrack: use power efficient workqueue

[Vincent Guittot]

conntrack uses the bounded system_long_wq workqueue for its works that
don't have to run on the cpu they have been queued.
Using bounded workqueue prevents the scheduler to make smart decision about
the best place to schedule the work.

This patch replaces system_long_wq with system_power_efficient_wq. the work
stays bounded to a cpu by default unless the CONFIG_WQ_POWER_EFFICIENT is
enable. In the latter case, the work can be scheduled on the best cpu from
a power or a performance point of view.

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 net/netfilter/nf_conntrack_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 0113039..ae7d3e4 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1083,7 +1083,7 @@ static void gc_worker(struct work_struct *work)
 	next_run = gc_work->next_gc_run;
 	gc_work->last_bucket = i;
 	gc_work->early_drop = false;
-	queue_delayed_work(system_long_wq, &gc_work->dwork, next_run);
+	queue_delayed_work(system_power_efficient_wq, &gc_work->dwork, next_run);
 }
 
 static void conntrack_gc_work_init(struct conntrack_gc_work *gc_work)
@@ -2084,7 +2084,7 @@ int nf_conntrack_init_start(void)
 		goto err_proto;
 
 	conntrack_gc_work_init(&conntrack_gc_work);
-	queue_delayed_work(system_long_wq, &conntrack_gc_work.dwork, HZ);
+	queue_delayed_work(system_power_efficient_wq, &conntrack_gc_work.dwork, HZ);
 
 	return 0;
 
-- 
2.7.4

Re: [PATCH] netfilter: conntrack: use power efficient workqueue

[Pablo Neira Ayuso]

On Thu, 2 Nov 2017 15:16:07 +0100 Vincent Guittot <vincent.guittot@linaro.org> wrote:
> conntrack uses the bounded system_long_wq workqueue for its works that
> don't have to run on the cpu they have been queued.  Using bounded
> workqueue prevents the scheduler to make smart decision about the best
> place to schedule the work.
>
> This patch replaces system_long_wq with system_power_efficient_wq.
> the work stays bounded to a cpu by default unless the
> CONFIG_WQ_POWER_EFFICIENT is enable. In the latter case, the work can
> be scheduled on the best cpu from a power or a performance point of
> view.

Applied, thanks.

Re: [PATCH] netfilter: conntrack: use power efficient workqueue

[Pablo Neira Ayuso]

On Mon, Nov 06, 2017 at 03:31:55PM +0100, Pablo Neira Ayuso wrote:
> On Thu, 2 Nov 2017 15:16:07 +0100 Vincent Guittot <vincent.guittot@linaro.org> wrote:
> > conntrack uses the bounded system_long_wq workqueue for its works that
> > don't have to run on the cpu they have been queued.  Using bounded
> > workqueue prevents the scheduler to make smart decision about the best
> > place to schedule the work.
> >
> > This patch replaces system_long_wq with system_power_efficient_wq.
> > the work stays bounded to a cpu by default unless the
> > CONFIG_WQ_POWER_EFFICIENT is enable. In the latter case, the work can
> > be scheduled on the best cpu from a power or a performance point of
> > view.
> 
> Applied, thanks.

I'm stepping back. According to what I'm reading
system_power_efficient_wq becomes system_wq when disabled, which is
not semantically the same as system_long_wq that we have now.

My concern is that the conntrack garbage collector may run for quite a
bit of time. Did you test this with a large conntrack table full of
entries?

Thanks.

Re: [PATCH] netfilter: conntrack: use power efficient workqueue

[Vincent Guittot]

Hi Pablo,

On 6 November 2017 at 15:56, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Nov 06, 2017 at 03:31:55PM +0100, Pablo Neira Ayuso wrote:
>> On Thu, 2 Nov 2017 15:16:07 +0100 Vincent Guittot <vincent.guittot@linaro.org> wrote:
>> > conntrack uses the bounded system_long_wq workqueue for its works that
>> > don't have to run on the cpu they have been queued.  Using bounded
>> > workqueue prevents the scheduler to make smart decision about the best
>> > place to schedule the work.
>> >
>> > This patch replaces system_long_wq with system_power_efficient_wq.
>> > the work stays bounded to a cpu by default unless the
>> > CONFIG_WQ_POWER_EFFICIENT is enable. In the latter case, the work can
>> > be scheduled on the best cpu from a power or a performance point of
>> > view.
>>
>> Applied, thanks.
>
> I'm stepping back. According to what I'm reading
> system_power_efficient_wq becomes system_wq when disabled, which is
> not semantically the same as system_long_wq that we have now.

When disabled, system_power_efficient_wq behaves like system_wq (and
system_long_wq) as the worqueue are bounded to a cpu but It stays a
different workqueue.

>
> My concern is that the conntrack garbage collector may run for quite a
> bit of time. Did you test this with a large conntrack table full of

No, I haven't done specific tests with a large conntrack table full of entries.

There is no system_power_efficient_long_wq. I was not convinced that
we should create one that's why I have used system_power_efficient_wq

Vincent

> entries?
>
> Thanks.

Re: [PATCH] netfilter: conntrack: use power efficient workqueue

[Pablo Neira Ayuso]

On Mon, Nov 06, 2017 at 04:15:42PM +0100, Vincent Guittot wrote:
> Hi Pablo,
> 
> On 6 November 2017 at 15:56, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Mon, Nov 06, 2017 at 03:31:55PM +0100, Pablo Neira Ayuso wrote:
> >> On Thu, 2 Nov 2017 15:16:07 +0100 Vincent Guittot <vincent.guittot@linaro.org> wrote:
> >> > conntrack uses the bounded system_long_wq workqueue for its works that
> >> > don't have to run on the cpu they have been queued.  Using bounded
> >> > workqueue prevents the scheduler to make smart decision about the best
> >> > place to schedule the work.
> >> >
> >> > This patch replaces system_long_wq with system_power_efficient_wq.
> >> > the work stays bounded to a cpu by default unless the
> >> > CONFIG_WQ_POWER_EFFICIENT is enable. In the latter case, the work can
> >> > be scheduled on the best cpu from a power or a performance point of
> >> > view.
> >>
> >> Applied, thanks.
> >
> > I'm stepping back. According to what I'm reading
> > system_power_efficient_wq becomes system_wq when disabled, which is
> > not semantically the same as system_long_wq that we have now.
> 
> When disabled, system_power_efficient_wq behaves like system_wq (and
> system_long_wq) as the worqueue are bounded to a cpu but It stays a
> different workqueue.
>
> > My concern is that the conntrack garbage collector may run for quite a
> > bit of time. Did you test this with a large conntrack table full of
> 
> No, I haven't done specific tests with a large conntrack table full of entries.
> 
> There is no system_power_efficient_long_wq. I was not convinced that
> we should create one that's why I have used system_power_efficient_wq

My concern is that this garbage collector may run intensively on busy
conntrack tables to get rid of expired entries, so my question is if
switching from system_long_wq to system_wq is a real issue.

Thanks.

Re: [PATCH] netfilter: conntrack: use power efficient workqueue

[Vincent Guittot]

On 6 November 2017 at 16:25, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Nov 06, 2017 at 04:15:42PM +0100, Vincent Guittot wrote:
>> Hi Pablo,
>>
>> On 6 November 2017 at 15:56, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > On Mon, Nov 06, 2017 at 03:31:55PM +0100, Pablo Neira Ayuso wrote:
>> >> On Thu, 2 Nov 2017 15:16:07 +0100 Vincent Guittot <vincent.guittot@linaro.org> wrote:
>> >> > conntrack uses the bounded system_long_wq workqueue for its works that
>> >> > don't have to run on the cpu they have been queued.  Using bounded
>> >> > workqueue prevents the scheduler to make smart decision about the best
>> >> > place to schedule the work.
>> >> >
>> >> > This patch replaces system_long_wq with system_power_efficient_wq.
>> >> > the work stays bounded to a cpu by default unless the
>> >> > CONFIG_WQ_POWER_EFFICIENT is enable. In the latter case, the work can
>> >> > be scheduled on the best cpu from a power or a performance point of
>> >> > view.
>> >>
>> >> Applied, thanks.
>> >
>> > I'm stepping back. According to what I'm reading
>> > system_power_efficient_wq becomes system_wq when disabled, which is
>> > not semantically the same as system_long_wq that we have now.
>>
>> When disabled, system_power_efficient_wq behaves like system_wq (and
>> system_long_wq) as the worqueue are bounded to a cpu but It stays a
>> different workqueue.
>>
>> > My concern is that the conntrack garbage collector may run for quite a
>> > bit of time. Did you test this with a large conntrack table full of
>>
>> No, I haven't done specific tests with a large conntrack table full of entries.
>>
>> There is no system_power_efficient_long_wq. I was not convinced that
>> we should create one that's why I have used system_power_efficient_wq
>
> My concern is that this garbage collector may run intensively on busy
> conntrack tables to get rid of expired entries, so my question is if
> switching from system_long_wq to system_wq is a real issue.

system_long_wq and system_wq have the same configuration. They are
just named differently so people will use one or the other but they
behave exactly the same.
Then system_power_efficient_wq is a 3rd workqueue that behaves like
system_wq and system_long_wq when CONFIG_WQ_POWER_EFFICIENT is disable
but uses ubounded workqueue when enabled

I don't think that switching from system_long_wq to
system_power_efficient_wq is an issue.
I haven't seen any problem so far but i'm haven't done specific tests
that stress the system with a large conntrack table full of entries.
Furthermore, these workqueue use default max active context (256)
which means that other work should be able to run.

Regards,
Vincent


>
> Thanks.

Re: [PATCH] netfilter: conntrack: use power efficient workqueue

[Pablo Neira Ayuso]

On Mon, Nov 06, 2017 at 05:33:43PM +0100, Vincent Guittot wrote:
> On 6 November 2017 at 16:25, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Mon, Nov 06, 2017 at 04:15:42PM +0100, Vincent Guittot wrote:
> >> Hi Pablo,
> >>
> >> On 6 November 2017 at 15:56, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >> > On Mon, Nov 06, 2017 at 03:31:55PM +0100, Pablo Neira Ayuso wrote:
> >> >> On Thu, 2 Nov 2017 15:16:07 +0100 Vincent Guittot <vincent.guittot@linaro.org> wrote:
> >> >> > conntrack uses the bounded system_long_wq workqueue for its works that
> >> >> > don't have to run on the cpu they have been queued.  Using bounded
> >> >> > workqueue prevents the scheduler to make smart decision about the best
> >> >> > place to schedule the work.
> >> >> >
> >> >> > This patch replaces system_long_wq with system_power_efficient_wq.
> >> >> > the work stays bounded to a cpu by default unless the
> >> >> > CONFIG_WQ_POWER_EFFICIENT is enable. In the latter case, the work can
> >> >> > be scheduled on the best cpu from a power or a performance point of
> >> >> > view.
> >> >>
> >> >> Applied, thanks.
> >> >
> >> > I'm stepping back. According to what I'm reading
> >> > system_power_efficient_wq becomes system_wq when disabled, which is
> >> > not semantically the same as system_long_wq that we have now.
> >>
> >> When disabled, system_power_efficient_wq behaves like system_wq (and
> >> system_long_wq) as the worqueue are bounded to a cpu but It stays a
> >> different workqueue.
> >>
> >> > My concern is that the conntrack garbage collector may run for quite a
> >> > bit of time. Did you test this with a large conntrack table full of
> >>
> >> No, I haven't done specific tests with a large conntrack table full of entries.
> >>
> >> There is no system_power_efficient_long_wq. I was not convinced that
> >> we should create one that's why I have used system_power_efficient_wq
> >
> > My concern is that this garbage collector may run intensively on busy
> > conntrack tables to get rid of expired entries, so my question is if
> > switching from system_long_wq to system_wq is a real issue.
> 
> system_long_wq and system_wq have the same configuration. They are
> just named differently so people will use one or the other but they
> behave exactly the same.

I see, this confirms what I've been reading in the code, so it's just
another queue with a different name.

> Then system_power_efficient_wq is a 3rd workqueue that behaves like
> system_wq and system_long_wq when CONFIG_WQ_POWER_EFFICIENT is disable
> but uses ubounded workqueue when enabled
> 
> I don't think that switching from system_long_wq to
> system_power_efficient_wq is an issue.
> I haven't seen any problem so far but i'm haven't done specific tests
> that stress the system with a large conntrack table full of entries.
> Furthermore, these workqueue use default max active context (256)
> which means that other work should be able to run.

Thanks for explaining, I think then system_long_wq is missing a bit
its purpose, but this is out of my scope a bit, so I have pushed out
this to nf-next.git. Thanks!