From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2FF64C433E2 for ; Wed, 16 Sep 2020 07:37:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D7E4E20771 for ; Wed, 16 Sep 2020 07:37:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="Wbmy2wm4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726139AbgIPHhh (ORCPT ); Wed, 16 Sep 2020 03:37:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52962 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726384AbgIPHhc (ORCPT ); Wed, 16 Sep 2020 03:37:32 -0400 Received: from mail-ot1-x344.google.com (mail-ot1-x344.google.com [IPv6:2607:f8b0:4864:20::344]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A4B00C061797 for ; Wed, 16 Sep 2020 00:37:31 -0700 (PDT) Received: by mail-ot1-x344.google.com with SMTP id g10so5776590otq.9 for ; Wed, 16 Sep 2020 00:37:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=Wbmy2wm4Y3Z7IHj2WQgnSxJHvWaqiIYxvJrRLzN5yosJdstiBZVZo9gvc44pSE/nA1 P9hDpmjXczq2zLOkqvx/WHir/QasP1J2aBD5X+7FKjnP91s5nxsyqJv2jhU8PZzNRGRF pjrrhaFUmLe21mq1kggp9iTxwlhfZdVWViCcQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=YIJFUp4jc8jnv6DFcI+gT2Sgyjf0ntFNLGVfPsCuoDI8yJPdwFwh2UKQJbKs82ec3y k5yrYJ/5hWTwoByY9HuzIGKmdX8E6put7xoeaBta/FtJ42DMim1fGdJLvQhRoGJ4othH 110urJ+lYgTkLp/mcuMDIkcLokiQ1V2Z41IAaGM3F7x/7DSEw/TT7EWvBXIh0l1LsWfz o+RSFr9qiGbzaH13B59/YUPVSF96bhShBHH2CqT+BCxRuFLYyaVa1MrvOrcEiCpUc6N0 zdha0PQRPolc7ozGUgidEJVzVlK6lhiqvhZHP3chXSURAmemh/usS9KCy0PbR8bAO2nz OWjQ== X-Gm-Message-State: AOAM530yxQ+rMQqPve+72lu6/J0/PNPz3jr2qz63MKsLSRdBPZLlOmt+ FDWf9EcfNKnM+xrtfrWIY6GTdJEY+N/+O2oPJIi2tA== X-Google-Smtp-Source: ABdhPJwGl0rb2zlMJB/+T8+6pHD/ce/ZZx4zrU0sObZFxXtLXBpNIcPMMRDqJU2JyoR0WPCWkLBmcTGbfcRbBMPy77o= X-Received: by 2002:a05:6830:14d9:: with SMTP id t25mr16390529otq.188.1600241849077; Wed, 16 Sep 2020 00:37:29 -0700 (PDT) MIME-Version: 1.0 References: <20200914204209.256266093@linutronix.de> <871rj4owfn.fsf@nanos.tec.linutronix.de> <87bli75t7v.fsf@nanos.tec.linutronix.de> In-Reply-To: From: Daniel Vetter Date: Wed, 16 Sep 2020 09:37:17 +0200 Message-ID: Subject: Re: [patch 00/13] preempt: Make preempt count unconditional To: Linus Torvalds Cc: Thomas Gleixner , Ard Biesheuvel , Herbert Xu , LKML , linux-arch , Sebastian Andrzej Siewior , Valentin Schneider , Richard Henderson , Ivan Kokshaysky , Matt Turner , alpha , Jeff Dike , Richard Weinberger , Anton Ivanov , linux-um , Brian Cain , linux-hexagon@vger.kernel.org, Geert Uytterhoeven , linux-m68k , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Will Deacon , Andrew Morton , Linux-MM , Ingo Molnar , Russell King , Linux ARM , Chris Zankel , Max Filippov , linux-xtensa@linux-xtensa.org, Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , David Airlie , intel-gfx , dri-devel , "Paul E. McKenney" , Josh Triplett , Mathieu Desnoyers , Lai Jiangshan , Shuah Khan , rcu@vger.kernel.org, "open list:KERNEL SELFTEST FRAMEWORK" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds wrote: > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner wrote: > > > > OTOH, having a working 'preemptible()' or maybe better named > > 'can_schedule()' check makes tons of sense to make decisions about > > allocation modes or other things. > > No. I think that those kinds of decisions about actual behavior are > always simply fundamentally wrong. > > Note that this is very different from having warnings about invalid > use. THAT is correct. It may not warn in all configurations, but that > doesn't matter: what matters is that it warns in common enough > configurations that developers will catch it. > > So having a warning in "might_sleep()" that doesn't always trigger, > because you have a limited configuration that can't even detect the > situation, that's fine and dandy and intentional. > > But having code like > > if (can_schedule()) > .. do something different .. > > is fundamentally complete and utter garbage. > > It's one thing if you test for "am I in hardware interrupt context". > Those tests aren't great either, but at least they make sense. > > But a driver - or some library routine - making a difference based on > some nebulous "can I schedule" is fundamentally and basically WRONG. > > If some code changes behavior, it needs to be explicit to the *caller* > of that code. > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule()) > do_something_atomic()" is pure shite. > > And I am not IN THE LEAST interested in trying to help people doing > pure shite. We need to fix them. Like the crypto code is getting > fixed. Just figured I'll throw my +1 in from reading too many (gpu) drivers. Code that tries to cleverly adjust its behaviour depending upon the context it's running in is harder to understand and blows up in more interesting ways. We still have drm_can_sleep() and it's mostly just used for debug code, and I've largely ended up just deleting everything that used it because when you're driver is blowing up the last thing you want is to realize your debug code and output can't be relied upon. Or worse, that the only Oops you have is the one in the debug code, because the real one scrolled away - the original idea behind drm_can_sleep was to make all the modeset code work automagically both in normal ioctl/kworker context and in the panic handlers or kgdb callbacks. Wishful thinking at best. Also at least for me that extends to everything, e.g. I much prefer explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for locks shared with interrupt handlers, since the former two gives me clear information from which contexts such function can be called. Other end is the memalloc_no*_save/restore functions, where I recently made a real big fool of myself because I didn't realize how much that impacts everything that's run within - suddenly "GFP_KERNEL for small stuff never fails" is wrong everywhere. It's all great for debugging and sanity checks (and we run with all that stuff enabled in our CI), but really semantic changes depending upon magic context checks freak my out :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B5E95C433E2 for ; Wed, 16 Sep 2020 07:37:33 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 292452078D for ; Wed, 16 Sep 2020 07:37:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="Wbmy2wm4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 292452078D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 85D4A6B0055; Wed, 16 Sep 2020 03:37:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8307C6B005A; Wed, 16 Sep 2020 03:37:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 747976B005C; Wed, 16 Sep 2020 03:37:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0223.hostedemail.com [216.40.44.223]) by kanga.kvack.org (Postfix) with ESMTP id 5F14D6B0055 for ; Wed, 16 Sep 2020 03:37:32 -0400 (EDT) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 272713624 for ; Wed, 16 Sep 2020 07:37:32 +0000 (UTC) X-FDA: 77268119544.02.plant99_1f0f67b27118 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin02.hostedemail.com (Postfix) with ESMTP id E4C6B100C9BD1 for ; Wed, 16 Sep 2020 07:37:31 +0000 (UTC) X-HE-Tag: plant99_1f0f67b27118 X-Filterd-Recvd-Size: 7998 Received: from mail-ot1-f65.google.com (mail-ot1-f65.google.com [209.85.210.65]) by imf21.hostedemail.com (Postfix) with ESMTP for ; Wed, 16 Sep 2020 07:37:31 +0000 (UTC) Received: by mail-ot1-f65.google.com with SMTP id o8so5822210otl.4 for ; Wed, 16 Sep 2020 00:37:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=Wbmy2wm4Y3Z7IHj2WQgnSxJHvWaqiIYxvJrRLzN5yosJdstiBZVZo9gvc44pSE/nA1 P9hDpmjXczq2zLOkqvx/WHir/QasP1J2aBD5X+7FKjnP91s5nxsyqJv2jhU8PZzNRGRF pjrrhaFUmLe21mq1kggp9iTxwlhfZdVWViCcQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=j1tCo7qDevY/A9fFlIkRfGmF4wP6LHrVSsAcpuActFfsmDWFvnira9Xybo/1Epem6B ZfHPYkNXE/cuxvtDrprPEy41mKrbFHhOKJE7CmRyPVrapdmf77pOpWrAWrP0cvYXei5E mPACN1qLrlOw1ach7ySn1RLAM9eEoJvKjDSZdzdO+lJQ/jfke3Eh3y2c14/7o5UOlEFe EXXrVIDlaBPGkesmuh9iF38WGYrfven7G4AbwoJbenDpCh0yvSLGafm8EOU5GEGH0O0/ LquoDCCLpr0KwGes1OCLVYaAI6w4UQhTX0GSZV5czJNGuo2V0hjNbL3ydZ+w0QzHaFYI dVvw== X-Gm-Message-State: AOAM5300Lz2PU94zh6DJ6JrUQvco7CTIKhcQLyDv8qV8vGH4hgSetZJ/ UHxAmGcDfHmUqk85dmZuiRshlOMTgACoeKCHyF22Bw== X-Google-Smtp-Source: ABdhPJwGl0rb2zlMJB/+T8+6pHD/ce/ZZx4zrU0sObZFxXtLXBpNIcPMMRDqJU2JyoR0WPCWkLBmcTGbfcRbBMPy77o= X-Received: by 2002:a05:6830:14d9:: with SMTP id t25mr16390529otq.188.1600241849077; Wed, 16 Sep 2020 00:37:29 -0700 (PDT) MIME-Version: 1.0 References: <20200914204209.256266093@linutronix.de> <871rj4owfn.fsf@nanos.tec.linutronix.de> <87bli75t7v.fsf@nanos.tec.linutronix.de> In-Reply-To: From: Daniel Vetter Date: Wed, 16 Sep 2020 09:37:17 +0200 Message-ID: Subject: Re: [patch 00/13] preempt: Make preempt count unconditional To: Linus Torvalds Cc: Thomas Gleixner , Ard Biesheuvel , Herbert Xu , LKML , linux-arch , Sebastian Andrzej Siewior , Valentin Schneider , Richard Henderson , Ivan Kokshaysky , Matt Turner , alpha , Jeff Dike , Richard Weinberger , Anton Ivanov , linux-um , Brian Cain , linux-hexagon@vger.kernel.org, Geert Uytterhoeven , linux-m68k , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Will Deacon , Andrew Morton , Linux-MM , Ingo Molnar , Russell King , Linux ARM , Chris Zankel , Max Filippov , linux-xtensa@linux-xtensa.org, Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , David Airlie , intel-gfx , dri-devel , "Paul E. McKenney" , Josh Triplett , Mathieu Desnoyers , Lai Jiangshan , Shuah Khan , rcu@vger.kernel.org, "open list:KERNEL SELFTEST FRAMEWORK" Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: E4C6B100C9BD1 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds wrote: > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner wrote: > > > > OTOH, having a working 'preemptible()' or maybe better named > > 'can_schedule()' check makes tons of sense to make decisions about > > allocation modes or other things. > > No. I think that those kinds of decisions about actual behavior are > always simply fundamentally wrong. > > Note that this is very different from having warnings about invalid > use. THAT is correct. It may not warn in all configurations, but that > doesn't matter: what matters is that it warns in common enough > configurations that developers will catch it. > > So having a warning in "might_sleep()" that doesn't always trigger, > because you have a limited configuration that can't even detect the > situation, that's fine and dandy and intentional. > > But having code like > > if (can_schedule()) > .. do something different .. > > is fundamentally complete and utter garbage. > > It's one thing if you test for "am I in hardware interrupt context". > Those tests aren't great either, but at least they make sense. > > But a driver - or some library routine - making a difference based on > some nebulous "can I schedule" is fundamentally and basically WRONG. > > If some code changes behavior, it needs to be explicit to the *caller* > of that code. > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule()) > do_something_atomic()" is pure shite. > > And I am not IN THE LEAST interested in trying to help people doing > pure shite. We need to fix them. Like the crypto code is getting > fixed. Just figured I'll throw my +1 in from reading too many (gpu) drivers. Code that tries to cleverly adjust its behaviour depending upon the context it's running in is harder to understand and blows up in more interesting ways. We still have drm_can_sleep() and it's mostly just used for debug code, and I've largely ended up just deleting everything that used it because when you're driver is blowing up the last thing you want is to realize your debug code and output can't be relied upon. Or worse, that the only Oops you have is the one in the debug code, because the real one scrolled away - the original idea behind drm_can_sleep was to make all the modeset code work automagically both in normal ioctl/kworker context and in the panic handlers or kgdb callbacks. Wishful thinking at best. Also at least for me that extends to everything, e.g. I much prefer explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for locks shared with interrupt handlers, since the former two gives me clear information from which contexts such function can be called. Other end is the memalloc_no*_save/restore functions, where I recently made a real big fool of myself because I didn't realize how much that impacts everything that's run within - suddenly "GFP_KERNEL for small stuff never fails" is wrong everywhere. It's all great for debugging and sanity checks (and we run with all that stuff enabled in our CI), but really semantic changes depending upon magic context checks freak my out :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4C89C35256 for ; Wed, 16 Sep 2020 07:37:33 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 634F820684 for ; Wed, 16 Sep 2020 07:37:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="Wbmy2wm4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 634F820684 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A62466E9C6; Wed, 16 Sep 2020 07:37:32 +0000 (UTC) Received: from mail-ot1-x343.google.com (mail-ot1-x343.google.com [IPv6:2607:f8b0:4864:20::343]) by gabe.freedesktop.org (Postfix) with ESMTPS id DB7626E2F3 for ; Wed, 16 Sep 2020 07:37:30 +0000 (UTC) Received: by mail-ot1-x343.google.com with SMTP id o8so5822211otl.4 for ; Wed, 16 Sep 2020 00:37:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=Wbmy2wm4Y3Z7IHj2WQgnSxJHvWaqiIYxvJrRLzN5yosJdstiBZVZo9gvc44pSE/nA1 P9hDpmjXczq2zLOkqvx/WHir/QasP1J2aBD5X+7FKjnP91s5nxsyqJv2jhU8PZzNRGRF pjrrhaFUmLe21mq1kggp9iTxwlhfZdVWViCcQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=myT0ClNnDYhwJLAytUXN2LpVy/4YOTNQivxrtG3PGnbVnYhiKWftW3VJAMzuasvjKu lzRTJ3AtFJOCwP/zWV4zuWI1qJY5rB0GBg9ym1XBWlDfC3GBtGU3gP9BbNcathaJ7P3W hYdTsLN1d4CNVOIGadh6vwoTHZ8MN2moMBauuPI6vfKi+VI23dF1VqSgkHvxtLi/Jnlk jSuEtwt+J2e6ldqS6Ki2hA3YvGCJ9skc8IPIHI2GB487/2wyYd1Hi381MUPhlE42wr9e yLa7aPmhDQASUQ6LoJmC6bj3Iw9yFnJSW5x9jLAQQH5vrnQU4FgO5YytoC+uPPM1U1om gkLg== X-Gm-Message-State: AOAM532ZwsDLxN6BjSwVFyxQsEzx7w43fuOo/f+KwjMGdGZTsnQoo1Y2 Ho4kp3EmK3iPCqWSOsfh/U6pegakFnReFtNzsPrZGQ== X-Google-Smtp-Source: ABdhPJwGl0rb2zlMJB/+T8+6pHD/ce/ZZx4zrU0sObZFxXtLXBpNIcPMMRDqJU2JyoR0WPCWkLBmcTGbfcRbBMPy77o= X-Received: by 2002:a05:6830:14d9:: with SMTP id t25mr16390529otq.188.1600241849077; Wed, 16 Sep 2020 00:37:29 -0700 (PDT) MIME-Version: 1.0 References: <20200914204209.256266093@linutronix.de> <871rj4owfn.fsf@nanos.tec.linutronix.de> <87bli75t7v.fsf@nanos.tec.linutronix.de> In-Reply-To: From: Daniel Vetter Date: Wed, 16 Sep 2020 09:37:17 +0200 Message-ID: Subject: Re: [patch 00/13] preempt: Make preempt count unconditional To: Linus Torvalds X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Peter Zijlstra , Sebastian Andrzej Siewior , Lai Jiangshan , dri-devel , Ben Segall , Linux-MM , "open list:KERNEL SELFTEST FRAMEWORK" , linux-hexagon@vger.kernel.org, Will Deacon , Ingo Molnar , Anton Ivanov , linux-arch , Vincent Guittot , Herbert Xu , Brian Cain , Richard Weinberger , Russell King , Ard Biesheuvel , David Airlie , Ingo Molnar , Geert Uytterhoeven , Mel Gorman , intel-gfx , Matt Turner , Valentin Schneider , linux-xtensa@linux-xtensa.org, Shuah Khan , "Paul E. McKenney" , Jeff Dike , linux-um , Josh Triplett , Steven Rostedt , rcu@vger.kernel.org, linux-m68k , Ivan Kokshaysky , Rodrigo Vivi , Thomas Gleixner , Dietmar Eggemann , Linux ARM , Richard Henderson , Chris Zankel , Max Filippov , LKML , alpha , Mathieu Desnoyers , Andrew Morton , Daniel Bristot de Oliveira Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds wrote: > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner wrote: > > > > OTOH, having a working 'preemptible()' or maybe better named > > 'can_schedule()' check makes tons of sense to make decisions about > > allocation modes or other things. > > No. I think that those kinds of decisions about actual behavior are > always simply fundamentally wrong. > > Note that this is very different from having warnings about invalid > use. THAT is correct. It may not warn in all configurations, but that > doesn't matter: what matters is that it warns in common enough > configurations that developers will catch it. > > So having a warning in "might_sleep()" that doesn't always trigger, > because you have a limited configuration that can't even detect the > situation, that's fine and dandy and intentional. > > But having code like > > if (can_schedule()) > .. do something different .. > > is fundamentally complete and utter garbage. > > It's one thing if you test for "am I in hardware interrupt context". > Those tests aren't great either, but at least they make sense. > > But a driver - or some library routine - making a difference based on > some nebulous "can I schedule" is fundamentally and basically WRONG. > > If some code changes behavior, it needs to be explicit to the *caller* > of that code. > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule()) > do_something_atomic()" is pure shite. > > And I am not IN THE LEAST interested in trying to help people doing > pure shite. We need to fix them. Like the crypto code is getting > fixed. Just figured I'll throw my +1 in from reading too many (gpu) drivers. Code that tries to cleverly adjust its behaviour depending upon the context it's running in is harder to understand and blows up in more interesting ways. We still have drm_can_sleep() and it's mostly just used for debug code, and I've largely ended up just deleting everything that used it because when you're driver is blowing up the last thing you want is to realize your debug code and output can't be relied upon. Or worse, that the only Oops you have is the one in the debug code, because the real one scrolled away - the original idea behind drm_can_sleep was to make all the modeset code work automagically both in normal ioctl/kworker context and in the panic handlers or kgdb callbacks. Wishful thinking at best. Also at least for me that extends to everything, e.g. I much prefer explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for locks shared with interrupt handlers, since the former two gives me clear information from which contexts such function can be called. Other end is the memalloc_no*_save/restore functions, where I recently made a real big fool of myself because I didn't realize how much that impacts everything that's run within - suddenly "GFP_KERNEL for small stuff never fails" is wrong everywhere. It's all great for debugging and sanity checks (and we run with all that stuff enabled in our CI), but really semantic changes depending upon magic context checks freak my out :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0851AC43461 for ; Wed, 16 Sep 2020 07:37:33 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8C4B32078D for ; Wed, 16 Sep 2020 07:37:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="Wbmy2wm4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8C4B32078D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E7BA488CFA; Wed, 16 Sep 2020 07:37:31 +0000 (UTC) Received: from mail-ot1-x343.google.com (mail-ot1-x343.google.com [IPv6:2607:f8b0:4864:20::343]) by gabe.freedesktop.org (Postfix) with ESMTPS id DAC7388CFA for ; Wed, 16 Sep 2020 07:37:30 +0000 (UTC) Received: by mail-ot1-x343.google.com with SMTP id 60so5824923otw.3 for ; Wed, 16 Sep 2020 00:37:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=Wbmy2wm4Y3Z7IHj2WQgnSxJHvWaqiIYxvJrRLzN5yosJdstiBZVZo9gvc44pSE/nA1 P9hDpmjXczq2zLOkqvx/WHir/QasP1J2aBD5X+7FKjnP91s5nxsyqJv2jhU8PZzNRGRF pjrrhaFUmLe21mq1kggp9iTxwlhfZdVWViCcQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=s0cFooHVbLIcfKCCGAEqFDmwbWjZeN7yaSrhWVypDA4ys084ejelOlsG7hheAyKPWu 78nfN1hI+1HlSgkmmpfG/xkQFGx8Y7ramcYAKAZ3yQNXT0gvFGkM+5PnPtPcpNgUD4Yh V0Gs3zJb1ZoFQMc7VHon/kHrafXRu3g2FMGWx/4ylyNLXG9q+r5E8SSIU5yjGeMBYhjo bcFZP45zeF9eQcZJs5+Zalcz3I2XwCVrh01tuKZlXKPBZcw3AvleJ9dCjK4ye7trYv1H NZxn9mYgAlViupJ/wsXCWFvbU8S+qQXbYxgUoZZyGVRrS6FbahjSGDZjzstQY8Ew0rxa G9vg== X-Gm-Message-State: AOAM531IpMZV3d9rsw1Uo+zbi9OlRISM9Os+y9lkT/iWzKjYPYSdVVxk G2k+1hz6TXL+k6qGcLSDb9qc6nASE6lc5jffmft/vw== X-Google-Smtp-Source: ABdhPJwGl0rb2zlMJB/+T8+6pHD/ce/ZZx4zrU0sObZFxXtLXBpNIcPMMRDqJU2JyoR0WPCWkLBmcTGbfcRbBMPy77o= X-Received: by 2002:a05:6830:14d9:: with SMTP id t25mr16390529otq.188.1600241849077; Wed, 16 Sep 2020 00:37:29 -0700 (PDT) MIME-Version: 1.0 References: <20200914204209.256266093@linutronix.de> <871rj4owfn.fsf@nanos.tec.linutronix.de> <87bli75t7v.fsf@nanos.tec.linutronix.de> In-Reply-To: From: Daniel Vetter Date: Wed, 16 Sep 2020 09:37:17 +0200 Message-ID: To: Linus Torvalds Subject: Re: [Intel-gfx] [patch 00/13] preempt: Make preempt count unconditional X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Peter Zijlstra , Sebastian Andrzej Siewior , Lai Jiangshan , dri-devel , Ben Segall , Linux-MM , "open list:KERNEL SELFTEST FRAMEWORK" , linux-hexagon@vger.kernel.org, Will Deacon , Ingo Molnar , Anton Ivanov , linux-arch , Herbert Xu , Brian Cain , Richard Weinberger , Russell King , Ard Biesheuvel , David Airlie , Ingo Molnar , Geert Uytterhoeven , Mel Gorman , intel-gfx , Matt Turner , Valentin Schneider , linux-xtensa@linux-xtensa.org, Shuah Khan , "Paul E. McKenney" , Jeff Dike , linux-um , Josh Triplett , Steven Rostedt , rcu@vger.kernel.org, linux-m68k , Ivan Kokshaysky , Thomas Gleixner , Dietmar Eggemann , Linux ARM , Richard Henderson , Chris Zankel , Max Filippov , LKML , alpha , Mathieu Desnoyers , Andrew Morton , Daniel Bristot de Oliveira Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds wrote: > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner wrote: > > > > OTOH, having a working 'preemptible()' or maybe better named > > 'can_schedule()' check makes tons of sense to make decisions about > > allocation modes or other things. > > No. I think that those kinds of decisions about actual behavior are > always simply fundamentally wrong. > > Note that this is very different from having warnings about invalid > use. THAT is correct. It may not warn in all configurations, but that > doesn't matter: what matters is that it warns in common enough > configurations that developers will catch it. > > So having a warning in "might_sleep()" that doesn't always trigger, > because you have a limited configuration that can't even detect the > situation, that's fine and dandy and intentional. > > But having code like > > if (can_schedule()) > .. do something different .. > > is fundamentally complete and utter garbage. > > It's one thing if you test for "am I in hardware interrupt context". > Those tests aren't great either, but at least they make sense. > > But a driver - or some library routine - making a difference based on > some nebulous "can I schedule" is fundamentally and basically WRONG. > > If some code changes behavior, it needs to be explicit to the *caller* > of that code. > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule()) > do_something_atomic()" is pure shite. > > And I am not IN THE LEAST interested in trying to help people doing > pure shite. We need to fix them. Like the crypto code is getting > fixed. Just figured I'll throw my +1 in from reading too many (gpu) drivers. Code that tries to cleverly adjust its behaviour depending upon the context it's running in is harder to understand and blows up in more interesting ways. We still have drm_can_sleep() and it's mostly just used for debug code, and I've largely ended up just deleting everything that used it because when you're driver is blowing up the last thing you want is to realize your debug code and output can't be relied upon. Or worse, that the only Oops you have is the one in the debug code, because the real one scrolled away - the original idea behind drm_can_sleep was to make all the modeset code work automagically both in normal ioctl/kworker context and in the panic handlers or kgdb callbacks. Wishful thinking at best. Also at least for me that extends to everything, e.g. I much prefer explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for locks shared with interrupt handlers, since the former two gives me clear information from which contexts such function can be called. Other end is the memalloc_no*_save/restore functions, where I recently made a real big fool of myself because I didn't realize how much that impacts everything that's run within - suddenly "GFP_KERNEL for small stuff never fails" is wrong everywhere. It's all great for debugging and sanity checks (and we run with all that stuff enabled in our CI), but really semantic changes depending upon magic context checks freak my out :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [patch 00/13] preempt: Make preempt count unconditional Date: Wed, 16 Sep 2020 09:37:17 +0200 Message-ID: References: <20200914204209.256266093@linutronix.de> <871rj4owfn.fsf@nanos.tec.linutronix.de> <87bli75t7v.fsf@nanos.tec.linutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ir83kA7yZmatKHQpy7nyYwdfudWOyhH141WksSrpLpI=; b=Wbmy2wm4Y3Z7IHj2WQgnSxJHvWaqiIYxvJrRLzN5yosJdstiBZVZo9gvc44pSE/nA1 P9hDpmjXczq2zLOkqvx/WHir/QasP1J2aBD5X+7FKjnP91s5nxsyqJv2jhU8PZzNRGRF pjrrhaFUmLe21mq1kggp9iTxwlhfZdVWViCcQ= In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Linus Torvalds Cc: Juri Lelli , Peter Zijlstra , Sebastian Andrzej Siewior , Lai Jiangshan , dri-devel , Ben Segall , Linux-MM , "open list:KERNEL SELFTEST FRAMEWORK" , linux-hexagon@vger.kernel.org, Will Deacon , Ingo Molnar , Anton Ivanov , linux-arch , Herbert Xu , Brian Cain , Richard Weinberger , Russell King , Ard Biesheuvel , David Airlie , Ingo Molnar , Geert Uytterhoeven , Mel Gorman , intel-gfx On Tue, Sep 15, 2020 at 7:35 PM Linus Torvalds wrote: > > On Tue, Sep 15, 2020 at 1:39 AM Thomas Gleixner wrote: > > > > OTOH, having a working 'preemptible()' or maybe better named > > 'can_schedule()' check makes tons of sense to make decisions about > > allocation modes or other things. > > No. I think that those kinds of decisions about actual behavior are > always simply fundamentally wrong. > > Note that this is very different from having warnings about invalid > use. THAT is correct. It may not warn in all configurations, but that > doesn't matter: what matters is that it warns in common enough > configurations that developers will catch it. > > So having a warning in "might_sleep()" that doesn't always trigger, > because you have a limited configuration that can't even detect the > situation, that's fine and dandy and intentional. > > But having code like > > if (can_schedule()) > .. do something different .. > > is fundamentally complete and utter garbage. > > It's one thing if you test for "am I in hardware interrupt context". > Those tests aren't great either, but at least they make sense. > > But a driver - or some library routine - making a difference based on > some nebulous "can I schedule" is fundamentally and basically WRONG. > > If some code changes behavior, it needs to be explicit to the *caller* > of that code. > > So this is why GFP_ATOMIC is fine, but "if (!can_schedule()) > do_something_atomic()" is pure shite. > > And I am not IN THE LEAST interested in trying to help people doing > pure shite. We need to fix them. Like the crypto code is getting > fixed. Just figured I'll throw my +1 in from reading too many (gpu) drivers. Code that tries to cleverly adjust its behaviour depending upon the context it's running in is harder to understand and blows up in more interesting ways. We still have drm_can_sleep() and it's mostly just used for debug code, and I've largely ended up just deleting everything that used it because when you're driver is blowing up the last thing you want is to realize your debug code and output can't be relied upon. Or worse, that the only Oops you have is the one in the debug code, because the real one scrolled away - the original idea behind drm_can_sleep was to make all the modeset code work automagically both in normal ioctl/kworker context and in the panic handlers or kgdb callbacks. Wishful thinking at best. Also at least for me that extends to everything, e.g. I much prefer explicit spin_lock and spin_lock_irq vs magic spin_lock_irqsave for locks shared with interrupt handlers, since the former two gives me clear information from which contexts such function can be called. Other end is the memalloc_no*_save/restore functions, where I recently made a real big fool of myself because I didn't realize how much that impacts everything that's run within - suddenly "GFP_KERNEL for small stuff never fails" is wrong everywhere. It's all great for debugging and sanity checks (and we run with all that stuff enabled in our CI), but really semantic changes depending upon magic context checks freak my out :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch