Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1691709

Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite()

From Jan Kara <jack@suse.cz>
Newsgroups linux.kernel
Subject Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite()
Date 2017-07-19 16:20 +0200
Message-ID <u4XZw-1ff-9@gated-at.bofh.it> (permalink)
References <tXtjQ-Wu-13@gated-at.bofh.it> <tXtjQ-Wu-25@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed 28-06-17 16:01:48, Ross Zwisler wrote:
> To be able to use the common 4k zero page in DAX we need to have our PTE
> fault path look more like our PMD fault path where a PTE entry can be
> marked as dirty and writeable as it is first inserted, rather than waiting
> for a follow-up dax_pfn_mkwrite() => finish_mkwrite_fault() call.
> 
> Right now we can rely on having a dax_pfn_mkwrite() call because we can
> distinguish between these two cases in do_wp_page():
> 
> 	case 1: 4k zero page => writable DAX storage
> 	case 2: read-only DAX storage => writeable DAX storage
> 
> This distinction is made by via vm_normal_page().  vm_normal_page() returns
> false for the common 4k zero page, though, just as it does for DAX ptes.
> Instead of special casing the DAX + 4k zero page case, we will simplify our
> DAX PTE page fault sequence so that it matches our DAX PMD sequence, and
> get rid of dax_pfn_mkwrite() completely.
> 
> This means that insert_pfn() needs to follow the lead of insert_pfn_pmd()
> and allow us to pass in a 'mkwrite' flag.  If 'mkwrite' is set insert_pfn()
> will do the work that was previously done by wp_page_reuse() as part of the
> dax_pfn_mkwrite() call path.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>

Just one small comment below.

> @@ -1658,14 +1658,26 @@ static int insert_pfn(struct vm_area_struct *vma, unsigned long addr,
>  	if (!pte)
>  		goto out;
>  	retval = -EBUSY;
> -	if (!pte_none(*pte))
> -		goto out_unlock;
> +	if (!pte_none(*pte)) {
> +		if (mkwrite) {
> +			entry = *pte;
> +			goto out_mkwrite;

Can we maybe check here that (pte_pfn(*pte) == pfn_t_to_pfn(pfn)) and
return -EBUSY otherwise? That way we are sure insert_pfn() isn't doing
anything we don't expect and if I understand the code right, we need to
invalidate all zero page mappings at given file offset (via
unmap_mapping_range()) before mapping an allocated block there and thus the
case of filling the hole won't be affected by this?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Back to linux.kernel | Previous | NextNext in thread | Find similar | Unroll thread


Thread

Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Jan Kara <jack@suse.cz> - 2017-07-19 16:20 +0200
  Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-07-19 20:00 +0200
    Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-07-20 00:00 +0200
      Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Ross Zwisler <ross.zwisler@linux.intel.com> - 2017-07-21 19:50 +0200
        Re: [PATCH v3 1/5] mm: add vm_insert_mixed_mkwrite() Jan Kara <jack@suse.cz> - 2017-07-24 13:30 +0200

csiph-web