From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753016AbeCUQxs (ORCPT ); Wed, 21 Mar 2018 12:53:48 -0400 Received: from ale.deltatee.com ([207.54.116.67]:56936 "EHLO ale.deltatee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751729AbeCUQxq (ORCPT ); Wed, 21 Mar 2018 12:53:46 -0400 To: Christoph Hellwig Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-nvme@lists.infradead.org, linux-rdma@vger.kernel.org, linux-nvdimm@lists.01.org, linux-block@vger.kernel.org, Stephen Bates , Jens Axboe , Keith Busch , Sagi Grimberg , Bjorn Helgaas , Jason Gunthorpe , Max Gurtovoy , Dan Williams , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Benjamin Herrenschmidt , Alex Williamson , Steve Wise References: <20180312193525.2855-1-logang@deltatee.com> <20180312193525.2855-12-logang@deltatee.com> <20180321092702.GC7098@lst.de> From: Logan Gunthorpe Message-ID: Date: Wed, 21 Mar 2018 10:52:25 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180321092702.GC7098@lst.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 172.16.1.162 X-SA-Exim-Rcpt-To: swise@opengridcomputing.com, alex.williamson@redhat.com, benh@kernel.crashing.org, jglisse@redhat.com, dan.j.williams@intel.com, maxg@mellanox.com, jgg@mellanox.com, bhelgaas@google.com, sagi@grimberg.me, keith.busch@intel.com, axboe@kernel.dk, sbates@raithlin.com, linux-block@vger.kernel.org, linux-nvdimm@lists.01.org, linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, hch@lst.de X-SA-Exim-Mail-From: logang@deltatee.com Subject: Re: [PATCH v3 11/11] nvmet: Optionally use PCI P2P memory X-SA-Exim-Version: 4.2.1 (built Tue, 02 Aug 2016 21:08:31 +0000) X-SA-Exim-Scanned: Yes (on ale.deltatee.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21/03/18 03:27 AM, Christoph Hellwig wrote: >> + const char *page, size_t count) >> +{ >> + struct nvmet_port *port = to_nvmet_port(item); >> + struct device *dev; >> + struct pci_dev *p2p_dev = NULL; >> + bool use_p2pmem; >> + >> + switch (page[0]) { >> + case 'y': >> + case 'Y': >> + case 'a': >> + case 'A': >> + use_p2pmem = true; >> + break; >> + case 'n': >> + case 'N': >> + use_p2pmem = false; >> + break; >> + default: >> + dev = bus_find_device_by_name(&pci_bus_type, NULL, page); >> + if (!dev) { >> + pr_err("No such PCI device: %s\n", page); >> + return -ENODEV; >> + } >> + >> + use_p2pmem = true; >> + p2p_dev = to_pci_dev(dev); >> + >> + if (!pci_has_p2pmem(p2p_dev)) { >> + pr_err("PCI device has no peer-to-peer memory: %s\n", >> + page); >> + pci_dev_put(p2p_dev); >> + return -ENODEV; >> + } >> + } > > Yikes. Shouldn't auto just be the normal yes case instead of this > string parsing mess? Sorry, I don't follow. The code, as is, should automatically select the device if the user sets it to "yes" or "auto" or "y" or similar. (Roughly similar to how kstrtobool() works, except '0' or '1' are not accepted seeing they could overlap with PCI device names). In other cases, it looks for the specific PCI device name to use exactly. Are you saying it shouldn't work this way or are you saying the code to implement it is too messy? >> + if (rsp->req.sg != &rsp->cmd->inline_sg) { >> + if (rsp->req.p2p_dev) >> + pci_p2pmem_free_sgl(rsp->req.p2p_dev, rsp->req.sg, >> + rsp->req.sg_cnt); >> + else >> + sgl_free(rsp->req.sg); >> + } > > Can we factor this into a helper, as the other target drivers (fc for now, > tcp soon) using sgl allocatins should share the code? > > (same for the alloc side) Sure. Would the helpers belong in core.c? Thanks, Logan