Re: [PATCH V1] fuse: give wakeup hints to the scheduler

From: Miklos Szeredi <miklos@szeredi.hu>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>,
	quic_stummala@quicinc.com, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, quic_pkondeti@quicinc.com,
	quic_sayalil@quicinc.com, quic_aiquny@quicinc.com,
	quic_zljing@quicinc.com, quic_blong@quicinc.com,
	quic_richardp@quicinc.com, quic_cdevired@quicinc.com,
	Pradeep P V K <quic_pragalla@quicinc.com>
Subject: Re: [PATCH V1] fuse: give wakeup hints to the scheduler
Date: Tue, 7 Dec 2021 11:20:59 +0100	[thread overview]
Message-ID: <CAJfpegsVg2K0CvrPvXGSu=Jz_R3VZZOy49Jw51rThQUJ1_9e6g@mail.gmail.com> (raw)
In-Reply-To: <Ya8ycLODlcvLx4xB@hirez.programming.kicks-ass.net>

On Tue, 7 Dec 2021 at 11:07, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Tue, Dec 07, 2021 at 10:07:45AM +0100, Miklos Szeredi wrote:
> > On Mon, 6 Dec 2021 at 09:47, Pradeep P V K <quic_pragalla@quicinc.com> wrote:
> > >
> > > The synchronous wakeup interface is available only for the
> > > interruptible wakeup. Add it for normal wakeup and use this
> > > synchronous wakeup interface to wakeup the userspace daemon.
> > > Scheduler can make use of this hint to find a better CPU for
> > > the waker task.
>
> That's a horrendoubly bad changelog :-/ Also, if you need it for
> UNINTERRUPTIBLE that's trivial to do ofc.
>
> > Ingo, Peter,
> >
> > What exactly does WF_SYNC do?   Does it try to give the waker's CPU
> > immediately to the waked?
> >
> > If that doesn't work (e.g. in this patch the wake up is done with a
> > spin lock held) does it do anything?
> >
> > Does it give a hint that the waked task should be scheduled on this
> > CPU at the next scheduling point?
>
> WF_SYNC is a hint to the scheduler that the waker is about to go sleep
> and as such it is reasonable to stack the woken thread on this CPU
> instead of going to find an idle CPU for it.
>
> Typically it also means that the waker and wakee share data, and thus
> having them share the CPU is beneficial for cache locality.

Okay, so it doesn't give up the CPU immediately to the woken task,
just marks the woken task as a "successor" when the current task goes
to sleep.  Right?

That may be good for fuse as the data is indeed shared.  It would be
even better if the woken task had already a known affinity to this
CPU, since that would mean thread local data wouldn't have to be
migrated each time...   So I'm not sure sync wakeup alone is worth it,
needs real life benchmarking.

Thanks,
Miklos