Re: [ATTEND] many topics

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 6067 bytes --]

On Wed, Jan 25 2017, Vlastimil Babka wrote:

> On 01/23/2017 08:34 PM, NeilBrown wrote:
>> On Tue, Jan 24 2017, Theodore Ts'o wrote:
>>
>>> On Sun, Jan 22, 2017 at 10:05:44PM -0800, Matthew Wilcox wrote:
>>>>
>>>> I don't have a clear picture in my mind of when Java promotes objects
>>>> from nursery to tenure
>>>
>>> It's typically on the order of minutes.   :-)
>>>
>>>> ... which is not too different from my lack of
>>>> understanding of what the MM layer considers "temporary" :-)  Is it
>>>> acceptable usage to allocate a SCSI command (guaranteed to be freed
>>>> within 30 seconds) from the temporary area?  Or should it only be used
>>>> for allocations where the thread of control is not going to sleep between
>>>> allocation and freeing?
>>>
>>> What the mm folks have said is that it's to prevent fragmentation.  If
>>> that's the optimization, whether or not you the process is allocating
>>> the memory sleeps for a few hundred milliseconds, or even seconds, is
>>> really in the noise compared with the average lifetime of an inode in
>>> the inode cache, or a page in the page cache....
>>>
>>> Why do you think it matters whether or not we sleep?  I've not heard
>>> any explanation for the assumption for why this might be important.
>>
>> Because "TEMPORARY" implies a limit to the amount of time, and sleeping
>> is the thing that causes a process to take a large amount of time.  It
>> seems like an obvious connection to me.
>
> There's no simple connection to time, it depends on the larger picture - what's 
> the state of the allocator and what other allocations/free's are happening 
> around this one. Perhaps let me try to explain what the flag does and what 
> benefits are expected.

If there is no simple connection to time, then I would discourage use of
the word "TEMPORARY" as that has a strong connection with the concept of time.

>
> GFP_TEMPORARY, compared to GFP_KERNEL, adds __GFP_RECLAIMABLE, which tries to 
> place the allocation within MIGRATE_RECLAIMABLE pageblocks - GFP_KERNEL implies 
> MIGRATE_UNMOVABLE pageblocks, and userspace allocations are typically 
> MIGRATE_MOVABLE. The main goal of this "mobility grouping" is to prevent the 
> unmovable pages spreading all over the memory, making it impossible to get 
> larger blocks by defragmentation (compaction). Ideally we would have all these 
> problematic pages fit neatly into the smallest possible number of pageblocks 
> that can accomodate them. But we can't know in advance how many, and we don't 
> know their lifetimes, so there are various heuristics for relabeling pageblocks 
> between the 3 types as we exceed the existing ones.
>
> Now GFP_TEMPORARY means we tell the allocator about the relatively shorter 
> lifetime, so it places the allocation within the RECLAIMABLE pageblocks, which 
> are also used for slab caches that have shrinkers. The expected benefit of this 
> is that we potentially prevent growing the number of UNMOVABLE pageblocks 
> (either directly by this allocation, or a subsequent GFP_KERNEL one, that would 
> otherwise fit within the existing pageblocks). While the RECLAIMABLE pages also 
> cannot be defragmented (at least currently, there are some proposals for the 
> slab caches...), we can at least shrink them, so the negative impact on 
> compaction is considered less severe in the longer term.

Hmmm...  this seems like a fuzzy heuristic.
I can use GFP_TEMPORARY as long  I'll free the memory eventually, or
there is some way for you to ask me to free the memory, though I don't
have to succeed - every.

If this heuristic actually works, and reduces fragmentation, then I
suspect it is more luck than good management.  You have maybe added
GFP_TEMPORARY in a few places which fit with your understanding of what
you want and which don't ruin the outcomes in your tests.  But without a
strong definition of when it can and cannot be used, it seems quite
likely that someone else will start using it in a way that fits within
your vague statement of requirements, but actually results in much more
fragmentation.

i.e. I think this is a fragile heuristic and not a long term solution
for anything.

I think it would be better if we could discard the idea of "reclaimable"
and just stick with "movable" and "unmovable".  Lots of things are not
movable at present, but could be made movable with relatively little
effort.  Once the interfaces are in place to allow arbitrary kernel code
to find out when things should be moved, I suspect that a lot of
allocations could become movable.

Before we reach that point, there might be some value in the heuristic
that "reclaimable" is sort-of close to "movable", but I don't think
that heuristic should appear in the public interface.  i.e. just 'or' in
__GFP_RECLAIMABLE where you think it is a good idea, and leave big
comment explaining why, and how it can be removed when we have proper
interfaces for moving things.

Thanks,
NeilBrown


>
>> Imagine I want to allocate a large contiguous region in the
>> ZONE_MOVEABLE region.  I find a mostly free region, so I just need to
>> move those last few pages.  If there is a limit on how long a process
>> can sleep while holding an allocation from ZONE_MOVEABLE, then I know
>> how long, at most, I need to wait before those pages become either free
>> or movable.  If those processes can wait indefinitely, then I might have
>> to wait indefinitely to get this large region.
>
> Yeah so this is not relevant, because GFP_TEMPORARY does not make the allocation 
> __GFP_MOVABLE, so it still is not allowed to end up within a ZONE_MOVABLE zone. 
> Unfortunately the issue similar to that you mention does still exist due to 
> uncontrolled pinning of the movable pages, which affects both ZONE_MOVABLE and 
> CMA, but that's another story...
>
>> "temporary" doesn't mean anything without a well defined time limit.
>>
>> But maybe I completely misunderstand.
>
> HTH,
> Vlastimil
>
>> NeilBrown
>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]