From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Jakub Sitnicki <jkbs@redhat.com>
Cc: netdev@vger.kernel.org, Neil Horman <nhorman@tuxdriver.com>,
Vlad Yasevich <vyasevich@gmail.com>,
linux-sctp@vger.kernel.org
Subject: Re: [PATCH v2 2/2] sctp: delay calls to sk_data_ready() as much as possible
Date: Thu, 07 Apr 2016 13:35:25 +0000 [thread overview]
Message-ID: <20160407133525.GD15005@localhost.localdomain> (raw)
In-Reply-To: <87egah3ktv.fsf@beetle.home>
On Thu, Apr 07, 2016 at 10:05:32AM +0200, Jakub Sitnicki wrote:
> On Wed, Apr 06, 2016 at 07:53 PM CEST, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
> > Currently, the processing of multiple chunks in a single SCTP packet
> > leads to multiple calls to sk_data_ready, causing multiple wake up
> > signals which are costly and doesn't make it wake up any faster.
> >
> > With this patch it will notice that the wake up is pending and will do it
> > before leaving the state machine interpreter, latest place possible to
> > do it realiably and cleanly.
> >
> > Note that sk_data_ready events are not dependent on asocs, unlike waking
> > up writers.
> >
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
>
> [...]
>
> > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> > index 7fe56d0acabf66cfd8fe29dfdb45f7620b470ac7..e7042f9ce63b0cfca50cae252f51b60b68cb5731 100644
> > --- a/net/sctp/sm_sideeffect.c
> > +++ b/net/sctp/sm_sideeffect.c
> > @@ -1742,6 +1742,11 @@ out:
> > error = sctp_outq_uncork(&asoc->outqueue, gfp);
> > } else if (local_cork)
> > error = sctp_outq_uncork(&asoc->outqueue, gfp);
> > +
> > + if (sctp_sk(ep->base.sk)->pending_data_ready) {
> > + ep->base.sk->sk_data_ready(ep->base.sk);
> > + sctp_sk(ep->base.sk)->pending_data_ready = 0;
> > + }
> > return error;
> > nomem:
> > error = -ENOMEM;
>
> Would it make sense to introduce a local variable for ep->base.sk (and
> make this function 535+1 lines long ;-)
>
> struct sock *sk = ep->base.sk;
>
> ... like sctp_ulpq_tail_event() does?
I guess so, yes. Same for sctp_sk() cast then. I´ll post a new version
later, thanks.
Marcelo
WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Jakub Sitnicki <jkbs@redhat.com>
Cc: netdev@vger.kernel.org, Neil Horman <nhorman@tuxdriver.com>,
Vlad Yasevich <vyasevich@gmail.com>,
linux-sctp@vger.kernel.org
Subject: Re: [PATCH v2 2/2] sctp: delay calls to sk_data_ready() as much as possible
Date: Thu, 7 Apr 2016 10:35:25 -0300 [thread overview]
Message-ID: <20160407133525.GD15005@localhost.localdomain> (raw)
In-Reply-To: <87egah3ktv.fsf@beetle.home>
On Thu, Apr 07, 2016 at 10:05:32AM +0200, Jakub Sitnicki wrote:
> On Wed, Apr 06, 2016 at 07:53 PM CEST, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
> > Currently, the processing of multiple chunks in a single SCTP packet
> > leads to multiple calls to sk_data_ready, causing multiple wake up
> > signals which are costly and doesn't make it wake up any faster.
> >
> > With this patch it will notice that the wake up is pending and will do it
> > before leaving the state machine interpreter, latest place possible to
> > do it realiably and cleanly.
> >
> > Note that sk_data_ready events are not dependent on asocs, unlike waking
> > up writers.
> >
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
>
> [...]
>
> > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> > index 7fe56d0acabf66cfd8fe29dfdb45f7620b470ac7..e7042f9ce63b0cfca50cae252f51b60b68cb5731 100644
> > --- a/net/sctp/sm_sideeffect.c
> > +++ b/net/sctp/sm_sideeffect.c
> > @@ -1742,6 +1742,11 @@ out:
> > error = sctp_outq_uncork(&asoc->outqueue, gfp);
> > } else if (local_cork)
> > error = sctp_outq_uncork(&asoc->outqueue, gfp);
> > +
> > + if (sctp_sk(ep->base.sk)->pending_data_ready) {
> > + ep->base.sk->sk_data_ready(ep->base.sk);
> > + sctp_sk(ep->base.sk)->pending_data_ready = 0;
> > + }
> > return error;
> > nomem:
> > error = -ENOMEM;
>
> Would it make sense to introduce a local variable for ep->base.sk (and
> make this function 535+1 lines long ;-)
>
> struct sock *sk = ep->base.sk;
>
> ... like sctp_ulpq_tail_event() does?
I guess so, yes. Same for sctp_sk() cast then. I´ll post a new version
later, thanks.
Marcelo
next prev parent reply other threads:[~2016-04-07 13:35 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-06 17:53 [PATCH v2 0/2] sctp: delay calls to sk_data_ready() as much as possible Marcelo Ricardo Leitner
2016-04-06 17:53 ` Marcelo Ricardo Leitner
2016-04-06 17:53 ` [PATCH v2 1/2] sctp: compress bit-wide flags to a bitfield on sctp_sock Marcelo Ricardo Leitner
2016-04-06 17:53 ` Marcelo Ricardo Leitner
2016-04-06 19:53 ` Joe Perches
2016-04-06 19:53 ` Joe Perches
2016-04-06 19:57 ` David Miller
2016-04-06 19:57 ` David Miller
2016-04-06 21:21 ` marcelo.leitner
2016-04-06 21:21 ` marcelo.leitner
2016-04-06 17:53 ` [PATCH v2 2/2] sctp: delay calls to sk_data_ready() as much as possible Marcelo Ricardo Leitner
2016-04-06 17:53 ` Marcelo Ricardo Leitner
2016-04-07 8:05 ` Jakub Sitnicki
2016-04-07 8:05 ` Jakub Sitnicki
2016-04-07 13:35 ` Marcelo Ricardo Leitner [this message]
2016-04-07 13:35 ` Marcelo Ricardo Leitner
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=20160407133525.GD15005@localhost.localdomain \
--to=marcelo.leitner@gmail.com \
--cc=jkbs@redhat.com \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=vyasevich@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.