Skip to content

Commit d5d2c51

Browse files
Sebastian Andrzej Siewiortorvalds
authored andcommitted
kcov: replace local_irq_save() with a local_lock_t
The kcov code mixes local_irq_save() and spin_lock() in kcov_remote_{start|end}(). This creates a warning on PREEMPT_RT because local_irq_save() disables interrupts and spin_lock_t is turned into a sleeping lock which can not be acquired in a section with disabled interrupts. The kcov_remote_lock is used to synchronize the access to the hash-list kcov_remote_map. The local_irq_save() block protects access to the per-CPU data kcov_percpu_data. There is no compelling reason to change the lock type to raw_spin_lock_t to make it work with local_irq_save(). Changing it would require to move memory allocation (in kcov_remote_add()) and deallocation outside of the locked section. Adding an unlimited amount of entries to the hashlist will increase the IRQ-off time during lookup. It could be argued that this is debug code and the latency does not matter. There is however no need to do so and it would allow to use this facility in an RT enabled build. Using a local_lock_t instead of local_irq_save() has the befit of adding a protection scope within the source which makes it obvious what is protected. On a !PREEMPT_RT && !LOCKDEP build the local_lock_irqsave() maps directly to local_irq_save() so there is overhead at runtime. Replace the local_irq_save() section with a local_lock_t. Link: https://lkml.kernel.org/r/[email protected] Link: https://lore.kernel.org/r/[email protected] Reported-by: Clark Williams <[email protected]> Signed-off-by: Sebastian Andrzej Siewior <[email protected]> Acked-by: Dmitry Vyukov <[email protected]> Acked-by: Marco Elver <[email protected]> Tested-by: Marco Elver <[email protected]> Reviewed-by: Andrey Konovalov <[email protected]> Cc: Steven Rostedt <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 22036ab commit d5d2c51

File tree

1 file changed

+17
-13
lines changed

1 file changed

+17
-13
lines changed

kernel/kcov.c

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ static struct list_head kcov_remote_areas = LIST_HEAD_INIT(kcov_remote_areas);
8888

8989
struct kcov_percpu_data {
9090
void *irq_area;
91+
local_lock_t lock;
9192

9293
unsigned int saved_mode;
9394
unsigned int saved_size;
@@ -96,7 +97,9 @@ struct kcov_percpu_data {
9697
int saved_sequence;
9798
};
9899

99-
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data);
100+
static DEFINE_PER_CPU(struct kcov_percpu_data, kcov_percpu_data) = {
101+
.lock = INIT_LOCAL_LOCK(lock),
102+
};
100103

101104
/* Must be called with kcov_remote_lock locked. */
102105
static struct kcov_remote *kcov_remote_find(u64 handle)
@@ -824,15 +827,15 @@ void kcov_remote_start(u64 handle)
824827
if (!in_task() && !in_serving_softirq())
825828
return;
826829

827-
local_irq_save(flags);
830+
local_lock_irqsave(&kcov_percpu_data.lock, flags);
828831

829832
/*
830833
* Check that kcov_remote_start() is not called twice in background
831834
* threads nor called by user tasks (with enabled kcov).
832835
*/
833836
mode = READ_ONCE(t->kcov_mode);
834837
if (WARN_ON(in_task() && kcov_mode_enabled(mode))) {
835-
local_irq_restore(flags);
838+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
836839
return;
837840
}
838841
/*
@@ -841,14 +844,15 @@ void kcov_remote_start(u64 handle)
841844
* happened while collecting coverage from a background thread.
842845
*/
843846
if (WARN_ON(in_serving_softirq() && t->kcov_softirq)) {
844-
local_irq_restore(flags);
847+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
845848
return;
846849
}
847850

848851
spin_lock(&kcov_remote_lock);
849852
remote = kcov_remote_find(handle);
850853
if (!remote) {
851-
spin_unlock_irqrestore(&kcov_remote_lock, flags);
854+
spin_unlock(&kcov_remote_lock);
855+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
852856
return;
853857
}
854858
kcov_debug("handle = %llx, context: %s\n", handle,
@@ -873,13 +877,13 @@ void kcov_remote_start(u64 handle)
873877

874878
/* Can only happen when in_task(). */
875879
if (!area) {
876-
local_irqrestore(flags);
880+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
877881
area = vmalloc(size * sizeof(unsigned long));
878882
if (!area) {
879883
kcov_put(kcov);
880884
return;
881885
}
882-
local_irq_save(flags);
886+
local_lock_irqsave(&kcov_percpu_data.lock, flags);
883887
}
884888

885889
/* Reset coverage size. */
@@ -891,7 +895,7 @@ void kcov_remote_start(u64 handle)
891895
}
892896
kcov_start(t, kcov, size, area, mode, sequence);
893897

894-
local_irq_restore(flags);
898+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
895899

896900
}
897901
EXPORT_SYMBOL(kcov_remote_start);
@@ -965,25 +969,25 @@ void kcov_remote_stop(void)
965969
if (!in_task() && !in_serving_softirq())
966970
return;
967971

968-
local_irq_save(flags);
972+
local_lock_irqsave(&kcov_percpu_data.lock, flags);
969973

970974
mode = READ_ONCE(t->kcov_mode);
971975
barrier();
972976
if (!kcov_mode_enabled(mode)) {
973-
local_irq_restore(flags);
977+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
974978
return;
975979
}
976980
/*
977981
* When in softirq, check if the corresponding kcov_remote_start()
978982
* actually found the remote handle and started collecting coverage.
979983
*/
980984
if (in_serving_softirq() && !t->kcov_softirq) {
981-
local_irq_restore(flags);
985+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
982986
return;
983987
}
984988
/* Make sure that kcov_softirq is only set when in softirq. */
985989
if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) {
986-
local_irq_restore(flags);
990+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
987991
return;
988992
}
989993

@@ -1013,7 +1017,7 @@ void kcov_remote_stop(void)
10131017
spin_unlock(&kcov_remote_lock);
10141018
}
10151019

1016-
local_irq_restore(flags);
1020+
local_unlock_irqrestore(&kcov_percpu_data.lock, flags);
10171021

10181022
/* Get in kcov_remote_start(). */
10191023
kcov_put(kcov);

0 commit comments

Comments
 (0)