archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <>
To: Oscar Salvador <>
Cc:,,,,,,,,,,, Oscar Salvador <>
Subject: Re: [RFC v2 2/2] mm/memory_hotplug: Shrink spanned pages when offlining memory
Date: Wed, 22 Aug 2018 10:11:40 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 22.08.2018 09:50, Oscar Salvador wrote:
> On Tue, Aug 21, 2018 at 03:17:10PM +0200, David Hildenbrand wrote:
>>> add_device_memory is in charge of
>> I wouldn't use the terminology of onlining/offlining here. That applies
>> rather to memory that is exposed to the rest of the system (e.g. buddy
>> allocator, has underlying memory block devices). I guess it is rather a
>> pure setup/teardown of that device memory.
> Hi David,
> I am not sure if you are referring to:
> "
> a) calling either arch_add_memory() or add_pages(), depending on whether
>    we want a linear mapping
> b) online the memory sections that correspond to the pfn range
> c) calling move_pfn_range_to_zone() being zone ZONE_DEVICE to
>    expand zone/pgdat spanned pages and initialize its pages
> "
> Well, that is partialy true.
> I mean, in order to make this work, we need to offline/online the memory
> sections, because shrink_pages will rely on that from now on.
> Is what we do when online/offline pages, but since device memory
> does not go through the "official" channels, we need to do it there
> as well.
> Sure I can use another terminology, but since that is what
> offline/online_mem_sections do, I just came up with that.

Okay, got it, so it is basically "mark the sections as online/offline".

>> I would really like to see the mem_hotplug_begin/end also getting moved
>> inside add_device_memory()/del_device_memory(). (just like for
>> add/remove_memory)
>> I wonder if kasan_ stuff actually requires this lock, or if it could
>> also be somehow moved inside add_device_memory/del_device_memory.
> Yes, that was my first approach, but then I saw that the kasan stuff is being
> handled whithin those locks, so I was not sure and I backed off leaving the
> mem_hotplug_begin/end where they were.
> Maybe Jerome can shed some light and, and we can just handle the kasan stuff
> out of the locks.
>> Maybe shorten that a bit
>> "HMM/devm memory does not have IORESOURCE_SYSTEM_RAM set. They use
>>  devm_request_mem_region/devm_release_mem_region to add/release a
>>  resource. Just back off here."
> Uhm, fair enough.
>> Any reason for these indirections?
> I wanted to hide the internals in the memory_hotplug code.
> I thought about removing them, but I finally left them.
> If people think that we are better off without them, I can just
> remove them.

I don't see a need for that. (everyone following the functions has to go
via one indirection that just passes on parameters). It is also not done
for other functions (a.g. add_memory)

>> I guess for readability, this patch could be split up into several
>> patches. E.g. factoring out of add_device_memory/del_device_memory,
>> release_mem_region_adjustable change ...
> Yes, really true.
> But I wanted first to gather feedback mainly from HMM/devm people to see
> if they saw an outright bug within the series because I am not so
> familiar with that part of the code.
> Feedback from Jerome/Dan will be appreciate as well to see if this is a good
> direction.

Yes, they probably know best how this all fits together.

> But you are right, in the end, this will have to be slipt up into several
> parts to ease the review.
> Thanks for reviewing this David!
> I will try to address your concerns.
> Thanks 



David / dhildenb

  reply	other threads:[~2018-08-22  8:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-17 15:41 [RFC v2 0/2] Do not touch pages in remove_memory path Oscar Salvador
2018-08-17 15:41 ` [RFC v2 1/2] mm/memory_hotplug: Add nid parameter to arch_remove_memory Oscar Salvador
2018-08-17 15:41 ` [RFC v2 2/2] mm/memory_hotplug: Shrink spanned pages when offlining memory Oscar Salvador
2018-08-21 13:17   ` David Hildenbrand
2018-08-22  7:50     ` Oscar Salvador
2018-08-22  8:11       ` David Hildenbrand [this message]
2018-08-29 23:09   ` Pasha Tatashin
2018-08-31 20:50     ` Oscar Salvador
2018-08-28 11:47 ` [RFC v2 0/2] Do not touch pages in remove_memory path Oscar Salvador
2018-08-29 17:04   ` Jerome Glisse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).