From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755859AbdKCO1O (ORCPT ); Fri, 3 Nov 2017 10:27:14 -0400 Received: from mail.kernel.org ([198.145.29.99]:41856 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755998AbdKCOZJ (ORCPT ); Fri, 3 Nov 2017 10:25:09 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 795EF21869 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=frederic@kernel.org X-Google-Smtp-Source: ABhQp+QCEHysLhGSyLZ42BdMGWScrJyPfuBXhfkWc/k1a+tbahmHLH6pmyOpcjwRUT3YIohVUUqUBFbmGFTvqUO8sjc= MIME-Version: 1.0 In-Reply-To: <20171022092025.GJ6524@worktop.programming.kicks-ass.net> References: <1508460971-8731-1-git-send-email-frederic@kernel.org> <1508460971-8731-8-git-send-email-frederic@kernel.org> <20171022092025.GJ6524@worktop.programming.kicks-ass.net> From: Frederic Weisbecker Date: Fri, 3 Nov 2017 15:25:07 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 07/14] x86: Use lockdep to assert IRQs are disabled/enabled To: Peter Zijlstra Cc: LKML , Tejun Heo , "David S . Miller" , Lai Jiangshan , Thomas Gleixner , "Paul E . McKenney" , Ingo Molnar Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-10-22 11:20 UTC+02:00, Peter Zijlstra : > On Fri, Oct 20, 2017 at 02:56:04AM +0200, Frederic Weisbecker wrote: >> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c >> index 03505ff..b4f3a55 100644 >> --- a/arch/x86/entry/common.c >> +++ b/arch/x86/entry/common.c >> @@ -186,7 +186,7 @@ __visible inline void prepare_exit_to_usermode(struct >> pt_regs *regs) >> >> addr_limit_user_check(); >> >> - if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled())) >> + if (!lockdep_assert_irqs_disabled()) >> local_irq_disable(); >> >> lockdep_sys_exit(); > > So this is the only site that ever uses the return value; and for this > you've chosen the wrong value for !CONFIG_PROVE_LOCKING (namely 1), > resulting in an unconditional CLI here for !lockdep kernels. Hmm, it should be the opposite. The assertion maps to 1 for !CONFIG_PROVE_LOCKING but we are testing with "!". > How about we replace that whole thing with a simple: > > lockdep_assert_irqs_disabled(); > > And leave it at that, allowing us to remove the return value thing > entirely. > > The whole if !disabled, disable logic is uber paranoid programming, but > I don't think we've ever seen that WARN trigger, and if it does (and > then burns the kernel) we at least know wtf happend. I certainly don't mind personally, as long as we get the warning. Now I've seen different opinions on matters such as this. Some prefer to have the auto-correction, some don't. I'll try as you say and see if anybody gets angry :)