Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
MIME-Version: 1.0
References: <20230915154856.1896062-1-lb@semihalf.com> <CAJfuBxyFYyGCtr5i=P7N=1oX3J=jmdp1VLGLt+z1fAnuvGK2aA@mail.gmail.com>
In-Reply-To: <CAJfuBxyFYyGCtr5i=P7N=1oX3J=jmdp1VLGLt+z1fAnuvGK2aA@mail.gmail.com>
From:   =?UTF-8?Q?=C5=81ukasz_Bartosik?= <lb@semihalf.com>
Date:   Mon, 18 Sep 2023 09:20:39 +0200
Message-ID: <CAK8ByeJBrPEQSgUc91LQO9Krzjh2pauhMTjEC82M8ozayE76Yg@mail.gmail.com>
Subject: Re: [PATCH v1] dynamic_debug: add support for logs destination
To:     jim.cromie@gmail.com
Cc:     Jason Baron <jbaron@akamai.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Kees Cook <keescook@chromium.org>,
        Douglas Anderson <dianders@chromium.org>,
        Guenter Roeck <groeck@google.com>,
        Yaniv Tzoreff <yanivt@google.com>,
        Benson Leung <bleung@google.com>, linux-kernel@vger.kernel.org,
        upstream@semihalf.com,
        Vincent Whitchurch <vincent.whitchurch@axis.com>,
        Steven Rostedt <rostedt@goodmis.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

pt., 15 wrz 2023 o 20:02 <jim.cromie@gmail.com> napisa=C5=82(a):
>
> On Fri, Sep 15, 2023 at 9:49=E2=80=AFAM =C5=81ukasz Bartosik <lb@semihalf=
.com> wrote:
> >
> > Add support for selection of dynamic debug logs destination.
> > When enabled it allows to configure destination of each callsite
> > individually. The option adds a framework (described by struct
> > ddebug_dst_ops) which provides flexible means to add a new destination
> > for debug logs. On top of the framework support for trace instance as
> > destination is added. By default destination of debug logs is syslog.
> > Syslog can be set explicitly by using dst keyword or is selected by
> > default when dst keyword is omitted, for example:
>
> A while back,
> Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
> proposed these patches:
>       https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchu=
rch@axis.com/
>
> his approach adds a single flag, +x or (my pref) +T
> which sends the pr_debug to tracefs, independent of +p.
> Steve Rostedt offered feedback on one of the versions,
> and one could read that as provisional acceptance.
>
> Ive worked his patchset since, it is here:
> https://github.com/jimc/linux/tree/dd-trc-11
> (it has some unrelated bits too)
>
> but it sits atop another patchset:
> https://lore.kernel.org/lkml/20230911230838.14461-1-jim.cromie@gmail.com/
> or for git remote add
> https://github.com/jimc/linux/tree/dd-fix-5i
>

I looked through the patches you pointed me to. The solution with
+T/+x is elegant and tailored to specific needs.
Our rationale to write pr_debug/dev_dbg logs to tracefs is similar to
Vincent's to aid us in debugging.
Unfortunately the solution with +T/+x does not cover our use case.

Our use case is different. We experience issues reported by Chromebook
users which are difficult to reproduce
that's why we would like to enable debug logs to be written to tracefs
on productions systems.
When users experiences an issues and sends us a feedback report the
debug logs written to tracefs would be
attached to the feedback report. We would like to write debug logs to
a separate trace instances based on
the subsystem/driver. Our initial areas of interest/issues are usbcore
and thunderbolt drivers/subsystems.
More may come in the future.

With my proposal this could be achieved for the thunderbolt subsystem
with the following steps:
* enable kernel configuration option CONFIG_DYNAMIC_DEBUG_CORE,
* enable kernel configuration option DYNAMIC_DEBUG_DST and set its
  operation mode to Static.
* add the option to the thunderbolt's drivers/thunderbolt/Makefile
in order to compile in debug logs:
        CFLAGS_nhi.o :=3D -DDYNAMIC_DEBUG_MODULE
* compile the kernel and update it on a target device,
* finally append the entry to the kernel boot command line:
        thunderbolt.dyndbg=3D"dst trace:thunderbolt =3Dp"

Both the solutions (I mean +x/+T and mine proposal) have pros and cons.
Maybe both could coexist together, +x/+T by default and this patch as
configurable option
which adds more flexibility ?

>
> syslog and/or tracefs seems sufficient, do you have a 3rd destination ?
>

I don't have a use case for the third destination, but for example
adding socket destination to my proposal would be relatively fast and
easy (that would be for limited use only after a network is up and
running).

Thanks,
Lukasz

> thanks