On 03.02.20 16:24, Chris Wilson wrote: > Quoting Kirill A. Shutemov (2020-02-03 15:18:44) >> On Fri, Jan 31, 2020 at 03:24:12PM +0000, Chris Wilson wrote: >>> Quoting Kirill A. Shutemov (2015-03-19 17:08:15) >>>> As far as I can see there's no users of PG_reserved on compound pages. >>>> Let's use NO_COMPOUND here. >>> >>> Much later than you would ever expect, but we just had a user update an >>> ancient device and trip over this. >>> https://gitlab.freedesktop.org/drm/intel/issues/1027 >>> >>> In drm_pci_alloc() we allocate a high-order page (for it to be physically >>> contiguous) and mark each page as Reserved. >>> >>> dmah->vaddr = dma_alloc_coherent(&dev->pdev->dev, size, >>> &dmah->busaddr, >>> GFP_KERNEL | __GFP_COMP); >>> >>> /* XXX - Is virt_to_page() legal for consistent mem? */ >>> /* Reserve */ >>> for (addr = (unsigned long)dmah->vaddr, sz = size; >>> sz > 0; addr += PAGE_SIZE, sz -= PAGE_SIZE) { >>> SetPageReserved(virt_to_page((void *)addr)); >>> } >>> >>> It's been doing that since >>> >>> commit ddf19b973be5a96d77c8467f657fe5bd7d126e0f >>> Author: Dave Airlie <airlied@xxxxxxxx> >>> Date: Sun Mar 19 18:56:12 2006 +1100 >>> >>> drm: fixup PCI DMA support >>> >>> I haven't found anything to say if we are meant to be reserving the >>> pages or not. So I bring it to your attention, asking for help. >> >> I don't see a real reason for these pages to be reserved. But I might be >> wrong here. >> >> I tried to look around: other users (infiniband/ethernet) of >> dma_alloc_coherent(__GFP_COMP) don't mess with PG_reserved. >> >> Could you try to drop it from DRM? > > That is the current plan. So long as there is nothing magical about > either the __GFP_COMP or SetPageReserved, we should be able to drop them > without any functional change. Only 2 very old bits of HW (r128, ancient > i915) depend on this routine, and i915 seems, touch wood, quite happy > with a plain dma_alloc_coherent(). I documented a while ago in include/linux/page-flags.h " Pages marked as PG_reserved include: [...] MMIO/DMA pages. Some architectures don't allow to ioremap pages that are not marked PG_reserved (as they might be in use by somebody else who does not respect the caching strategy). " I also removed a bunch of users back then (and even had a patch to remove this code here), but for this code I think I came to the conclusion that it might be relevant for some archs. git grep -o PageReserved | grep ioremap arch/mips/mm/ioremap.c:PageReserved arch/nios2/mm/ioremap.c:PageReserved arch/parisc/mm/ioremap.c:PageReserved arch/x86/mm/ioremap.c:PageReserved It would be good to clarify if this here is actually needed (and in addition the same pattern in other driver/ paths) and eventually update the documentation. -- Thanks, David / dhildenb
