LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Riccardo Mancini <rickyman7@gmail.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ian Rogers <irogers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>, Jiri Olsa <jolsa@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-perf-users <linux-perf-users@vger.kernel.org>,
	Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Subject: Re: [RFC PATCH v3 08/15] perf workqueue: add queue_work and flush_workqueue functions
Date: Tue, 31 Aug 2021 18:23:27 +0200	[thread overview]
Message-ID: <0952ba94284218e3ac43c2bdf416866fc3dc0700.camel@gmail.com> (raw)
In-Reply-To: <CAM9d7ch4RM5rKrYLKrny3yt3ciK87aqzJ8Wt3ze87u9KBHjyXg@mail.gmail.com>

Hi,

On Tue, 2021-08-24 at 12:40 -0700, Namhyung Kim wrote:
> On Fri, Aug 20, 2021 at 3:54 AM Riccardo Mancini <rickyman7@gmail.com> wrote:
> > 
> > This patch adds functions to queue and wait work_structs, and
> > related tests.
> > 
> > When a new work item is added, the workqueue first checks if there
> > are threads to wake up. If so, it wakes it up with the given work item,
> > otherwise it will pick the next round-robin thread and queue the work
> > item to its queue. A thread which completes its queue will go to sleep.
> > 
> > The round-robin mechanism is implemented through the next_worker
> > attibute which will point to the next worker to be chosen for queueing.
> > When work is assigned to that worker or when the worker goes to sleep,
> > the pointer is moved to the next worker in the busy_list, if any.
> > When a worker is woken up, it is added in the busy list just before the
> > next_worker, so that it will be chosen as last (it's just been assigned
> > a work item).
> 
> Do we really need this?  I think some of the complexity comes
> because of this.  Can we simply put the works in a list in wq
> and workers take it out with a lock?  Then the kernel will
> distribute the works among the threads for us.
> 
> Maybe we can get rid of worker->lock too..

Having a per-thread queue has some benefits which are very useful in our case:
 - it should be able to scale to bigger machines than a shared queue (looking at
both tests from jiri, it looks like this version is somewhat better than v2, but
they're done in different conditions, so some other tests comparing the two
versions on big machines would be useful).
 - it is possible to choose the worker to execute work on, which is used in the
evlist patchset (where threads can be pinned to a cpu and evlist operations can
be done on them).

Of course, it adds some complexity over the shared queue, for example:
 - the next_worker pointer to implement the round-robin policy, for which maybe
there's a cleaner way to do it.
 - the thread "self-registration", which I think can be dropped in favor of an
array inside the workqueue (the max number of threads is limited, so having a
self-registration does not really add much flexibility and it adds contention on
the workqueue lock when threads are spun-up). Getting rid of it could reduce
workqueue spinup and stop time.

Thanks,
Riccardo

> 
> Thanks,
> Namhyung



  reply	other threads:[~2021-08-31 16:23 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20 10:53 [RFC PATCH v3 00/15] perf: add workqueue library and use it in synthetic-events Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 01/15] perf workqueue: threadpool creation and destruction Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 02/15] perf tests: add test for workqueue Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 03/15] perf workqueue: add threadpool start and stop functions Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 04/15] perf workqueue: add threadpool execute and wait functions Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 05/15] tools: add sparse context/locking annotations in compiler-types.h Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 06/15] perf workqueue: introduce workqueue struct Riccardo Mancini
2021-08-24 19:27   ` Namhyung Kim
2021-08-31 16:13     ` Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 07/15] perf workqueue: implement worker thread and management Riccardo Mancini
2021-08-30  7:22   ` Jiri Olsa
2021-08-20 10:53 ` [RFC PATCH v3 08/15] perf workqueue: add queue_work and flush_workqueue functions Riccardo Mancini
2021-08-24 19:40   ` Namhyung Kim
2021-08-31 16:23     ` Riccardo Mancini [this message]
2021-08-20 10:53 ` [RFC PATCH v3 09/15] perf workqueue: spinup threads when needed Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 10/15] perf workqueue: create global workqueue Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 11/15] perf workqueue: add utility to execute a for loop in parallel Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 12/15] perf record: setup global workqueue Riccardo Mancini
2021-08-20 10:53 ` [RFC PATCH v3 13/15] perf top: " Riccardo Mancini
2021-08-20 10:54 ` [RFC PATCH v3 14/15] perf test/synthesis: " Riccardo Mancini
2021-08-20 10:54 ` [RFC PATCH v3 15/15] perf synthetic-events: use workqueue parallel_for Riccardo Mancini
2021-08-29 21:59 ` [RFC PATCH v3 00/15] perf: add workqueue library and use it in synthetic-events Jiri Olsa
2021-08-31 15:46   ` Jiri Olsa
2021-08-31 16:57     ` Riccardo Mancini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0952ba94284218e3ac43c2bdf416866fc3dc0700.camel@gmail.com \
    --to=rickyman7@gmail.com \
    --cc=acme@kernel.org \
    --cc=alexey.v.bayduraev@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --subject='Re: [RFC PATCH v3 08/15] perf workqueue: add queue_work and flush_workqueue functions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).