| Message ID | 20240514145631.2128-1-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/print: Introduce drm_line_printer | expand |
On Tue, 14 May 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote: > This drm printer wrapper can be used to increase the robustness of > the captured output generated by any other drm_printer to make sure > we didn't lost any intermediate lines of the output by adding line > numbers to each output line. Helpful for capturing some crash data. Interesting. Nothing another level of abstraction can't solve. ;) Except maybe it'll make adding function names to debug printers harder. No strong feelings either way about it, I'll let others chime in. A few comments inline. BR, Jani. > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > drivers/gpu/drm/drm_print.c | 9 +++++++++ > include/drm/drm_print.h | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > index cf2efb44722c..d6fb50d3407a 100644 > --- a/drivers/gpu/drm/drm_print.c > +++ b/drivers/gpu/drm/drm_print.c > @@ -214,6 +214,15 @@ void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf) > } > EXPORT_SYMBOL(__drm_printfn_err); > > +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf) > +{ > + unsigned int line = (uintptr_t)(++p->prefix); Subtle. Might warrant adding a union in struct drm_printer for clarity. > + struct drm_printer *dp = p->arg; > + > + drm_printf(dp, "%u: %pV", line, vaf); > +} > +EXPORT_SYMBOL(__drm_printfn_line); > + > /** > * drm_puts - print a const string to a &drm_printer stream > * @p: the &drm printer > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index 089950ad8681..58cc73c53853 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -186,6 +186,7 @@ void __drm_puts_seq_file(struct drm_printer *p, const char *str); > void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); > void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf); > void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf); > +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf); > > __printf(2, 3) > void drm_printf(struct drm_printer *p, const char *f, ...); > @@ -357,6 +358,42 @@ static inline struct drm_printer drm_err_printer(struct drm_device *drm, > return p; > } > > +/** > + * drm_line_printer - construct a &drm_printer that prefixes outputs with line numbers > + * @dp: the &struct drm_printer which actually generates the output > + * > + * This printer can be used to increase the robustness of the captured output > + * to make sure we didn't lost any intermediate lines of the output. Helpful > + * while capturing some crash data. > + * > + * For example:: > + * > + * void crash_dump(struct drm_device *drm) > + * { > + * struct drm_printer dp = drm_err_printer(drm, "crash"); > + * struct drm_printer lp = drm_line_printer(&dp); > + * > + * drm_printf(&lp, "foo"); > + * drm_printf(&lp, "bar"); > + * } > + * > + * Above code will print into the dmesg something like:: > + * > + * [ ] 0000:00:00.0: [drm] *ERROR* crash 1: foo > + * [ ] 0000:00:00.0: [drm] *ERROR* crash 2: bar > + * > + * RETURNS: > + * The &drm_printer object > + */ > +static inline struct drm_printer drm_line_printer(struct drm_printer *dp) Just p is customary for the drm_printer. "dp" gives me too much Display Port vibes. > +{ > + struct drm_printer lp = { > + .printfn = __drm_printfn_line, > + .arg = dp, > + }; > + return lp; > +} > + > /* > * struct device based logging > *
On 15.05.2024 11:56, Jani Nikula wrote: > On Tue, 14 May 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote: >> This drm printer wrapper can be used to increase the robustness of >> the captured output generated by any other drm_printer to make sure >> we didn't lost any intermediate lines of the output by adding line >> numbers to each output line. Helpful for capturing some crash data. > > Interesting. Nothing another level of abstraction can't solve. ;) > > Except maybe it'll make adding function names to debug printers harder. but why? primary printer should work in the same way with or without this line printer > > No strong feelings either way about it, I'll let others chime in. forgot to mention that this new printer was aimed to simplify the manual and error prone work done as part of [1] but I'm afraid there is little traction to have any kind of generic solution at all, since it is considered as 'over engineering', even at the cost of trashing other printers that don't need extra robustness [1] https://patchwork.freedesktop.org/patch/593223/?series=133349&rev=1 > > A few comments inline. > > > BR, > Jani. > >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> --- >> drivers/gpu/drm/drm_print.c | 9 +++++++++ >> include/drm/drm_print.h | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 46 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c >> index cf2efb44722c..d6fb50d3407a 100644 >> --- a/drivers/gpu/drm/drm_print.c >> +++ b/drivers/gpu/drm/drm_print.c >> @@ -214,6 +214,15 @@ void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf) >> } >> EXPORT_SYMBOL(__drm_printfn_err); >> >> +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf) >> +{ >> + unsigned int line = (uintptr_t)(++p->prefix); > > Subtle. Might warrant adding a union in struct drm_printer for clarity. good idea > >> + struct drm_printer *dp = p->arg; >> + >> + drm_printf(dp, "%u: %pV", line, vaf); >> +} >> +EXPORT_SYMBOL(__drm_printfn_line); >> + >> /** >> * drm_puts - print a const string to a &drm_printer stream >> * @p: the &drm printer >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >> index 089950ad8681..58cc73c53853 100644 >> --- a/include/drm/drm_print.h >> +++ b/include/drm/drm_print.h >> @@ -186,6 +186,7 @@ void __drm_puts_seq_file(struct drm_printer *p, const char *str); >> void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); >> void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf); >> void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf); >> +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf); >> >> __printf(2, 3) >> void drm_printf(struct drm_printer *p, const char *f, ...); >> @@ -357,6 +358,42 @@ static inline struct drm_printer drm_err_printer(struct drm_device *drm, >> return p; >> } >> >> +/** >> + * drm_line_printer - construct a &drm_printer that prefixes outputs with line numbers >> + * @dp: the &struct drm_printer which actually generates the output >> + * >> + * This printer can be used to increase the robustness of the captured output >> + * to make sure we didn't lost any intermediate lines of the output. Helpful >> + * while capturing some crash data. >> + * >> + * For example:: >> + * >> + * void crash_dump(struct drm_device *drm) >> + * { >> + * struct drm_printer dp = drm_err_printer(drm, "crash"); >> + * struct drm_printer lp = drm_line_printer(&dp); >> + * >> + * drm_printf(&lp, "foo"); >> + * drm_printf(&lp, "bar"); >> + * } >> + * >> + * Above code will print into the dmesg something like:: >> + * >> + * [ ] 0000:00:00.0: [drm] *ERROR* crash 1: foo >> + * [ ] 0000:00:00.0: [drm] *ERROR* crash 2: bar >> + * >> + * RETURNS: >> + * The &drm_printer object >> + */ >> +static inline struct drm_printer drm_line_printer(struct drm_printer *dp) > > Just p is customary for the drm_printer. "dp" gives me too much Display > Port vibes. ha, it was 'p' initially, but then it was still asymmetric compared to __drm_printfn_line where 'p' was actually referring to the line printer maybe best option would be to don't have local vars at all: drm_printf((struct drm_printer *)p->arg, "%u: %pV", p->line, vaf); > >> +{ >> + struct drm_printer lp = { >> + .printfn = __drm_printfn_line, >> + .arg = dp, >> + }; >> + return lp; >> +} >> + >> /* >> * struct device based logging >> * >
On Wed, 15 May 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote: > On 15.05.2024 11:56, Jani Nikula wrote: >> On Tue, 14 May 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote: >>> This drm printer wrapper can be used to increase the robustness of >>> the captured output generated by any other drm_printer to make sure >>> we didn't lost any intermediate lines of the output by adding line >>> numbers to each output line. Helpful for capturing some crash data. >> >> Interesting. Nothing another level of abstraction can't solve. ;) >> >> Except maybe it'll make adding function names to debug printers harder. > > but why? primary printer should work in the same way with or without > this line printer Because of __builtin_return_address(0). But currently __drm_printfn_dbg() doesn't use that anyway, because it would already be off... Not really a meaningful argument against this patch, to be fair. >> >> No strong feelings either way about it, I'll let others chime in. > > forgot to mention that this new printer was aimed to simplify the manual > and error prone work done as part of [1] but I'm afraid there is little > traction to have any kind of generic solution at all, since it is > considered as 'over engineering', even at the cost of trashing other > printers that don't need extra robustness > > [1] https://patchwork.freedesktop.org/patch/593223/?series=133349&rev=1 > >> >> A few comments inline. >> >> >> BR, >> Jani. >> >>> >>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >>> --- >>> drivers/gpu/drm/drm_print.c | 9 +++++++++ >>> include/drm/drm_print.h | 37 +++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 46 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c >>> index cf2efb44722c..d6fb50d3407a 100644 >>> --- a/drivers/gpu/drm/drm_print.c >>> +++ b/drivers/gpu/drm/drm_print.c >>> @@ -214,6 +214,15 @@ void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf) >>> } >>> EXPORT_SYMBOL(__drm_printfn_err); >>> >>> +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf) >>> +{ >>> + unsigned int line = (uintptr_t)(++p->prefix); >> >> Subtle. Might warrant adding a union in struct drm_printer for clarity. > > good idea > >> >>> + struct drm_printer *dp = p->arg; >>> + >>> + drm_printf(dp, "%u: %pV", line, vaf); >>> +} >>> +EXPORT_SYMBOL(__drm_printfn_line); >>> + >>> /** >>> * drm_puts - print a const string to a &drm_printer stream >>> * @p: the &drm printer >>> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >>> index 089950ad8681..58cc73c53853 100644 >>> --- a/include/drm/drm_print.h >>> +++ b/include/drm/drm_print.h >>> @@ -186,6 +186,7 @@ void __drm_puts_seq_file(struct drm_printer *p, const char *str); >>> void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); >>> void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf); >>> void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf); >>> +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf); >>> >>> __printf(2, 3) >>> void drm_printf(struct drm_printer *p, const char *f, ...); >>> @@ -357,6 +358,42 @@ static inline struct drm_printer drm_err_printer(struct drm_device *drm, >>> return p; >>> } >>> >>> +/** >>> + * drm_line_printer - construct a &drm_printer that prefixes outputs with line numbers >>> + * @dp: the &struct drm_printer which actually generates the output >>> + * >>> + * This printer can be used to increase the robustness of the captured output >>> + * to make sure we didn't lost any intermediate lines of the output. Helpful >>> + * while capturing some crash data. >>> + * >>> + * For example:: >>> + * >>> + * void crash_dump(struct drm_device *drm) >>> + * { >>> + * struct drm_printer dp = drm_err_printer(drm, "crash"); >>> + * struct drm_printer lp = drm_line_printer(&dp); >>> + * >>> + * drm_printf(&lp, "foo"); >>> + * drm_printf(&lp, "bar"); >>> + * } >>> + * >>> + * Above code will print into the dmesg something like:: >>> + * >>> + * [ ] 0000:00:00.0: [drm] *ERROR* crash 1: foo >>> + * [ ] 0000:00:00.0: [drm] *ERROR* crash 2: bar >>> + * >>> + * RETURNS: >>> + * The &drm_printer object >>> + */ >>> +static inline struct drm_printer drm_line_printer(struct drm_printer *dp) >> >> Just p is customary for the drm_printer. "dp" gives me too much Display >> Port vibes. > > ha, it was 'p' initially, but then it was still asymmetric compared to > __drm_printfn_line where 'p' was actually referring to the line printer > > maybe best option would be to don't have local vars at all: > > drm_printf((struct drm_printer *)p->arg, "%u: %pV", p->line, vaf); The cast is actually not required. BR, Jani. > >> >>> +{ >>> + struct drm_printer lp = { >>> + .printfn = __drm_printfn_line, >>> + .arg = dp, >>> + }; >>> + return lp; >>> +} >>> + >>> /* >>> * struct device based logging >>> * >>
On 5/23/2024 16:54, Daniele Ceraolo Spurio wrote: > -------- Forwarded Message -------- > Subject: [RFC] drm/print: Introduce drm_line_printer > Date: Tue, 14 May 2024 16:56:31 +0200 > From: Michal Wajdeczko <michal.wajdeczko@intel.com> > To: dri-devel@lists.freedesktop.org > > > > This drm printer wrapper can be used to increase the robustness of > the captured output generated by any other drm_printer to make sure > we didn't lost any intermediate lines of the output by adding line > numbers to each output line. Helpful for capturing some crash data. > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > --- > drivers/gpu/drm/drm_print.c | 9 +++++++++ > include/drm/drm_print.h | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > index cf2efb44722c..d6fb50d3407a 100644 > --- a/drivers/gpu/drm/drm_print.c > +++ b/drivers/gpu/drm/drm_print.c > @@ -214,6 +214,15 @@ void __drm_printfn_err(struct drm_printer *p, > struct va_format *vaf) > } > EXPORT_SYMBOL(__drm_printfn_err); > +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf) > +{ > + unsigned int line = (uintptr_t)(++p->prefix); The prefix field is officially supposed to be a const char *. There is no documentation to say that this is intended to be used as a private data field by random printer wrappers. So overloading it like this feels very hacky and dangerous. Also, you are mixing types - uintptr_t then uint. So an arch with 64-bit pointers but only 32-bit ints would hit a truncated compiler warning? > + struct drm_printer *dp = p->arg; > + > + drm_printf(dp, "%u: %pV", line, vaf); This is insufficient. As previously commented, there needs to be a global counter as well as a local line counter. The global count must be global to at least whatever entity is generating a specific set of prints. Being global to a higher level, e.g. kernel global, is fine. But without that, two concurrent dumps that get interleaved can be impossible to separate resulting in a useless bug report/log. The prefix field could potentially be split into a 16:16 global:local index with the global master just being a static u16 inside that function. With the first print call to a given drm_printer object being defined by the global value being zero. And it then sets the global value to the next increment skipping over zero on a 16-bit wrap around. But see above about prefix not being intended for such purposes. So now you are just piling hacks upon hacks. Plus it would be much nicer output to have the ability to put an arbitrary prefix in front of the G.L number, as per the original implementation. The whole point of this is to aid identification of otherwise uniform data such as hexdumps. So anything that makes it less clear is bad. John. > +} > +EXPORT_SYMBOL(__drm_printfn_line); > + > /** > * drm_puts - print a const string to a &drm_printer stream > * @p: the &drm printer > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index 089950ad8681..58cc73c53853 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -186,6 +186,7 @@ void __drm_puts_seq_file(struct drm_printer *p, > const char *str); > void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); > void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf); > void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf); > +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf); > __printf(2, 3) > void drm_printf(struct drm_printer *p, const char *f, ...); > @@ -357,6 +358,42 @@ static inline struct drm_printer > drm_err_printer(struct drm_device *drm, > return p; > } > +/** > + * drm_line_printer - construct a &drm_printer that prefixes outputs > with line numbers > + * @dp: the &struct drm_printer which actually generates the output > + * > + * This printer can be used to increase the robustness of the > captured output > + * to make sure we didn't lost any intermediate lines of the output. > Helpful > + * while capturing some crash data. > + * > + * For example:: > + * > + * void crash_dump(struct drm_device *drm) > + * { > + * struct drm_printer dp = drm_err_printer(drm, "crash"); > + * struct drm_printer lp = drm_line_printer(&dp); > + * > + * drm_printf(&lp, "foo"); > + * drm_printf(&lp, "bar"); > + * } > + * > + * Above code will print into the dmesg something like:: > + * > + * [ ] 0000:00:00.0: [drm] *ERROR* crash 1: foo > + * [ ] 0000:00:00.0: [drm] *ERROR* crash 2: bar > + * > + * RETURNS: > + * The &drm_printer object > + */ > +static inline struct drm_printer drm_line_printer(struct drm_printer *dp) > +{ > + struct drm_printer lp = { > + .printfn = __drm_printfn_line, > + .arg = dp, > + }; > + return lp; > +} > + > /* > * struct device based logging > * > -- > 2.43.0 >
On Thu, 23 May 2024, John Harrison <john.c.harrison@intel.com> wrote: > On 5/23/2024 16:54, Daniele Ceraolo Spurio wrote: >> -------- Forwarded Message -------- >> Subject: [RFC] drm/print: Introduce drm_line_printer >> Date: Tue, 14 May 2024 16:56:31 +0200 >> From: Michal Wajdeczko <michal.wajdeczko@intel.com> >> To: dri-devel@lists.freedesktop.org >> >> >> >> This drm printer wrapper can be used to increase the robustness of >> the captured output generated by any other drm_printer to make sure >> we didn't lost any intermediate lines of the output by adding line >> numbers to each output line. Helpful for capturing some crash data. >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> --- >> drivers/gpu/drm/drm_print.c | 9 +++++++++ >> include/drm/drm_print.h | 37 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 46 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c >> index cf2efb44722c..d6fb50d3407a 100644 >> --- a/drivers/gpu/drm/drm_print.c >> +++ b/drivers/gpu/drm/drm_print.c >> @@ -214,6 +214,15 @@ void __drm_printfn_err(struct drm_printer *p, >> struct va_format *vaf) >> } >> EXPORT_SYMBOL(__drm_printfn_err); >> +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf) >> +{ >> + unsigned int line = (uintptr_t)(++p->prefix); > The prefix field is officially supposed to be a const char *. There is > no documentation to say that this is intended to be used as a private > data field by random printer wrappers. So overloading it like this feels > very hacky and dangerous. Also, you are mixing types - uintptr_t then > uint. So an arch with 64-bit pointers but only 32-bit ints would hit a > truncated compiler warning? I already commented on abusing the type in another reply. I think making prefix a union would dodge that issue. Otherwise, I think each printer can pretty much use the arg and prefix members as they see fit. It's not mucking the printer being wrapped. > >> + struct drm_printer *dp = p->arg; >> + >> + drm_printf(dp, "%u: %pV", line, vaf); > This is insufficient. As previously commented, there needs to be a > global counter as well as a local line counter. The global count must be > global to at least whatever entity is generating a specific set of > prints. Being global to a higher level, e.g. kernel global, is fine. But > without that, two concurrent dumps that get interleaved can be > impossible to separate resulting in a useless bug report/log. > > The prefix field could potentially be split into a 16:16 global:local > index with the global master just being a static u16 inside that > function. With the first print call to a given drm_printer object being > defined by the global value being zero. And it then sets the global > value to the next increment skipping over zero on a 16-bit wrap around. > But see above about prefix not being intended for such purposes. So now > you are just piling hacks upon hacks. To tackle that issue, I think I'd add a "unique id" kind of parameter to drm_line_printer(). If the user needs more uniqueness, they can maintain it locally (in a data structure or static or whatever) and id++ it. Could even skip it in printing if set to 0 if it's not likely the caller needs it. This dodges the issue of having to store global stuff in the printer, and keeps output lean when not needed. For example: static int id; struct drm_printer dp = drm_err_printer(drm, "crash"); struct drm_printer lp = drm_line_printer(&dp, ++id); > Plus it would be much nicer output to have the ability to put an > arbitrary prefix in front of the G.L number, as per the original > implementation. The whole point of this is to aid identification of > otherwise uniform data such as hexdumps. So anything that makes it less > clear is bad. The prefix in the printer being wrapped is intact, so you could add it there. In the above example, it's "crash". BR, Jani. > > John. > > >> +} >> +EXPORT_SYMBOL(__drm_printfn_line); >> + >> /** >> * drm_puts - print a const string to a &drm_printer stream >> * @p: the &drm printer >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h >> index 089950ad8681..58cc73c53853 100644 >> --- a/include/drm/drm_print.h >> +++ b/include/drm/drm_print.h >> @@ -186,6 +186,7 @@ void __drm_puts_seq_file(struct drm_printer *p, >> const char *str); >> void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); >> void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf); >> void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf); >> +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf); >> __printf(2, 3) >> void drm_printf(struct drm_printer *p, const char *f, ...); >> @@ -357,6 +358,42 @@ static inline struct drm_printer >> drm_err_printer(struct drm_device *drm, >> return p; >> } >> +/** >> + * drm_line_printer - construct a &drm_printer that prefixes outputs >> with line numbers >> + * @dp: the &struct drm_printer which actually generates the output >> + * >> + * This printer can be used to increase the robustness of the >> captured output >> + * to make sure we didn't lost any intermediate lines of the output. >> Helpful >> + * while capturing some crash data. >> + * >> + * For example:: >> + * >> + * void crash_dump(struct drm_device *drm) >> + * { >> + * struct drm_printer dp = drm_err_printer(drm, "crash"); >> + * struct drm_printer lp = drm_line_printer(&dp); >> + * >> + * drm_printf(&lp, "foo"); >> + * drm_printf(&lp, "bar"); >> + * } >> + * >> + * Above code will print into the dmesg something like:: >> + * >> + * [ ] 0000:00:00.0: [drm] *ERROR* crash 1: foo >> + * [ ] 0000:00:00.0: [drm] *ERROR* crash 2: bar >> + * >> + * RETURNS: >> + * The &drm_printer object >> + */ >> +static inline struct drm_printer drm_line_printer(struct drm_printer *dp) >> +{ >> + struct drm_printer lp = { >> + .printfn = __drm_printfn_line, >> + .arg = dp, >> + }; >> + return lp; >> +} >> + >> /* >> * struct device based logging >> * >> -- >> 2.43.0 >>
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index cf2efb44722c..d6fb50d3407a 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -214,6 +214,15 @@ void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf) } EXPORT_SYMBOL(__drm_printfn_err); +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf) +{ + unsigned int line = (uintptr_t)(++p->prefix); + struct drm_printer *dp = p->arg; + + drm_printf(dp, "%u: %pV", line, vaf); +} +EXPORT_SYMBOL(__drm_printfn_line); + /** * drm_puts - print a const string to a &drm_printer stream * @p: the &drm printer diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 089950ad8681..58cc73c53853 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -186,6 +186,7 @@ void __drm_puts_seq_file(struct drm_printer *p, const char *str); void __drm_printfn_info(struct drm_printer *p, struct va_format *vaf); void __drm_printfn_dbg(struct drm_printer *p, struct va_format *vaf); void __drm_printfn_err(struct drm_printer *p, struct va_format *vaf); +void __drm_printfn_line(struct drm_printer *p, struct va_format *vaf); __printf(2, 3) void drm_printf(struct drm_printer *p, const char *f, ...); @@ -357,6 +358,42 @@ static inline struct drm_printer drm_err_printer(struct drm_device *drm, return p; } +/** + * drm_line_printer - construct a &drm_printer that prefixes outputs with line numbers + * @dp: the &struct drm_printer which actually generates the output + * + * This printer can be used to increase the robustness of the captured output + * to make sure we didn't lost any intermediate lines of the output. Helpful + * while capturing some crash data. + * + * For example:: + * + * void crash_dump(struct drm_device *drm) + * { + * struct drm_printer dp = drm_err_printer(drm, "crash"); + * struct drm_printer lp = drm_line_printer(&dp); + * + * drm_printf(&lp, "foo"); + * drm_printf(&lp, "bar"); + * } + * + * Above code will print into the dmesg something like:: + * + * [ ] 0000:00:00.0: [drm] *ERROR* crash 1: foo + * [ ] 0000:00:00.0: [drm] *ERROR* crash 2: bar + * + * RETURNS: + * The &drm_printer object + */ +static inline struct drm_printer drm_line_printer(struct drm_printer *dp) +{ + struct drm_printer lp = { + .printfn = __drm_printfn_line, + .arg = dp, + }; + return lp; +} + /* * struct device based logging *
This drm printer wrapper can be used to increase the robustness of the captured output generated by any other drm_printer to make sure we didn't lost any intermediate lines of the output by adding line numbers to each output line. Helpful for capturing some crash data. Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> --- drivers/gpu/drm/drm_print.c | 9 +++++++++ include/drm/drm_print.h | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+)