> On Mar 4, 2020, at 10:12 AM, Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 3/4/20 10:08 AM, Andrii Nakryiko wrote:
>> On Wed, Mar 4, 2020 at 9:53 AM Yonghong Song <yhs@xxxxxx> wrote:
>>>
>>> Added one test, send_signal_sched_switch, to test bpf_send_signal()
>>> helper triggered by sched/sched_switch tracepoint. This test can be used
>>> to verify kernel deadlocks fixed by the previous commit. The test itself
>>> is heavily borrowed from Commit eac9153f2b58 ("bpf/stackmap: Fix deadlock
>>> with rq_lock in bpf_get_stack()").
>>>
>>> Cc: Song Liu <songliubraving@xxxxxx>
>>> Signed-off-by: Yonghong Song <yhs@xxxxxx>
>>> ---
>>> .../bpf/prog_tests/send_signal_sched_switch.c | 89 +++++++++++++++++++
>>> .../bpf/progs/test_send_signal_kern.c | 6 ++
>>> 2 files changed, 95 insertions(+)
>>> create mode 100644 tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c b/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c
>>> new file mode 100644
>>> index 000000000000..f5c9dbdeb173
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal_sched_switch.c
>>> @@ -0,0 +1,89 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +#include <test_progs.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <sys/mman.h>
>>> +#include <pthread.h>
>>> +#include <sys/types.h>
>>> +#include <sys/stat.h>
>>> +#include <fcntl.h>
>>> +#include "test_send_signal_kern.skel.h"
>>> +
>>> +static void sigusr1_handler(int signum)
>>> +{
>>> +}
>>> +
>>> +#define THREAD_COUNT 100
>>> +
>>> +static char *filename;
>>> +
>>> +static void *worker(void *p)
>>> +{
>>> + int err, fd, i = 0;
>>> + u32 duration = 0;
>>> + char *pptr;
>>> + void *ptr;
>>> +
>>> + fd = open(filename, O_RDONLY);
>>> + if (CHECK(fd < 0, "open", "open failed %s\n", strerror(errno)))
>>> + return NULL;
>>> +
>>> + while (i < 100) {
>>> + struct timespec ts = {0, 1000 + rand() % 2000};
>>> +
>>> + ptr = mmap(NULL, 4096 * 64, PROT_READ, MAP_PRIVATE, fd, 0);
>>> + err = errno;
>>> + usleep(1);
>>> + if (CHECK(ptr == MAP_FAILED, "mmap", "mmap failed: %s\n",
>>> + strerror(err)))
>>> + break;
>>> +
>>> + munmap(ptr, 4096 * 64);
>>> + usleep(1);
>>> + pptr = malloc(1);
>>> + usleep(1);
>>> + pptr[0] = 1;
>>> + usleep(1);
>>> + free(pptr);
>>> + usleep(1);
>>> + nanosleep(&ts, NULL);
>>> + i++;
>> Any specific reason to do mmap()/munmap() in a loop? Would, say,
>> getpid syscall work just fine as well to trigger a bunch of context
>> switches? Or event just usleep(1) alone?
>
> No particular reason. Copied from Song's original reproducer and
> it is working. Maybe Song can comment why it is written this way.
In that test, mmap/munmap are used to lock mmap_sem. I guess we
don't really need them in this test.
Thanks,
song
