On 07/15/2016 09:48 AM, Mel Gorman wrote:
> On Thu, Jul 14, 2016 at 03:40:11PM +0200, Vlastimil Babka wrote:
>>> @@ -4,6 +4,26 @@
>>> #include <linux/huge_mm.h>
>>> #include <linux/swap.h>
>>>
>>> +#ifdef CONFIG_HIGHMEM
>>> +extern atomic_t highmem_file_pages;
>>> +
>>> +static inline void acct_highmem_file_pages(int zid, enum lru_list lru,
>>> + int nr_pages)
>>> +{
>>> + if (is_highmem_idx(zid) && is_file_lru(lru)) {
>>> + if (nr_pages > 0)
>>
>> This seems like a unnecessary branch, atomic_add should handle negative
>> nr_pages just fine?
>>
>
> On x86 it would but the interface makes no guarantees it'll handle
> signed types properly on all architectures.
Hmm really? At least some drivers do that in an easily grepable way:
drivers/tty/serial/dz.c: atomic_add(-1, &mux->map_guard);
drivers/tty/serial/sb1250-duart.c: atomic_add(-1, &duart->map_guard);
drivers/tty/serial/zs.c: atomic_add(-1, &scc->irq_guard);
And our own __mod_zone_page_state() can get both negative and positive
vales and boils down to atomic_long_add() (I assume the long variant wouldn't
be different in this aspect).
>>> @@ -1456,14 +1461,27 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
>>> unsigned long available;
>>> enum compact_result compact_result;
>>>
>>> + if (last_pgdat == zone->zone_pgdat)
>>> + continue;
>>> +
>>> + /*
>>> + * This over-estimates the number of pages available for
>>> + * reclaim/compaction but walking the LRU would take too
>>> + * long. The consequences are that compaction may retry
>>> + * longer than it should for a zone-constrained allocation
>>> + * request.
>>
>> The comment above says that we don't retry zone-constrained at all. Is this
>> an obsolete comment, or does it refer to the ZONE_NORMAL constraint? (as
>> opposed to HIGHMEM, MOVABLE etc?).
>>
>
> It can still over-estimate the amount of memory available if
> ZONE_MOVABLE exists even if the request is not zone-constrained.
OK.
>>> @@ -3454,6 +3455,15 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
>>> return false;
>>>
>>> /*
>>> + * Blindly retry lowmem allocation requests that are often ignored by
>>> + * the OOM killer up to MAX_RECLAIM_RETRIES as we not have a reliable
>>> + * and fast means of calculating reclaimable, dirty and writeback pages
>>> + * in eligible zones.
>>> + */
>>> + if (ac->high_zoneidx < ZONE_NORMAL)
>>> + goto out;
>>
>> A goto inside two nested for cycles? Is there no hope for sanity? :(
>>
>
> None, hand it in at the door.
Mine's long gone, was thinking for the future newbies :)
> It can be pulled out and put past the "return false" at the end. It's
> just not necessarily any better.
I see...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>