From: Liang Li <liliang324@gmail.com> To: Mike Kravetz <mike.kravetz@oracle.com> Cc: Alexander Duyck <alexander.duyck@gmail.com>, Mel Gorman <mgorman@techsingularity.net>, Andrew Morton <akpm@linux-foundation.org>, Andrea Arcangeli <aarcange@redhat.com>, Dan Williams <dan.j.williams@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>, Jason Wang <jasowang@redhat.com>, Dave Hansen <dave.hansen@intel.com>, Michal Hocko <mhocko@suse.com>, Liang Li <liliangleo@didiglobal.com>, linux-mm <linux-mm@kvack.org>, LKML <linux-kernel@vger.kernel.org>, virtualization@lists.linux-foundation.org, qemu-devel@nongnu.org Subject: Re: [RFC PATCH 1/3] mm: support hugetlb free page reporting Date: Thu, 24 Dec 2020 09:31:21 +0800 [thread overview] Message-ID: <CA+2MQi_bxnzgS+S=GqL7UCYUKqAHZ7VC_ZTHCHcfrBHPsY+9Pg@mail.gmail.com> (raw) In-Reply-To: <e9d835e1-5d7f-d0ca-bf42-1cfa64416db6@oracle.com> > >>> +static int > >>> +hugepage_reporting_cycle(struct page_reporting_dev_info *prdev, > >>> + struct hstate *h, unsigned int nid, > >>> + struct scatterlist *sgl, unsigned int *offset) > >>> +{ > >>> + struct list_head *list = &h->hugepage_freelists[nid]; > >>> + unsigned int page_len = PAGE_SIZE << h->order; > >>> + struct page *page, *next; > >>> + long budget; > >>> + int ret = 0, scan_cnt = 0; > >>> + > >>> + /* > >>> + * Perform early check, if free area is empty there is > >>> + * nothing to process so we can skip this free_list. > >>> + */ > >>> + if (list_empty(list)) > >>> + return ret; > >> > >> Do note that not all entries on the hugetlb free lists are free. Reserved > >> entries are also on the free list. The actual number of free entries is > >> 'h->free_huge_pages - h->resv_huge_pages'. > >> Is the intention to process reserved pages as well as free pages? > > > > Yes, Reserved pages was treated as 'free pages' > > If that is true, then this code breaks hugetlb. hugetlb code assumes that > h->free_huge_pages is ALWAYS >= h->resv_huge_pages. This code would break > that assumption. If you really want to add support for hugetlb pages, then > you will need to take reserved pages into account. I didn't know that. thanks! > P.S. There might be some confusion about 'reservations' based on the > commit message. My comments are directed at hugetlb reservations described > in Documentation/vm/hugetlbfs_reserv.rst. > > >>> + /* Attempt to pull page from list and place in scatterlist */ > >>> + if (*offset) { > >>> + isolate_free_huge_page(page, h, nid); > >> > >> Once a hugetlb page is isolated, it can not be used and applications that > >> depend on hugetlb pages can start to fail. > >> I assume that is acceptable/expected behavior. Correct? > >> On some systems, hugetlb pages are a precious resource and the sysadmin > >> carefully configures the number needed by applications. Removing a hugetlb > >> page (even for a very short period of time) could cause serious application > >> failure. > > > > That' true, especially for 1G pages. Any suggestions? > > Let the hugepage allocator be aware of this situation and retry ? > > I would hate to add that complexity to the allocator. > > This question is likely based on my lack of understanding of virtio-balloon > usage and this reporting mechanism. But, why do the hugetlb pages have to > be 'temporarily' allocated for reporting purposes? The link here will give your more detail about how page reporting works, https://www.kernel.org/doc/html/latest//vm/free_page_reporting.html the virtio-balloon driver is based on this framework and will report the free pages information to QEMU&KVM, host can unmap the memory region corresponding to reported free pages and reclaim the memory for other use, it's useful for memory overcommit. Allocated the pages 'temporarily' before reporting is necessary, it make sure guests will not use the page when the host side unmap the region. or it will break the guest. Now I realized we should solve this issue first, it seems adding a lock will help. Thanks
WARNING: multiple messages have this Message-ID (diff)
From: Liang Li <liliang324@gmail.com> To: Mike Kravetz <mike.kravetz@oracle.com> Cc: Andrea Arcangeli <aarcange@redhat.com>, Michal Hocko <mhocko@suse.com>, "Michael S. Tsirkin" <mst@redhat.com>, qemu-devel@nongnu.org, Jason Wang <jasowang@redhat.com>, David Hildenbrand <david@redhat.com>, Liang Li <liliangleo@didiglobal.com>, Alexander Duyck <alexander.duyck@gmail.com>, LKML <linux-kernel@vger.kernel.org>, linux-mm <linux-mm@kvack.org>, Dave Hansen <dave.hansen@intel.com>, Andrew Morton <akpm@linux-foundation.org>, virtualization@lists.linux-foundation.org, Mel Gorman <mgorman@techsingularity.net>, Dan Williams <dan.j.williams@intel.com> Subject: Re: [RFC PATCH 1/3] mm: support hugetlb free page reporting Date: Thu, 24 Dec 2020 09:31:21 +0800 [thread overview] Message-ID: <CA+2MQi_bxnzgS+S=GqL7UCYUKqAHZ7VC_ZTHCHcfrBHPsY+9Pg@mail.gmail.com> (raw) In-Reply-To: <e9d835e1-5d7f-d0ca-bf42-1cfa64416db6@oracle.com> > >>> +static int > >>> +hugepage_reporting_cycle(struct page_reporting_dev_info *prdev, > >>> + struct hstate *h, unsigned int nid, > >>> + struct scatterlist *sgl, unsigned int *offset) > >>> +{ > >>> + struct list_head *list = &h->hugepage_freelists[nid]; > >>> + unsigned int page_len = PAGE_SIZE << h->order; > >>> + struct page *page, *next; > >>> + long budget; > >>> + int ret = 0, scan_cnt = 0; > >>> + > >>> + /* > >>> + * Perform early check, if free area is empty there is > >>> + * nothing to process so we can skip this free_list. > >>> + */ > >>> + if (list_empty(list)) > >>> + return ret; > >> > >> Do note that not all entries on the hugetlb free lists are free. Reserved > >> entries are also on the free list. The actual number of free entries is > >> 'h->free_huge_pages - h->resv_huge_pages'. > >> Is the intention to process reserved pages as well as free pages? > > > > Yes, Reserved pages was treated as 'free pages' > > If that is true, then this code breaks hugetlb. hugetlb code assumes that > h->free_huge_pages is ALWAYS >= h->resv_huge_pages. This code would break > that assumption. If you really want to add support for hugetlb pages, then > you will need to take reserved pages into account. I didn't know that. thanks! > P.S. There might be some confusion about 'reservations' based on the > commit message. My comments are directed at hugetlb reservations described > in Documentation/vm/hugetlbfs_reserv.rst. > > >>> + /* Attempt to pull page from list and place in scatterlist */ > >>> + if (*offset) { > >>> + isolate_free_huge_page(page, h, nid); > >> > >> Once a hugetlb page is isolated, it can not be used and applications that > >> depend on hugetlb pages can start to fail. > >> I assume that is acceptable/expected behavior. Correct? > >> On some systems, hugetlb pages are a precious resource and the sysadmin > >> carefully configures the number needed by applications. Removing a hugetlb > >> page (even for a very short period of time) could cause serious application > >> failure. > > > > That' true, especially for 1G pages. Any suggestions? > > Let the hugepage allocator be aware of this situation and retry ? > > I would hate to add that complexity to the allocator. > > This question is likely based on my lack of understanding of virtio-balloon > usage and this reporting mechanism. But, why do the hugetlb pages have to > be 'temporarily' allocated for reporting purposes? The link here will give your more detail about how page reporting works, https://www.kernel.org/doc/html/latest//vm/free_page_reporting.html the virtio-balloon driver is based on this framework and will report the free pages information to QEMU&KVM, host can unmap the memory region corresponding to reported free pages and reclaim the memory for other use, it's useful for memory overcommit. Allocated the pages 'temporarily' before reporting is necessary, it make sure guests will not use the page when the host side unmap the region. or it will break the guest. Now I realized we should solve this issue first, it seems adding a lock will help. Thanks
next prev parent reply other threads:[~2020-12-24 1:32 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-22 7:46 [RFC PATCH 1/3] mm: support hugetlb free page reporting Liang Li 2020-12-22 7:46 ` Liang Li 2020-12-22 19:59 ` Alexander Duyck 2020-12-22 19:59 ` Alexander Duyck 2020-12-22 19:59 ` Alexander Duyck 2020-12-22 19:59 ` Alexander Duyck 2020-12-22 22:55 ` Mike Kravetz 2020-12-23 3:42 ` Liang Li 2020-12-23 3:42 ` Liang Li 2020-12-23 3:42 ` Liang Li 2020-12-23 3:38 ` Liang Li 2020-12-23 3:38 ` Liang Li 2020-12-23 3:38 ` Liang Li 2020-12-23 16:39 ` Alexander Duyck 2020-12-23 16:39 ` Alexander Duyck 2020-12-23 16:39 ` Alexander Duyck 2020-12-23 16:39 ` Alexander Duyck 2020-12-24 0:45 ` Liang Li 2020-12-24 0:45 ` Liang Li 2020-12-24 0:45 ` Liang Li 2020-12-22 22:30 ` Mike Kravetz 2020-12-23 3:57 ` Liang Li 2020-12-23 3:57 ` Liang Li 2020-12-23 3:57 ` Liang Li 2020-12-23 18:47 ` Mike Kravetz 2020-12-23 18:47 ` Mike Kravetz 2020-12-24 1:31 ` Liang Li [this message] 2020-12-24 1:31 ` Liang Li 2020-12-24 1:31 ` Liang Li
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: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CA+2MQi_bxnzgS+S=GqL7UCYUKqAHZ7VC_ZTHCHcfrBHPsY+9Pg@mail.gmail.com' \ --to=liliang324@gmail.com \ --cc=aarcange@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=alexander.duyck@gmail.com \ --cc=dan.j.williams@intel.com \ --cc=dave.hansen@intel.com \ --cc=david@redhat.com \ --cc=jasowang@redhat.com \ --cc=liliangleo@didiglobal.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@techsingularity.net \ --cc=mhocko@suse.com \ --cc=mike.kravetz@oracle.com \ --cc=mst@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.