Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE.
On 23.03.21 08:23, Dmitry Vyukov wrote:
>> diff --git a/kernel/kcov.c b/kernel/kcov.c
>> index 80bfe71bbe13..1f727043146a 100644
>> --- a/kernel/kcov.c
>> +++ b/kernel/kcov.c
>> @@ -24,6 +24,7 @@
>> #include
>> #include
>> #include
>> +#include
>
> Is this for __always_inline?
> __always_inline is defined in include/linux/compiler_types.h.
>
This is for the symbols marking start and end of the text segment
(_stext/_etext).
>
>>
>> #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__)
>>
>> @@ -151,10 +152,8 @@ static void kcov_remote_area_put(struct
>> kcov_remote_area *area,
>> list_add(>list, _remote_areas);
>> }
>>
>> -static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct
>> task_struct *t)
>> +static __always_inline notrace bool check_kcov_mode(enum kcov_mode
>> needed_mode, struct task_struct *t, unsigned int *mode)
>> {
>> - unsigned int mode;
>> -
>> /*
>> * We are interested in code coverage as a function of a syscall
>> inputs,
>> * so we ignore code executed in interrupts, unless we are in a
>> remote
>> @@ -162,7 +161,7 @@ static notrace bool check_kcov_mode(enum kcov_mode
>> needed_mode, struct task_stru
>> */
>> if (!in_task() && !(in_serving_softirq() && t->kcov_softirq))
>> return false;
>> - mode = READ_ONCE(t->kcov_mode);
>> + *mode = READ_ONCE(t->kcov_mode);
>> /*
>> * There is some code that runs in interrupts but for which
>> * in_interrupt() returns false (e.g. preempt_schedule_irq()).
>> @@ -171,7 +170,7 @@ static notrace bool check_kcov_mode(enum kcov_mode
>> needed_mode, struct task_stru
>> * kcov_start().
>> */
>> barrier();
>> - return mode == needed_mode;
>> + return ((int)(*mode & (KCOV_IN_CTXSW | needed_mode))) > 0;
>
> This logic and the rest of the patch looks good to me.
>
> Thanks
Thx.
>
>> }
>>
>> static notrace unsigned long canonicalize_ip(unsigned long ip)
>> @@ -191,18 +190,27 @@ void notrace __sanitizer_cov_trace_pc(void)
>> struct task_struct *t;
>> unsigned long *area;
>> unsigned long ip = canonicalize_ip(_RET_IP_);
>> - unsigned long pos;
>> + unsigned long pos, idx;
>> + unsigned int mode;
>>
>> t = current;
>> - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
>> + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t,
>> ))
>> return;
>>
>> area = t->kcov_area;
>> - /* The first 64-bit word is the number of subsequent PCs. */
>> - pos = READ_ONCE(area[0]) + 1;
>> - if (likely(pos < t->kcov_size)) {
>> - area[pos] = ip;
>> - WRITE_ONCE(area[0], pos);
>> + if (likely(mode == KCOV_MODE_TRACE_PC)) {
>> + /* The first 64-bit word is the number of subsequent PCs. */
>> + pos = READ_ONCE(area[0]) + 1;
>> + if (likely(pos < t->kcov_size)) {
>> + area[pos] = ip;
>> + WRITE_ONCE(area[0], pos);
>> + }
>> + } else {
>> + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4;
>> + pos = idx % BITS_PER_LONG;
>> + idx /= BITS_PER_LONG;
>> + if (likely(idx < t->kcov_size))
>> + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L <<
>> pos);
>> }
>> }
>> EXPORT_SYMBOL(__sanitizer_cov_trace_pc);
>> @@ -213,9 +221,10 @@ static void notrace write_comp_data(u64 type, u64 arg1,
>> u64 arg2, u64 ip)
>> struct task_struct *t;
>> u64 *area;
>> u64 count, start_index, end_pos, max_pos;
>> + unsigned int mode;
>>
>> t = current;
>> - if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t))
>> + if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t, ))
>> return;
>>
>> ip = canonicalize_ip(ip);
>> @@ -362,7 +371,7 @@ void kcov_task_init(struct task_struct *t)
>> static void kcov_reset(struct kcov *kcov)
>> {
>> kcov->t = NULL;
>> - kcov->mode = KCOV_MODE_INIT;
>> + kcov->mode = KCOV_MODE_INIT_TRACE;
>> kcov->remote = false;
>> kcov->remote_size = 0;
>> kcov->sequence++;
>> @@ -468,12 +477,13 @@ static int kcov_mmap(struct file *filep, struct
>> vm_area_struct *vma)
>>
>> spin_lock_irqsave(>lock, flags);
>> size = kcov->size * sizeof(unsigned long);
>> - if (kcov->mode != KCOV_MODE_INIT || vma->vm_pgoff != 0 ||
>> + if (kcov->mode & ~(KCOV_INIT_TRACE | KCOV_INIT_UNIQUE) ||
>> vma->vm_pgoff != 0 ||
>> vma->vm_end - vma->vm_start != size) {
>> res = -EINVAL;
>> goto exit;
>> }
>> if (!kcov->area) {
>> + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size);
>> kcov->area = area;
>> vma->vm_flags |= VM_DONTEXPAND;
>>
Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE.
On Sun, Mar 21, 2021 at 7:44 PM Alexander Lochmann
wrote:
>
> It simply stores the executed PCs.
> The execution order is discarded.
> Each bit in the shared buffer represents every fourth
> byte of the text segment.
> Since a call instruction on every supported
> architecture is at least four bytes, it is safe
> to just store every fourth byte of the text segment.
> In contrast to KCOV_MODE_TRACE_PC, the shared buffer
> cannot overflow. Thus, all executed PCs are recorded.
>
> Signed-off-by: Alexander Lochmann
> ---
> Documentation/dev-tools/kcov.rst | 80 +++
> include/linux/kcov.h | 12 ++--
> include/uapi/linux/kcov.h| 10
> kernel/kcov.c| 94
> 4 files changed, 169 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/dev-tools/kcov.rst
> b/Documentation/dev-tools/kcov.rst
> index d2c4c27e1702..e105ffe6b6e3 100644
> --- a/Documentation/dev-tools/kcov.rst
> +++ b/Documentation/dev-tools/kcov.rst
> @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov,
> enables trace mode,
> mmaps coverage buffer and then forks child processes in a loop. Child
> processes
> only need to enable coverage (disable happens automatically on thread end).
>
> +If someone is interested in a set of executed PCs, and does not care about
> +execution order, he or she can advise KCOV to do so:
> +
> +.. code-block:: c
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define KCOV_INIT_TRACE_IOR('c', 1, unsigned long)
> +#define KCOV_INIT_UNIQUE_IOR('c', 2, unsigned long)
> +#define KCOV_ENABLE_IO('c', 100)
> +#define KCOV_DISABLE _IO('c', 101)
> +
> +#define BITS_PER_LONG 64
> +#define KCOV_TRACE_PC 0
> +#define KCOV_TRACE_CMP 1
> +#define KCOV_UNIQUE_PC 2
> +/*
> + * Determine start of text segment via 'nm vmlinux | grep _stext | cut
> -d " " -f1',
> + * and fill in.
> + */
> +#define STEXT_START 0x8100
> +
> +
> +
> +int main(int argc, char **argv)
> +{
> + int fd;
> + unsigned long *cover, n, i;
> +
> + /* A single fd descriptor allows coverage collection on a single
> +* thread.
> +*/
> + fd = open("/sys/kernel/debug/kcov", O_RDWR);
> + if (fd == -1)
> + perror("open"), exit(1);
> + /* Setup trace mode and trace size. */
> + if ((n = ioctl(fd, KCOV_INIT_UNIQUE, 0)) < 0)
> + perror("ioctl"), exit(1);
> + /* Mmap buffer shared between kernel- and user-space. */
> + cover = (unsigned long*)mmap(NULL, n,
> +PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> 0);
> + if ((void*)cover == MAP_FAILED)
> + perror("mmap"), exit(1);
> + /* Enable coverage collection on the current thread. */
> + if (ioctl(fd, KCOV_ENABLE, KCOV_UNIQUE_PC))
> + perror("ioctl"), exit(1);
> + /* That's the target syscal call. */
> + read(-1, NULL, 0);
> + /* Disable coverage collection for the current thread. After this call
> +* coverage can be enabled for a different thread.
> +*/
> + if (ioctl(fd, KCOV_DISABLE, 0))
> + perror("ioctl"), exit(1);
> +/* Convert byte size into element size */
> +n /= sizeof(unsigned long);
> +/* Print executed PCs in sorted order */
> +for (i = 0; i < n; i++) {
> +for (int j = 0; j < BITS_PER_LONG; j++) {
> +if (cover[i] & (1L << j)) {
> +printf("0x%jx\n", (uintmax_t)(STEXT_START + (i *
> BITS_PER_LONG + j) * 4));
> +}
> +}
> +}
> + /* Free resources. */
> + if (munmap(cover, n * sizeof(unsigned long)))
> + perror("munmap"), exit(1);
> + if (close(fd))
> + perror("close"), exit(1);
> + return 0;
> +}
> +
> Comparison operands collection
> --
>
> diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> index 4e3037dc1204..d72dd73388d1 100644
> --- a/include/linux/kcov.h
> +++ b/include/linux/kcov.h
> @@ -12,17 +12,21 @@ enum kcov_mode {
> /* Coverage collection is not enabled yet. */
> KCOV_MODE_DISABLED = 0,
> /* KCOV was initialized, but tracing mode hasn't been chosen yet. */
> - KCOV_MODE_INIT = 1,
> + KCOV_MODE_INIT_TRACE = 1,
> + /* KCOV was initialized, but recording of unique PCs hasn't been
> chosen yet. */
> + KCOV_MODE_INIT_UNQIUE = 2,
> /*
> * Tracing coverage collection mode.
> * Covered PCs are collected in a per-task buffer.
> */
> - KCOV_MODE_TRACE_PC = 2,
> +
Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE.
On 22.03.21 13:17, Miguel Ojeda wrote:
> Hi Alexander,
>
> On Sun, Mar 21, 2021 at 8:14 PM Alexander Lochmann
> wrote:
>>
>> diff --git a/Documentation/dev-tools/kcov.rst
>> b/Documentation/dev-tools/kcov.rst
>> index d2c4c27e1702..e105ffe6b6e3 100644
>> --- a/Documentation/dev-tools/kcov.rst
>> +++ b/Documentation/dev-tools/kcov.rst
>> @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov,
>> enables trace mode,
>> mmaps coverage buffer and then forks child processes in a loop. Child
>> processes
>> only need to enable coverage (disable happens automatically on thread end).
>>
>> +If someone is interested in a set of executed PCs, and does not care about
>> +execution order, he or she can advise KCOV to do so:
>
> Please mention explicitly that KCOV_INIT_UNIQUE should be used for
> that, i.e. readers of the example shouldn't need to read every line to
> figure it out.
>
>> +#define KCOV_INIT_TRACE_IOR('c', 1, unsigned long)
>
> Trace is not used in the example.
>
>> + /* KCOV was initialized, but recording of unique PCs hasn't been
>> chosen yet. */
>> + KCOV_MODE_INIT_UNQIUE = 2,
>
> Typo? It isn't used?
It is a typo. It should be used...
>
> PS: not sure why I was Cc'd, but I hope that helps.
Thx for your feedback. get_maintainer.pl told me to include you in Cc.
Cheers,
Alex
>
> Cheers,
> Miguel
>
--
Alexander LochmannPGP key: 0xBC3EF6FD
Heiliger Weg 72 phone: +49.231.28053964
D-44141 Dortmund mobile: +49.151.15738323
Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE.
Hi Alexander,
On Sun, Mar 21, 2021 at 8:14 PM Alexander Lochmann
wrote:
>
> diff --git a/Documentation/dev-tools/kcov.rst
> b/Documentation/dev-tools/kcov.rst
> index d2c4c27e1702..e105ffe6b6e3 100644
> --- a/Documentation/dev-tools/kcov.rst
> +++ b/Documentation/dev-tools/kcov.rst
> @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov,
> enables trace mode,
> mmaps coverage buffer and then forks child processes in a loop. Child
> processes
> only need to enable coverage (disable happens automatically on thread end).
>
> +If someone is interested in a set of executed PCs, and does not care about
> +execution order, he or she can advise KCOV to do so:
Please mention explicitly that KCOV_INIT_UNIQUE should be used for
that, i.e. readers of the example shouldn't need to read every line to
figure it out.
> +#define KCOV_INIT_TRACE_IOR('c', 1, unsigned long)
Trace is not used in the example.
> + /* KCOV was initialized, but recording of unique PCs hasn't been
> chosen yet. */
> + KCOV_MODE_INIT_UNQIUE = 2,
Typo? It isn't used?
PS: not sure why I was Cc'd, but I hope that helps.
Cheers,
Miguel
