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: <cover.1599072725.git.nachukannan@gmail.com> <CAEXW_YSt49yGH+Wc63zfm4i7n1M_YxETbAA8nxmfDH1B3WFQXg@mail.gmail.com>
 <CAL_JsqK7QRgCcD01MfVermvTGgLPj8KC412kxSQg2zsp_46fgQ@mail.gmail.com>
In-Reply-To: <CAL_JsqK7QRgCcD01MfVermvTGgLPj8KC412kxSQg2zsp_46fgQ@mail.gmail.com>
From:   Joel Fernandes <joel@joelfernandes.org>
Date:   Thu, 10 Sep 2020 21:25:11 -0400
Message-ID: <CAEXW_YTr9VaEY78S_C5iN1EH3hySZOED8F37t1A=7PNgbQK9CA@mail.gmail.com>
Subject: Re: [RFC PATCH 0/7] Trace events to pstore
To:     Rob Herring <robh+dt@kernel.org>
Cc:     Nachammai Karuppiah <nachukannan@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>,
        Frank Rowand <frowand.list@gmail.com>,
        Kees Cook <keescook@chromium.org>,
        Anton Vorontsov <anton@enomsg.org>,
        Colin Cross <ccross@android.com>,
        Tony Luck <tony.luck@intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
        Brian Norris <computersforpeace@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Rob,
(Back from holidays, digging through the email pile). Reply below:

On Thu, Sep 3, 2020 at 2:09 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Wed, Sep 2, 2020 at 3:47 PM Joel Fernandes <joel@joelfernandes.org> wr=
ote:
> >
> > On Wed, Sep 2, 2020 at 4:01 PM Nachammai Karuppiah
> > <nachukannan@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > This patch series adds support to store trace events in pstore.
> > >
> > > Storing trace entries in persistent RAM would help in understanding w=
hat
> > > happened just before the system went down. The trace events that led =
to the
> > > crash can be retrieved from the pstore after a warm reboot. This will=
 help
> > > debug what happened before machine=E2=80=99s last breath. This has to=
 be done in a
> > > scalable way so that tracing a live system does not impact the perfor=
mance
> > > of the system.
> >
> > Just to add, Nachammai was my intern in the recent outreachy program
> > and we designed together a way for trace events to be written to
> > pstore backed memory directory instead of regular memory. The basic
> > idea is to allocate frace's ring buffer on pstore memory and have it
> > right there. Then recover it on reboot. Nachammai wrote the code with
> > some guidance :) . I talked to Steve as well in the past about the
> > basic of idea of this. Steve is on vacation this week though.
>
> ramoops is already the RAM backend for pstore and ramoops already has
> an ftrace region defined. What am I missing?

ramoops is too slow for tracing. Honestly, the ftrace functionality in
ramoops should be removed in favor of Nachammai's patches (she did it
for events but function tracing could be trivially added). No one uses
the current ftrace in pstore because it is darned slow. ramoops sits
in between the writing of the ftrace record and the memory being
written to adding more overhead in the process, while also writing
ftrace records in a non-ftrace format. So ramoop's API and
infrastructure fundamentally does not meet the requirements of high
speed persistent tracing.  The idea of this work is to keep the trace
events enabled for a long period time (possibly even in production)
and low overhead until the problem like machine crashing happens.

> From a DT standpoint, we already have a reserved persistent RAM
> binding too. There's already too much kernel specifics on how it is
> used, we don't need more of that in DT. We're not going to add another
> separate region (actually, you can have as many regions defined as you
> want. They will just all be 'ramoops' compatible).

I agree with the sentiment here on DT. Maybe the DT can be generalized
to provide a ram region to which either ramoops or ramtrace can
attach.

 - Joel