On Fri, Oct 23, 2020 at 03:53:47PM -0400, Michael Jeanson wrote:
> -#define __DO_TRACE(tp, proto, args, cond, rcuidle)                   \
> +#define __DO_TRACE(tp, proto, args, cond, rcuidle, tp_flags)         \
>       do {                                                            \
>               struct tracepoint_func *it_func_ptr;                    \
>               void *it_func;                                          \
>               void *__data;                                           \
>               int __maybe_unused __idx = 0;                           \
> +             bool maysleep = (tp_flags) & TRACEPOINT_MAYSLEEP;       \
>                                                                       \
>               if (!(cond))                                            \
>                       return;                                         \
> @@ -170,8 +178,13 @@ static inline struct tracepoint 
> *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>               /* srcu can't be used from NMI */                       \
>               WARN_ON_ONCE(rcuidle && in_nmi());                      \
>                                                                       \
> -             /* keep srcu and sched-rcu usage consistent */          \
> -             preempt_disable_notrace();                              \
> +             if (maysleep) {                                         \
> +                     might_sleep();                                  \

The main purpose of the patch set is to access user memory in tracepoints, 
right?
In such case I suggest to use stronger might_fault() here.
We used might_sleep() in sleepable bpf and it wasn't enough to catch
a combination where sleepable hook was invoked while mm->mmap_lock was
taken which may cause a deadlock.

> +                     rcu_read_lock_trace();                          \
> +             } else {                                                \
> +                     /* keep srcu and sched-rcu usage consistent */  \
> +                     preempt_disable_notrace();                      \
> +             }                                                       \

Reply via email to