From patchwork Wed Dec 2 09:31:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sagi Grimberg X-Patchwork-Id: 7743721 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 781F7BEEE1 for ; Wed, 2 Dec 2015 09:31:49 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 95BF4205E1 for ; Wed, 2 Dec 2015 09:31:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 54CFF205DF for ; Wed, 2 Dec 2015 09:31:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757027AbbLBJbp (ORCPT ); Wed, 2 Dec 2015 04:31:45 -0500 Received: from mail-wm0-f47.google.com ([74.125.82.47]:33920 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757000AbbLBJbn (ORCPT ); Wed, 2 Dec 2015 04:31:43 -0500 Received: by wmvv187 with SMTP id v187so245423348wmv.1 for ; Wed, 02 Dec 2015 01:31:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dev-mellanox-co-il.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-type:content-transfer-encoding; bh=6aCuBNGDToIq5yhvYfa/h1QVPVuQNR1D88N1xkicX28=; b=pDyn8bTs1pc8MipxAfXF7CPG9KaRzUGYtgn9V8/xyDJ0rFzvLxPU7w+w4lvWvRUs1u NWKjhJQ9JifbVH8u3Yp48bLxqd+7AJyzKcsL6M0opw9VEi0aLIhBPfamBbdVcYIQrnVf Fb6HUG6gilC0DPH9cCxuYXKF2O4wKFjKH9PtRm3JuKsxWFwkCXugjS7hqARNPOMarP4o 71kU5FlbLCOA4QXeTxVZXcydmaww6VLv9u1Jn1JtWKWebLdaBMy2kcosBUjqFyqtQvkG vPHRgInf9Q3WqEOMuZ5myy2eWr8xDYWn5W2Lh9aZsVtnpINu7v4zmkKvoos4H2PXNsgv M8gQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=6aCuBNGDToIq5yhvYfa/h1QVPVuQNR1D88N1xkicX28=; b=PfspUiKvCBP7rF3+ErwNSK3r2K1Ok4gSioqr7cU4I8+J7E3eFsx5z9iAfSpBB9bD5R jfHoNk+pRRoOVUj9Ch+pcdEtkndUO5nIklEpv9eueAiHUZ3rKtCOmNNpu7HEH74RkmRi 29CeLKuujs0XIGdxareIb9U5iaPIkCUC1UIIiDPompmdC/LmEY6RWAXSRwQIE8O2wUhm HwhJNe3DKkAgiuHlN+EqM9B2nOBP7OSiREUkChmTBRWB2LU5tXdi4URNuR8tA/3MtfId J7niFXYHg930erSOLlo7G2dKbN7yCDDqXD5ryRi2KguxESxWJd5hBNvm3LgX4TsnEAH1 ExAA== X-Gm-Message-State: ALoCoQkVVNi07Vojsy2hUvI2Cr4vU7lPZRNH26rv36+8CzYQ2kl+pL1XJPH0ZRZHSk3NGBXQOnOF X-Received: by 10.28.218.17 with SMTP id r17mr4377438wmg.90.1449048701785; Wed, 02 Dec 2015 01:31:41 -0800 (PST) Received: from [10.223.3.75] ([193.47.165.251]) by smtp.gmail.com with ESMTPSA id z10sm30081997wmg.4.2015.12.02.01.31.39 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 02 Dec 2015 01:31:40 -0800 (PST) Subject: Re: [PATCH 5/6] IB core: Fix ib_sg_to_pages() To: Bart Van Assche , Doug Ledford References: <565DE3EC.2070002@sandisk.com> <565DE49D.4020102@sandisk.com> <565DE7D0.4080408@dev.mellanox.co.il> <565DF0A5.6040102@sandisk.com> Cc: Christoph Hellwig , Sebastian Parschauer , "linux-rdma@vger.kernel.org" From: Sagi Grimberg Message-ID: <565EBA78.3050201@dev.mellanox.co.il> Date: Wed, 2 Dec 2015 11:31:36 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <565DF0A5.6040102@sandisk.com> Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 01/12/2015 21:10, Bart Van Assche wrote: > On 12/01/2015 10:32 AM, Sagi Grimberg wrote: >>> Fix the code for detecting gaps and disable the code for chunking. >>> A gap occurs not only if the second or later scatterlist element >>> is not aligned but also if any scatterlist element other than the >>> last does not end at a page boundary. Disable the code for chunking. >> >> So can you explain what went wrong with the code? If ib_sg_to_pages() >> supports chunking, then sg elements are allowed not to end at a page >> boundary if it is physically contig to the next sg and then the next >> is chunked. Care to explain how did ib_sg_to_pages mess up? > > With the upstream implementation, if the previous element ends at a page > boundary (last_end_dma_addr == dma_addr) but the current element does > not start at a page boundary (page_addr != dma_addr) then the current > element is mapped. I think this is wrong because this condition > indicates a gap and hence that ib_sg_to_pages() should stop iterating in > this case. Why is (last_end_dma_addr == dma_addr) implying that the previous element ends at a page boundary? it tests if the current is contig to the prev (dma_addr is not necessarily the page address). But I think I see whats wrong. If the prev sg didn't end aligned to page we won't get into the above test at all and continue mapping. Does this make the problem go away? --- Note that I don't mind making the chunking code go away if no one wants it (I think Steve specifically asked for it). I'm just trying to understand the exact mistake here. > How ib_sg_to_pages() reports to its caller that mapping the first > scatterlist element failed is not important to me. I included that > change in this patch because I noticed that in the upstream SRP > initiator does not consider ib_map_mr_sg() returning zero as an error. I > think either ib_sg_to_pages() or ib_map_mr_sg() should be modified such > that "no pages have been mapped" is handled as a mapping failure. I don't either, but the patch introduces inconsistency in a case where the caller passed sg_nents=0 (which will return 0 and not -errno). Would it make better sense to have srp treat 0 return as error? note that all other ULPs treat return_val != sg_nents as error. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index 043a60e..23457fe 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -1544,7 +1544,7 @@ int ib_sg_to_pages(struct ib_mr *mr, u64 end_dma_addr = dma_addr + dma_len; u64 page_addr = dma_addr & page_mask; - if (i && page_addr != dma_addr) { + if (i && (page_addr != dma_addr || last_page_off)) { if (last_end_dma_addr != dma_addr) { /* gap */ goto done;