Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1437302

Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when terminating freeing scanner

Show all headers | View raw

On 07/05/2016 11:01 PM, David Rientjes wrote:
> On Thu, 30 Jun 2016, Vlastimil Babka wrote:
> 
>>>  Note: I really dislike the low watermark check in split_free_page() and
>>>  consider it poor software engineering.  The function should split a free
>>>  page, nothing more.  Terminating memory compaction because of a low
>>>  watermark check when we're simply trying to migrate memory seems like an
>>>  arbitrary heuristic.  There was an objection to removing it in the first
>>>  proposed patch, but I think we should really consider removing that
>>>  check so this is simpler.
>>
>> There's a patch changing it to min watermark (you were CC'd on the series). We
>> could argue whether it belongs to split_free_page() or some wrapper of it, but
>> I don't think removing it completely should be done. If zone is struggling
>> with order-0 pages, a functionality for making higher-order pages shouldn't
>> make it even worse. It's also not that arbitrary, even if we succeeded the
>> migration and created a high-order page, the higher-order allocation would
>> still fail due to watermark checks. Worse, __compact_finished() would keep
>> telling the compaction to continue, creating an even longer lag, which is also
>> against your recent patches.
>>
> 
> I'm suggesting we shouldn't check any zone watermark in split_free_page(): 
> that function should just split the free page.
> 
> I don't find our current watermark checks to determine if compaction is 
> worthwhile to be invalid, but I do think that we should avoid checking or 
> acting on any watermark in isolate_freepages() itself.  We could do more 
> effective checking in __compact_finished() to determine if we should 
> terminate compaction, but the freeing scanner feels like the wrong place 
> to do it -- it's also expensive to check while gathering free pages for 
> memory that we have already successfully isolated as part of the 
> iteration.
> 
> Do you have any objection to this fix for 4.7?

No.

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> Joonson and/or Minchan, does this address the issue that you reported?
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[patch for-4.7] mm, compaction: prevent VM_BUG_ON when terminating  freeing scanner David Rientjes <rientjes@google.com> - 2016-06-30 00:00 +0200
  Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Greg KH <greg@kroah.com> - 2016-06-30 00:50 +0200
  Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Vlastimil Babka <vbabka@suse.cz> - 2016-06-30 09:20 +0200
    Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when terminating  freeing scanner David Rientjes <rientjes@google.com> - 2016-07-05 23:10 +0200
      Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Vlastimil Babka <vbabka@suse.cz> - 2016-07-05 23:40 +0200
      Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-06 03:40 +0200
        Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Minchan Kim <minchan@kernel.org> - 2016-07-06 04:00 +0200
  Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-06 03:40 +0200
  Re: [patch for-4.7] mm, compaction: prevent VM_BUG_ON when  terminating freeing scanner Minchan Kim <minchan@kernel.org> - 2016-07-06 08:50 +0200

csiph-web