Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 23 Sep 2019 11:03:49 -0700 | Subject | Re: [PATCH v4 05/10] mm: Return faster for non-fatal signals in user mode faults |
| |
On Sun, Sep 22, 2019 at 9:26 PM Peter Xu <peterx@redhat.com> wrote: > > This patch is a preparation of removing that special path by allowing > the page fault to return even faster if we were interrupted by a > non-fatal signal during a user-mode page fault handling routine.
So I really wish saome other vm person would also review these things, but looking over this series once more, this is the patch I probably like the least.
And the reason I like it the least is that I have a hard time explaining to myself what the code does and why, and why it's so full of this pattern:
> - if ((fault & VM_FAULT_RETRY) && fatal_signal_pending(current)) > + if ((fault & VM_FAULT_RETRY) && > + fault_should_check_signal(user_mode(regs))) > return;
which isn't all that pretty.
Why isn't this just
static bool fault_signal_pending(unsigned int fault_flags, struct pt_regs *regs) { return (fault_flags & VM_FAULT_RETRY) && (fatal_signal_pending(current) || (user_mode(regs) && signal_pending(current))); }
and then most of the users would be something like
if (fault_signal_pending(fault, regs)) return;
and the exceptions could do their own thing.
Now the code is prettier and more understandable, I feel.
And if something doesn't follow this pattern, maybe it either _should_ follow that pattern or it should just not use the helper but explain why it has an unusual pattern.
Linus
| |