All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jkbs@redhat.com>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.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 08:05:32 +0000	[thread overview]
Message-ID: <87egah3ktv.fsf@beetle.home> (raw)
In-Reply-To: <703257ed516669b180fcce57e6745b1853da9a95.1459952558.git.marcelo.leitner@gmail.com>

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?

Thanks,
Jakub

WARNING: multiple messages have this Message-ID (diff)
From: Jakub Sitnicki <jkbs@redhat.com>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.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 10:05:32 +0200	[thread overview]
Message-ID: <87egah3ktv.fsf@beetle.home> (raw)
In-Reply-To: <703257ed516669b180fcce57e6745b1853da9a95.1459952558.git.marcelo.leitner@gmail.com>

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?

Thanks,
Jakub

  reply	other threads:[~2016-04-07  8:05 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 [this message]
2016-04-07  8:05     ` Jakub Sitnicki
2016-04-07 13:35     ` Marcelo Ricardo Leitner
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=87egah3ktv.fsf@beetle.home \
    --to=jkbs@redhat.com \
    --cc=linux-sctp@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --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.