On Sat, May 06, 2023, Yan Zhao wrote: > On Sat, May 06, 2023 at 02:35:41PM +0800, Yan Zhao wrote: > > > > Maybe the checking of PageTransHuge(cur_page) and bailing out is not necessary. > > > > If a page is not transparent huge, but there are 512 contigous 4K > > > > pages, I think it's still good to map them in IOMMU in 2M. > > > > See vfio_pin_map_dma() who does similar things. > > > > > > I agree that bailing isn't strictly necessary, and processing "blindly" should > > > Just Work for HugeTLB and other hugepage types. I was going to argue that it > > > would be safer to add this and then drop it at the end, but I think that's a > > > specious argument. If not checking the page type is unsafe, then the existing > > > code is buggy, and this changelog literally states that the check for contiguous > > > pages guards against any such problems. > > > > > > I do think there's a (very, very theoretical) issue though. For "CONFIG_SPARSEMEM=y > > > && CONFIG_SPARSEMEM_VMEMMAP=n", struct pages aren't virtually contiguous with respect > > > to their pfns, i.e. it's possible (again, very theoretically) that two struct pages > > > could be virtually contiguous but physically discontiguous. I suspect I'm being > > > ridiculously paranoid, but for the efficient cases where pages are guaranteed to > > > be contiguous, the extra page_to_pfn() checks should be optimized away by the > > > compiler, i.e. there's no meaningful downside to the paranoia. > > To make sure I understand it correctly: > > There are 3 conditions: > > (1) Two struct pages aren't virtually contiguous, but there PFNs are contiguous. > > (2) Two struct pages are virtually contiguous but their PFNs aren't contiguous. > > (Looks this will not happen?) > > (3) Two struct pages are virtually contiguous, and their PFNs are contiguous, too. > > But they have different backends, e.g. > > PFN 1 and PFN 2 are contiguous, while PFN 1 belongs to RAM, and PFN 2 > > belongs to DEVMEM. > > > > I think you mean condition (3) is problematic, am I right? > Oh, I got it now. > You are saying about condition (2), with "CONFIG_SPARSEMEM=y && > CONFIG_SPARSEMEM_VMEMMAP=n". > Two struct pages are contiguous if one is at one section's tail and another at > another section's head, but the two sections aren't for contiguous PFNs. Yep, exactly.
