Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753562AbbGOT4j (ORCPT ); Wed, 15 Jul 2015 15:56:39 -0400 Received: from mail-ig0-f178.google.com ([209.85.213.178]:32915 "EHLO mail-ig0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753313AbbGOT4g (ORCPT ); Wed, 15 Jul 2015 15:56:36 -0400 MIME-Version: 1.0 In-Reply-To: <20150714230727.GD29441@lerouge> References: <20150714230727.GD29441@lerouge> Date: Wed, 15 Jul 2015 12:56:35 -0700 X-Google-Sender-Auth: ECY-r_2cp_fOKijeZZlcqp0cJfQ Message-ID: Subject: Re: [tip:x86/asm] x86/entry: Add new, comprehensible entry and exit handlers written in C From: Linus Torvalds To: Frederic Weisbecker Cc: Kees Cook , Peter Zijlstra , Denys Vlasenko , Ingo Molnar , Brian Gerst , Andy Lutomirski , Borislav Petkov , Andrew Lutomirski , Oleg Nesterov , Peter Anvin , Linux Kernel Mailing List , Denys Vlasenko , Thomas Gleixner , Rik van Riel , "linux-tip-commits@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2629 Lines: 72 On Tue, Jul 14, 2015 at 4:07 PM, Frederic Weisbecker wrote: > On Tue, Jul 07, 2015 at 03:51:48AM -0700, tip-bot for Andy Lutomirski wrote: >> + while (true) { >> + u32 cached_flags = >> + READ_ONCE(pt_regs_to_thread_info(regs)->flags); >> + >> + if (!(cached_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | >> + _TIF_UPROBE | _TIF_NEED_RESCHED))) >> + break; >> + >> + /* We have work to do. */ >> + local_irq_enable(); >> + >> + if (cached_flags & _TIF_NEED_RESCHED) >> + schedule(); >> + >> + if (cached_flags & _TIF_UPROBE) >> + uprobe_notify_resume(regs); >> + >> + /* deal with pending signal delivery */ >> + if (cached_flags & _TIF_SIGPENDING) >> + do_signal(regs); >> + >> + if (cached_flags & _TIF_NOTIFY_RESUME) { >> + clear_thread_flag(TIF_NOTIFY_RESUME); >> + tracehook_notify_resume(regs); >> + } >> + >> + if (cached_flags & _TIF_USER_RETURN_NOTIFY) >> + fire_user_return_notifiers(); >> + >> + /* Disable IRQs and retry */ >> + local_irq_disable(); >> + } > > I dreamed so many times about this loop in C! So this made me look at it again, and now I'm worried. There's that "early break", but it doesn't check _TIF_USER_RETURN_NOTIFY. So if *only* USER_RETURN_NOTIFY is set, we're screwed. It migth be that that doesn't happen for some reason, but I'm not seeing what that reason would be. The other thing that worries me is that this depends on all the handler routines to clear the flags (except for tracehook_notify_resume()). Which they hopefully do. But that means that just looking at this locally, it's not at all obvious that it works right. So wouldn't it be much nicer to do: u32 cached_flags = READ_ONCE(pt_regs_to_thread_info(regs)->flags); cached_flags &= _TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_USER_RETURN_NOTIFY | _TIF_UPROBE | _TIF_NEED_RESCHED; if (!cached_flags) break; atomic_clear_mask(cached_flags, &pt_regs_to_thread_info(regs)->flags); and then have those bit tests after that? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/