Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1595333413-30052-1-git-send-email-sumit.garg@linaro.org>
 <CAFA6WYMN=na4Pxnu1LYRVAAZRdV==5EwU-Vcq-QkRb_jaLiPmw@mail.gmail.com>
 <20200811135801.GA416071@kroah.com> <CAFA6WYMN8i96rEZuHLnskB+4k0o=K9vF1_we83P04h2BSoGjmQ@mail.gmail.com>
 <20200811145816.GA424033@kroah.com> <CAD=FV=UD=cTn6jwpYS-C-=1ORd-4azZ8ZiBR6om++2sMS1nmMg@mail.gmail.com>
 <CAFA6WYPBdOiVsKR_hSLpigN_1b9jimXKaqyRZjvKSx3xpAmLjA@mail.gmail.com>
 <CAD=FV=WccmFRkV4UUTLSYR9+7210h00Si=nG4tRs3BBuweA6ng@mail.gmail.com>
 <CAD=FV=V8UhQVQvcAp6XCmT3=6FYM=_zPELy4FTj4kMKUswaR8Q@mail.gmail.com>
 <CAFA6WYPxieH6ZTa_BFdaLuiwbqAs6r7eKmxG7ci4XtyRONGN7g@mail.gmail.com> <CAD=FV=WGh-+GTsg3-UDr-Ht48n3sRqAJ76PJVFcFuJ1ruFEqOw@mail.gmail.com>
In-Reply-To: <CAD=FV=WGh-+GTsg3-UDr-Ht48n3sRqAJ76PJVFcFuJ1ruFEqOw@mail.gmail.com>
From:   Sumit Garg <sumit.garg@linaro.org>
Date:   Fri, 14 Aug 2020 18:20:05 +0530
Message-ID: <CAFA6WYPOF6_+jxG+PCtUS1BHPsnzYtAHmcWRMpMnvorQQ+M3wg@mail.gmail.com>
Subject: Re: [RFC 0/5] Introduce NMI aware serial drivers
To:     Doug Anderson <dianders@chromium.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Daniel Thompson <daniel.thompson@linaro.org>,
        linux-serial@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
        Jiri Slaby <jslaby@suse.com>,
        Russell King - ARM Linux admin <linux@armlinux.org.uk>,
        Jason Wessel <jason.wessel@windriver.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 13 Aug 2020 at 20:56, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Thu, Aug 13, 2020 at 2:25 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > > One other idea occurred to me that's maybe simpler.  You could in
> > > theory just poll the serial port periodically to accomplish.  It would
> > > actually probably even work to call the normal serial port interrupt
> > > routine from any random CPU.  On many serial drivers the entire
> > > interrupt handler is wrapped with:
> > >
> > > spin_lock_irqsave(&uap->port.lock, flags);
> > > ...
> > > spin_unlock_irqrestore(&uap->port.lock, flags);
> > >
> > > And a few (the ones I was involved in fixing) have the similar pattern
> > > of using uart_unlock_and_check_sysrq().
> > >
> > > Any serial drivers following this pattern could have their interrupt
> > > routine called periodically just to poll for characters and it'd be
> > > fine, right?  ...and having it take a second before a sysrq comes in
> > > this case is probably not the end of the world?
> > >
> >
> > Are you proposing to have complete RX operation in polling mode with
> > RX interrupt disabled (eg. using a kernel thread)?
>
> No, I'm suggesting a hybrid approach.  Leave the interrupts enabled as
> usual, but _also_ poll every 500 ms or 1 second (maybe make it
> configurable?).  In some serial drivers (ones that hold the lock for
> the whole interrupt routine) this polling function could actually be
> the same as the normal interrupt handler so it'd be trivially easy to
> implement and maintain.
>
> NOTE: This is not the same type of polling that kgdb does today.  The
> existing polling is really only intended to work when we're dropped
> into the debugger.  This would be more like a "poll_irq" type function
> that would do all the normal work the interrupt did and is really just
> there in the case that the CPU that the interrupt is routed to is
> locked up.
>

Your idea sounds interesting. I think where we are reaching is to have
an ever active listener to serial port that can be scheduled to any
random active CPU. And to keep its CPU overhead negligible, it can
sleep and only wake-up and listen once every 500 ms or 1 second
(configurable).

I will try to think more about it and probably give it a try with a PoC.

-Sumit

>
> > > One nice benefit of this is that it would actually work _better_ on
> > > SMP systems for any sysrqs that aren't NMI safe.  Specifically with
> > > your patch series those would be queued with irq_work_queue() which
> > > means they'd be blocked if the CPU processing the NMI is stuck with
> > > IRQs disabled.
> >
> > Yes, the sysrq handlers which aren't NMI safe will behave similarly to
> > existing IRQ based sysrq handlers.
> >
> > > With the polling mechanism they'd nicely just run on a
> > > different CPU.
> >
> > It looks like polling would cause much CPU overhead. So I am not sure
> > if that is the preferred approach.
>
> Maybe now it's clearer that there should be almost no overhead.  When
> dealing with a SYSRQ it's fine if there's a bit of a delay before it's
> processed, so polling every 1 second is probably fine.
>
> -Doug