Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Sep 2019 09:54:09 -0600
From:      Ian Lepore <ian@freebsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        "O'Connor, Daniel" <darius@dons.net.au>, "freebsd-arm@FreeBSD.org" <freebsd-arm@FreeBSD.org>
Subject:   Re: Is it a good idea to use a usb-serial adapter for PPS input? Yes, it is.
Message-ID:  <3af74fe770557e2fee098a6353e4685bab504dc2.camel@freebsd.org>
In-Reply-To: <20190910144059.GA2559@kib.kiev.ua>
References:  <B9EFA4D4-C1AD-4181-B421-F6BD53434FA5@dons.net.au> <bfd784f6ac9c939dbe6a243336bc3b6eab02d4f5.camel@freebsd.org> <61B1AAF3-40F6-47BC-8F05-7491C13BF288@dons.net.au> <9E142F1A-5E8C-4410-91F5-7C80B3D0A15B@dons.net.au> <9D2ACA87-C2DB-40D9-9638-B0E215A4EEC0@dons.net.au> <0A7796DA-508B-4FE6-B5C0-391EC5F46C86@dons.net.au> <20190908134205.GO2559@kib.kiev.ua> <25BF53F1-23CF-4619-AB13-110566D6DC82@dons.net.au> <20190909164501.GT2559@kib.kiev.ua> <b5c8493ef95dcc49ed21fd7c7cf52808e5f0bfe8.camel@freebsd.org> <20190910144059.GA2559@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 2019-09-10 at 17:40 +0300, Konstantin Belousov wrote:
> On Mon, Sep 09, 2019 at 04:19:48PM -0600, Ian Lepore wrote:
> > On Mon, 2019-09-09 at 19:45 +0300, Konstantin Belousov wrote:
> > > [...]
> > > Still the question is why your system have the negative impact from
> > > reducing the number of timehands.  Interrupt should never interrupt
> > > tc_windup() because it is protected by spinlock.  Silly question, are
> > > spinlocks functional on this machine ?
> > > 
> > 
> > <dropping usb@ from the cc, since this is purely arm stuff>
> > 
> > Yep, spinlocks work fine.
> > 
> > The problem sequence that happens, as I remember it, is that the system
> > is sleeping in cpu_idle() which has stopped the single core with the
> > WFI (wait-for-interrupt) instruction.  The wakeup from WFI is an
> > eventtimer event that was scheduled to handle state->nexthard to keep
> > the timecounter updated.  As part of handling that event, the system
> > calls the timecounter's tc_poll_pps function (which is dmtpps_poll() in
> > arm/ti/am335x/am335x_dmtpps.c).  That captures the pps event time using
> > the current timehands, then schedules a taskqueue task to finish
> > processing the captured data.  By the time the taskqueue task runs,
> > tc_windup() has been called more times than there are timehands, so
> > pps_event() rejects the data because th->th_generation captured in
> > dmtpps_poll() does not match th_generation in that set of timehands
> > anymore.
> 
> Is it really needed to schedule task for pps_event() ?
> 
> We don't for tc_windup(), and pps_event() is quite similar.
> And for pps_event(), we might use same opportunistic locking as
> for tc_windup().
> 

I think it is not necessary anymore, and I should fix that.  It was
necessary when the code was originally written.  The call to
pps_event() has to be protected by a mutex because the same data is
accessed via ioctl().

The timecounter polling routine is called in primary interrupt context
where sleeping is not allowed, so the old way to structure a hardware-
capture driver was to do the pps_capture() (which doesn't need mutex
protection) in the polling routine called from a filter handler and
defer the pps_event() processing to context where sleeping is allowed.

In May 2015 I committed some changes to the pps api stuff that allows
using a spin mutex, so I could change the kind of mutex used by the
driver and eliminate the taskqueue stuff completely.

-- Ian




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3af74fe770557e2fee098a6353e4685bab504dc2.camel>