From: Sudeep Holla <sudeep.holla@arm.com>
To: Ionela Voinescu <ionela.voinescu@arm.com>,
Sudeep Holla <sudeep.holla@arm.com>
Cc: Jassi Brar <jassisinghbrar@gmail.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Kevin Hilman <khilman@baylibre.com>,
"open list:ARM/Amlogic Meson..."
<linux-amlogic@lists.infradead.org>, Da Xue <da@libre.computer>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mailbox: cancel timer before starting it
Date: Fri, 16 Oct 2020 12:52:02 +0100 [thread overview]
Message-ID: <CAPKp9uZu0MggcMGyxK__UeGRONo3VHHWrqKwjOtZt-pguS=6JA@mail.gmail.com> (raw)
In-Reply-To: <20201016085217.GA12323@arm.com>
On Fri, Oct 16, 2020 at 12:32 PM Ionela Voinescu
<ionela.voinescu@arm.com> wrote:
>
> On Thursday 15 Oct 2020 at 13:45:54 (-0500), Jassi Brar wrote:
> [..]
> > > >> --- a/drivers/mailbox/mailbox.c
> > > >> +++ b/drivers/mailbox/mailbox.c
> > > >> @@ -82,9 +82,13 @@ static void msg_submit(struct mbox_chan *chan)
> > > >> exit:
> > > >> spin_unlock_irqrestore(&chan->lock, flags);
> > > >>
> > > >> - if (!err && (chan->txdone_method & TXDONE_BY_POLL))
> > > >> - /* kick start the timer immediately to avoid delays */
> > > >> + if (!err && (chan->txdone_method & TXDONE_BY_POLL)) {
> > > >> + /* Disable the timer if already active ... */
> > > >> + hrtimer_cancel(&chan->mbox->poll_hrt);
> > > >> +
> > > >> + /* ... and kick start it immediately to avoid delays */
> > > >> hrtimer_start(&chan->mbox->poll_hrt, 0, HRTIMER_MODE_REL);
> > > >> + }
> > > >> }
> > > >>
> > > >> static void tx_tick(struct mbox_chan *chan, int r)
> > > >
> > > > I've tracked a regression back to this commit. Details to reproduce:
> > >
> > > Hi Ionela,
> > >
> > > I don't have access to your platform and I don't get what is going on
> > > from the log below.
> > >
> > > Could you please give us a bit more details about what is going on ?
> > >
> > > All this patch does is add hrtimer_cancel().
> > > * It is needed if the timer had already been started, which is
> > > appropriate AFAIU
> > > * It is a NO-OP is the timer is not active.
> > >
> > Can you please try using hrtimer_try_to_cancel() instead ?
> >
>
> Yes, using hrtimer_try_to_cancel() instead works for me. But doesn't
> this limit how effective this change is? AFAIU, this will possibly only
> reduce the chances for the race condition, but not solve it.
>
Indeed, I tried the same and got a lot of -ETIME failures. hrtimer_cancel
uses hrtimer_try_to_cancel in loop until it succeeds so that the following
hrtimer_start can be done w/o any pending timers.
--
Regards,
Sudeep
prev parent reply other threads:[~2020-10-16 11:52 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-23 12:39 [PATCH] mailbox: cancel timer before starting it Jerome Brunet
2020-09-24 16:50 ` Kevin Hilman
2020-10-15 13:46 ` Ionela Voinescu
2020-10-15 13:58 ` Jerome Brunet
2020-10-15 14:29 ` Ionela Voinescu
2020-10-16 8:44 ` Sudeep Holla
2020-10-16 9:02 ` Jerome Brunet
2020-10-16 9:34 ` Sudeep Holla
2020-10-16 11:47 ` Sudeep Holla
2020-10-15 18:45 ` Jassi Brar
2020-10-16 8:52 ` Ionela Voinescu
2020-10-16 9:00 ` Jerome Brunet
2020-10-16 13:52 ` Sudeep Holla
2020-10-16 17:33 ` Jassi Brar
2020-10-16 18:54 ` Jerome Brunet
2020-10-16 19:27 ` Jassi Brar
2022-07-19 7:08 ` Da Xue
2020-10-16 11:52 ` Sudeep Holla [this message]
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='CAPKp9uZu0MggcMGyxK__UeGRONo3VHHWrqKwjOtZt-pguS=6JA@mail.gmail.com' \
--to=sudeep.holla@arm.com \
--cc=da@libre.computer \
--cc=ionela.voinescu@arm.com \
--cc=jassisinghbrar@gmail.com \
--cc=jbrunet@baylibre.com \
--cc=khilman@baylibre.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).