Re: [PATCH v1 00/10] mm: online/offline 4MB chunks controlled by device driver

[Michal Hocko]

On Wed 18-07-18 11:56:29, David Hildenbrand wrote:
> On 16.07.2018 22:05, Michal Hocko wrote:
> > On Mon 16-07-18 21:48:59, David Hildenbrand wrote:
> >> On 11.06.2018 14:33, David Hildenbrand wrote:
> >>> On 11.06.2018 13:56, Michal Hocko wrote:
> >>>> On Mon 11-06-18 13:53:49, David Hildenbrand wrote:
> >>>>> On 24.05.2018 23:07, David Hildenbrand wrote:
> >>>>>> On 24.05.2018 16:22, Michal Hocko wrote:
> >>>>>>> I will go over the rest of the email later I just wanted to make this
> >>>>>>> point clear because I suspect we are talking past each other.
> >>>>>>
> >>>>>> It sounds like we are now talking about how to solve the problem. I like
> >>>>>> that :)
> >>>>>>
> >>>>>
> >>>>> Hi Michal,
> >>>>>
> >>>>> did you have time to think about the details of your proposed idea?
> >>>>
> >>>> Not really. Sorry about that. It's been busy time. I am planning to
> >>>> revisit after merge window closes.
> >>>>
> >>>
> >>> Sure no worries, I still have a bunch of other things to work on. But it
> >>> would be nice to clarify soon in which direction I have to head to get
> >>> this implemented and upstream (e.g. what I proposed, what you proposed
> >>> or maybe something different).
> >>>
> >> I would really like to make progress here.
> >>
> >> I pointed out basic problems/questions with the proposed alternative. I
> >> think I answered all your questions. But you also said that you are not
> >> going to accept the current approach. So some decision has to be made.
> >>
> >> Although it's very demotivating and frustrating (I hope not all work in
> >> the MM area will be like this), if there is no guidance on how to
> >> proceed, I'll have to switch to adding/removing/onlining/offlining whole
> >> segments. This is not what I want, but maybe this has a higher chance of
> >> getting reviews/acks.
> >>
> >> Understanding that you are busy, please if you make suggestions, follow
> >> up on responses.
> > 
> > I plan to get back to this. It's busy time with too many things
> > happening both upstream and on my work table as well. Sorry about that.
> > I do understand your frustration but there is only that much time I
> > have. There are not that many people to review this code unfortunately.
> > 
> > In principle though, I still maintain my position that the memory
> > hotplug code is way too subtle to add more on top. Maybe the code can be
> > reworked to be less section oriented but that will be a lot of work.
> > If you _really_ need a smaller granularity I do not have a better
> > suggestion than to emulate that on top of sections. I still have to go
> > back to your last emails though.
> > 
> 
> The only way I see doing the stuff on top will be using a new bit for
> marking pages as offline (PageOffline - Patch 1).
> 
> When a section is added, all pages are initialized to PageOffline.
> 
> online_pages() can be then hindered to online specific pages using the
> well known hook set_online_page_callback().

Not really. You just make those pages unavailable from the page
allocator and mark them reserved. You can keep them linked at your
convenience. If you need to put them back to the allocator, just do so
and drop the reserved bit.
Once you gather a section worth of pages then you can simply offline and
remove the whole section.

> In my driver, I can manually "soft offline" parts, setting them to
> PageOffline or "soft online" them again (including clearing PageOffline).
> 
> offline_pages() can then skip all pages that are already "soft offline"
> - PageOffline set - and effectively set the section offline.
> 
> 
> Without this new bit offline_pages() cannot know if a page is actually
> offline or simply reserved by some other part of the system. Imagine
> that all parts of a section are "soft offline". Now I want to offline
> the section and remove the memory. I would have to temporarily online
> all pages again, adding them to the buddy in order to properly offline
> them using offline_pages(). Prone to races as these pages must not be
> touched.

Not really. Once the page is reserved it is yours. You can reuse any
parts of the struct page as you wish. You can call the state PageOffline
and teach the offlining code to skip them (with some protocol to ensure
they will not become online all of the sudden of course). I have no
problem to integrate that part into the generic hotplug code. It should
be a trivial check at a single place. But I do not think you really need
a new page flag for that.

-- 
Michal Hocko
SUSE Labs