[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200403174559.GC220160@carbon.lan>
Date: Fri, 3 Apr 2020 10:50:08 -0700
From: Roman Gushchin <guro@...com>
To: Joonsoo Kim <js1304@...il.com>
CC: Andrew Morton <akpm@...ux-foundation.org>,
Vlastimil Babka <vbabka@...e.cz>,
Rik van Riel <riel@...riel.com>,
Linux Memory Management List <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>, <kernel-team@...com>,
Qian Cai <cai@....pw>,
Mel Gorman <mgorman@...hsingularity.net>,
Anshuman Khandual <anshuman.khandual@....com>,
Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: [PATCH] mm,page_alloc,cma: conditionally prefer cma pageblocks
for movable allocations
On Fri, Apr 03, 2020 at 01:34:45PM +0900, Joonsoo Kim wrote:
> 2020년 4월 3일 (금) 오전 4:42, Roman Gushchin <guro@...com>님이 작성:
> > > In fact, I've tested this patch and your fixes for migration problem
> > > and found that there is
> > > still migration problem and failure rate is increased by this patch.
> >
> > Do you mind sharing any details? What kind of pages are those?
>
> I don't investigate more since I had not enough time to do. If I
> remember correctly,
> it's the page used by journaling. I attach my test script below to
> help you reproduce it.
> My test setup is:
> - virtual machine, 8 cpus and 1024 MB mem (256 MB cma mem)
> - ubuntu 16.04 with custom kernel
> - filesystem is ext4
>
> > I'm using the following patch to dump failed pages:
Thank you! I'll take a look.
> >
> > @@ -1455,6 +1455,9 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
> > private, page, pass > 2, mode,
> > reason);
> >
> > + if (rc && reason == MR_CONTIG_RANGE)
> > + dump_page(page, "unmap_and_move");
> > +
> > switch(rc) {
> > case -ENOMEM:
> > /*
> >
> >
> > > However, given that
> > > there is no progress on this area for a long time, I think that
> > > applying the change aggressively
> > > is required to break the current situation.
> >
> > I totally agree!
> >
> > Btw, I've found that cma_release() grabs the cma->lock mutex,
> > so it can't be called from the atomic context (I've got a lockdep warning).
> >
> > Of course, I can change the calling side, but I think it's better to change
> > the cma code to make cma_release() more accepting. What do you think
> > about the following patch?
>
> For 2GB CMA area, we need to check 8192(?) bytes in worst case scenario and
> I don't think it's small enough for spinlock. Even, there is no limit
> on the size of
> the cma area. If cma area is bigger, it takes more. So, I think that
> spinlock() isn't
> good here.
Ok, I'll try to implement the other approach.
Thanks!
Roman
Powered by blists - more mailing lists