Re: [PATCH 00/13] Fix the DAX-gup mistake

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

 



On Tue, Sep 06, 2022 at 05:54:54PM -0700, Dan Williams wrote:
> Dan Williams wrote:
> > Jason Gunthorpe wrote:
> > > On Tue, Sep 06, 2022 at 11:37:36AM -0700, Dan Williams wrote:
> > > > Jason Gunthorpe wrote:
> > > > > On Tue, Sep 06, 2022 at 10:23:41AM -0700, Dan Williams wrote:
> > > > > 
> > > > > > > Can we continue to have the weird page->refcount behavior and still
> > > > > > > change the other things?
> > > > > > 
> > > > > > No at a minimum the pgmap vs page->refcount problem needs to be solved
> > > > > > first.
> > > > > 
> > > > > So who will do the put page after the PTE/PMD's are cleared out? In
> > > > > the normal case the tlb flusher does it integrated into zap..
> > > > 
> > > > AFAICS the zap manages the _mapcount not _refcount. Are you talking
> > > > about page_remove_rmap() or some other reference count drop?
> > > 
> > > No, page refcount.
> > > 
> > > __tlb_remove_page() eventually causes a put_page() via
> > > tlb_batch_pages_flush() calling free_pages_and_swap_cache()
> > > 
> > > Eg:
> > > 
> > >  *  MMU_GATHER_NO_GATHER
> > >  *
> > >  *  If the option is set the mmu_gather will not track individual pages for
> > >  *  delayed page free anymore. A platform that enables the option needs to
> > >  *  provide its own implementation of the __tlb_remove_page_size() function to
> > >  *  free pages.
> > 
> > Ok, yes, that is a vm_normal_page() mechanism which I was going to defer
> > since it is incremental to the _refcount handling fix and maintain that
> > DAX pages are still !vm_normal_page() in this set.
> > 
> > > > > Can we safely have the put page in the fsdax side after the zap?
> > > > 
> > > > The _refcount is managed from the lifetime insert_page() to
> > > > truncate_inode_pages(), where for DAX those are managed from
> > > > dax_insert_dentry() to dax_delete_mapping_entry().
> > > 
> > > As long as we all understand the page doesn't become re-allocatable
> > > until the refcount reaches 0 and the free op is called it may be OK!
> > 
> > Yes, but this does mean that page_maybe_dma_pinned() is not sufficient for
> > when the filesystem can safely reuse the page, it really needs to wait
> > for the reference count to drop to 0 similar to how it waits for the
> > page-idle condition today.
> 
> This gets tricky with break_layouts(). For example xfs_break_layouts()
> wants to ensure that the page is gup idle while holding the mmap lock.
> If the page is not gup idle it needs to drop locks and retry. It is
> possible the path to drop a page reference also needs to acquire
> filesystem locs. Consider odd cases like DMA from one offset to another
> in the same file. So waiting with filesystem locks held is off the
> table, which also means that deferring the wait until
> dax_delete_mapping_entry() time is also off the table.
> 
> That means that even after the conversion to make DAX page references
> 0-based it will still be the case that filesystem code will be waiting
> for the 2 -> 1 transition to indicate "mapped DAX page has no more
> external references".

Why?

If you are doing the break layouts wouldn't you first zap the ptes,
which will bring the reference to 0 if there are not other users.

If the reference did not become 0 then you have to drop all locks,
sleep until it reaches zero, and retry?

How does adding 2->1 help anything?

Jason



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux