Re: [PATCH v2] drm/ttm: Do not put non-struct page memory into PUD/PMDs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 22, 2021 at 03:57:42PM -0300, Jason Gunthorpe wrote:
> On Thu, Oct 21, 2021 at 01:41:39PM +0200, Daniel Vetter wrote:
> > On Wed, Oct 20, 2021 at 04:37:02PM -0300, Jason Gunthorpe wrote:
> > > On Wed, Oct 20, 2021 at 08:41:24AM +0200, Christian König wrote:
> > > 
> > > > > I think the patch subject needs updating to reflect that we're disabling
> > > > > PUD/PMDs completely.
> > > > > With that fixed,
> > > 
> > > Everyone is OK with this?
> > > 
> > > drm/ttm: remove ttm_bo_vm_insert_huge()
> > > 
> > > The huge page functionality in TTM does not work safely because PUD and
> > > PMD entries do not have a special bit.
> > > 
> > > get_user_pages_fast() considers any page that passed pmd_huge() as
> > > usable:
> > > 
> > > 	if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) ||
> > > 		     pmd_devmap(pmd))) {
> > > 
> > > And vmf_insert_pfn_pmd_prot() unconditionally sets
> > > 
> > > 	entry = pmd_mkhuge(pfn_t_pmd(pfn, prot));
> > > 
> > > eg on x86 the page will be _PAGE_PRESENT | PAGE_PSE.
> > > 
> > > As such gup_huge_pmd() will try to deref a struct page:
> > > 
> > > 	head = try_grab_compound_head(pmd_page(orig), refs, flags);
> > > 
> > > and thus crash.
> > > 
> > > So, iomem cannot be installed using vmf_insert_pfn_pud/pmd_prot().
> > > 
> > > Thomas further notices that the drivers are not expecting the struct page
> > > to be used by anything - in particular the refcount incr above will cause
> > > them to malfunction. This means even the struct page memory cannot be
> > > used.
> > > 
> > > Therefore everything about this is not able to fully work correctly
> > > considering GUP_fast. Delete it entirely. It can return someday along with
> > > a proper PMD/PUD_SPECIAL bit in the page table itself to gate GUP_fast.
> > > 
> > > Fixes: 314b6580adc5 ("drm/ttm, drm/vmwgfx: Support huge TTM pagefaults")
> > > Reviewed-by: Christian König <christian.koenig@xxxxxxx>
> > > Reviewed-by: Thomas Hellström <thomas.helllstrom@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > 
> > Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > 
> > I think we also want cc: stable here.
> 
> Ok
>  
> > Do you plan to land this through some dedicated pull for -rc? I think that
> > makes sense to highlight it, but I can also smash this into some
> > drm-fixes.
> 
> I was hoping you'd take it? Do want a v3?

Hm totally lost this, I'm trying to not be too responsible for mm changes
since it scares me :-) Plus dropping this very late in the release feels a
bit risky.

Ok if I stuff this into drm-next instead?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux