Re: [PATCH] mm,page_alloc,cma: conditionally prefer cma pageblocks for movable allocations

[Roman Gushchin]

On Fri, Apr 03, 2020 at 01:34:45PM +0900, Joonsoo Kim wrote:
> 2020ë 4ì 3ì (ê) ìì 4:42, Roman Gushchin <guro@xxxxxx>ëì ìì:
> > > 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