Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1550661 > unrolled thread
| Started by | Mel Gorman <mgorman@techsingularity.net> |
|---|---|
| First post | 2017-01-04 12:20 +0100 |
| Last post | 2017-01-09 11:00 +0100 |
| Articles | 7 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests Mel Gorman <mgorman@techsingularity.net> - 2017-01-04 12:20 +0100
Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests Jesper Dangaard Brouer <brouer@redhat.com> - 2017-01-04 15:30 +0100
Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-01-06 04:30 +0100
Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests Mel Gorman <mgorman@techsingularity.net> - 2017-01-06 11:20 +0100
Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-01-09 04:20 +0100
Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests Mel Gorman <mgorman@techsingularity.net> - 2017-01-09 10:50 +0100
Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-01-09 11:00 +0100
| From | Mel Gorman <mgorman@techsingularity.net> |
|---|---|
| Date | 2017-01-04 12:20 +0100 |
| Subject | [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests |
| Message-ID | <sVRLP-3XT-9@gated-at.bofh.it> |
Many workloads that allocate pages are not handling an interrupt at a
time. As allocation requests may be from IRQ context, it's necessary to
disable/enable IRQs for every page allocation. This cost is the bulk
of the free path but also a significant percentage of the allocation
path.
This patch alters the locking and checks such that only irq-safe allocation
requests use the per-cpu allocator. All others acquire the irq-safe
zone->lock and allocate from the buddy allocator. It relies on disabling
preemption to safely access the per-cpu structures. It could be slightly
modified to avoid soft IRQs using it but it's not clear it's worthwhile.
This modification may slow allocations from IRQ context slightly but the main
gain from the per-cpu allocator is that it scales better for allocations
from multiple contexts. There is an implicit assumption that intensive
allocations from IRQ contexts on multiple CPUs from a single NUMA node are
rare and that the fast majority of scaling issues are encountered in !IRQ
contexts such as page faulting. It's worth noting that this patch is not
required for a bulk page allocator but it significantly reduces the overhead.
The following is results from a page allocator micro-benchmark. Only
order-0 is interesting as higher orders do not use the per-cpu allocator
4.10.0-rc2 4.10.0-rc2
vanilla irqsafe-v1r5
Amean alloc-odr0-1 287.15 ( 0.00%) 219.00 ( 23.73%)
Amean alloc-odr0-2 221.23 ( 0.00%) 183.23 ( 17.18%)
Amean alloc-odr0-4 187.00 ( 0.00%) 151.38 ( 19.05%)
Amean alloc-odr0-8 167.54 ( 0.00%) 132.77 ( 20.75%)
Amean alloc-odr0-16 156.00 ( 0.00%) 123.00 ( 21.15%)
Amean alloc-odr0-32 149.00 ( 0.00%) 118.31 ( 20.60%)
Amean alloc-odr0-64 138.77 ( 0.00%) 116.00 ( 16.41%)
Amean alloc-odr0-128 145.00 ( 0.00%) 118.00 ( 18.62%)
Amean alloc-odr0-256 136.15 ( 0.00%) 125.00 ( 8.19%)
Amean alloc-odr0-512 147.92 ( 0.00%) 121.77 ( 17.68%)
Amean alloc-odr0-1024 147.23 ( 0.00%) 126.15 ( 14.32%)
Amean alloc-odr0-2048 155.15 ( 0.00%) 129.92 ( 16.26%)
Amean alloc-odr0-4096 164.00 ( 0.00%) 136.77 ( 16.60%)
Amean alloc-odr0-8192 166.92 ( 0.00%) 138.08 ( 17.28%)
Amean alloc-odr0-16384 159.00 ( 0.00%) 138.00 ( 13.21%)
Amean free-odr0-1 165.00 ( 0.00%) 89.00 ( 46.06%)
Amean free-odr0-2 113.00 ( 0.00%) 63.00 ( 44.25%)
Amean free-odr0-4 99.00 ( 0.00%) 54.00 ( 45.45%)
Amean free-odr0-8 88.00 ( 0.00%) 47.38 ( 46.15%)
Amean free-odr0-16 83.00 ( 0.00%) 46.00 ( 44.58%)
Amean free-odr0-32 80.00 ( 0.00%) 44.38 ( 44.52%)
Amean free-odr0-64 72.62 ( 0.00%) 43.00 ( 40.78%)
Amean free-odr0-128 78.00 ( 0.00%) 42.00 ( 46.15%)
Amean free-odr0-256 80.46 ( 0.00%) 57.00 ( 29.16%)
Amean free-odr0-512 96.38 ( 0.00%) 64.69 ( 32.88%)
Amean free-odr0-1024 107.31 ( 0.00%) 72.54 ( 32.40%)
Amean free-odr0-2048 108.92 ( 0.00%) 78.08 ( 28.32%)
Amean free-odr0-4096 113.38 ( 0.00%) 82.23 ( 27.48%)
Amean free-odr0-8192 112.08 ( 0.00%) 82.85 ( 26.08%)
Amean free-odr0-16384 110.38 ( 0.00%) 81.92 ( 25.78%)
Amean total-odr0-1 452.15 ( 0.00%) 308.00 ( 31.88%)
Amean total-odr0-2 334.23 ( 0.00%) 246.23 ( 26.33%)
Amean total-odr0-4 286.00 ( 0.00%) 205.38 ( 28.19%)
Amean total-odr0-8 255.54 ( 0.00%) 180.15 ( 29.50%)
Amean total-odr0-16 239.00 ( 0.00%) 169.00 ( 29.29%)
Amean total-odr0-32 229.00 ( 0.00%) 162.69 ( 28.96%)
Amean total-odr0-64 211.38 ( 0.00%) 159.00 ( 24.78%)
Amean total-odr0-128 223.00 ( 0.00%) 160.00 ( 28.25%)
Amean total-odr0-256 216.62 ( 0.00%) 182.00 ( 15.98%)
Amean total-odr0-512 244.31 ( 0.00%) 186.46 ( 23.68%)
Amean total-odr0-1024 254.54 ( 0.00%) 198.69 ( 21.94%)
Amean total-odr0-2048 264.08 ( 0.00%) 208.00 ( 21.24%)
Amean total-odr0-4096 277.38 ( 0.00%) 219.00 ( 21.05%)
Amean total-odr0-8192 279.00 ( 0.00%) 220.92 ( 20.82%)
Amean total-odr0-16384 269.38 ( 0.00%) 219.92 ( 18.36%)
This is the alloc, free and total overhead of allocating order-0 pages in
batches of 1 page up to 16384 pages. Avoiding disabling/enabling overhead
massively reduces overhead. Alloc overhead is roughly reduced by 14-20% in
most cases. The free path is reduced by 26-46% and the total reduction
is significant.
Many users require zeroing of pages from the page allocator which is the
vast cost of allocation. Hence, the impact on a basic page faulting benchmark
is not that significant
4.10.0-rc2 4.10.0-rc2
vanilla irqsafe-v1r5
Hmean page_test 656632.98 ( 0.00%) 675536.13 ( 2.88%)
Hmean brk_test 3845502.67 ( 0.00%) 3867186.94 ( 0.56%)
Stddev page_test 10543.29 ( 0.00%) 4104.07 ( 61.07%)
Stddev brk_test 33472.36 ( 0.00%) 15538.39 ( 53.58%)
CoeffVar page_test 1.61 ( 0.00%) 0.61 ( 62.15%)
CoeffVar brk_test 0.87 ( 0.00%) 0.40 ( 53.84%)
Max page_test 666513.33 ( 0.00%) 678640.00 ( 1.82%)
Max brk_test 3882800.00 ( 0.00%) 3887008.66 ( 0.11%)
This is from aim9 and the most notable outcome is that fault variability
is reduced by the patch. The headline improvement is small as the overall
fault cost, zeroing, page table insertion etc dominate relative to
disabling/enabling IRQs in the per-cpu allocator.
Similarly, little benefit was seen on networking benchmarks both localhost
and between physical server/clients where other costs dominate. It's
possible that this will only be noticable on very high speed networks.
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
mm/page_alloc.c | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4a602b7f258d..01b09f9da288 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1087,10 +1087,10 @@ static void free_pcppages_bulk(struct zone *zone, int count,
{
int migratetype = 0;
int batch_free = 0;
- unsigned long nr_scanned;
+ unsigned long nr_scanned, flags;
bool isolated_pageblocks;
- spin_lock(&zone->lock);
+ spin_lock_irqsave(&zone->lock, flags);
isolated_pageblocks = has_isolate_pageblock(zone);
nr_scanned = node_page_state(zone->zone_pgdat, NR_PAGES_SCANNED);
if (nr_scanned)
@@ -1139,7 +1139,7 @@ static void free_pcppages_bulk(struct zone *zone, int count,
trace_mm_page_pcpu_drain(page, 0, mt);
} while (--count && --batch_free && !list_empty(list));
}
- spin_unlock(&zone->lock);
+ spin_unlock_irqrestore(&zone->lock, flags);
}
static void free_one_page(struct zone *zone,
@@ -1147,8 +1147,8 @@ static void free_one_page(struct zone *zone,
unsigned int order,
int migratetype)
{
- unsigned long nr_scanned;
- spin_lock(&zone->lock);
+ unsigned long nr_scanned, flags;
+ spin_lock_irqsave(&zone->lock, flags);
nr_scanned = node_page_state(zone->zone_pgdat, NR_PAGES_SCANNED);
if (nr_scanned)
__mod_node_page_state(zone->zone_pgdat, NR_PAGES_SCANNED, -nr_scanned);
@@ -1158,7 +1158,7 @@ static void free_one_page(struct zone *zone,
migratetype = get_pfnblock_migratetype(page, pfn);
}
__free_one_page(page, pfn, zone, order, migratetype);
- spin_unlock(&zone->lock);
+ spin_unlock_irqrestore(&zone->lock, flags);
}
static void __meminit __init_single_page(struct page *page, unsigned long pfn,
@@ -1236,7 +1236,6 @@ void __meminit reserve_bootmem_region(phys_addr_t start, phys_addr_t end)
static void __free_pages_ok(struct page *page, unsigned int order)
{
- unsigned long flags;
int migratetype;
unsigned long pfn = page_to_pfn(page);
@@ -1244,10 +1243,8 @@ static void __free_pages_ok(struct page *page, unsigned int order)
return;
migratetype = get_pfnblock_migratetype(page, pfn);
- local_irq_save(flags);
- __count_vm_events(PGFREE, 1 << order);
+ count_vm_events(PGFREE, 1 << order);
free_one_page(page_zone(page), page, pfn, order, migratetype);
- local_irq_restore(flags);
}
static void __init __free_pages_boot_core(struct page *page, unsigned int order)
@@ -2219,8 +2216,9 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
int migratetype, bool cold)
{
int i, alloced = 0;
+ unsigned long flags;
- spin_lock(&zone->lock);
+ spin_lock_irqsave(&zone->lock, flags);
for (i = 0; i < count; ++i) {
struct page *page = __rmqueue(zone, order, migratetype);
if (unlikely(page == NULL))
@@ -2256,7 +2254,7 @@ static int rmqueue_bulk(struct zone *zone, unsigned int order,
* pages added to the pcp list.
*/
__mod_zone_page_state(zone, NR_FREE_PAGES, -(i << order));
- spin_unlock(&zone->lock);
+ spin_unlock_irqrestore(&zone->lock, flags);
return alloced;
}
@@ -2444,7 +2442,6 @@ void free_hot_cold_page(struct page *page, bool cold)
{
struct zone *zone = page_zone(page);
struct per_cpu_pages *pcp;
- unsigned long flags;
unsigned long pfn = page_to_pfn(page);
int migratetype;
@@ -2453,8 +2450,8 @@ void free_hot_cold_page(struct page *page, bool cold)
migratetype = get_pfnblock_migratetype(page, pfn);
set_pcppage_migratetype(page, migratetype);
- local_irq_save(flags);
- __count_vm_event(PGFREE);
+ preempt_disable();
+ count_vm_event(PGFREE);
/*
* We only track unmovable, reclaimable and movable on pcp lists.
@@ -2484,7 +2481,7 @@ void free_hot_cold_page(struct page *page, bool cold)
}
out:
- local_irq_restore(flags);
+ preempt_enable();
}
/*
@@ -2647,9 +2644,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone,
struct list_head *list;
bool cold = ((gfp_flags & __GFP_COLD) != 0);
struct page *page;
- unsigned long flags;
- local_irq_save(flags);
+ preempt_disable();
pcp = &this_cpu_ptr(zone->pageset)->pcp;
list = &pcp->lists[migratetype];
page = __rmqueue_pcplist(zone, order, gfp_flags, migratetype,
@@ -2658,7 +2654,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone,
__count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order);
zone_statistics(preferred_zone, zone, gfp_flags);
}
- local_irq_restore(flags);
+ preempt_enable();
return page;
}
@@ -2674,7 +2670,7 @@ struct page *rmqueue(struct zone *preferred_zone,
unsigned long flags;
struct page *page;
- if (likely(order == 0))
+ if (likely(order == 0) && !in_interrupt())
return rmqueue_pcplist(preferred_zone, zone, order,
gfp_flags, migratetype);
@@ -3919,7 +3915,7 @@ EXPORT_SYMBOL(get_zeroed_page);
void __free_pages(struct page *page, unsigned int order)
{
if (put_page_testzero(page)) {
- if (order == 0)
+ if (order == 0 && !in_interrupt())
free_hot_cold_page(page, false);
else
__free_pages_ok(page, order);
--
2.11.0
[toc] | [next] | [standalone]
| From | Jesper Dangaard Brouer <brouer@redhat.com> |
|---|---|
| Date | 2017-01-04 15:30 +0100 |
| Subject | Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests |
| Message-ID | <sVUJI-5S7-25@gated-at.bofh.it> |
| In reply to | #1550661 |
On Wed, 4 Jan 2017 11:10:48 +0000 Mel Gorman <mgorman@techsingularity.net> wrote: > Many workloads that allocate pages are not handling an interrupt at a > time. As allocation requests may be from IRQ context, it's necessary to > disable/enable IRQs for every page allocation. This cost is the bulk > of the free path but also a significant percentage of the allocation > path. > > This patch alters the locking and checks such that only irq-safe allocation > requests use the per-cpu allocator. All others acquire the irq-safe > zone->lock and allocate from the buddy allocator. It relies on disabling > preemption to safely access the per-cpu structures. I love this idea and patch :-) > It could be slightly > modified to avoid soft IRQs using it but it's not clear it's worthwhile. NICs usually refill their RX-ring from SoftIRQ context (NAPI). Thus, we do want this optimization to work in softirq. > This modification may slow allocations from IRQ context slightly but the main > gain from the per-cpu allocator is that it scales better for allocations > from multiple contexts. There is an implicit assumption that intensive > allocations from IRQ contexts on multiple CPUs from a single NUMA node are > rare and that the fast majority of scaling issues are encountered in !IRQ > contexts such as page faulting. IHMO, I agree with this implicit assumption. > It's worth noting that this patch is not > required for a bulk page allocator but it significantly reduces the overhead. > > The following is results from a page allocator micro-benchmark. Only > order-0 is interesting as higher orders do not use the per-cpu allocator I'm seeing approx 34% reduction in a order-0 micro-benchmark! amazing! :-) [1] https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/mm/bench/ > 4.10.0-rc2 4.10.0-rc2 > vanilla irqsafe-v1r5 > Amean alloc-odr0-1 287.15 ( 0.00%) 219.00 ( 23.73%) > Amean alloc-odr0-2 221.23 ( 0.00%) 183.23 ( 17.18%) > Amean alloc-odr0-4 187.00 ( 0.00%) 151.38 ( 19.05%) > Amean alloc-odr0-8 167.54 ( 0.00%) 132.77 ( 20.75%) > Amean alloc-odr0-16 156.00 ( 0.00%) 123.00 ( 21.15%) > Amean alloc-odr0-32 149.00 ( 0.00%) 118.31 ( 20.60%) > Amean alloc-odr0-64 138.77 ( 0.00%) 116.00 ( 16.41%) > Amean alloc-odr0-128 145.00 ( 0.00%) 118.00 ( 18.62%) > Amean alloc-odr0-256 136.15 ( 0.00%) 125.00 ( 8.19%) > Amean alloc-odr0-512 147.92 ( 0.00%) 121.77 ( 17.68%) > Amean alloc-odr0-1024 147.23 ( 0.00%) 126.15 ( 14.32%) > Amean alloc-odr0-2048 155.15 ( 0.00%) 129.92 ( 16.26%) > Amean alloc-odr0-4096 164.00 ( 0.00%) 136.77 ( 16.60%) > Amean alloc-odr0-8192 166.92 ( 0.00%) 138.08 ( 17.28%) > Amean alloc-odr0-16384 159.00 ( 0.00%) 138.00 ( 13.21%) > Amean free-odr0-1 165.00 ( 0.00%) 89.00 ( 46.06%) > Amean free-odr0-2 113.00 ( 0.00%) 63.00 ( 44.25%) > Amean free-odr0-4 99.00 ( 0.00%) 54.00 ( 45.45%) > Amean free-odr0-8 88.00 ( 0.00%) 47.38 ( 46.15%) > Amean free-odr0-16 83.00 ( 0.00%) 46.00 ( 44.58%) > Amean free-odr0-32 80.00 ( 0.00%) 44.38 ( 44.52%) > Amean free-odr0-64 72.62 ( 0.00%) 43.00 ( 40.78%) > Amean free-odr0-128 78.00 ( 0.00%) 42.00 ( 46.15%) > Amean free-odr0-256 80.46 ( 0.00%) 57.00 ( 29.16%) > Amean free-odr0-512 96.38 ( 0.00%) 64.69 ( 32.88%) > Amean free-odr0-1024 107.31 ( 0.00%) 72.54 ( 32.40%) > Amean free-odr0-2048 108.92 ( 0.00%) 78.08 ( 28.32%) > Amean free-odr0-4096 113.38 ( 0.00%) 82.23 ( 27.48%) > Amean free-odr0-8192 112.08 ( 0.00%) 82.85 ( 26.08%) > Amean free-odr0-16384 110.38 ( 0.00%) 81.92 ( 25.78%) > Amean total-odr0-1 452.15 ( 0.00%) 308.00 ( 31.88%) > Amean total-odr0-2 334.23 ( 0.00%) 246.23 ( 26.33%) > Amean total-odr0-4 286.00 ( 0.00%) 205.38 ( 28.19%) > Amean total-odr0-8 255.54 ( 0.00%) 180.15 ( 29.50%) > Amean total-odr0-16 239.00 ( 0.00%) 169.00 ( 29.29%) > Amean total-odr0-32 229.00 ( 0.00%) 162.69 ( 28.96%) > Amean total-odr0-64 211.38 ( 0.00%) 159.00 ( 24.78%) > Amean total-odr0-128 223.00 ( 0.00%) 160.00 ( 28.25%) > Amean total-odr0-256 216.62 ( 0.00%) 182.00 ( 15.98%) > Amean total-odr0-512 244.31 ( 0.00%) 186.46 ( 23.68%) > Amean total-odr0-1024 254.54 ( 0.00%) 198.69 ( 21.94%) > Amean total-odr0-2048 264.08 ( 0.00%) 208.00 ( 21.24%) > Amean total-odr0-4096 277.38 ( 0.00%) 219.00 ( 21.05%) > Amean total-odr0-8192 279.00 ( 0.00%) 220.92 ( 20.82%) > Amean total-odr0-16384 269.38 ( 0.00%) 219.92 ( 18.36%) > > This is the alloc, free and total overhead of allocating order-0 pages in > batches of 1 page up to 16384 pages. Avoiding disabling/enabling overhead > massively reduces overhead. Alloc overhead is roughly reduced by 14-20% in > most cases. The free path is reduced by 26-46% and the total reduction > is significant. > [...] > > Similarly, little benefit was seen on networking benchmarks both localhost > and between physical server/clients where other costs dominate. It's > possible that this will only be noticable on very high speed networks. The networking results highly depend on NIC drivers. As you mention in the cover-letter, (1) some drivers (e.g mlx4) alloc high-order pages to work-around order-0 pages and DMA-map being too slow (for their HW use-case), (2) drivers that do use order-0 pages have driver specific page-recycling tricks (e.g. mlx5 and ixgbe). The page_pool target making a more generic recycle mechanism for drivers to use. I'm very excited to see improvements in this area! :-))) -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
[toc] | [prev] | [next] | [standalone]
| From | "Hillf Danton" <hillf.zj@alibaba-inc.com> |
|---|---|
| Date | 2017-01-06 04:30 +0100 |
| Message-ID | <sWto5-420-1@gated-at.bofh.it> |
| In reply to | #1550661 |
On Wednesday, January 04, 2017 7:11 PM Mel Gorman wrote: > @@ -2647,9 +2644,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > struct list_head *list; > bool cold = ((gfp_flags & __GFP_COLD) != 0); > struct page *page; > - unsigned long flags; > > - local_irq_save(flags); > + preempt_disable(); > pcp = &this_cpu_ptr(zone->pageset)->pcp; > list = &pcp->lists[migratetype]; > page = __rmqueue_pcplist(zone, order, gfp_flags, migratetype, > @@ -2658,7 +2654,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > __count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order); > zone_statistics(preferred_zone, zone, gfp_flags); > } > - local_irq_restore(flags); > + preempt_enable(); > return page; > } > With PREEMPT configured, preempt_enable() adds entry point to schedule(). Is that needed when we try to allocate a page? Hillf
[toc] | [prev] | [next] | [standalone]
| From | Mel Gorman <mgorman@techsingularity.net> |
|---|---|
| Date | 2017-01-06 11:20 +0100 |
| Subject | Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests |
| Message-ID | <sWzMS-iN-7@gated-at.bofh.it> |
| In reply to | #1552495 |
On Fri, Jan 06, 2017 at 11:26:46AM +0800, Hillf Danton wrote: > > On Wednesday, January 04, 2017 7:11 PM Mel Gorman wrote: > > @@ -2647,9 +2644,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > struct list_head *list; > > bool cold = ((gfp_flags & __GFP_COLD) != 0); > > struct page *page; > > - unsigned long flags; > > > > - local_irq_save(flags); > > + preempt_disable(); > > pcp = &this_cpu_ptr(zone->pageset)->pcp; > > list = &pcp->lists[migratetype]; > > page = __rmqueue_pcplist(zone, order, gfp_flags, migratetype, > > @@ -2658,7 +2654,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > __count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order); > > zone_statistics(preferred_zone, zone, gfp_flags); > > } > > - local_irq_restore(flags); > > + preempt_enable(); > > return page; > > } > > > With PREEMPT configured, preempt_enable() adds entry point to schedule(). > Is that needed when we try to allocate a page? > Not necessarily but what are you proposing as an alternative? get_cpu() is not an alternative and the point is to avoid disabling interrupts which is a much more expensive operation. -- Mel Gorman SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | "Hillf Danton" <hillf.zj@alibaba-inc.com> |
|---|---|
| Date | 2017-01-09 04:20 +0100 |
| Message-ID | <sXyF3-70e-1@gated-at.bofh.it> |
| In reply to | #1552662 |
> Sent: Friday, January 06, 2017 6:16 PM Mel Gorman wrote: > > On Fri, Jan 06, 2017 at 11:26:46AM +0800, Hillf Danton wrote: > > > > On Wednesday, January 04, 2017 7:11 PM Mel Gorman wrote: > > > @@ -2647,9 +2644,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > struct list_head *list; > > > bool cold = ((gfp_flags & __GFP_COLD) != 0); > > > struct page *page; > > > - unsigned long flags; > > > > > > - local_irq_save(flags); > > > + preempt_disable(); > > > pcp = &this_cpu_ptr(zone->pageset)->pcp; > > > list = &pcp->lists[migratetype]; > > > page = __rmqueue_pcplist(zone, order, gfp_flags, migratetype, > > > @@ -2658,7 +2654,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > __count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order); > > > zone_statistics(preferred_zone, zone, gfp_flags); > > > } > > > - local_irq_restore(flags); > > > + preempt_enable(); > > > return page; > > > } > > > > > With PREEMPT configured, preempt_enable() adds entry point to schedule(). > > Is that needed when we try to allocate a page? > > > > Not necessarily but what are you proposing as an alternative? preempt_enable_no_resched() looks at first glance a choice for us to avoid flipping interrupts. > get_cpu() > is not an alternative and the point is to avoid disabling interrupts > which is a much more expensive operation. > Agree with every word. Hillf
[toc] | [prev] | [next] | [standalone]
| From | Mel Gorman <mgorman@techsingularity.net> |
|---|---|
| Date | 2017-01-09 10:50 +0100 |
| Subject | Re: [PATCH 3/4] mm, page_allocator: Only use per-cpu allocator for irq-safe requests |
| Message-ID | <sXEKt-2tb-17@gated-at.bofh.it> |
| In reply to | #1554027 |
On Mon, Jan 09, 2017 at 11:14:29AM +0800, Hillf Danton wrote: > > Sent: Friday, January 06, 2017 6:16 PM Mel Gorman wrote: > > > > On Fri, Jan 06, 2017 at 11:26:46AM +0800, Hillf Danton wrote: > > > > > > On Wednesday, January 04, 2017 7:11 PM Mel Gorman wrote: > > > > @@ -2647,9 +2644,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > > struct list_head *list; > > > > bool cold = ((gfp_flags & __GFP_COLD) != 0); > > > > struct page *page; > > > > - unsigned long flags; > > > > > > > > - local_irq_save(flags); > > > > + preempt_disable(); > > > > pcp = &this_cpu_ptr(zone->pageset)->pcp; > > > > list = &pcp->lists[migratetype]; > > > > page = __rmqueue_pcplist(zone, order, gfp_flags, migratetype, > > > > @@ -2658,7 +2654,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > > __count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order); > > > > zone_statistics(preferred_zone, zone, gfp_flags); > > > > } > > > > - local_irq_restore(flags); > > > > + preempt_enable(); > > > > return page; > > > > } > > > > > > > With PREEMPT configured, preempt_enable() adds entry point to schedule(). > > > Is that needed when we try to allocate a page? > > > > > > > Not necessarily but what are you proposing as an alternative? > > preempt_enable_no_resched() looks at first glance a choice for us to > avoid flipping interrupts. > Ok, I wasn't sure if you were proposing something more drastic. I can make it this although I have no reason to believe it will really matter. The path should be short enough that it's unlikely a scheduler event would ever occur at that point. Still, no harm in doing what you suggest. Thanks. -- Mel Gorman SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | "Hillf Danton" <hillf.zj@alibaba-inc.com> |
|---|---|
| Date | 2017-01-09 11:00 +0100 |
| Message-ID | <sXEUf-2wC-35@gated-at.bofh.it> |
| In reply to | #1554174 |
On Monday, January 09, 2017 5:48 PM Mel Gorman wrote: > On Mon, Jan 09, 2017 at 11:14:29AM +0800, Hillf Danton wrote: > > > On Friday, January 06, 2017 6:16 PM Mel Gorman wrote: > > > > > > On Fri, Jan 06, 2017 at 11:26:46AM +0800, Hillf Danton wrote: > > > > > > > > On Wednesday, January 04, 2017 7:11 PM Mel Gorman wrote: > > > > > @@ -2647,9 +2644,8 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > > > struct list_head *list; > > > > > bool cold = ((gfp_flags & __GFP_COLD) != 0); > > > > > struct page *page; > > > > > - unsigned long flags; > > > > > > > > > > - local_irq_save(flags); > > > > > + preempt_disable(); > > > > > pcp = &this_cpu_ptr(zone->pageset)->pcp; > > > > > list = &pcp->lists[migratetype]; > > > > > page = __rmqueue_pcplist(zone, order, gfp_flags, migratetype, > > > > > @@ -2658,7 +2654,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone, > > > > > __count_zid_vm_events(PGALLOC, page_zonenum(page), 1 << order); > > > > > zone_statistics(preferred_zone, zone, gfp_flags); > > > > > } > > > > > - local_irq_restore(flags); > > > > > + preempt_enable(); > > > > > return page; > > > > > } > > > > > > > > > With PREEMPT configured, preempt_enable() adds entry point to schedule(). > > > > Is that needed when we try to allocate a page? > > > > > > > > > > Not necessarily but what are you proposing as an alternative? > > > > preempt_enable_no_resched() looks at first glance a choice for us to > > avoid flipping interrupts. > > > > Ok, I wasn't sure if you were proposing something more drastic. I can > make it this although I have no reason to believe it will really matter. > The path should be short enough that it's unlikely a scheduler event > would ever occur at that point. Still, no harm in doing what you > suggest. > If spin, feel free to add Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com> to the patchset.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web