| Message ID | 20211126134209.17332-3-david@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-mem: prepare for granularity smaller than MAX_ORDER - 1 | expand |
Hi, On 2021/11/26 21:42, David Hildenbrand wrote: > Let's prepare our fake page onlining code for subblock size smaller than > MAX_ORDER - 1: we might get called for ranges not covering properly > aligned MAX_ORDER - 1 pages. We have to detect the order to use > dynamically. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > drivers/virtio/virtio_mem.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c > index 03e1c5743699..50de7582c9f8 100644 > --- a/drivers/virtio/virtio_mem.c > +++ b/drivers/virtio/virtio_mem.c > @@ -1121,15 +1121,18 @@ static void virtio_mem_clear_fake_offline(unsigned long pfn, > */ > static void virtio_mem_fake_online(unsigned long pfn, unsigned long nr_pages) > { > - const unsigned long max_nr_pages = MAX_ORDER_NR_PAGES; > + unsigned long order = MAX_ORDER - 1; > unsigned long i; > > /* > - * We are always called at least with MAX_ORDER_NR_PAGES > - * granularity/alignment (e.g., the way subblocks work). All pages > - * inside such a block are alike. > + * We might get called for ranges that don't cover properly aligned > + * MAX_ORDER - 1 pages; however, we can only online properly aligned > + * pages with an order of MAX_ORDER - 1 at maximum. > */ > - for (i = 0; i < nr_pages; i += max_nr_pages) { > + while (!IS_ALIGNED(pfn | nr_pages, 1 << order)) > + order--; > + > + for (i = 0; i < nr_pages; i += 1 << order) { > struct page *page = pfn_to_page(pfn + i); > > /* > @@ -1139,14 +1142,12 @@ static void virtio_mem_fake_online(unsigned long pfn, unsigned long nr_pages) > * alike. > */ > if (PageDirty(page)) { > - virtio_mem_clear_fake_offline(pfn + i, max_nr_pages, > - false); > - generic_online_page(page, MAX_ORDER - 1); > + virtio_mem_clear_fake_offline(pfn + i, 1 << order, false); > + generic_online_page(page, order); > } else { > - virtio_mem_clear_fake_offline(pfn + i, max_nr_pages, > - true); > - free_contig_range(pfn + i, max_nr_pages); > - adjust_managed_page_count(page, max_nr_pages); > + virtio_mem_clear_fake_offline(pfn + i, 1 << order, true); > + free_contig_range(pfn + i, 1 << order); > + adjust_managed_page_count(page, 1 << order); In the loop, pfn + i, 1 << order are repeatedly calculated. 1 << order is a step size, pfn + i is each step position. Better to figure the numer once each iter? LGTL. LGTM. Reviewed-by: Eric Ren <renzhengeek@gmail.com> > } > } > } > @@ -2477,7 +2478,6 @@ static int virtio_mem_init_hotplug(struct virtio_mem *vm) > /* > * We want subblocks to span at least MAX_ORDER_NR_PAGES and > * pageblock_nr_pages pages. This: > - * - Simplifies our fake page onlining code (virtio_mem_fake_online). > * - Is required for now for alloc_contig_range() to work reliably - > * it doesn't properly handle smaller granularity on ZONE_NORMAL. > */
Hi Eric, thanks for the review! >> if (PageDirty(page)) { >> - virtio_mem_clear_fake_offline(pfn + i, max_nr_pages, >> - false); >> - generic_online_page(page, MAX_ORDER - 1); >> + virtio_mem_clear_fake_offline(pfn + i, 1 << order, false); >> + generic_online_page(page, order); >> } else { >> - virtio_mem_clear_fake_offline(pfn + i, max_nr_pages, >> - true); >> - free_contig_range(pfn + i, max_nr_pages); >> - adjust_managed_page_count(page, max_nr_pages); >> + virtio_mem_clear_fake_offline(pfn + i, 1 << order, true); >> + free_contig_range(pfn + i, 1 << order); >> + adjust_managed_page_count(page, 1 << order); > In the loop, pfn + i, 1 << order are repeatedly calculated. 1 << order > is a step size, pfn + i is each step position. > Better to figure the numer once each iter? The compiler better be smart enough to calculate such constants once :) > > LGTL. > LGTM. > Reviewed-by: Eric Ren <renzhengeek@gmail.com> Thanks!
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c index 03e1c5743699..50de7582c9f8 100644 --- a/drivers/virtio/virtio_mem.c +++ b/drivers/virtio/virtio_mem.c @@ -1121,15 +1121,18 @@ static void virtio_mem_clear_fake_offline(unsigned long pfn, */ static void virtio_mem_fake_online(unsigned long pfn, unsigned long nr_pages) { - const unsigned long max_nr_pages = MAX_ORDER_NR_PAGES; + unsigned long order = MAX_ORDER - 1; unsigned long i; /* - * We are always called at least with MAX_ORDER_NR_PAGES - * granularity/alignment (e.g., the way subblocks work). All pages - * inside such a block are alike. + * We might get called for ranges that don't cover properly aligned + * MAX_ORDER - 1 pages; however, we can only online properly aligned + * pages with an order of MAX_ORDER - 1 at maximum. */ - for (i = 0; i < nr_pages; i += max_nr_pages) { + while (!IS_ALIGNED(pfn | nr_pages, 1 << order)) + order--; + + for (i = 0; i < nr_pages; i += 1 << order) { struct page *page = pfn_to_page(pfn + i); /* @@ -1139,14 +1142,12 @@ static void virtio_mem_fake_online(unsigned long pfn, unsigned long nr_pages) * alike. */ if (PageDirty(page)) { - virtio_mem_clear_fake_offline(pfn + i, max_nr_pages, - false); - generic_online_page(page, MAX_ORDER - 1); + virtio_mem_clear_fake_offline(pfn + i, 1 << order, false); + generic_online_page(page, order); } else { - virtio_mem_clear_fake_offline(pfn + i, max_nr_pages, - true); - free_contig_range(pfn + i, max_nr_pages); - adjust_managed_page_count(page, max_nr_pages); + virtio_mem_clear_fake_offline(pfn + i, 1 << order, true); + free_contig_range(pfn + i, 1 << order); + adjust_managed_page_count(page, 1 << order); } } } @@ -2477,7 +2478,6 @@ static int virtio_mem_init_hotplug(struct virtio_mem *vm) /* * We want subblocks to span at least MAX_ORDER_NR_PAGES and * pageblock_nr_pages pages. This: - * - Simplifies our fake page onlining code (virtio_mem_fake_online). * - Is required for now for alloc_contig_range() to work reliably - * it doesn't properly handle smaller granularity on ZONE_NORMAL. */
Let's prepare our fake page onlining code for subblock size smaller than MAX_ORDER - 1: we might get called for ranges not covering properly aligned MAX_ORDER - 1 pages. We have to detect the order to use dynamically. Signed-off-by: David Hildenbrand <david@redhat.com> --- drivers/virtio/virtio_mem.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)