Bug 210337 - KCOV: allow nested remote coverage sections
Summary: KCOV: allow nested remote coverage sections
Status: NEW
Alias: None
Product: Memory Management
Classification: Unclassified
Component: Sanitizers (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: MM/Sanitizers virtual assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-23 22:44 UTC by Andrey Konovalov
Modified: 2024-05-20 21:18 UTC (History)
2 users (show)

See Also:
Kernel Version: upstream
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Andrey Konovalov 2020-11-23 22:44:57 UTC
KCOV currently supports collecting coverage when an interrupt with a remote coverage collection section comes in the middle of a remote coverage collection section for a task. However, there's no support for having nested remote coverage collection sections when both happen in the task context. Support for this would be useful at least for USB/IP, see [1] for details.


[1] https://lkml.org/lkml/2020/10/16/592
Comment 1 Dmitry Vyukov 2020-11-24 08:03:12 UTC
2 partial, but potentially simpler solutions, just for discussion.

1. If we assume that recursion only happens for the same target KCOV descriptor, then instead of full recursion support kcov_remote_start could just check that we need to collect coverage for the already active KCOV descriptor and increment a recursion counter. Then kcov_remote_stop will decrement the recursion counter and if it's not 0, bail out.
This will allow us to avoid the WARNING and collect coverage for USB without full fledged recursion support.

2. Maybe USB code could be restructured slightly. Namely instead of:

void foo() {
  kcov_remote_start(...);
  ...
  kcov_remote_stop(...);
}

void bar() {
  kcov_remote_start(...);
  ...
  foo();
  ...
  kcov_remote_stop(...);
}

we do:

void __foo() {
  kcov_remote_start(...);
  ...
  kcov_remote_stop(...);
}

void foo() {
  kcov_remote_start(...);
  __foo();
  kcov_remote_stop(...);
}

void bar() {
  kcov_remote_start(...);
  ...
  __foo();
  ...
  kcov_remote_stop(...);
}

In some sense similar to "locked" and "unlocked" versions of the same function. If you already hold the lock, you need to call the internal "unlocked" function.
So this does not too unreasonable.

I have not looked at the USB code, though.
Comment 2 Andrey Konovalov 2024-05-20 21:18:12 UTC
We also need to support nested remote coverage collection sections in the softirq context: while the BH workqueue handles a softirq, another softirq might arrive; see [1] for details.

[1] https://lore.kernel.org/linux-usb/20240520205856.162910-1-andrey.konovalov@linux.dev/T/#u

Note You need to log in before you can comment on or make changes to this bug.