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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1270DC32772 for ; Tue, 23 Aug 2022 12:32:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8AD5A8D0001; Tue, 23 Aug 2022 08:32:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 836096B0074; Tue, 23 Aug 2022 08:32:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6D64B8D0001; Tue, 23 Aug 2022 08:32:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 57C416B0073 for ; Tue, 23 Aug 2022 08:32:52 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2C9FD120C14 for ; Tue, 23 Aug 2022 12:32:52 +0000 (UTC) X-FDA: 79830796584.25.95372CC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf14.hostedemail.com (Postfix) with ESMTP id 6915A10000F for ; Tue, 23 Aug 2022 12:32:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1661257970; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JFZpUrH9edUOuu2AArKyngbcz1gI3F8tDFjZRMkRQFs=; b=Ui+eZfqHhvNr5Nv3x+b5NMJVXJCILL/clpbEKfGwsdFqA6TiPc5zWbyvMK/4DJsR177nyz Rc1KAEiwygrI8mUk6lRCtZiq06lFa5FUlS/pn1m1IRlzq1uqp0YYKQxUuY6pmyByXebRnR qy+kSNpYyDMQoByBtrKwN4INyS3GuOo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-470-cQTcaUiDMyaszofSEUWFVQ-1; Tue, 23 Aug 2022 08:32:47 -0400 X-MC-Unique: cQTcaUiDMyaszofSEUWFVQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4C03918E5341; Tue, 23 Aug 2022 12:32:47 +0000 (UTC) Received: from localhost (ovpn-12-31.pek2.redhat.com [10.72.12.31]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F022940C141D; Tue, 23 Aug 2022 12:32:45 +0000 (UTC) Date: Tue, 23 Aug 2022 20:32:42 +0800 From: Baoquan He To: Christophe Leroy Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "akpm@linux-foundation.org" , "hch@infradead.org" , "agordeev@linux.ibm.com" , "wangkefeng.wang@huawei.com" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH v2 02/11] mm: ioremap: fixup the physical address and page prot Message-ID: References: <20220820003125.353570-1-bhe@redhat.com> <20220820003125.353570-3-bhe@redhat.com> <54b7afcc-056d-7f33-6858-d451a3222c70@csgroup.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <54b7afcc-056d-7f33-6858-d451a3222c70@csgroup.eu> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661257971; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=JFZpUrH9edUOuu2AArKyngbcz1gI3F8tDFjZRMkRQFs=; b=3F5hPARMD2hqVdsg1R8uGaDIeQ3gzYG0SurnCgEl8jpqUefLtJJ9IQmk0rnPf6cepVvHv2 sWL/dgPLCkFrYLE5ckWWKdMIqMgxUWj06ZSUvsO0TLBfx6/J6OvKQqlsDxs25ioOoQSvfY X5+GEKZy4NKImWGSksT5MR6fKKqRMpc= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Ui+eZfqH; spf=pass (imf14.hostedemail.com: domain of bhe@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bhe@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661257971; a=rsa-sha256; cv=none; b=3A4wAFR5gEiQjF+eWij4pkwx6thSc1KWklJbp0iSoVFPGfW/coM4M0CIYalQnHZOlUtW/I cpyrDcBBoPI2UXUQ2Cm/KSN38GXqQH5Ely7oMTSlcxQPfVl1rrY1slkQkLf9xEH3ZFgLol T7BDl7iX5zHnnep9UFOnx16Azc+NXpI= X-Stat-Signature: pf1drq6nh3q9dy3p6octet9jajbhq15j X-Rspamd-Queue-Id: 6915A10000F X-Rspam-User: Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Ui+eZfqH; spf=pass (imf14.hostedemail.com: domain of bhe@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bhe@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspamd-Server: rspam01 X-HE-Tag: 1661257971-497279 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 08/23/22 at 05:33am, Christophe Leroy wrote: > > > Le 23/08/2022 à 03:19, Baoquan He a écrit : > > On 08/22/22 at 06:30am, Christophe Leroy wrote: > >> > >> > >> Le 20/08/2022 à 02:31, Baoquan He a écrit : > >>> On some architectures, the physical address need be fixed up before > >>> doing mapping, e.g, parisc. And on architectures, e.g arc, the > >>> parameter 'prot' passed into ioremap_prot() need be adjusted too. > >>> > >>> In oder to convert them to take GENERIC_IOREMAP method, we need wrap > >>> the address fixing up code and page prot adjusting code into arch_ioremap() > >>> and pass the new address and 'prot' out for ioremap_prot() handling. > >> > >> Is it really the best approach ? Wouldn't it be better to have helpers > >> to do that, those helpers being called by the ioremap_prot(), instead of > >> doing it inside the arch_ioremap() function ? > > > > This is suggested too by Alexander during his v1 reviewing. I tried, but > > feel the current way taken in this patchset is better. Because not all > > architecutres need the address fix up, only parisc, and only few need > > adjust the 'prot'. Introducing other helpers seems too much, that only > > increases the complexity of of ioremap() and the generic GENERIC_IOREMAP > > method for people to understand and take. > > I can't understand. Why is it difficult to do something like: > > #ifndef ioremap_adjust_prot > static inline unsigned long ioremap_adjust_prot(unsigned long flags) > { > return flags; > } > #endif > > Then for arc you do > > static inline unsigned long ioremap_adjust_prot(unsigned long flags) > { > return pgprot_val(pgprot_noncached(__pgprot(flags))); > } > #define ioremap_adjust_prot ioremap_adjust_prot My thinking is we have four things to do in the added hookers. 1) check if we should do ioremap on ARCHes. If not, return NULL from ioremap_prot(); 2) handling the mapping io address specifically on ARCHes, e.g arc, ia64, s390; 3) the original physical address passed into ioremap_prot() need be fixed up, e.g arc; 4) the 'prot' passed into ioremap_prot() need be adjusted, e.g on arc and xtensa. With Kefeng's patches, the case 1) is handled with introduced ioremap_allowed()/iounmap_allowed(). In this patchset, what I do is rename the hooks as arch_ioremap() and arch_iounmap(), then put case 1), 2), 3), 4) handling into arch_ioremap(). Adding helpers to cover each case is not difficult from my side. I worry that as time goes by, those several hooks my cause issue, e.g if a new adjustment need be done, should we introduce a new helper or make do with the existed hook; how When I investigated this, one arch_ioremap() looks not complicated since not all ARCHes need cover all above 4 cases. That's why I finally choose one hook. I am open to new idea, please let me know if we should change it to introduce several different helpers. > > > By the way, could be a good opportunity to change ioremap_prot() flags > type from unsigned long to pgprot_t Tend to agree, I will give it a shot.