lkml.org 
[lkml]   [2017]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] mm, page_allocator: Only use per-cpu allocator for irq-safe requests
On Thu, Jan 12, 2017 at 06:02:38PM +0100, Vlastimil Babka wrote:
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>
> > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Very promising! But I have some worries. Should we put something like
> VM_BUG_ON(in_interrupt()) into free_hot_cold_page() and rmqueue_pcplist() to
> catch future potential misuses and also document this requirement? Also
> free_hot_cold_page() has other call sites besides __free_pages() and I'm not
> sure if those are all guaranteed to be !IRQ? E.g. free_hot_cold_page_list()
> which is called by release_page() which uses irq-safe lock operations...
>

They are not guaranteed to be !irq but the API is easier to call incorrectly
than it could be. I think the checks can be pushed further down without
excessive overhead.

> Smaller nit below:
>
> > @@ -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);
>
> AFAICS preempt_disable() is enough for using __count_vm_event(), no?
>

It is, I'll fix it.

Thanks.

--
Mel Gorman
SUSE Labs

\
 
 \ /
  Last update: 2017-01-13 11:19    [W:0.050 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site