Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1653244 > unrolled thread

Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Vlastimil Babka <vbabka.lkml@gmail.com> - 2017-05-30 15:50 +0200
    Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Lu Baolu <baolu.lu@linux.intel.com> - 2017-05-31 05:30 +0200
      Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Steven Rostedt <rostedt@goodmis.org> - 2017-05-31 08:30 +0200
        Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Lu Baolu <baolu.lu@linux.intel.com> - 2017-06-01 05:40 +0200
      Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Vlastimil Babka <vbabka@suse.cz> - 2017-05-31 11:40 +0200
        Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Lu Baolu <baolu.lu@linux.intel.com> - 2017-06-01 05:40 +0200
          Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Vlastimil Babka <vbabka@suse.cz> - 2017-06-01 10:20 +0200
            Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Peter Zijlstra <peterz@infradead.org> - 2017-06-01 14:30 +0200
              Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability Steven Rostedt <rostedt@goodmis.org> - 2017-06-02 02:50 +0200

#1653244 — Re: [PATCH v8 2/5] usb: early: add driver for xhci debug capability

On 03/21/2017 09:01 AM, Lu Baolu wrote:
> XHCI debug capability (DbC) is an optional but standalone
> functionality provided by an xHCI host controller. Software
> learns this capability by walking through the extended
> capability list of the host. XHCI specification describes
> DbC in the section 7.6.
> 
> This patch introduces the code to probe and initialize the
> debug capability hardware during early boot. With hardware
> initialized, the debug target (system on which this code is
> running) will present a debug device through the debug port
> (normally the first USB3 port). The debug device is fully
> compliant with the USB framework and provides the equivalent
> of a very high performance (USB3) full-duplex serial link
> between the debug host and target. The DbC functionality is
> independent of the xHCI host. There isn't any precondition
> from the xHCI host side for the DbC to work.
> 
> One use for this feature is kernel debugging, for example
> when your machine crashes very early before the regular
> console code is initialized. Other uses include simpler,
> lockless logging instead of a full-blown printk console
> driver and klogd.
> 
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

...

> +
> +#define XDBC_TRACE
> +#ifdef XDBC_TRACE
> +#define	xdbc_trace	trace_printk

Did you forget to remove the #define XDBC_TRACE?

Enabling this driver brings the "trace_printk() being used. Allocating
extra memory. This means that this is a DEBUG kernel and it is unsafe
for production use." message in 4.12-rcX dmesg.

Thanks,
Vlastimil

[toc] | [next] | [standalone]

#1653798

Hi,

On 05/30/2017 09:46 PM, Vlastimil Babka wrote:
> On 03/21/2017 09:01 AM, Lu Baolu wrote:
>> XHCI debug capability (DbC) is an optional but standalone
>> functionality provided by an xHCI host controller. Software
>> learns this capability by walking through the extended
>> capability list of the host. XHCI specification describes
>> DbC in the section 7.6.
>>
>> This patch introduces the code to probe and initialize the
>> debug capability hardware during early boot. With hardware
>> initialized, the debug target (system on which this code is
>> running) will present a debug device through the debug port
>> (normally the first USB3 port). The debug device is fully
>> compliant with the USB framework and provides the equivalent
>> of a very high performance (USB3) full-duplex serial link
>> between the debug host and target. The DbC functionality is
>> independent of the xHCI host. There isn't any precondition
>> from the xHCI host side for the DbC to work.
>>
>> One use for this feature is kernel debugging, for example
>> when your machine crashes very early before the regular
>> console code is initialized. Other uses include simpler,
>> lockless logging instead of a full-blown printk console
>> driver and klogd.
>>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ...
>
>> +
>> +#define XDBC_TRACE
>> +#ifdef XDBC_TRACE
>> +#define	xdbc_trace	trace_printk
> Did you forget to remove the #define XDBC_TRACE?
>
> Enabling this driver brings the "trace_printk() being used. Allocating
> extra memory. This means that this is a DEBUG kernel and it is unsafe
> for production use." message in 4.12-rcX dmesg.

This feature is only for a DEBUG kernel, should not be enabled for
any production kernel. This was the reason I enabled trace_printk()
by default.

Best regards,
Lu Baolu

> Thanks,
> Vlastimil
>

[toc] | [prev] | [next] | [standalone]

#1653872

On Wed, 31 May 2017 11:27:19 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

  
> >> +
> >> +#define XDBC_TRACE
> >> +#ifdef XDBC_TRACE
> >> +#define	xdbc_trace	trace_printk  
> > Did you forget to remove the #define XDBC_TRACE?
> >
> > Enabling this driver brings the "trace_printk() being used.
> > Allocating extra memory. This means that this is a DEBUG kernel and
> > it is unsafe for production use." message in 4.12-rcX dmesg.  
> 
> This feature is only for a DEBUG kernel, should not be enabled for
> any production kernel. This was the reason I enabled trace_printk()
> by default.

Yes, it is perfectly fine to use trace_printk() for debug only configs.
But if if you have it there and it is helpful to debug something that
happens in a production system, you may want to look into creating a
real tracepoint for the locations.

-- Steve

[toc] | [prev] | [next] | [standalone]

#1654784

Hi,

On 05/31/2017 02:24 PM, Steven Rostedt wrote:
> On Wed, 31 May 2017 11:27:19 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
>   
>>>> +
>>>> +#define XDBC_TRACE
>>>> +#ifdef XDBC_TRACE
>>>> +#define	xdbc_trace	trace_printk  
>>> Did you forget to remove the #define XDBC_TRACE?
>>>
>>> Enabling this driver brings the "trace_printk() being used.
>>> Allocating extra memory. This means that this is a DEBUG kernel and
>>> it is unsafe for production use." message in 4.12-rcX dmesg.  
>> This feature is only for a DEBUG kernel, should not be enabled for
>> any production kernel. This was the reason I enabled trace_printk()
>> by default.
> Yes, it is perfectly fine to use trace_printk() for debug only configs.
> But if if you have it there and it is helpful to debug something that
> happens in a production system, you may want to look into creating a
> real tracepoint for the locations.

Yes. Good suggestion. I will try this.

Thank you!

Best regards,
Lu Baolu

>
> -- Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[toc] | [prev] | [next] | [standalone]

#1654057

On 05/31/2017 05:27 AM, Lu Baolu wrote:
> Hi,
> 
> On 05/30/2017 09:46 PM, Vlastimil Babka wrote:
>> On 03/21/2017 09:01 AM, Lu Baolu wrote:
>>> XHCI debug capability (DbC) is an optional but standalone
>>> functionality provided by an xHCI host controller. Software
>>> learns this capability by walking through the extended
>>> capability list of the host. XHCI specification describes
>>> DbC in the section 7.6.
>>>
>>> This patch introduces the code to probe and initialize the
>>> debug capability hardware during early boot. With hardware
>>> initialized, the debug target (system on which this code is
>>> running) will present a debug device through the debug port
>>> (normally the first USB3 port). The debug device is fully
>>> compliant with the USB framework and provides the equivalent
>>> of a very high performance (USB3) full-duplex serial link
>>> between the debug host and target. The DbC functionality is
>>> independent of the xHCI host. There isn't any precondition
>>> from the xHCI host side for the DbC to work.
>>>
>>> One use for this feature is kernel debugging, for example
>>> when your machine crashes very early before the regular
>>> console code is initialized. Other uses include simpler,
>>> lockless logging instead of a full-blown printk console
>>> driver and klogd.
>>>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ...
>>
>>> +
>>> +#define XDBC_TRACE
>>> +#ifdef XDBC_TRACE
>>> +#define	xdbc_trace	trace_printk
>> Did you forget to remove the #define XDBC_TRACE?
>>
>> Enabling this driver brings the "trace_printk() being used. Allocating
>> extra memory. This means that this is a DEBUG kernel and it is unsafe
>> for production use." message in 4.12-rcX dmesg.
> 
> This feature is only for a DEBUG kernel, should not be enabled for
> any production kernel. This was the reason I enabled trace_printk()
> by default.

Hmm, but it seems we generally enable all these early printk
features/drivers in our distro kernels. They are not active without a
early_printk=X bootparam anyway, right? It's much more convenient to
e.g. just tell customer to change a param when debugging something than
to install a debug kernel. So I wouldn't agree that only a DEBUG kernel
should have this compiled in.

Thanks,
Vlastimil

> Best regards,
> Lu Baolu
> 
>> Thanks,
>> Vlastimil
>>
> 

[toc] | [prev] | [next] | [standalone]

#1654787

Hi,

On 05/31/2017 05:38 PM, Vlastimil Babka wrote:
> On 05/31/2017 05:27 AM, Lu Baolu wrote:
>> Hi,
>>
>> On 05/30/2017 09:46 PM, Vlastimil Babka wrote:
>>> On 03/21/2017 09:01 AM, Lu Baolu wrote:
>>>> XHCI debug capability (DbC) is an optional but standalone
>>>> functionality provided by an xHCI host controller. Software
>>>> learns this capability by walking through the extended
>>>> capability list of the host. XHCI specification describes
>>>> DbC in the section 7.6.
>>>>
>>>> This patch introduces the code to probe and initialize the
>>>> debug capability hardware during early boot. With hardware
>>>> initialized, the debug target (system on which this code is
>>>> running) will present a debug device through the debug port
>>>> (normally the first USB3 port). The debug device is fully
>>>> compliant with the USB framework and provides the equivalent
>>>> of a very high performance (USB3) full-duplex serial link
>>>> between the debug host and target. The DbC functionality is
>>>> independent of the xHCI host. There isn't any precondition
>>>> from the xHCI host side for the DbC to work.
>>>>
>>>> One use for this feature is kernel debugging, for example
>>>> when your machine crashes very early before the regular
>>>> console code is initialized. Other uses include simpler,
>>>> lockless logging instead of a full-blown printk console
>>>> driver and klogd.
>>>>
>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>> Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>> ...
>>>
>>>> +
>>>> +#define XDBC_TRACE
>>>> +#ifdef XDBC_TRACE
>>>> +#define	xdbc_trace	trace_printk
>>> Did you forget to remove the #define XDBC_TRACE?
>>>
>>> Enabling this driver brings the "trace_printk() being used. Allocating
>>> extra memory. This means that this is a DEBUG kernel and it is unsafe
>>> for production use." message in 4.12-rcX dmesg.
>> This feature is only for a DEBUG kernel, should not be enabled for
>> any production kernel. This was the reason I enabled trace_printk()
>> by default.
> Hmm, but it seems we generally enable all these early printk
> features/drivers in our distro kernels. They are not active without a
> early_printk=X bootparam anyway, right? It's much more convenient to
> e.g. just tell customer to change a param when debugging something than
> to install a debug kernel. So I wouldn't agree that only a DEBUG kernel
> should have this compiled in.

Okay, I will try to find a fix.

Best regards,
Lu Baolu

>
> Thanks,
> Vlastimil
>
>> Best regards,
>> Lu Baolu
>>
>>> Thanks,
>>> Vlastimil
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[toc] | [prev] | [next] | [standalone]

#1654915

On 06/01/2017 05:37 AM, Lu Baolu wrote:
> Hi,
> 
> On 05/31/2017 05:38 PM, Vlastimil Babka wrote:
>> On 05/31/2017 05:27 AM, Lu Baolu wrote:
>>> Hi,
>>>
>>> On 05/30/2017 09:46 PM, Vlastimil Babka wrote:
>>>> On 03/21/2017 09:01 AM, Lu Baolu wrote:
>>>> Did you forget to remove the #define XDBC_TRACE?
>>>>
>>>> Enabling this driver brings the "trace_printk() being used. Allocating
>>>> extra memory. This means that this is a DEBUG kernel and it is unsafe
>>>> for production use." message in 4.12-rcX dmesg.
>>> This feature is only for a DEBUG kernel, should not be enabled for
>>> any production kernel. This was the reason I enabled trace_printk()
>>> by default.
>> Hmm, but it seems we generally enable all these early printk
>> features/drivers in our distro kernels. They are not active without a
>> early_printk=X bootparam anyway, right? It's much more convenient to
>> e.g. just tell customer to change a param when debugging something than
>> to install a debug kernel. So I wouldn't agree that only a DEBUG kernel
>> should have this compiled in.
> 
> Okay, I will try to find a fix.

Thanks. I didn't make it clear that the trace_printk() warning is there
even if the code using it doesn't actually execute (i.e. I didn't
specify any early_printk bootparam). There are some roastedy tricks to
detect the potential users, so that the buffers can be allocated in
advance to allow the first trace_printk() from any context, I guess.

I'm not sure if there's a way to change it so that your driver reports
the trace_printk usage only in response to the bootparam (which could
also be a safe point to allocate ftrace buffers?).

Vlastimil

[toc] | [prev] | [next] | [standalone]

#1655089

On Thu, Jun 01, 2017 at 10:15:24AM +0200, Vlastimil Babka wrote:
> Thanks. I didn't make it clear that the trace_printk() warning is there
> even if the code using it doesn't actually execute (i.e. I didn't
> specify any early_printk bootparam). There are some roastedy tricks to
> detect the potential users, so that the buffers can be allocated in
> advance to allow the first trace_printk() from any context, I guess.
> 
> I'm not sure if there's a way to change it so that your driver reports
> the trace_printk usage only in response to the bootparam (which could
> also be a safe point to allocate ftrace buffers?).

No, nor do we want to. There should not be a single caller to
trace_printk() in normal kernels.

[toc] | [prev] | [next] | [standalone]

#1655824

On Thu, 1 Jun 2017 14:25:51 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Jun 01, 2017 at 10:15:24AM +0200, Vlastimil Babka wrote:
> > Thanks. I didn't make it clear that the trace_printk() warning is
> > there even if the code using it doesn't actually execute (i.e. I
> > didn't specify any early_printk bootparam). There are some roastedy
> > tricks to detect the potential users, so that the buffers can be
> > allocated in advance to allow the first trace_printk() from any
> > context, I guess.
> > 
> > I'm not sure if there's a way to change it so that your driver
> > reports the trace_printk usage only in response to the bootparam
> > (which could also be a safe point to allocate ftrace buffers?).  
> 
> No, nor do we want to. There should not be a single caller to
> trace_printk() in normal kernels.

Correct. If you find a trace_printk() that would be useful in a
production environment, then make it a tracepoint.

That's why I have that nasty banner, to make sure trace_printk()s are
not the "quick way" to add tracepoint hacks.

-- Steve

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web