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.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 32985C433ED for ; Tue, 4 May 2021 00:12:59 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 AC0B861186 for ; Tue, 4 May 2021 00:12:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AC0B861186 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nvidia.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-nvme-bounces+linux-nvme=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:CC:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=K0CAKCRhb7ehiEIwjVYHR5dzSTNUqMJ6/lUTW95MUxg=; b=A/esRQDAX01QvXB5F66beImlk vpjrVPi0U83eSNm4Pn7zmvrPHdq5mo//u0++1gbME4zNcC3eCm6V6r/6BgKfO5NdoOKjXZN+kfn5K 7+u3h63HjKZ/Oe0KM2TlBaOyeBO668pC/Wlff4CGlSdKTgoTave/XMv22Imulbsayu6EhRXqTwgOo tderCcov4Qhr8tXKTFWuNZnz3BJpzL7AC7HZFjmXxBcnamdvRLnCadDZbVma8zL90NmrEjrAXh3+F ymaEdA28OzWDwSTLv4z3sFhNcqm3dE99qIsAftmkvDiujuQ92PBYkiIp5vm8kPJ5QQbXkMChf/a5d 5Q9oHZLJw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1ldig8-00F6HS-K9; Tue, 04 May 2021 00:12:44 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1ldig5-00F6HE-TO for linux-nvme@desiato.infradead.org; Tue, 04 May 2021 00:12:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:From:References:CC:To: Subject:Sender:Reply-To:Content-ID:Content-Description; bh=j6wsklSkLPlyhjHfYFFzs3hqmGcJWrEzyR3+KtX004M=; b=R82qJeXASLWTiwGpNsZ0ch3R7U roxQ2goiDSrg5uVy4vziq5bL8C5G6aGBcozRBNmK4w65T+6xMyllG1TbZmGlSvEk4a9lh1vvh/xG0 SSkgHWAYmEqlJDUCaQYP2OqdW8JBr25WORp45gsCf6lkbEYTzDwU4B5i61VpJgzq3QE7SKe7mooAh vcmFle3DwB8f8iiE0KG4JaW8QQ0Zbe7p3vlcRUORYqs7I3cG6pjqhyp9yN5v97T2bIaOzE3nQE44I kxf+i8JHdFgI1j1NLt8wzCk9ruB9J3O081XXD9wyqyLURh5TGFUVU7TJYqliSFtB8NNCqLNzzQeUl jtAmxjuA==; Received: from mail-mw2nam10on2059.outbound.protection.outlook.com ([40.107.94.59] helo=NAM10-MW2-obe.outbound.protection.outlook.com) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1ldig2-003ZaH-RQ for linux-nvme@lists.infradead.org; Tue, 04 May 2021 00:12:40 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Dxg7qIGI4vgpG5JjF3CGJhhGL/BL/JnproycOe8Jc0C789lbTnGJBwYVc+/PEWynvBnTnWNuyMy9SZhPuZl75JVGSH1A4fuK0Wg0BrZ6V6TdCNjELemOEfg4GMIeqZZtA3mVCD6TXp/apubCAgphaDA5mqPS4vGtofp9XYo4sIf+BffrfuGdFEUu3+y+RMQ7I5tZBpZVtOjdejktQeyA8A6gtjJVYlej7xpStEfV1EjZf+fwAEWCfCaZXgXszPwiNSvTTVRIsyOehWkM1jDWbLarrd8nVLaiRaFw76V5h2VRr9VMNxQHzIvuikutk5twlXtZN5gkVnXgX+/T84Spkw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=j6wsklSkLPlyhjHfYFFzs3hqmGcJWrEzyR3+KtX004M=; b=AkNfb8GjlTDOaAZogwMWkawBccLPSyAOVoNrIT+MYuq0kv/eU9JpREZcXQrACsgxO9LpcRAM5ceecYSd2AF6Vpwt7ZZTrKreKOb9Kb1m11i1v4DukZEi2snUDEAlkNVzCEDVLorV8bTQN3Din768E7Drdcv0m5V7SVdmi6K9jmU8s6bhDmcpY7oYaammNX2JvJlOCZTsDS9AYAy1nX4qjRELQ4S70mYg0BD+YsTrlA065RTiuDYy35m84aJBJor7e9z4PApNL2aVZoVjvZgp/pfXNriD9Ppg8R20DMz8O3zGP0osspIgIqyjTLKkr97MSvkoK3uUS4Gvo4VWK8aqCA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 216.228.112.34) smtp.rcpttodomain=raithlin.com smtp.mailfrom=nvidia.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=nvidia.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=j6wsklSkLPlyhjHfYFFzs3hqmGcJWrEzyR3+KtX004M=; b=DJVaaIGcrDZOHF2Tkb7yWlb3/ve6nWjc29n8pF5QcBpiaQhlQPlVAs0LYlYWVi5Wur+4ftP4rvU8neL8rD/Ap3UkQGN5BFwdqvixPOGVUxPInfUrGQ32lqcnCSkRMnqh8RbIutKjyYH0OVqqXrUfRjm4mrFF50BDHCKyJGCMfc2hk8Z0FUMi8Q4eG3z0g5oe55iJdkS1YzIRonOxJ7EY6rLbGJc0vefqf6KddSl/qOTS0fYf6aXQ28BynGtJM7z4onKkRiiqyrQoq5C6nlUDRuCZYlU1oMkTkVEcW1tOpOaULt4wlc49HXfoiPCB/McrGWdQiMGpalquNvNzRFvneQ== Received: from DM6PR03CA0004.namprd03.prod.outlook.com (2603:10b6:5:40::17) by BN7PR12MB2738.namprd12.prod.outlook.com (2603:10b6:408:2d::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4087.40; Tue, 4 May 2021 00:12:33 +0000 Received: from DM6NAM11FT021.eop-nam11.prod.protection.outlook.com (2603:10b6:5:40:cafe::4e) by DM6PR03CA0004.outlook.office365.com (2603:10b6:5:40::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4087.25 via Frontend Transport; Tue, 4 May 2021 00:12:33 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 216.228.112.34) smtp.mailfrom=nvidia.com; raithlin.com; dkim=none (message not signed) header.d=none;raithlin.com; dmarc=pass action=none header.from=nvidia.com; Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates 216.228.112.34 as permitted sender) receiver=protection.outlook.com; client-ip=216.228.112.34; helo=mail.nvidia.com; Received: from mail.nvidia.com (216.228.112.34) by DM6NAM11FT021.mail.protection.outlook.com (10.13.173.76) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.20.4087.32 via Frontend Transport; Tue, 4 May 2021 00:12:33 +0000 Received: from [10.2.50.162] (172.20.145.6) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Tue, 4 May 2021 00:12:32 +0000 Subject: Re: [PATCH 09/16] dma-direct: Support PCI P2PDMA pages in dma-direct map_sg To: Logan Gunthorpe , , , , , , CC: Stephen Bates , Christoph Hellwig , Dan Williams , Jason Gunthorpe , =?UTF-8?Q?Christian_K=c3=b6nig?= , Don Dutile , Matthew Wilcox , Daniel Vetter , Jakowski Andrzej , Minturn Dave B , Jason Ekstrand , Dave Hansen , Xiong Jianxin , Bjorn Helgaas , Ira Weiny , Robin Murphy References: <20210408170123.8788-1-logang@deltatee.com> <20210408170123.8788-10-logang@deltatee.com> <37fa46c7-2c24-1808-16e9-e543f4601279@nvidia.com> From: John Hubbard Message-ID: <7a44c5b2-2ca8-514f-0952-711166acb502@nvidia.com> Date: Mon, 3 May 2021 17:12:32 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Originating-IP: [172.20.145.6] X-ClientProxiedBy: HQMAIL101.nvidia.com (172.20.187.10) To HQMAIL107.nvidia.com (172.20.187.13) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: af1c0519-6a29-4789-42be-08d90e9150b1 X-MS-TrafficTypeDiagnostic: BN7PR12MB2738: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: rsIKbooXENDu8IPLgv751tEvwS/p94XEeWTCN+AqxhlJ6LcSEW7LRgNCR1WStjPYosCu8quPqGrzi/guVh2+51oTtILAnZuSYvs5Mqrp1nvYYPMpLRBM4QWLosqhhQf78xvSbPp2gGDN+jeR5DJi8zYKj0v5pGgvmw/xq6699HuVA37EIHFK3OqIBoxEeHnww2pyEd0ho8zC/Yf1hhDGUdWAGKvN0OhiapdUKlAvsYvmO4ZsdvajJl8rrQ8ICBcuT/GfzmUzPdd72lqsJrmNOKxML1fVWRMbpRg3yH2degKydLxpd2KCALnzRssd640ZOtEh3x/3luENythODzcHOqOqXtZWG1ZO2WToNNwi1h/hFreJYB4R7hqAL/DwQSiYfBdWaNJHfBH7defryPjR6AlNisHXxk9Fy9+0tw5i7h/YUirgps5IBFg0HwBUrVCb08JDzMRRxpNWw95ZMW9diitSd+UW6tKMGNwG3R3Tq1EifLIhNTxXb7TuISbJ4jlgeJID+j7SuV0GY73sStz9Zl5H9Co9dy9YHvOUUOG7FKl9Txql9J6ksY9Ed5lmyUvqQxiQXCic5qWFD8K5HLvs6GR/MJCccP4voipcFGZw6pmG9SAhzoaJmD3VQ1VWdBxjfNV5mpiba7ML4qStnz/f4c/fotVyISfAJDg7+i4SWCkYd5YOI55V/2G9r8elaiXmhtVi1AT/DxHYn/H93+aCMw== X-Forefront-Antispam-Report: CIP:216.228.112.34; CTRY:US; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:mail.nvidia.com; PTR:schybrid03.nvidia.com; CAT:NONE; SFS:(4636009)(39860400002)(346002)(376002)(136003)(396003)(46966006)(36840700001)(426003)(54906003)(36906005)(82740400003)(8676002)(110136005)(4326008)(7636003)(70586007)(316002)(70206006)(2906002)(478600001)(82310400003)(5660300002)(86362001)(36860700001)(16526019)(31696002)(16576012)(83380400001)(53546011)(31686004)(26005)(47076005)(356005)(7416002)(2616005)(336012)(8936002)(36756003)(186003)(2101003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2021 00:12:33.5307 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: af1c0519-6a29-4789-42be-08d90e9150b1 X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a; Ip=[216.228.112.34]; Helo=[mail.nvidia.com] X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT021.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR12MB2738 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210503_171238_903874_08661EF4 X-CRM114-Status: GOOD ( 28.78 ) X-BeenThere: linux-nvme@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "Linux-nvme" Errors-To: linux-nvme-bounces+linux-nvme=archiver.kernel.org@lists.infradead.org On 5/3/21 9:55 AM, Logan Gunthorpe wrote: ... >> The same thing can be achieved with fewer lines and a bit more clarity. >> Can we please do it like this instead: >> >> for_each_sg(sgl, sg, nents, i) { >> if (sg_is_pci_p2pdma(sg)) >> sg_unmark_pci_p2pdma(sg); >> else >> dma_direct_unmap_page(dev, sg->dma_address, >> sg_dma_len(sg), dir, attrs); >> } >> >> > > That's debatable (the way I did it emphasizes the common case). But I'll > consider changing it. > Thanks for considering it. >> >> Also here, a block comment for the function would be nice. How about >> approximately this: >> >> /* >> * Maps each SG segment. Returns the number of entries mapped, or 0 upon >> * failure. If any entry could not be mapped, then no entries are mapped. >> */ >> >> I'll stop complaining about the pre-existing return code conventions, >> since by now you know what I was thinking of saying. :) > > Not really part of this patchset... Seems like if you think there should > be a comment like that here, you should send a patch. But this patch > starts returning a negative value here. OK, that's fine. Like you say, that comment is rather beyond this patchset. >>> for_each_sg(sgl, sg, nents, i) { >>> + if (is_pci_p2pdma_page(sg_page(sg))) { >>> + ret = pci_p2pdma_map_segment(&p2pdma_state, dev, sg, >>> + attrs); >>> + if (ret < 0) { >>> + goto out_unmap; >>> + } else if (ret) { >>> + ret = 0; >>> + continue; >> >> Is this a bug? If neither of those "if" branches fires (ret == 0), then >> the code (probably unintentionally) falls through and continues on to >> attempt to call dma_direct_map_page()--despite being a PCI_P2PDMA page! > > No, it's not a bug. Per the documentation of pci_p2pdma_map_segment(), > if it returns zero the segment should be mapped normally. P2PDMA pages > must be mapped with physical addresses (or IOVA addresses) if the TLPS > for the transaction will go through the host bridge. Could we maybe put a little comment there, to that effect? It would be nice to call out that point, especially since it is common to miss one case (negative, 0, positive) when handling return values. Sort of like we used to put "// fallthrough" in the case statements. Not a big deal of course. > >> See below for suggestions: >> >>> + } >>> + } >>> + >>> sg->dma_address = dma_direct_map_page(dev, sg_page(sg), >>> sg->offset, sg->length, dir, attrs); >>> if (sg->dma_address == DMA_MAPPING_ERROR) >> >> This is another case in which "continue" is misleading and not as good >> as "else". Because unless I'm wrong above, you really only want to take >> one path *or* the other. > > No, per above, it's not one path or the other. If it's a P2PDMA page it > may still need to be mapped normally. > Right. That follows. thanks, -- John Hubbard NVIDIA _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme