From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Anna-Maria Gleixner <anna-maria@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
keescook@chromium.org, John Stultz <john.stultz@linaro.org>,
Thomas Gleixner <tglx@linutronix.de>,
Marc Kleine-Budde <mkl@pengutronix.de>,
linux-can@vger.kernel.org
Subject: Re: [PATCH 19/25] can/bcm: Replace hrtimer_tasklet with softirq based hrtimer
Date: Sat, 2 Sep 2017 19:56:36 +0200 [thread overview]
Message-ID: <b3aceee5-2ec4-90fb-bce9-b6ad6f17f458@hartkopp.net> (raw)
In-Reply-To: <20170831105827.099510718@linutronix.de>
On 08/31/2017 02:23 PM, Anna-Maria Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> Switch the timer to CLOCK_MONOTONIC_SOFT, which executed the timer
> callback in softirq context and remove the hrtimer_tasklet.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Thanks,
Oliver
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: linux-can@vger.kernel.org
> ---
> net/can/bcm.c | 150 ++++++++++++++++++----------------------------------------
> 1 file changed, 49 insertions(+), 101 deletions(-)
>
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -102,7 +102,6 @@ struct bcm_op {
> unsigned long frames_abs, frames_filtered;
> struct bcm_timeval ival1, ival2;
> struct hrtimer timer, thrtimer;
> - struct tasklet_struct tsklet, thrtsklet;
> ktime_t rx_stamp, kt_ival1, kt_ival2, kt_lastmsg;
> int rx_ifindex;
> int cfsiz;
> @@ -364,25 +363,34 @@ static void bcm_send_to_user(struct bcm_
> }
> }
>
> -static void bcm_tx_start_timer(struct bcm_op *op)
> +static bool bcm_tx_set_expiry(struct bcm_op *op, struct hrtimer *hrt)
> {
> + ktime_t ival;
> +
> if (op->kt_ival1 && op->count)
> - hrtimer_start(&op->timer,
> - ktime_add(ktime_get(), op->kt_ival1),
> - HRTIMER_MODE_ABS);
> + ival = op->kt_ival1;
> else if (op->kt_ival2)
> - hrtimer_start(&op->timer,
> - ktime_add(ktime_get(), op->kt_ival2),
> - HRTIMER_MODE_ABS);
> + ival = op->kt_ival2;
> + else
> + return false;
> +
> + hrtimer_set_expires(hrt, ktime_add(ktime_get(), ival));
> + return true;
> }
>
> -static void bcm_tx_timeout_tsklet(unsigned long data)
> +static void bcm_tx_start_timer(struct bcm_op *op)
> {
> - struct bcm_op *op = (struct bcm_op *)data;
> + if (bcm_tx_set_expiry(op, &op->timer))
> + hrtimer_start_expires(&op->timer, HRTIMER_MODE_ABS);
> +}
> +
> +/* bcm_tx_timeout_handler - performs cyclic CAN frame transmissions */
> +static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
> +{
> + struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
> struct bcm_msg_head msg_head;
>
> if (op->kt_ival1 && (op->count > 0)) {
> -
> op->count--;
> if (!op->count && (op->flags & TX_COUNTEVT)) {
>
> @@ -399,22 +407,12 @@ static void bcm_tx_timeout_tsklet(unsign
> }
> bcm_can_tx(op);
>
> - } else if (op->kt_ival2)
> + } else if (op->kt_ival2) {
> bcm_can_tx(op);
> + }
>
> - bcm_tx_start_timer(op);
> -}
> -
> -/*
> - * bcm_tx_timeout_handler - performs cyclic CAN frame transmissions
> - */
> -static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
> -{
> - struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
> -
> - tasklet_schedule(&op->tsklet);
> -
> - return HRTIMER_NORESTART;
> + return bcm_tx_set_expiry(op, &op->timer) ?
> + HRTIMER_RESTART : HRTIMER_NORESTART;
> }
>
> /*
> @@ -542,11 +540,18 @@ static void bcm_rx_starttimer(struct bcm
> hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL);
> }
>
> -static void bcm_rx_timeout_tsklet(unsigned long data)
> +/* bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out */
> +static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
> {
> - struct bcm_op *op = (struct bcm_op *)data;
> + struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
> struct bcm_msg_head msg_head;
>
> + /* if user wants to be informed, when cyclic CAN-Messages come back */
> + if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
> + /* clear received CAN frames to indicate 'nothing received' */
> + memset(op->last_frames, 0, op->nframes * op->cfsiz);
> + }
> +
> /* create notification to user */
> msg_head.opcode = RX_TIMEOUT;
> msg_head.flags = op->flags;
> @@ -557,25 +562,6 @@ static void bcm_rx_timeout_tsklet(unsign
> msg_head.nframes = 0;
>
> bcm_send_to_user(op, &msg_head, NULL, 0);
> -}
> -
> -/*
> - * bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out
> - */
> -static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
> -{
> - struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
> -
> - /* schedule before NET_RX_SOFTIRQ */
> - tasklet_hi_schedule(&op->tsklet);
> -
> - /* no restart of the timer is done here! */
> -
> - /* if user wants to be informed, when cyclic CAN-Messages come back */
> - if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
> - /* clear received CAN frames to indicate 'nothing received' */
> - memset(op->last_frames, 0, op->nframes * op->cfsiz);
> - }
>
> return HRTIMER_NORESTART;
> }
> @@ -583,14 +569,12 @@ static enum hrtimer_restart bcm_rx_timeo
> /*
> * bcm_rx_do_flush - helper for bcm_rx_thr_flush
> */
> -static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
> - unsigned int index)
> +static inline int bcm_rx_do_flush(struct bcm_op *op, unsigned int index)
> {
> struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;
>
> if ((op->last_frames) && (lcf->flags & RX_THR)) {
> - if (update)
> - bcm_rx_changed(op, lcf);
> + bcm_rx_changed(op, lcf);
> return 1;
> }
> return 0;
> @@ -598,11 +582,8 @@ static inline int bcm_rx_do_flush(struct
>
> /*
> * bcm_rx_thr_flush - Check for throttled data and send it to the userspace
> - *
> - * update == 0 : just check if throttled data is available (any irq context)
> - * update == 1 : check and send throttled data to userspace (soft_irq context)
> */
> -static int bcm_rx_thr_flush(struct bcm_op *op, int update)
> +static int bcm_rx_thr_flush(struct bcm_op *op)
> {
> int updated = 0;
>
> @@ -611,24 +592,16 @@ static int bcm_rx_thr_flush(struct bcm_o
>
> /* for MUX filter we start at index 1 */
> for (i = 1; i < op->nframes; i++)
> - updated += bcm_rx_do_flush(op, update, i);
> + updated += bcm_rx_do_flush(op, i);
>
> } else {
> /* for RX_FILTER_ID and simple filter */
> - updated += bcm_rx_do_flush(op, update, 0);
> + updated += bcm_rx_do_flush(op, 0);
> }
>
> return updated;
> }
>
> -static void bcm_rx_thr_tsklet(unsigned long data)
> -{
> - struct bcm_op *op = (struct bcm_op *)data;
> -
> - /* push the changed data to the userspace */
> - bcm_rx_thr_flush(op, 1);
> -}
> -
> /*
> * bcm_rx_thr_handler - the time for blocked content updates is over now:
> * Check for throttled data and send it to the userspace
> @@ -637,9 +610,7 @@ static enum hrtimer_restart bcm_rx_thr_h
> {
> struct bcm_op *op = container_of(hrtimer, struct bcm_op, thrtimer);
>
> - tasklet_schedule(&op->thrtsklet);
> -
> - if (bcm_rx_thr_flush(op, 0)) {
> + if (bcm_rx_thr_flush(op)) {
> hrtimer_forward(hrtimer, ktime_get(), op->kt_ival2);
> return HRTIMER_RESTART;
> } else {
> @@ -735,23 +706,8 @@ static struct bcm_op *bcm_find_op(struct
>
> static void bcm_remove_op(struct bcm_op *op)
> {
> - if (op->tsklet.func) {
> - while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
> - test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
> - hrtimer_active(&op->timer)) {
> - hrtimer_cancel(&op->timer);
> - tasklet_kill(&op->tsklet);
> - }
> - }
> -
> - if (op->thrtsklet.func) {
> - while (test_bit(TASKLET_STATE_SCHED, &op->thrtsklet.state) ||
> - test_bit(TASKLET_STATE_RUN, &op->thrtsklet.state) ||
> - hrtimer_active(&op->thrtimer)) {
> - hrtimer_cancel(&op->thrtimer);
> - tasklet_kill(&op->thrtsklet);
> - }
> - }
> + hrtimer_cancel(&op->timer);
> + hrtimer_cancel(&op->thrtimer);
>
> if ((op->frames) && (op->frames != &op->sframe))
> kfree(op->frames);
> @@ -979,15 +935,13 @@ static int bcm_tx_setup(struct bcm_msg_h
> op->ifindex = ifindex;
>
> /* initialize uninitialized (kzalloc) structure */
> - hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + hrtimer_init(&op->timer, CLOCK_MONOTONIC_SOFT,
> + HRTIMER_MODE_REL);
> op->timer.function = bcm_tx_timeout_handler;
>
> - /* initialize tasklet for tx countevent notification */
> - tasklet_init(&op->tsklet, bcm_tx_timeout_tsklet,
> - (unsigned long) op);
> -
> /* currently unused in tx_ops */
> - hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC_SOFT,
> + HRTIMER_MODE_REL);
>
> /* add this bcm_op to the list of the tx_ops */
> list_add(&op->list, &bo->tx_ops);
> @@ -1150,20 +1104,14 @@ static int bcm_rx_setup(struct bcm_msg_h
> op->rx_ifindex = ifindex;
>
> /* initialize uninitialized (kzalloc) structure */
> - hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + hrtimer_init(&op->timer, CLOCK_MONOTONIC_SOFT,
> + HRTIMER_MODE_REL);
> op->timer.function = bcm_rx_timeout_handler;
>
> - /* initialize tasklet for rx timeout notification */
> - tasklet_init(&op->tsklet, bcm_rx_timeout_tsklet,
> - (unsigned long) op);
> -
> - hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC_SOFT,
> + HRTIMER_MODE_REL);
> op->thrtimer.function = bcm_rx_thr_handler;
>
> - /* initialize tasklet for rx throttle handling */
> - tasklet_init(&op->thrtsklet, bcm_rx_thr_tsklet,
> - (unsigned long) op);
> -
> /* add this bcm_op to the list of the rx_ops */
> list_add(&op->list, &bo->rx_ops);
>
> @@ -1209,7 +1157,7 @@ static int bcm_rx_setup(struct bcm_msg_h
> */
> op->kt_lastmsg = 0;
> hrtimer_cancel(&op->thrtimer);
> - bcm_rx_thr_flush(op, 1);
> + bcm_rx_thr_flush(op);
> }
>
> if ((op->flags & STARTTIMER) && op->kt_ival1)
>
>
next prev parent reply other threads:[~2017-09-02 17:57 UTC|newest]
Thread overview: 86+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-31 12:23 [PATCH 00/25] hrtimer: Provide softirq context hrtimers Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 01/25] hrtimer: Use predefined function for updating next_timer Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 02/25] hrtimer: Correct blantanly wrong comment Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 03/25] hrtimer: Fix kerneldoc for struct hrtimer_cpu_base Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 04/25] hrtimer: Cleanup clock argument in schedule_hrtimeout_range_clock() Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 05/25] hrtimer: Switch for loop to _ffs() evaluation Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 07/25] hrtimer: Reduce conditional code (hres_active) Anna-Maria Gleixner
2017-09-25 13:55 ` Peter Zijlstra
2017-09-25 14:28 ` Thomas Gleixner
2017-08-31 12:23 ` [PATCH 06/25] hrtimer: Store running timer in hrtimer_clock_base Anna-Maria Gleixner
2017-09-25 14:44 ` Peter Zijlstra
2017-09-28 12:45 ` Thomas Gleixner
2017-08-31 12:23 ` [PATCH 08/25] hrtimer: Reduce conditional code (expires_next, next_timer) Anna-Maria Gleixner
2017-09-26 12:10 ` Peter Zijlstra
2017-09-28 8:07 ` Thomas Gleixner
2017-08-31 12:23 ` [PATCH 09/25] hrtimer: Reduce conditional code (hrtimer_reprogram()) Anna-Maria Gleixner
2017-09-26 12:07 ` Peter Zijlstra
2017-08-31 12:23 ` [PATCH 10/25] hrtimer: Make handling of hrtimer reprogramming and enqueuing not conditional Anna-Maria Gleixner
2017-09-26 12:14 ` Peter Zijlstra
2017-09-28 8:09 ` Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 11/25] hrtimer: Allow remote hrtimer enqueue with "expires_next" as expiry time Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 13/25] hrtimer: Split out code from hrtimer_start_range_ns() for reuse Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 12/25] hrtimer: Simplify hrtimer_reprogram() call Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 14/25] hrtimer: Split out code from __hrtimer_get_next_event() for reuse Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 15/25] hrtimer: Add clock bases for soft irq context Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 16/25] hrtimer: Allow function reuse for softirq based hrtimer Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 18/25] hrtimer: Enable soft and hard hrtimer Anna-Maria Gleixner
2017-09-26 12:52 ` Peter Zijlstra
2017-09-27 14:18 ` Anna-Maria Gleixner
2017-09-27 15:54 ` Thomas Gleixner
2017-09-27 16:47 ` Peter Zijlstra
2017-08-31 12:23 ` [PATCH 17/25] hrtimer: Implementation of softirq hrtimer handling Anna-Maria Gleixner
2017-09-26 12:40 ` Peter Zijlstra
2017-09-26 15:03 ` Peter Zijlstra
2017-09-27 14:22 ` Anna-Maria Gleixner
2017-09-27 16:46 ` Peter Zijlstra
2017-09-27 16:40 ` Peter Zijlstra
2017-09-28 7:59 ` Thomas Gleixner
2017-09-28 8:15 ` Peter Zijlstra
2017-12-19 8:58 ` Sebastian Andrzej Siewior
2017-12-19 13:21 ` Peter Zijlstra
2017-12-20 8:44 ` Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 20/25] mac80211_hwsim: Replace hrtimer tasklet with softirq hrtimer Anna-Maria Gleixner
2017-08-31 12:23 ` Anna-Maria Gleixner
2017-09-05 7:03 ` Johannes Berg
2017-09-05 8:49 ` Thomas Gleixner
2017-09-05 8:51 ` Johannes Berg
2017-08-31 12:23 ` [PATCH 19/25] can/bcm: Replace hrtimer_tasklet with softirq based hrtimer Anna-Maria Gleixner
2017-09-01 15:49 ` Oliver Hartkopp
2017-09-01 15:56 ` Thomas Gleixner
2017-09-01 17:02 ` Oliver Hartkopp
2017-09-02 17:56 ` Oliver Hartkopp [this message]
2017-08-31 12:23 ` [PATCH 21/25] xfrm: Replace hrtimer tasklet with softirq hrtimer Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 23/25] ALSA/dummy: Replace " Anna-Maria Gleixner
2017-08-31 14:21 ` Takashi Sakamoto
2017-08-31 14:21 ` Takashi Sakamoto
2017-08-31 14:26 ` Takashi Iwai
2017-08-31 14:26 ` Takashi Iwai
2017-08-31 15:36 ` Takashi Iwai
2017-08-31 15:36 ` Takashi Iwai
2017-09-01 10:25 ` Takashi Sakamoto
2017-09-01 10:25 ` Takashi Sakamoto
2017-09-01 11:58 ` Takashi Iwai
2017-09-01 11:58 ` Takashi Iwai
2017-09-02 1:19 ` Takashi Sakamoto
2017-09-02 1:19 ` Takashi Sakamoto
2017-09-04 12:45 ` Takashi Sakamoto
2017-09-05 15:53 ` [PATCH 23/25 v2] " Sebastian Andrzej Siewior
2017-09-05 15:53 ` Sebastian Andrzej Siewior
2017-09-05 16:02 ` Takashi Iwai
2017-09-05 16:02 ` Takashi Iwai
2017-09-05 16:05 ` Takashi Sakamoto
2017-09-05 16:18 ` [PATCH 23/25 v3] " Sebastian Andrzej Siewior
2017-09-05 16:18 ` Sebastian Andrzej Siewior
2017-09-06 4:30 ` Takashi Sakamoto
2017-09-06 4:30 ` Takashi Sakamoto
2017-09-08 8:28 ` [alsa-devel] " Takashi Iwai
2017-09-08 8:28 ` Takashi Iwai
2017-08-31 12:23 ` [PATCH 22/25] softirq: Remove tasklet_hrtimer Anna-Maria Gleixner
2017-08-31 12:23 ` [PATCH 25/25] usb/gadget/NCM: Replace tasklet with softirq hrtimer Anna-Maria Gleixner
2017-10-24 9:45 ` Felipe Balbi
2017-08-31 12:23 ` [PATCH 24/25] net/cdc_ncm: " Anna-Maria Gleixner
2017-08-31 13:33 ` Greg Kroah-Hartman
2017-08-31 13:57 ` Bjørn Mork
2017-09-05 15:42 ` [PATCH 24/25 v2] " Sebastian Andrzej Siewior
2017-08-31 12:36 ` [PATCH 00/25] hrtimer: Provide softirq context hrtimers Anna-Maria Gleixner
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=b3aceee5-2ec4-90fb-bce9-b6ad6f17f458@hartkopp.net \
--to=socketcan@hartkopp.net \
--cc=anna-maria@linutronix.de \
--cc=john.stultz@linaro.org \
--cc=keescook@chromium.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mkl@pengutronix.de \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
/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.