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.9 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 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 08978C43465 for ; Sun, 20 Sep 2020 06:41:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C875B2078D for ; Sun, 20 Sep 2020 06:41:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726290AbgITGlG (ORCPT ); Sun, 20 Sep 2020 02:41:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59996 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726192AbgITGlG (ORCPT ); Sun, 20 Sep 2020 02:41:06 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93D03C061755; Sat, 19 Sep 2020 23:41:05 -0700 (PDT) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds Cc: LKML , linux-arch , Paul McKenney , the arch/x86 maintainers , Sebastian Andrzej Siewior , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Will Deacon , Andrew Morton , Linux-MM , Russell King , Linux ARM , Chris Zankel , Max Filippov , linux-xtensa@linux-xtensa.org, Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , David Airlie , Daniel Vetter , intel-gfx , dri-devel , Ard Biesheuvel , Herbert Xu , Vineet Gupta , "open list\:SYNOPSYS ARC ARCHITECTURE" , Arnd Bergmann , Guo Ren , linux-csky@vger.kernel.org, Michal Simek , Thomas Bogendoerfer , linux-mips@vger.kernel.org, Nick Hu , Greentime Hu , Vincent Chen , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , linuxppc-dev , "David S. Miller" , linux-sparc Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Gleixner Date: Sun, 20 Sep 2020 06:41:02 +0000 Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends Message-Id: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> List-Id: References: <20200919091751.011116649@linutronix.de> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Linus Torvalds Cc: Juri Lelli , Peter Zijlstra , Sebastian Andrzej Siewior , dri-devel , linux-mips@vger.kernel.org, Ben Segall , Max Filippov , Guo Ren , linux-sparc , Vincent Chen , Will Deacon , Ard Biesheuvel , linux-arch , Vincent Guittot , Herbert Xu , Michael Ellerman , the arch/x86 maintainers , Russell King , linux-csky@vger.kernel.org, David Airlie , Mel Gorman , "open list:SYNOPSYS ARC ARCHITECTURE" , linux-xtensa@linux-xtensa.org, Paul McKenney , intel-gfx , linuxppc-dev , Steven Rostedt , Rodrigo Vivi , Dietmar Eggemann , Linux ARM , Chris Zankel , Michal Simek , Thomas Bogendoerfer , Nick Hu , Linux-MM , Vineet Gupta , LKML , Arnd Bergmann , Paul Mackerras , Andrew Morton , Daniel Bristot de Oliveira , "David S. Miller" , Greentime Hu On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx 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.9 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 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 A80EEC4346A for ; Sun, 20 Sep 2020 06:41:08 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2704F207C3 for ; Sun, 20 Sep 2020 06:41:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2704F207C3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D1A456B005A; Sun, 20 Sep 2020 02:41:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CA6A66B005C; Sun, 20 Sep 2020 02:41:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B6ACE6B005D; Sun, 20 Sep 2020 02:41:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0229.hostedemail.com [216.40.44.229]) by kanga.kvack.org (Postfix) with ESMTP id 94D386B005A for ; Sun, 20 Sep 2020 02:41:06 -0400 (EDT) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 51CB81EE6 for ; Sun, 20 Sep 2020 06:41:06 +0000 (UTC) X-FDA: 77282492532.08.van48_1410bd12713a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin08.hostedemail.com (Postfix) with ESMTP id 338A91819E621 for ; Sun, 20 Sep 2020 06:41:06 +0000 (UTC) X-HE-Tag: van48_1410bd12713a X-Filterd-Recvd-Size: 7179 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf37.hostedemail.com (Postfix) with ESMTP for ; Sun, 20 Sep 2020 06:41:05 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds Cc: LKML , linux-arch , Paul McKenney , the arch/x86 maintainers , Sebastian Andrzej Siewior , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Will Deacon , Andrew Morton , Linux-MM , Russell King , Linux ARM , Chris Zankel , Max Filippov , linux-xtensa@linux-xtensa.org, Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , David Airlie , Daniel Vetter , intel-gfx , dri-devel , Ard Biesheuvel , Herbert Xu , Vineet Gupta , "open list\:SYNOPSYS ARC ARCHITECTURE" , Arnd Bergmann , Guo Ren , linux-csky@vger.kernel.org, Michal Simek , Thomas Bogendoerfer , linux-mips@vger.kernel.org, Nick Hu , Greentime Hu , Vincent Chen , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , linuxppc-dev , "David S. Miller" , linux-sparc Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain 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 Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx 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 D0994C43465 for ; Sun, 20 Sep 2020 06:42:49 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (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 0931D2085B for ; Sun, 20 Sep 2020 06:42:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0931D2085B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4BvJ0k5P3nzDqyH for ; Sun, 20 Sep 2020 16:42:46 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=linutronix.de (client-ip=193.142.43.55; helo=galois.linutronix.de; envelope-from=tglx@linutronix.de; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=linutronix.de header.i=@linutronix.de header.a=rsa-sha256 header.s=2020 header.b=ZbudrrXW; dkim=pass header.d=linutronix.de header.i=@linutronix.de header.a=ed25519-sha256 header.s=2020e header.b=hJcFT5y1; dkim-atps=neutral X-Greylist: delayed 75060 seconds by postgrey-1.36 at bilbo; Sun, 20 Sep 2020 16:41:09 AEST Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4BvHys1NHvzDqPb for ; Sun, 20 Sep 2020 16:41:08 +1000 (AEST) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Peter Zijlstra , Sebastian Andrzej Siewior , Joonas Lahtinen , dri-devel , linux-mips@vger.kernel.org, Ben Segall , Max Filippov , Guo Ren , linux-sparc , Vincent Chen , Will Deacon , Ard Biesheuvel , linux-arch , Vincent Guittot , Herbert Xu , the arch/x86 maintainers , Russell King , linux-csky@vger.kernel.org, David Airlie , Mel Gorman , "open list:SYNOPSYS ARC ARCHITECTURE" , linux-xtensa@linux-xtensa.org, Paul McKenney , intel-gfx , linuxppc-dev , Steven Rostedt , Jani Nikula , Rodrigo Vivi , Dietmar Eggemann , Linux ARM , Chris Zankel , Michal Simek , Thomas Bogendoerfer , Nick Hu , Linux-MM , Vineet Gupta , LKML , Arnd Bergmann , Daniel Vetter , Paul Mackerras , Andrew Morton , Daniel Bristot de Oliveira , "David S. Miller" , Greentime Hu Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx 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=-5.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,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 DC6EEC43468 for ; Sun, 20 Sep 2020 06:41:13 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 747542078D for ; Sun, 20 Sep 2020 06:41:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="LOxSRwGN"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 747542078D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-snps-arc-bounces+linux-snps-arc=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=auG75ejgdGRTB07SOQAKjSL4ixFIYGNe7DZWZs3alCs=; b=LOxSRwGNkRo2ATAQbxHyqCtU5 lFGOUvQxBwKxprwoQktPf59SQucSc+BStiOHofw56ZVq30+4P6tIE0+ttbh4R78bsMeoNuTYDY6Ac DL44JV1Rcqs2VnC/gS9rgeRuN+S6U2nS2nHL/JhxpXp8lXshIetguxWSsWyExVvDXVJSPcwwjZDeC qXcGgAQtbEP2b8u495obC+yjddTR1yXE3B4g/i2N6QzrEM9Jib+I8iLlz3H5hOMYrcfySike6HfTf aZlvtC/Fptu657prczcJroenNMVUCOkBH5cFg4LQmhRFuOtT3C512Q8o9g/k4yiwcba+IE+BU2fj9 2pt3eMQ4Q==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJt28-000782-Jl; Sun, 20 Sep 2020 06:41:12 +0000 Received: from galois.linutronix.de ([193.142.43.55]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJt21-00075i-ON; Sun, 20 Sep 2020 06:41:07 +0000 From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200920_024106_007957_12B72304 X-CRM114-Status: GOOD ( 30.85 ) X-BeenThere: linux-snps-arc@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on Synopsys ARC Processors List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Peter Zijlstra , Benjamin Herrenschmidt , Sebastian Andrzej Siewior , Joonas Lahtinen , dri-devel , linux-mips@vger.kernel.org, Ben Segall , Max Filippov , Guo Ren , linux-sparc , Vincent Chen , Will Deacon , Ard Biesheuvel , linux-arch , Vincent Guittot , Herbert Xu , Michael Ellerman , the arch/x86 maintainers , Russell King , linux-csky@vger.kernel.org, David Airlie , Mel Gorman , "open list:SYNOPSYS ARC ARCHITECTURE" , linux-xtensa@linux-xtensa.org, Paul McKenney , intel-gfx , linuxppc-dev , Steven Rostedt , Jani Nikula , Rodrigo Vivi , Dietmar Eggemann , Linux ARM , Chris Zankel , Michal Simek , Thomas Bogendoerfer , Nick Hu , Linux-MM , Vineet Gupta , LKML , Arnd Bergmann , Daniel Vetter , Paul Mackerras , Andrew Morton , Daniel Bristot de Oliveira , "David S. Miller" , Greentime Hu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-snps-arc" Errors-To: linux-snps-arc-bounces+linux-snps-arc=archiver.kernel.org@lists.infradead.org On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx _______________________________________________ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc 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=-5.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,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 4F6B1C43466 for ; Sun, 20 Sep 2020 06:42:58 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 F3BF52085B for ; Sun, 20 Sep 2020 06:42:57 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="PEbJvj0q"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F3BF52085B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=uJR87k550ZDsXA67h3JkGHMlSVvvx9WalxZUq0EZecA=; b=PEbJvj0qhOF6byi+VX9i4o/qH D96YoMbbS0dJ+65cXkeoyT37ZPDtEc1kzJa9zuqZw7OjR9Tsy2Tk/MuF9mfO245JuFsFFj6/XHF3H 2ous/rKVPuiqDR52WJkBuwkRIQMC0IPkx0rBJGD0J6EapH2hyjswLM6uUl/8nsl0SdUhTojZrUDwg nBhDEiB5nCjrmPri7h24z4+favFswV+k4LFjramXQblMN1Vf9XHB5ZoZKNYc4igtbvblBZ0b9i+7w dcNEcMaPLMvjaojWFWzh19KgTRW/am9pM+cWGQyb9a0BBA/yN/c2Voz38/AveBGvCzWo/emAgHDMk lk//mq9+g==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJt26-00077J-QL; Sun, 20 Sep 2020 06:41:10 +0000 Received: from galois.linutronix.de ([193.142.43.55]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kJt21-00075i-ON; Sun, 20 Sep 2020 06:41:07 +0000 From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200920_024106_007957_12B72304 X-CRM114-Status: GOOD ( 30.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Peter Zijlstra , Benjamin Herrenschmidt , Sebastian Andrzej Siewior , Joonas Lahtinen , dri-devel , linux-mips@vger.kernel.org, Ben Segall , Max Filippov , Guo Ren , linux-sparc , Vincent Chen , Will Deacon , Ard Biesheuvel , linux-arch , Vincent Guittot , Herbert Xu , Michael Ellerman , the arch/x86 maintainers , Russell King , linux-csky@vger.kernel.org, David Airlie , Mel Gorman , "open list:SYNOPSYS ARC ARCHITECTURE" , linux-xtensa@linux-xtensa.org, Paul McKenney , intel-gfx , linuxppc-dev , Steven Rostedt , Jani Nikula , Rodrigo Vivi , Dietmar Eggemann , Linux ARM , Chris Zankel , Michal Simek , Thomas Bogendoerfer , Nick Hu , Linux-MM , Vineet Gupta , LKML , Arnd Bergmann , Daniel Vetter , Paul Mackerras , Andrew Morton , Daniel Bristot de Oliveira , "David S. Miller" , Greentime Hu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 A9713C43465 for ; Mon, 21 Sep 2020 07:36:30 +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 563BF21D91 for ; Mon, 21 Sep 2020 07:36:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 563BF21D91 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de 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 A94F06E225; Mon, 21 Sep 2020 07:36:27 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5C018892E0; Sun, 20 Sep 2020 06:41:05 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds Subject: Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 X-Mailman-Approved-At: Mon, 21 Sep 2020 07:34:45 +0000 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 , dri-devel , linux-mips@vger.kernel.org, Ben Segall , Max Filippov , Guo Ren , linux-sparc , Vincent Chen , Will Deacon , Ard Biesheuvel , linux-arch , Vincent Guittot , Herbert Xu , Michael Ellerman , the arch/x86 maintainers , Russell King , linux-csky@vger.kernel.org, David Airlie , Mel Gorman , "open list:SYNOPSYS ARC ARCHITECTURE" , linux-xtensa@linux-xtensa.org, Paul McKenney , intel-gfx , linuxppc-dev , Steven Rostedt , Rodrigo Vivi , Dietmar Eggemann , Linux ARM , Chris Zankel , Michal Simek , Thomas Bogendoerfer , Nick Hu , Linux-MM , Vineet Gupta , LKML , Arnd Bergmann , Paul Mackerras , Andrew Morton , Daniel Bristot de Oliveira , "David S. Miller" , Greentime Hu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx _______________________________________________ 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.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 8D3A1C43469 for ; Sun, 20 Sep 2020 06:41:08 +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 3903D2085B for ; Sun, 20 Sep 2020 06:41:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="ZbudrrXW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="hJcFT5y1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3903D2085B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de 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 8F145892E0; Sun, 20 Sep 2020 06:41:07 +0000 (UTC) Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5C018892E0; Sun, 20 Sep 2020 06:41:05 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=ZbudrrXW3BmBlMSesKPCFcaOuHlyqPwGJ2rDhOMbxt3mPAnInwIbTwlFWdXYV8k68cqtaX KfEuU6M33k6TNgEv2z3BoREDlGO2O9Fw5TgP2+5hcpWK7uStRqjbWtSKwO3gS4QCuyivwy 26cuDkJvS5X9yMAUHVa9s0UJUt+J6543xHxV5P3DaWXrCCPeOCRdXjTt3mHzSFT/GSvW6o JaVOcgOKbttRfnVUr6cHm/Y19mE/tWAhhxXhM/hU7XhieUh+8YV+lRCkQmefuoxYeKpcBS Vf7k79LGubJaArfFULHNmfuj7TyKvSea5/MDMs+6uMgB8Li/jQ7m/qvIhWo6HA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1600584063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CoLGj4bgeeSGrBtXaSAQtCzCSINs+vZ2LYYnDPrnZo8=; b=hJcFT5y1jFrtCoruYoZORnqggnYSJ5Wm/G+vHz1SccOLs4xq3SNs6bWtqMz5RIfu775hAi dKKPo41u4g/pn4AQ== To: Linus Torvalds In-Reply-To: References: <20200919091751.011116649@linutronix.de> Date: Sun, 20 Sep 2020 08:41:02 +0200 Message-ID: <87mu1lc5mp.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 Subject: Re: [Intel-gfx] [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends 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 , Benjamin Herrenschmidt , Sebastian Andrzej Siewior , dri-devel , linux-mips@vger.kernel.org, Ben Segall , Max Filippov , Guo Ren , linux-sparc , Vincent Chen , Will Deacon , Ard Biesheuvel , linux-arch , Herbert Xu , Michael Ellerman , the arch/x86 maintainers , Russell King , linux-csky@vger.kernel.org, David Airlie , Mel Gorman , "open list:SYNOPSYS ARC ARCHITECTURE" , linux-xtensa@linux-xtensa.org, Paul McKenney , intel-gfx , linuxppc-dev , Steven Rostedt , Dietmar Eggemann , Linux ARM , Chris Zankel , Michal Simek , Thomas Bogendoerfer , Nick Hu , Linux-MM , Vineet Gupta , LKML , Arnd Bergmann , Paul Mackerras , Andrew Morton , Daniel Bristot de Oliveira , "David S. Miller" , Greentime Hu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Sat, Sep 19 2020 at 10:18, Linus Torvalds wrote: > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner wrote: >> >> this provides a preemptible variant of kmap_atomic & related >> interfaces. This is achieved by: > > Ack. This looks really nice, even apart from the new capability. > > The only thing I really reacted to is that the name doesn't make sense > to me: "kmap_temporary()" seems a bit odd. Yeah. Couldn't come up with something useful. > Particularly for an interface that really is basically meant as a > better replacement of "kmap_atomic()" (but is perhaps also a better > replacement for "kmap()"). > > I think I understand how the name came about: I think the "temporary" > is there as a distinction from the "longterm" regular kmap(). So I > think it makes some sense from an internal implementation angle, but I > don't think it makes a lot of sense from an interface name. > > I don't know what might be a better name, but if we want to emphasize > that it's thread-private and a one-off, maybe "local" would be a > better naming, and make it distinct from the "global" nature of the > old kmap() interface? Right, _local or _thread would be more intuitive. > However, another solution might be to just use this new preemptible > "local" kmap(), and remove the old global one entirely. Yes, the old > global one caches the page table mapping and that sounds really > efficient and nice. But it's actually horribly horribly bad, because > it means that we need to use locking for them. Your new "temporary" > implementation seems to be fundamentally better locking-wise, and only > need preemption disabling as locking (and is equally fast for the > non-highmem case). > > So I wonder if the single-page TLB flush isn't a better model, and > whether it wouldn't be a lot simpler to just get rid of the old > complex kmap() entirely, and replace it with this? > > I agree we can't replace the kmap_atomic() version, because maybe > people depend on the preemption disabling it also implied. But what > about replacing the non-atomic kmap()? > > Maybe I've missed something. Is it because the new interface still > does "pagefault_disable()" perhaps? > > But does it even need the pagefault_disable() at all? Yes, the > *atomic* one obviously needed it. But why does this new one need to > disable page faults? It disables pagefaults because it can be called from atomic and non-atomic context. That was the point to get rid of if (preeemptible()) kmap(); else kmap_atomic(); If it does not disable pagefaults, then it's just a lightweight variant of kmap() for short lived mappings. > But apart from that question about naming (and perhaps replacing > kmap() entirely), I very much like it. I thought about it, but then I figured that kmap pointers can be handed to other contexts from the thread which sets up the mapping because it's 'permanent'. I'm not sure whether that actually happens, so we'd need to audit all kmap() users to be sure. If there is no such use case, then we surely can get of rid of kmap() completely. It's only 300+ instances to stare at and quite some of them are wrapped into other functions. Highmem sucks no matter what and the only sane solution is to remove it completely. Thanks, tglx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx