On Tue, Jun 04, 2024 at 09:36:08AM +0000, Bernd Schubert wrote:
> On 6/4/24 11:26, Peter Zijlstra wrote:
> > On Thu, May 30, 2024 at 04:37:29PM -0400, Josef Bacik wrote:
> >> On Wed, May 29, 2024 at 08:00:50PM +0200, Bernd Schubert wrote:
> >>> This is needed by fuse-over-io-uring to wake up the waiting
> >>> application thread on the core it was submitted from.
> >>> Avoiding core switching is actually a major factor for
> >>> fuse performance improvements of fuse-over-io-uring.
> >>>
> >>> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
> >>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> >>> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> >>> Cc: Andrei Vagin <avagin@xxxxxxxxxx>
> >>
> >> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> >>
> >> Probably best to submit this as a one-off so the sched guys can take it and it's
> >> not in the middle of a fuse patchset they may be ignoring. Thanks,
> >
> > On its own its going to not be applied. Never merge an EXPORT without a
> > user.
> >
> > As is, I don't have enough of the series to even see the user, so yeah,
> > not happy :/
> >
> > And as hch said, this very much needs to be a GPL export.
>
> Sorry, accidentally done without the _GPL. What is the right way to get this merged?
> First merge the entire fuse-io-uring series and then add on this? I already have these
> optimization patches at the end of the series... The user for this is in the next patch
Yeah, but you didn't send me the next patch, did you? So I have no
clue.. :-)
Anyway, if you could add a wee comment to __wake_up_con_current_cpu()
along with the EXPORT_SYMBOL_GPL() that might be nice. I suppose you can
copy paste from __wake_up() and then edit a wee bit.
> [PATCH RFC v2 16/19] fuse: {uring} Wake requests on the the current cpu
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index c7fd3849a105..851c5fa99946 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -333,7 +333,10 @@ void fuse_request_end(struct fuse_req *req)
> spin_unlock(&fc->bg_lock);
> } else {
> /* Wake up waiter sleeping in request_wait_answer() */
> - wake_up(&req->waitq);
> + if (fuse_per_core_queue(fc))
> + __wake_up_on_current_cpu(&req->waitq, TASK_NORMAL, NULL);
> + else
> + wake_up(&req->waitq);
> }
>
> if (test_bit(FR_ASYNC, &req->flags))
Fair enough, although do we want a helper like wake_up() -- something
like wake_up_on_current_cpu() ?
