Linux Virtualization - Jun 2018 - [PATCH v33 1/4] mm: add a function to get free page blocks

On Fri, Jun 15, 2018 at 2:08 PM Wei Wang <wei.w.wang at intel.com>
wrote:
>
> This patch adds a function to get free pages blocks from a free page
> list. The obtained free page blocks are hints about free pages, because
> there is no guarantee that they are still on the free page list after
> the function returns.
Ack. This is the kind of simple interface where I don't need to worry
about the MM code calling out to random drivers or subsystems.

I think that "order" should be checked for validity, but from a MM
standpoint I think this is fine.

                Linus

On Saturday, June 16, 2018 7:09 AM, Linus Torvalds
wrote:
> On Fri, Jun 15, 2018 at 2:08 PM Wei Wang <wei.w.wang at intel.com>
wrote:
> >
> > This patch adds a function to get free pages blocks from a free page
> > list. The obtained free page blocks are hints about free pages,
> > because there is no guarantee that they are still on the free page
> > list after the function returns.
> 
> Ack. This is the kind of simple interface where I don't need to worry
about
> the MM code calling out to random drivers or subsystems.
> 
> I think that "order" should be checked for validity, but from a
MM standpoint
> I think this is fine.
> 
Thanks, will add a validity check for "order".

Best,
Wei

On Sat, Jun 16, 2018 at 08:08:53AM +0900, Linus Torvalds
wrote:
> On Fri, Jun 15, 2018 at 2:08 PM Wei Wang <wei.w.wang at intel.com>
wrote:
> >
> > This patch adds a function to get free pages blocks from a free page
> > list. The obtained free page blocks are hints about free pages,
because
> > there is no guarantee that they are still on the free page list after
> > the function returns.
...
> > +uint32_t get_from_free_page_list(int order, __le64 buf[], uint32_t
size)
...
> 
> Ack. This is the kind of simple interface where I don't need to worry
> about the MM code calling out to random drivers or subsystems.
> 
> I think that "order" should be checked for validity, but from a
MM
> standpoint I think this is fine.
> 
>                 Linus
The only issue seems to be getting hold of buf that's large enough -
and we don't really know what the size is, or whether one
buf would be enough.

Linus, do you think it would be ok to have get_from_free_page_list
actually pop entries from the free list and use them as the buffer
to store PAs?

Caller would be responsible for freeing the returned entries.

-- 
MST

[ Sorry for slow reply, my travels have made a mess of my inbox ]

On Mon, Jun 25, 2018 at 6:55 PM Michael S. Tsirkin <mst at redhat.com>
wrote:
>
> Linus, do you think it would be ok to have get_from_free_page_list
> actually pop entries from the free list and use them as the buffer
> to store PAs?
Honestly, what I think the best option would be is to get rid of this
interface *entirely*, and just have the balloon code do

    #define GFP_MINFLAGS (__GFP_NORETRY | __GFP_NOWARN |
__GFP_THISNODE | __GFP_NOMEMALLOC)

    struct page *page =  alloc_pages(GFP_MINFLAGS, MAX_ORDER-1);

 which is not a new interface, and simply removes the max-order page
from the list if at all possible.

The above has the advantage of "just working", and not having any
races.

Now, because you don't want to necessarily *entirely* deplete the max
order, I'd suggest that the *one* new interface you add is just a "how
many max-order pages are there" interface. So then you can query
(either before or after getting the max-order page) just how many of
them there were and whether you want to give that page back.

Notice? No need for any page lists or physical addresses. No races. No
complex new functions.

The physical address you can just get from the "struct page" you got.

And if you run out of memory because of getting a page, you get all
the usual "hey, we ran out of memory" responses..

Wouldn't the above be sufficient?

            Linus