On Thu, Jan 21, 2021 at 04:12:23PM +0300, Andrey Gruzdev wrote:
> > > +/* KRETPROBE for handle_userfault(). */
> > > +int retprobe_handle_userfault(struct pt_regs *ctx)
> > > +{
> > > + u64 pid = (u32) bpf_get_current_pid_tgid();
> > > + u64 *addr_p;
> > > +
> > > + /*
> > > + * Here we just ignore the return value. In case of spurious wakeup
> > > + * or pending signal we'll still get (at least for v5.8.0 kernel)
> > > + * VM_FAULT_RETRY or (VM_FAULT_RETRY | VM_FAULT_MAJOR) here.
> > > + * Anyhow, handle_userfault() would be re-entered if such case
> > > happens,
> > > + * keeping initial timestamp unchanged for the faulting thread.
> > AFAIU this comment is not matching what the code does. But I agree it's
> > not a
> > big problem because we won't miss any long delays (because the one long
> > delayed
> > sample will just be split into two or multiple delays, which will still be
> > reflected in the histogram at last). Or am I wrong?
>
> Mm, not really sure about comment.. I need to read kernel code again.
Not relevant to kernel; I was only talking about the last sentence where we
won't "keeping initial timestamp unchanged" but we'll do the statistic anyways.
Because exactly as you said we'll get VM_FAULT_RETRY unconditionally while we
won't be able to identify whether the page fault request is resolved or not.
--
Peter Xu
