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


Groups > linux.kernel > #1527231

Re: [HMM v13 02/18] mm/ZONE_DEVICE/unaddressable: add support for un-addressable device memory

From Anshuman Khandual <khandual@linux.vnet.ibm.com>
Newsgroups linux.kernel
Subject Re: [HMM v13 02/18] mm/ZONE_DEVICE/unaddressable: add support for un-addressable device memory
Date 2016-11-22 06:20 +0100
Message-ID <sGbER-2Ps-1@gated-at.bofh.it> (permalink)
References <sEUZr-1B7-3@gated-at.bofh.it> <sEV97-1Es-5@gated-at.bofh.it> <sFRPP-74b-5@gated-at.bofh.it> <sFW37-1bK-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 11/21/2016 06:03 PM, Jerome Glisse wrote:
> On Mon, Nov 21, 2016 at 01:36:57PM +0530, Anshuman Khandual wrote:
>> On 11/18/2016 11:48 PM, Jérôme Glisse wrote:
>>> This add support for un-addressable device memory. Such memory is hotpluged
>>> only so we can have struct page but should never be map. This patch add code
>>
>> struct pages inside the system RAM range unlike the vmem_altmap scheme
>> where the struct pages can be inside the device memory itself. This
>> possibility does not arise for un addressable device memory. May be we
>> will have to block the paths where vmem_altmap is requested along with
>> un addressable device memory.
> 
> I did not think checking for that explicitly was necessary, sounded like shooting
> yourself in the foot and that it would be obvious :)

dev_memremap_pages() is kind of an important interface for getting
device memory into kernel through ZONE_DEVICE. So it should actually
enforce all these checks. Also we should document these things clearly
above the function.

> 
> [...]
> 
>>> diff --git a/include/linux/memremap.h b/include/linux/memremap.h
>>> index 9341619..fe61dca 100644
>>> --- a/include/linux/memremap.h
>>> +++ b/include/linux/memremap.h
>>> @@ -41,22 +41,34 @@ static inline struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start)
>>>   * @res: physical address range covered by @ref
>>>   * @ref: reference count that pins the devm_memremap_pages() mapping
>>>   * @dev: host device of the mapping for debug
>>> + * @flags: memory flags (look for MEMORY_FLAGS_NONE in memory_hotplug.h)
>>
>> ^^^^^^^^^^^^^ device memory flags instead ?
> 
> Well maybe it will be use for something else than device memory in the future
> but yes for now it is only device memory so i can rename it.
> 
>>>   */
>>>  struct dev_pagemap {
>>>  	struct vmem_altmap *altmap;
>>>  	const struct resource *res;
>>>  	struct percpu_ref *ref;
>>>  	struct device *dev;
>>> +	int flags;
>>>  };
>>>  
>>>  #ifdef CONFIG_ZONE_DEVICE
>>>  void *devm_memremap_pages(struct device *dev, struct resource *res,
>>> -		struct percpu_ref *ref, struct vmem_altmap *altmap);
>>> +			  struct percpu_ref *ref, struct vmem_altmap *altmap,
>>> +			  struct dev_pagemap **ppgmap, int flags);
>>>  struct dev_pagemap *find_dev_pagemap(resource_size_t phys);
>>> +
>>> +static inline bool is_addressable_page(const struct page *page)
>>> +{
>>> +	return ((page_zonenum(page) != ZONE_DEVICE) ||
>>> +		!(page->pgmap->flags & MEMORY_UNADDRESSABLE));
>>> +}
>>>  #else
>>>  static inline void *devm_memremap_pages(struct device *dev,
>>> -		struct resource *res, struct percpu_ref *ref,
>>> -		struct vmem_altmap *altmap)
>>> +					struct resource *res,
>>> +					struct percpu_ref *ref,
>>> +					struct vmem_altmap *altmap,
>>> +					struct dev_pagemap **ppgmap,
>>> +					int flags)
>>
>>
>> As I had mentioned before devm_memremap_pages() should be changed not
>> to accept a valid altmap along with request for un-addressable memory.
> 
> If you fear such case yes sure.
> 
> 
> [...]
> 
>>> diff --git a/kernel/memremap.c b/kernel/memremap.c
>>> index 07665eb..438a73aa2 100644
>>> --- a/kernel/memremap.c
>>> +++ b/kernel/memremap.c
>>> @@ -246,7 +246,7 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
>>>  	/* pages are dead and unused, undo the arch mapping */
>>>  	align_start = res->start & ~(SECTION_SIZE - 1);
>>>  	align_size = ALIGN(resource_size(res), SECTION_SIZE);
>>> -	arch_remove_memory(align_start, align_size, MEMORY_DEVICE);
>>> +	arch_remove_memory(align_start, align_size, pgmap->flags);
>>>  	untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
>>>  	pgmap_radix_release(res);
>>>  	dev_WARN_ONCE(dev, pgmap->altmap && pgmap->altmap->alloc,
>>> @@ -270,6 +270,8 @@ struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
>>>   * @res: "host memory" address range
>>>   * @ref: a live per-cpu reference count
>>>   * @altmap: optional descriptor for allocating the memmap from @res
>>> + * @ppgmap: pointer set to new page dev_pagemap on success
>>> + * @flags: flag for memory (look for MEMORY_FLAGS_NONE in memory_hotplug.h)
>>>   *
>>>   * Notes:
>>>   * 1/ @ref must be 'live' on entry and 'dead' before devm_memunmap_pages() time
>>> @@ -280,7 +282,8 @@ struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
>>>   *    this is not enforced.
>>>   */
>>>  void *devm_memremap_pages(struct device *dev, struct resource *res,
>>> -		struct percpu_ref *ref, struct vmem_altmap *altmap)
>>> +			  struct percpu_ref *ref, struct vmem_altmap *altmap,
>>> +			  struct dev_pagemap **ppgmap, int flags)
>>>  {
>>>  	resource_size_t key, align_start, align_size, align_end;
>>>  	pgprot_t pgprot = PAGE_KERNEL;
>>> @@ -322,6 +325,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
>>>  	}
>>>  	pgmap->ref = ref;
>>>  	pgmap->res = &page_map->res;
>>> +	pgmap->flags = flags | MEMORY_DEVICE;
>>
>> So the caller of devm_memremap_pages() should not have give out MEMORY_DEVICE
>> in the flag it passed on to this function ? Hmm, else we should just check
>> that the flags contains all appropriate bits before proceeding.
> 
> Here i was just trying to be on the safe side, yes caller should already have set
> the flag but this function is only use for device memory so it did not seem like
> it would hurt to be extra safe. I can add a BUG_ON() but it seems people have mix
> feeling about BUG_ON()

We dont have to do BUG_ON(), just a check that all expected flags are
in there, else fail the call. Now this function does not return any
value to be checked inside driver, in that case we can just do a error
message print and move on.

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


Thread

[HMM v13 00/18] HMM (Heterogeneous Memory Management) v13 Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:20 +0100
  [HMM v13 13/18] mm/hmm/mirror: device page fault handler Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:20 +0100
  [HMM v13 12/18] mm/hmm/mirror: helper to snapshot CPU page table Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
  [HMM v13 02/18] mm/ZONE_DEVICE/unaddressable: add support for un-addressable device memory Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
    Re: [HMM v13 02/18] mm/ZONE_DEVICE/unaddressable: add support for  un-addressable device memory Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 09:10 +0100
      Re: [HMM v13 02/18] mm/ZONE_DEVICE/unaddressable: add support for  un-addressable device memory Jerome Glisse <jglisse@redhat.com> - 2016-11-21 13:40 +0100
        Re: [HMM v13 02/18] mm/ZONE_DEVICE/unaddressable: add support for  un-addressable device memory Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-22 06:20 +0100
  [HMM v13 09/18] mm/hmm/mirror: mirror process address space on device with HMM helpers Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
    Re: [HMM v13 09/18] mm/hmm/mirror: mirror process address space on  device with HMM helpers Balbir Singh <bsingharora@gmail.com> - 2016-11-21 03:50 +0100
      Re: [HMM v13 09/18] mm/hmm/mirror: mirror process address space on  device with HMM helpers Jerome Glisse <jglisse@redhat.com> - 2016-11-21 06:20 +0100
  [HMM v13 05/18] mm/ZONE_DEVICE/devmem_pages_remove: allow early removal of device memory Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
    Re: [HMM v13 05/18] mm/ZONE_DEVICE/devmem_pages_remove: allow early  removal of device memory Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 11:40 +0100
      Re: [HMM v13 05/18] mm/ZONE_DEVICE/devmem_pages_remove: allow early  removal of device memory Jerome Glisse <jglisse@redhat.com> - 2016-11-21 13:40 +0100
        Re: [HMM v13 05/18] mm/ZONE_DEVICE/devmem_pages_remove: allow early  removal of device memory Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-22 06:00 +0100
  [HMM v13 04/18] mm/ZONE_DEVICE/free-page: callback when page is freed Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
    Re: [HMM v13 04/18] mm/ZONE_DEVICE/free-page: callback when page is  freed Balbir Singh <bsingharora@gmail.com> - 2016-11-21 03:00 +0100
      Re: [HMM v13 04/18] mm/ZONE_DEVICE/free-page: callback when page is  freed Jerome Glisse <jglisse@redhat.com> - 2016-11-21 06:00 +0100
    Re: [HMM v13 04/18] mm/ZONE_DEVICE/free-page: callback when page is  freed Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 09:30 +0100
      Re: [HMM v13 04/18] mm/ZONE_DEVICE/free-page: callback when page is  freed Jerome Glisse <jglisse@redhat.com> - 2016-11-21 13:40 +0100
        Re: [HMM v13 04/18] mm/ZONE_DEVICE/free-page: callback when page is  freed Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-22 06:10 +0100
  [HMM v13 10/18] mm/hmm/mirror: add range lock helper, prevent CPU page table update for the range Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
  [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap for unaddressable Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
    Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Balbir Singh <bsingharora@gmail.com> - 2016-11-21 03:10 +0100
      Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Jerome Glisse <jglisse@redhat.com> - 2016-11-21 06:10 +0100
        Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Balbir Singh <bsingharora@gmail.com> - 2016-11-22 03:30 +0100
          Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Jerome Glisse <jglisse@redhat.com> - 2016-11-22 15:00 +0100
      Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 12:20 +0100
    Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-21 12:00 +0100
      Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Jerome Glisse <jglisse@redhat.com> - 2016-11-21 13:50 +0100
        Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2016-11-22 05:50 +0100
          Re: [HMM v13 06/18] mm/ZONE_DEVICE/unaddressable: add special swap  for unaddressable Jerome Glisse <jglisse@redhat.com> - 2016-11-24 15:00 +0100
  [HMM v13 11/18] mm/hmm/mirror: add range monitor helper, to monitor CPU page table update Jérôme Glisse <jglisse@redhat.com> - 2016-11-18 18:30 +0100
  Re: [HMM v13 00/18] HMM (Heterogeneous Memory Management) v13 John Hubbard <jhubbard@nvidia.com> - 2016-11-19 01:50 +0100
    Re: [HMM v13 00/18] HMM (Heterogeneous Memory Management) v13 "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> - 2016-11-19 16:00 +0100

csiph-web