Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-04-25 Thread Andrew Morton
On Thu, 13 Mar 2025 11:43:15 + Alessandro Carminati  
wrote:

> Some unit tests intentionally trigger warning backtraces by passing bad
> parameters to kernel API functions. Such unit tests typically check the
> return value from such calls, not the existence of the warning backtrace.

I've had this series in mm.git's mm-new branch for a while.  I didn't
send it up for 6.15-rc1 due to what I believe to be unresolved review
issues.

I'll drop this v4 series.  Please resend if/when suitable.

Some notes I have taken are:

https://lkml.kernel.org/r/202504190918.jlnurgvb-...@intel.com
https://lkml.kernel.org/r/20250402074550.gq5...@noisy.programming.kicks-ass.net
#arm64-add-support-for-suppressing-warning-backtraces.patch: check review

Some fixes I had merged which presumably should be carried forward are
https://lore.kernel.org/all/20250330212934.3f898c4c...@smtp.kernel.org/T/
https://lkml.kernel.org/r/20250330212739.85827c4c...@smtp.kernel.org

Thanks.



Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-04-05 Thread Shuah Khan

On 3/13/25 16:05, Andrew Morton wrote:

On Thu, 13 Mar 2025 11:31:12 -0700 Guenter Roeck  wrote:


On Thu, Mar 13, 2025 at 06:24:25PM +0100, Maxime Ripard wrote:


Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
very noisy tests much easier to deal with.


And for the record, we're also affected by this in DRM and would very
much like to get it merged in one shape or another.



I was unable to get maintainers of major architectures interested enough
to provide feedback, and did not see a path forward. Maybe Alessandro
has more success than me.


I'll put them into mm.git, to advance things a bit.


I haven't heard from kunit maintainers yet. This thread got lost
in inbox due to travel.

David/Brendan/Rae, Okay to take this series?

Andrew, Okay to take this through your tree - this needs merging.

thanks,
-- Shuah




Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-04-05 Thread David Gow
On Thu, 13 Mar 2025 at 19:44, Alessandro Carminati  wrote:
>
> Some unit tests intentionally trigger warning backtraces by passing bad
> parameters to kernel API functions. Such unit tests typically check the
> return value from such calls, not the existence of the warning backtrace.
>
> Such intentionally generated warning backtraces are neither desirable
> nor useful for a number of reasons.
> - They can result in overlooked real problems.
> - A warning that suddenly starts to show up in unit tests needs to be
>   investigated and has to be marked to be ignored, for example by
>   adjusting filter scripts. Such filters are ad-hoc because there is
>   no real standard format for warnings. On top of that, such filter
>   scripts would require constant maintenance.
>
> One option to address problem would be to add messages such as "expected
> warning backtraces start / end here" to the kernel log.  However, that
> would again require filter scripts, it might result in missing real
> problematic warning backtraces triggered while the test is running, and
> the irrelevant backtrace(s) would still clog the kernel log.
>
> Solve the problem by providing a means to identify and suppress specific
> warning backtraces while executing test code. Support suppressing multiple
> backtraces while at the same time limiting changes to generic code to the
> absolute minimum. Architecture specific changes are kept at minimum by
> retaining function names only if both CONFIG_DEBUG_BUGVERBOSE and
> CONFIG_KUNIT are enabled.
>
> The first patch of the series introduces the necessary infrastructure.
> The second patch introduces support for counting suppressed backtraces.
> This capability is used in patch three to implement unit tests.
> Patch four documents the new API.
> The next two patches add support for suppressing backtraces in drm_rect
> and dev_addr_lists unit tests. These patches are intended to serve as
> examples for the use of the functionality introduced with this series.
> The remaining patches implement the necessary changes for all
> architectures with GENERIC_BUG support.
>
> With CONFIG_KUNIT enabled, image size increase with this series applied is
> approximately 1%. The image size increase (and with it the functionality
> introduced by this series) can be avoided by disabling
> CONFIG_KUNIT_SUPPRESS_BACKTRACE.
>
> This series is based on the RFC patch and subsequent discussion at
> https://patchwork.kernel.org/project/linux-kselftest/patch/02546e59-1afe-4b08-ba81-d94f3b691c9a@moroto.mountain/
> and offers a more comprehensive solution of the problem discussed there.
>
> Design note:
>   Function pointers are only added to the __bug_table section if both
>   CONFIG_KUNIT_SUPPRESS_BACKTRACE and CONFIG_DEBUG_BUGVERBOSE are enabled
>   to avoid image size increases if CONFIG_KUNIT is disabled. There would be
>   some benefits to adding those pointers all the time (reduced complexity,
>   ability to display function names in BUG/WARNING messages). That change,
>   if desired, can be made later.
>
> Checkpatch note:
>   Remaining checkpatch errors and warnings were deliberately ignored.
>   Some are triggered by matching coding style or by comments interpreted
>   as code, others by assembler macros which are disliked by checkpatch.
>   Suggestions for improvements are welcome.
>
> Changes since RFC:
> - Introduced CONFIG_KUNIT_SUPPRESS_BACKTRACE
> - Minor cleanups and bug fixes
> - Added support for all affected architectures
> - Added support for counting suppressed warnings
> - Added unit tests using those counters
> - Added patch to suppress warning backtraces in dev_addr_lists tests
>
> Changes since v1:
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
>   [I retained those tags since there have been no functional changes]
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option, enabled by
>   default.
>
> Changes since v2:
> - Rebased to v6.9-rc2
> - Added comments to drm warning suppression explaining why it is needed.
> - Added patch to move conditional code in arch/sh/include/asm/bug.h
>   to avoid kerneldoc warning
> - Added architecture maintainers to Cc: for architecture specific patches
> - No functional changes
>
> Changes since v3:
> - Rebased to v6.14-rc6
> - Dropped net: "kunit: Suppress lock warning noise at end of dev_addr_lists 
> tests"
>   since 3db3b62955cd6d73afde05a17d7e8e106695c3b9
> - Added __kunit_ and KUNIT_ prefixes.
> - Tested on interessed architectures.
>
> 

Sorry: I also thought this had already landed.

I'm definitely in favour of us taking this, though agree that we
definitely can't afford to break the s390x build.

I've (re-)reviewed the early patches as well, and am generally acking
the series (though some of the architecture-specific patches are
definitely beyond my expertise to review fully).

Acked-by: David Gow 

Cheers,
-- David


> Guenter Roeck (14):
>   bug/kunit: Core support for suppressing warning backtraces
>   kunit: bug:

Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-28 Thread Andrew Morton
On Fri, 28 Mar 2025 16:14:55 -0600 Shuah Khan  wrote:

> On 3/13/25 16:05, Andrew Morton wrote:
> > On Thu, 13 Mar 2025 11:31:12 -0700 Guenter Roeck  wrote:
> > 
> >> On Thu, Mar 13, 2025 at 06:24:25PM +0100, Maxime Ripard wrote:
> 
>  Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
>  very noisy tests much easier to deal with.
> >>>
> >>> And for the record, we're also affected by this in DRM and would very
> >>> much like to get it merged in one shape or another.
> >>>
> >>
> >> I was unable to get maintainers of major architectures interested enough
> >> to provide feedback, and did not see a path forward. Maybe Alessandro
> >> has more success than me.
> > 
> > I'll put them into mm.git, to advance things a bit.
> 
> I haven't heard from kunit maintainers yet. This thread got lost
> in inbox due to travel.
> 
> David/Brendan/Rae, Okay to take this series?
> 
>
> Andrew, Okay to take this through your tree - this needs merging.


The review for 07/14 made me expect an update - perhaps tweak the asm
constraints and add a comment.  This can be addressed later, as long as
we don't forget.

However
https://lkml.kernel.org/r/20250324104702.12139e73-...@linux.ibm.com
needs to be addressed before a merge.  The series in mm.git breaks the
s390 build.





Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-28 Thread Maxime Ripard
On Fri, Mar 28, 2025 at 11:38:23AM +0100, Mickaël Salaün wrote:
> On Thu, Mar 13, 2025 at 06:24:25PM +0100, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Mar 13, 2025 at 10:17:49AM -0700, Kees Cook wrote:
> > > On Thu, Mar 13, 2025 at 11:43:15AM +, Alessandro Carminati wrote:
> > > > Some unit tests intentionally trigger warning backtraces by passing bad
> > > > parameters to kernel API functions. Such unit tests typically check the
> > > > return value from such calls, not the existence of the warning 
> > > > backtrace.
> > > 
> > > Thanks for picking this series back up! I honestly thought this had
> > > already landed. :)
> > > 
> > > > With CONFIG_KUNIT enabled, image size increase with this series applied 
> > > > is
> > > > approximately 1%. The image size increase (and with it the functionality
> > > > introduced by this series) can be avoided by disabling
> > > > CONFIG_KUNIT_SUPPRESS_BACKTRACE.
> > > 
> > > Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
> > > very noisy tests much easier to deal with.
> > 
> > And for the record, we're also affected by this in DRM and would very
> > much like to get it merged in one shape or another.
> 
> Here is another case:
> https://lore.kernel.org/all/20250328.ahc0thi6c...@digikod.net/
> 
> It would be very useful to have this feature merged.  Without it, we may
> need to remove useful tests.

AFAIK, it's been merged in next a couple of weeks ago, so it should be
in 6.15.

Maxime


signature.asc
Description: PGP signature


Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-28 Thread Mickaël Salaün
On Thu, Mar 13, 2025 at 06:24:25PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Mar 13, 2025 at 10:17:49AM -0700, Kees Cook wrote:
> > On Thu, Mar 13, 2025 at 11:43:15AM +, Alessandro Carminati wrote:
> > > Some unit tests intentionally trigger warning backtraces by passing bad
> > > parameters to kernel API functions. Such unit tests typically check the
> > > return value from such calls, not the existence of the warning backtrace.
> > 
> > Thanks for picking this series back up! I honestly thought this had
> > already landed. :)
> > 
> > > With CONFIG_KUNIT enabled, image size increase with this series applied is
> > > approximately 1%. The image size increase (and with it the functionality
> > > introduced by this series) can be avoided by disabling
> > > CONFIG_KUNIT_SUPPRESS_BACKTRACE.
> > 
> > Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
> > very noisy tests much easier to deal with.
> 
> And for the record, we're also affected by this in DRM and would very
> much like to get it merged in one shape or another.

Here is another case:
https://lore.kernel.org/all/20250328.ahc0thi6c...@digikod.net/

It would be very useful to have this feature merged.  Without it, we may
need to remove useful tests.



Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-13 Thread Andrew Morton
On Thu, 13 Mar 2025 11:31:12 -0700 Guenter Roeck  wrote:

> On Thu, Mar 13, 2025 at 06:24:25PM +0100, Maxime Ripard wrote:
> > > 
> > > Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
> > > very noisy tests much easier to deal with.
> > 
> > And for the record, we're also affected by this in DRM and would very
> > much like to get it merged in one shape or another.
> > 
> 
> I was unable to get maintainers of major architectures interested enough
> to provide feedback, and did not see a path forward. Maybe Alessandro
> has more success than me.

I'll put them into mm.git, to advance things a bit.

If someone wants to merge via a different tree, please speak up.

Hopefully the various arch maintainers will review at least their parts
of the series.




Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-13 Thread Guenter Roeck
On Thu, Mar 13, 2025 at 06:24:25PM +0100, Maxime Ripard wrote:
> > 
> > Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
> > very noisy tests much easier to deal with.
> 
> And for the record, we're also affected by this in DRM and would very
> much like to get it merged in one shape or another.
> 

I was unable to get maintainers of major architectures interested enough
to provide feedback, and did not see a path forward. Maybe Alessandro
has more success than me.

Guenter



Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-13 Thread Maxime Ripard
Hi,

On Thu, Mar 13, 2025 at 10:17:49AM -0700, Kees Cook wrote:
> On Thu, Mar 13, 2025 at 11:43:15AM +, Alessandro Carminati wrote:
> > Some unit tests intentionally trigger warning backtraces by passing bad
> > parameters to kernel API functions. Such unit tests typically check the
> > return value from such calls, not the existence of the warning backtrace.
> 
> Thanks for picking this series back up! I honestly thought this had
> already landed. :)
> 
> > With CONFIG_KUNIT enabled, image size increase with this series applied is
> > approximately 1%. The image size increase (and with it the functionality
> > introduced by this series) can be avoided by disabling
> > CONFIG_KUNIT_SUPPRESS_BACKTRACE.
> 
> Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
> very noisy tests much easier to deal with.

And for the record, we're also affected by this in DRM and would very
much like to get it merged in one shape or another.

Maxime


signature.asc
Description: PGP signature


Re: [PATCH v4 00/14] Add support for suppressing warning backtraces

2025-03-13 Thread Kees Cook
On Thu, Mar 13, 2025 at 11:43:15AM +, Alessandro Carminati wrote:
> Some unit tests intentionally trigger warning backtraces by passing bad
> parameters to kernel API functions. Such unit tests typically check the
> return value from such calls, not the existence of the warning backtrace.

Thanks for picking this series back up! I honestly thought this had
already landed. :)

> With CONFIG_KUNIT enabled, image size increase with this series applied is
> approximately 1%. The image size increase (and with it the functionality
> introduced by this series) can be avoided by disabling
> CONFIG_KUNIT_SUPPRESS_BACKTRACE.

Yeah, as with my prior review, I'm a fan of this. It makes a bunch of my
very noisy tests much easier to deal with.

-Kees

-- 
Kees Cook