| Message ID | 1509635767-12253-1-git-send-email-vincent.guittot@linaro.org |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: conntrack: use power efficient workqueue | expand |
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)