linux-kernel - Re: [HMM 09/15] mm/hmm/devmem: device memory hotplug using ZONE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64f74f48-ca5e-e16d-7546-0d57163f6f93@nvidia.com>
Date:   Fri, 23 Jun 2017 20:54:44 -0700
From:   John Hubbard <jhubbard@...dia.com>
To:     Jérôme Glisse <jglisse@...hat.com>,
        <akpm@...ux-foundation.org>, <linux-kernel@...r.kernel.org>,
        <linux-mm@...ck.org>
CC:     Dan Williams <dan.j.williams@...el.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Evgeny Baskakov <ebaskakov@...dia.com>,
        Mark Hairgrove <mhairgrove@...dia.com>,
        Sherry Cheung <SCheung@...dia.com>,
        Subhash Gutti <sgutti@...dia.com>,
        Evgeny Baskakov <ebaskakov@...dia.com>
Subject: Re: [HMM 09/15] mm/hmm/devmem: device memory hotplug using
 ZONE_DEVICE v5

On 05/24/2017 10:20 AM, Jérôme Glisse wrote:
[...]
> +/*
> + * hmm_devmem_fault_range() - migrate back a virtual range of memory
> + *
> + * @devmem: hmm_devmem struct use to track and manage the ZONE_DEVICE memory
> + * @vma: virtual memory area containing the range to be migrated
> + * @ops: migration callback for allocating destination memory and copying
> + * @src: array of unsigned long containing source pfns
> + * @dst: array of unsigned long containing destination pfns
> + * @start: start address of the range to migrate (inclusive)
> + * @addr: fault address (must be inside the range)
> + * @end: end address of the range to migrate (exclusive)
> + * @private: pointer passed back to each of the callback
> + * Returns: 0 on success, VM_FAULT_SIGBUS on error
> + *
> + * This is a wrapper around migrate_vma() which checks the migration status
> + * for a given fault address and returns the corresponding page fault handler
> + * status. That will be 0 on success, or VM_FAULT_SIGBUS if migration failed
> + * for the faulting address.
> + *
> + * This is a helper intendend to be used by the ZONE_DEVICE fault handler.
> + */
> +int hmm_devmem_fault_range(struct hmm_devmem *devmem,
> +			   struct vm_area_struct *vma,
> +			   const struct migrate_vma_ops *ops,
> +			   unsigned long *src,
> +			   unsigned long *dst,
> +			   unsigned long start,
> +			   unsigned long addr,
> +			   unsigned long end,
> +			   void *private)
> +{
> +	if (migrate_vma(ops, vma, start, end, src, dst, private))
> +		return VM_FAULT_SIGBUS;
> +
> +	if (dst[(addr - start) >> PAGE_SHIFT] & MIGRATE_PFN_ERROR)
> +		return VM_FAULT_SIGBUS;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(hmm_devmem_fault_range);
> +#endif /* IS_ENABLED(CONFIG_HMM_DEVMEM) */
> 

Hi Jerome (+Evgeny),

After some time and testing, I'd like to recommend that we delete the above 
hmm_dev_fault_range() function from the patchset. Reasons:

1. Our driver code is actually easier to follow if we call migrate_vma() directly, for CPU 
faults. That's because there are a lot of hmm_* calls in both directions (driver <--> 
core), already, and it takes some time to remember which direction each one goes.

2. The helper is a little confusing to use, what with a start, end, *and* an addr argument.

3. ...and it doesn't add anything that the driver can't trivially do itself.

So, let's just remove it. Less is more this time. :)

thanks,
--
John Hubbard
NVIDIA