Message-ID: <a218d5bd-93cd-2332-de95-195ccbe41995@deltatee.com>
Date:   Tue, 15 Oct 2019 10:12:11 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Christoph Hellwig <hch@...radead.org>,
        Alan Mikhak <alan.mikhak@...ive.com>
Cc:     linux-kernel@...r.kernel.org, martin.petersen@...cle.com,
        alexios.zavras@...el.com, ming.lei@...hat.com,
        gregkh@...uxfoundation.org, tglx@...utronix.de,
        Jason Gunthorpe <jgg@...pe.ca>, christophe.leroy@....fr,
        Palmer Dabbelt <palmer@...ive.com>,
        Paul Walmsley <paul.walmsley@...ive.com>
Subject: Re: [PATCH] scatterlist: Validate page before calling PageSlab()



On 2019-10-15 3:55 a.m., Christoph Hellwig wrote:
> On Mon, Oct 07, 2019 at 02:13:51PM -0700, Alan Mikhak wrote:
>>> My goal is to not modify the Linux NVMe target code at all. The NVMe
>>> endpoint function driver currently does the work that is required.
> 
> You will have to do some modifications, as for example in PCIe you can
> have a n:1 relationship between SQs and CQs.  And you need to handle
> the Create/Delete SQ/CQ commands, but not the fabrics commands.  And
> modifying subsystems in Linux is perfectly acceptable, that is how they
> improve.
> 
> Do you have a pointer to your code?
> 
>>> In my current platform, there are no page struct backing for the PCIe
>>> memory address space.
> 
> In Linux there aren't struct pages for physical memory remapped using
> ioremap().  But if you want to feed them to the I/O subsystem you have
> to use devm_memremap_pages to create a page backing.  Assuming you are
> on a RISC-V platform given your affiliation you'll need to ensure your
> kernel allows for ZONE_DEVICE pages, which Logan (added to Cc) has been
> working on.  I don't remember what the current status is.

The last patchset submission was here [1]. It had some issues but the
main one was that they wanted the page tables to be created and removed
dynamically. I took a crack at it and ran into some issues and haven't
had time to touch it since. I was waiting to see how arm64[2] solves
similar problems and then maybe it can be made common.

It's also a bit of a PITA because the RISC-V hardware we have with hacky
PCI support is fragile and stopped working on recent kernels, last I
tried. So this hasn't been a priority for us.

>> Please consider the following information and cost estimate in
>> bytes for requiring page structs for PCI memory if used with
>> scatterlists. For example, a 128GB PCI memory address space
>> could require as much as 256MB of system memory just for
>> page struct backing. In a 1GB 64-bit system with flat memory
>> model, that consumes 25% of available memory. However,
>> not all of the 128GB PCI memory may be mapped for use at
>> a given time depending on the application. The cost of PCI
>> page structs is an upfront cost to be paid at system start.
> 
> I know the pages are costly.  But once you want to feed them through
> subsystems that do expect pages you'll have to do that.  And anything
> using scatterlists currently does.  A little hack here and there isn't
> going to solve that.
> 

Agreed. I tried expanding the SG-list to allow for page-less entries and
it was a much bigger mess than what you describe.

Also, I think your analysis is a bit unfair, we don't need to create
pages for the entire 128GB of PCI memory space, we typically only need
the BARs of a subset of devices which is far less. If a system has only
1GB of memory it probably doesn't actually have 128GB of PCI bar spaces
that are sensibly usable.

Yes, it would be nice to get rid of this overhead, but that's a much
bigger long-term project.

Logan


[1]
https://lore.kernel.org/linux-riscv/20190327213643.23789-1-logang@deltatee.com/
[2]
https://lore.kernel.org/linux-arm-kernel/1570788852-12402-1-git-send-email-anshuman.khandual@arm.com/

Powered by blists - more mailing lists