On Mon, Apr 3, 2017 at 4:12 PM, Logan Gunthorpe <logang@xxxxxxxxxxxx> wrote: > > > On 03/04/17 04:47 PM, Dan Williams wrote: >> I wouldn't necessarily conflate supporting pfn_t in the scatterlist >> with the stalled stuct-page-less DMA effor. A pfn_t_to_page() >> conversion will still work and be required. However you're right, the >> minute we use pfn_t for this we're into the realm of special case >> drivers that understand scatterlists with special "I/O-pfn_t" entries. > > Well yes, it would certainly be possible to convert the scatterlist code > from page_link to pfn_t. (The only slightly tricky thing is that > scatterlist uses extra chaining bits and pfn_t uses extra flag bits so > they'd have to be harmonized somehow). But if we aren't moving toward > struct-page-less DMA, I fail to see the point of the conversion. > > I'll definitely need IO scatterlists of some form or another and I like > pfn_t but right now it just seems like extra work with unclear benefit. > (Though, if someone told me that I can't use a third bit in the > page_link field then maybe that would be a good reason to move to pfn_t.) > >> However, maybe that's what we want? I think peer-to-peer DMA is not a >> general purpose feature unless/until we get it standardized in PCI. So >> maybe drivers with special case scatterlist support is exactly what we >> want for now. > > Well, I think this should be completely independent from PCI code. I see > no reason why we can't have infrastructure for DMA on iomem from any > bus. Largely all the work I've done in this area is completely agnostic > to the bus in use. (Except for any kind of white/black list when it is > used.) The completely agnostic part is where I get worried, but I shouldn't say anymore until I actually read the patch.The worry is cases where this agnostic enabling allows unsuspecting code paths to do the wrong thing. Like bypass iomem safety. > The "special case scatterlist" is essentially what I'm proposing in the > patch I sent upthread, it just stores the flag in the page_link instead > of in a pfn_t. Makes sense. The suggestion of pfn_t was to try to get more type safety throughout the stack. So that, again, unsuspecting code paths that get an I/O pfn aren't able to do things like page_address() or kmap() without failing. I'll stop commenting now and set aside some time to go read the patches.
