* Re: [PATCH v3 2/6] mm, gup: Ensure real head page is ref-counted when using hugepages
From: Punit Agrawal @ 2017-05-23 15:43 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: akpm, linux-mm, linux-kernel, linux-arm-kernel, catalin.marinas,
will.deacon, n-horiguchi, kirill.shutemov, mike.kravetz,
steve.capper, mark.rutland, hillf.zj, linux-arch, aneesh.kumar,
Michal Hocko
In-Reply-To: <20170523131312.aim6obne2t5sxtdr@node.shutemov.name>
"Kirill A. Shutemov" <kirill@shutemov.name> writes:
> On Mon, May 22, 2017 at 02:36:00PM +0100, Punit Agrawal wrote:
>> When speculatively taking references to a hugepage using
>> page_cache_add_speculative() in gup_huge_pmd(), it is assumed that the
>> page returned by pmd_page() is the head page. Although normally true,
>> this assumption doesn't hold when the hugepage comprises of successive
>> page table entries such as when using contiguous bit on arm64 at PTE or
>> PMD levels.
>>
>> This can be addressed by ensuring that the page passed to
>> page_cache_add_speculative() is the real head or by de-referencing the
>> head page within the function.
>>
>> We take the first approach to keep the usage pattern aligned with
>> page_cache_get_speculative() where users already pass the appropriate
>> page, i.e., the de-referenced head.
>>
>> Apply the same logic to fix gup_huge_[pud|pgd]() as well.
>
> Hm. Okay. But I'm kinda surprise that this is the only place that need to
> be adjusted.
>
> Have you validated all other pmd_page() use-cases?
I came across the gup issues were found while investigating a failing
test from mce-tests.
I think the problem here is not due to the use of pmd_page() but because
page_cache_[add|get]_speculative() don't ensure they ref-count the head
page as is done in get_page().
Having said that, I had a quick look at the other uses of pmd_page() -
Quite a few of them are followed by an explicit BUG_ON() to check that
the page returned is a head page. All other instances seem to be dealing
with transparent hugepages where contiguous hugepages are not supported.
I don't see any call sites that ring alarm bells.
Did you have any particular part of the code in mind where pmd_page()
usage might be a problem?
Thanks,
Punit
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3.1 4/6] mm/hugetlb: Allow architectures to override huge_pte_clear()
From: Punit Agrawal @ 2017-05-23 14:53 UTC (permalink / raw)
To: Martin Schwidefsky
Cc: akpm, linux-mm, linux-kernel, linux-arm-kernel, catalin.marinas,
will.deacon, n-horiguchi, kirill.shutemov, mike.kravetz,
steve.capper, mark.rutland, linux-arch, aneesh.kumar,
Heiko Carstens, Arnd Bergmann
In-Reply-To: <20170523072629.06163fa6@mschwideX1>
Martin Schwidefsky <schwidefsky@de.ibm.com> writes:
> On Mon, 22 May 2017 17:25:55 +0100
> Punit Agrawal <punit.agrawal@arm.com> wrote:
>
>> When unmapping a hugepage range, huge_pte_clear() is used to clear the
>> page table entries that are marked as not present. huge_pte_clear()
>> internally just ends up calling pte_clear() which does not correctly
>> deal with hugepages consisting of contiguous page table entries.
>>
>> Add a size argument to address this issue and allow architectures to
>> override huge_pte_clear() by wrapping it in a #ifndef block.
>>
>> Update s390 implementation with the size parameter as well.
>>
>> Note that the change only affects huge_pte_clear() - the other generic
>> hugetlb functions don't need any change.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
>> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> Cc: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>>
>> Changes since v3
>>
>> * Drop weak function and use #ifndef block to allow architecture override
>> * Drop unnecessary move of s390 function definition
>>
>> arch/s390/include/asm/hugetlb.h | 2 +-
>> include/asm-generic/hugetlb.h | 4 +++-
>> mm/hugetlb.c | 2 +-
>> 3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
>> index cd546a245c68..c0443500baec 100644
>> --- a/arch/s390/include/asm/hugetlb.h
>> +++ b/arch/s390/include/asm/hugetlb.h
>> @@ -39,7 +39,7 @@ static inline int prepare_hugepage_range(struct file *file,
>> #define arch_clear_hugepage_flags(page) do { } while (0)
>>
>> static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
>> - pte_t *ptep)
>> + pte_t *ptep, unsigned long sz)
>> {
>> if ((pte_val(*ptep) & _REGION_ENTRY_TYPE_MASK) == _REGION_ENTRY_TYPE_R3)
>> pte_val(*ptep) = _REGION3_ENTRY_EMPTY;
>
> For the nop-change for s390:
> Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Applied the tag locally. Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3.1 4/6] mm/hugetlb: Allow architectures to override huge_pte_clear()
From: Punit Agrawal @ 2017-05-23 14:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, Linux-MM, Linux Kernel Mailing List, Linux ARM,
Catalin Marinas, Will Deacon, n-horiguchi, Kirill A . Shutemov,
mike.kravetz, steve.capper, Mark Rutland, linux-arch,
Aneesh Kumar K.V, Martin Schwidefsky, Heiko Carstens
In-Reply-To: <CAK8P3a0WppXFJ5==nymNHeqrKvixpLQ1AetFRGVv9Y3q8kT9Ew@mail.gmail.com>
Arnd Bergmann <arnd@arndb.de> writes:
> On Mon, May 22, 2017 at 6:25 PM, Punit Agrawal <punit.agrawal@arm.com> wrote:
>> When unmapping a hugepage range, huge_pte_clear() is used to clear the
>> page table entries that are marked as not present. huge_pte_clear()
>> internally just ends up calling pte_clear() which does not correctly
>> deal with hugepages consisting of contiguous page table entries.
>>
>> Add a size argument to address this issue and allow architectures to
>> override huge_pte_clear() by wrapping it in a #ifndef block.
>>
>> Update s390 implementation with the size parameter as well.
>>
>> Note that the change only affects huge_pte_clear() - the other generic
>> hugetlb functions don't need any change.
>>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
>> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> Cc: Mike Kravetz <mike.kravetz@oracle.com>
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Thanks, Arnd. I've applied the tag locally.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [Question] Mlocked count will not be decreased
From: Kefeng Wang @ 2017-05-23 14:41 UTC (permalink / raw)
To: linux-mm, linux-kernel, zhongjiang, Qiuxishi, Yisheng Xie,
wangkefeng.wang
Hi All,
Mlocked in meminfo will be increasing with an small testcase, and never be released in mainline,
here is a testcase[1] to reproduce the issue, but the centos7.2/7.3 will not increase.
Is it normal?
Thanks,
Kefeng
[1] testcase
linux:~ # cat test_mlockall.sh
grep Mlocked /proc/meminfo
for j in `seq 0 10`
do
for i in `seq 4 15`
do
./p_mlockall >> log &
done
sleep 0.2
done
grep Mlocked /proc/meminfo
linux:~ # cat p_mlockall.c
#include <sys/mman.h>
#include <stdlib.h>
#include <stdio.h>
#define SPACE_LEN 4096
int main(int argc, char ** argv)
{
int ret;
void *adr = malloc(SPACE_LEN);
if (!adr)
return -1;
ret = mlockall(MCL_CURRENT | MCL_FUTURE);
printf("mlcokall ret = %d\n", ret);
ret = munlockall();
printf("munlcokall ret = %d\n", ret);
free(adr);
return 0;
}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] mm: make kswapd try harder to keep active pages in cache
From: Josef Bacik @ 2017-05-23 14:23 UTC (permalink / raw)
To: akpm, kernel-team, riel, hannes, linux-mm
When testing a slab heavy workload I noticed that we often would barely
reclaim anything at all from slab when kswapd started doing reclaim.
This is because we use the ratio of nr_scanned / nr_lru to determine how
much of slab we should reclaim. But in a slab only/mostly workload we
will not have much page cache to reclaim, and thus our ratio will be
really low and not at all related to where the memory on the system is.
Instead we want to use a ratio of the reclaimable slab to the actual
reclaimable space on the system. That way if we are slab heavy we work
harder to reclaim slab.
The other part of this that hurts is when we are running close to full
memory with our working set. If we start putting a lot of reclaimable
slab pressure on the system (think find /, or some other silliness), we
will happily evict the active pages over the slab cache. This is kind
of backwards as we want to do all that we can to keep the active working
set in memory, and instead evict these short lived objects. The same
thing occurs when say you do a yum update of a few packages while your
working set takes up most of RAM, you end up with inactive lists being
relatively small and so we reclaim active pages even though we could
reclaim these short lived inactive pages.
My approach here is twofold. First, keep track of the difference in
inactive and slab pages since the last time kswapd ran. In the first
run this will just be the overall counts of inactive and slab, but for
each subsequent run we'll have a good idea of where the memory pressure
is coming from. Then we use this information to put pressure on either
the inactive lists or the slab caches, depending on where the pressure
is coming from.
If this optimization does not work, then we fall back to the previous
methods of reclaiming space with a slight adjustment. Instead of using
the overall scan rate of page cache to determine the scan rate for slab,
we instead use the total usage of slab compared to the reclaimable page
cache on the box. This will allow us to put an appropriate amount of
pressure on the slab shrinkers if we are a mostly slab workload.
I have two tests I was using to watch either side of this problem. The
first test kept 2 files that took up 3/4 of the memory, and then started
creating a bunch of empty files. Without this patch we would have to
re-read both files in their entirety at least 3 times during the run.
With this patch the active pages are never evicted.
The second test was a test that would read and stat all the files in a
directory, which again would take up about 3/4 of the memory with slab
cache. Then I cat'ed a 100gib file into /dev/null and checked to see if
any of the files were evicted and verified that none of the files were
evicted.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
mm/vmscan.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 171 insertions(+), 23 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 8ad39bb..481e14e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -110,11 +110,20 @@ struct scan_control {
/* One of the zones is ready for compaction */
unsigned int compaction_ready:1;
+ /* Only reclaim inactive page cache or slab. */
+ unsigned int inactive_only:1;
+
/* Incremented by the number of inactive pages that were scanned */
unsigned long nr_scanned;
/* Number of pages freed so far during a call to shrink_zones() */
unsigned long nr_reclaimed;
+
+ /* Number of inactive pages added since last kswapd run. */
+ unsigned long inactive_diff;
+
+ /* Number of slab pages added since last kswapd run. */
+ unsigned long slab_diff;
};
#ifdef ARCH_HAS_PREFETCH
@@ -308,7 +317,8 @@ EXPORT_SYMBOL(unregister_shrinker);
static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
struct shrinker *shrinker,
unsigned long nr_scanned,
- unsigned long nr_eligible)
+ unsigned long nr_eligible,
+ unsigned long *slab_scanned)
{
unsigned long freed = 0;
unsigned long long delta;
@@ -409,6 +419,9 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
next_deferred -= scanned;
else
next_deferred = 0;
+ if (slab_scanned)
+ (*slab_scanned) += scanned;
+
/*
* move the unused scan count back into the shrinker in a
* manner that handles concurrent updates. If we exhausted the
@@ -455,7 +468,8 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
struct mem_cgroup *memcg,
unsigned long nr_scanned,
- unsigned long nr_eligible)
+ unsigned long nr_eligible,
+ unsigned long *slab_scanned)
{
struct shrinker *shrinker;
unsigned long freed = 0;
@@ -463,9 +477,6 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
if (memcg && (!memcg_kmem_enabled() || !mem_cgroup_online(memcg)))
return 0;
- if (nr_scanned == 0)
- nr_scanned = SWAP_CLUSTER_MAX;
-
if (!down_read_trylock(&shrinker_rwsem)) {
/*
* If we would return 0, our callers would understand that we
@@ -496,7 +507,8 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
if (!(shrinker->flags & SHRINKER_NUMA_AWARE))
sc.nid = 0;
- freed += do_shrink_slab(&sc, shrinker, nr_scanned, nr_eligible);
+ freed += do_shrink_slab(&sc, shrinker, nr_scanned, nr_eligible,
+ slab_scanned);
}
up_read(&shrinker_rwsem);
@@ -515,7 +527,7 @@ void drop_slab_node(int nid)
freed = 0;
do {
freed += shrink_slab(GFP_KERNEL, nid, memcg,
- 1000, 1000);
+ 1000, 1000, NULL);
} while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)) != NULL);
} while (freed > 10);
}
@@ -2102,6 +2114,7 @@ enum scan_balance {
SCAN_FRACT,
SCAN_ANON,
SCAN_FILE,
+ SCAN_INACTIVE,
};
/*
@@ -2128,6 +2141,11 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg,
unsigned long ap, fp;
enum lru_list lru;
+ if (sc->inactive_only) {
+ scan_balance = SCAN_INACTIVE;
+ goto out;
+ }
+
/* If we have no swap space, do not bother scanning anon pages. */
if (!sc->may_swap || mem_cgroup_get_nr_swap_pages(memcg) <= 0) {
scan_balance = SCAN_FILE;
@@ -2292,6 +2310,15 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg,
scan = 0;
}
break;
+ case SCAN_INACTIVE:
+ if (file && !is_active_lru(lru)) {
+ if (scan && size > sc->nr_to_reclaim)
+ scan = sc->nr_to_reclaim;
+ } else {
+ size = 0;
+ scan = 0;
+ }
+ break;
default:
/* Look ma, no brain */
BUG();
@@ -2509,8 +2536,62 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
{
struct reclaim_state *reclaim_state = current->reclaim_state;
unsigned long nr_reclaimed, nr_scanned;
+ unsigned long nr_reclaim, nr_slab, total_high_wmark = 0, nr_inactive;
+ int z;
bool reclaimable = false;
+ bool skip_slab = false;
+
+ nr_slab = sum_zone_node_page_state(pgdat->node_id,
+ NR_SLAB_RECLAIMABLE);
+ nr_inactive = node_page_state(pgdat, NR_INACTIVE_FILE);
+ nr_reclaim = pgdat_reclaimable_pages(pgdat);
+
+ for (z = 0; z < MAX_NR_ZONES; z++) {
+ struct zone *zone = &pgdat->node_zones[z];
+ if (!managed_zone(zone))
+ continue;
+ total_high_wmark += high_wmark_pages(zone);
+ }
+
+ /*
+ * If we don't have a lot of inactive or slab pages then there's no
+ * point in trying to free them exclusively, do the normal scan stuff.
+ */
+ if (nr_inactive < total_high_wmark && nr_slab < total_high_wmark)
+ sc->inactive_only = 0;
+ /*
+ * We don't have historical information, we can't make good decisions
+ * about ratio's and where we should put pressure, so just apply
+ * pressure based on overall consumption ratios.
+ */
+ if (!sc->slab_diff && !sc->inactive_diff)
+ sc->inactive_only = 0;
+
+ /*
+ * We still want to slightly prefer slab over inactive, so if the
+ * inactive on this node is large enough and what is pushing us into
+ * reclaim terretitory then limit our flushing to the inactive list for
+ * the first go around.
+ *
+ * The idea is that with a memcg configured system we will still reclaim
+ * memcg aware shrinkers, which includes the super block shrinkers. So
+ * if our steady state is keeping fs objects in cache for our workload
+ * we'll still put a certain amount of pressure on them anyway. To
+ * avoid evicting things we actually care about we want to skip slab
+ * reclaim altogether.
+ *
+ * However we still want to account for slab and inactive growing at the
+ * same rate, so if that is the case just carry on shrinking inactive
+ * and slab together.
+ */
+ if (nr_inactive > total_high_wmark &&
+ sc->inactive_diff > sc->slab_diff) {
+ unsigned long tmp = sc->inactive_diff >> 1;
+
+ if (tmp >= sc->slab_diff)
+ skip_slab = true;
+ }
do {
struct mem_cgroup *root = sc->target_mem_cgroup;
struct mem_cgroup_reclaim_cookie reclaim = {
@@ -2518,6 +2599,8 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
.priority = sc->priority,
};
unsigned long node_lru_pages = 0;
+ unsigned long slab_reclaimed = 0;
+ unsigned long slab_scanned = 0;
struct mem_cgroup *memcg;
nr_reclaimed = sc->nr_reclaimed;
@@ -2543,10 +2626,27 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
shrink_node_memcg(pgdat, memcg, sc, &lru_pages);
node_lru_pages += lru_pages;
- if (memcg)
- shrink_slab(sc->gfp_mask, pgdat->node_id,
- memcg, sc->nr_scanned - scanned,
- lru_pages);
+ /*
+ * We don't want to put a lot of pressure on all of the
+ * slabs if a memcg is mostly full, so use the ratio of
+ * the lru size to the total reclaimable space on the
+ * system. If we have sc->inactive_only set then we
+ * want to use the ratio of the difference between the
+ * two since the last kswapd run so we apply pressure to
+ * the consumer appropriately.
+ */
+ if (memcg && !skip_slab) {
+ unsigned long numerator = lru_pages;
+ unsigned long denominator = nr_reclaim;
+ if (sc->inactive_only) {
+ numerator = sc->slab_diff;
+ denominator = sc->inactive_diff;
+ }
+ slab_reclaimed +=
+ shrink_slab(sc->gfp_mask, pgdat->node_id,
+ memcg, numerator, denominator,
+ &slab_scanned);
+ }
/* Record the group's reclaim efficiency */
vmpressure(sc->gfp_mask, memcg, false,
@@ -2570,14 +2670,18 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
}
} while ((memcg = mem_cgroup_iter(root, memcg, &reclaim)));
- /*
- * Shrink the slab caches in the same proportion that
- * the eligible LRU pages were scanned.
- */
- if (global_reclaim(sc))
- shrink_slab(sc->gfp_mask, pgdat->node_id, NULL,
- sc->nr_scanned - nr_scanned,
- node_lru_pages);
+ if (!skip_slab && global_reclaim(sc)) {
+ unsigned long numerator = nr_slab;
+ unsigned long denominator = nr_reclaim;
+ if (sc->inactive_only) {
+ numerator = sc->slab_diff;
+ denominator = sc->inactive_diff;
+ }
+ slab_reclaimed += shrink_slab(sc->gfp_mask,
+ pgdat->node_id, NULL,
+ numerator, denominator,
+ &slab_scanned);
+ }
if (reclaim_state) {
sc->nr_reclaimed += reclaim_state->reclaimed_slab;
@@ -2589,9 +2693,27 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
sc->nr_scanned - nr_scanned,
sc->nr_reclaimed - nr_reclaimed);
- if (sc->nr_reclaimed - nr_reclaimed)
+ if (sc->nr_reclaimed - nr_reclaimed) {
reclaimable = true;
+ } else if (sc->inactive_only && !skip_slab) {
+ unsigned long percent;
+ /*
+ * We didn't reclaim anything this go around, so the
+ * inactive list is likely spent. If we're reclaiming
+ * less than half of the objects in slab that we're
+ * scanning then just stop doing the inactive only scan.
+ * Otherwise ramp up the pressure on the slab caches
+ * hoping that eventually we'll start freeing enough
+ * objects to reclaim space.
+ */
+ percent = (slab_reclaimed * 100 / slab_scanned);
+ if (percent < 50)
+ sc->inactive_only = 0;
+ else
+ nr_slab <<= 1;
+ }
+ skip_slab = false;
} while (should_continue_reclaim(pgdat, sc->nr_reclaimed - nr_reclaimed,
sc->nr_scanned - nr_scanned, sc));
@@ -3234,7 +3356,8 @@ static bool kswapd_shrink_node(pg_data_t *pgdat,
* or lower is eligible for reclaim until at least one usable zone is
* balanced.
*/
-static int balance_pgdat(pg_data_t *pgdat, int order, int classzone_idx)
+static int balance_pgdat(pg_data_t *pgdat, int order, int classzone_idx,
+ unsigned long inactive_diff, unsigned long slab_diff)
{
int i;
unsigned long nr_soft_reclaimed;
@@ -3247,6 +3370,9 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int classzone_idx)
.may_writepage = !laptop_mode,
.may_unmap = 1,
.may_swap = 1,
+ .inactive_only = 1,
+ .inactive_diff = inactive_diff,
+ .slab_diff = slab_diff,
};
count_vm_event(PAGEOUTRUN);
@@ -3466,7 +3592,7 @@ static int kswapd(void *p)
unsigned int classzone_idx = MAX_NR_ZONES - 1;
pg_data_t *pgdat = (pg_data_t*)p;
struct task_struct *tsk = current;
-
+ unsigned long nr_slab = 0, nr_inactive = 0;
struct reclaim_state reclaim_state = {
.reclaimed_slab = 0,
};
@@ -3496,6 +3622,7 @@ static int kswapd(void *p)
pgdat->kswapd_order = 0;
pgdat->kswapd_classzone_idx = MAX_NR_ZONES;
for ( ; ; ) {
+ unsigned long slab_diff, inactive_diff;
bool ret;
alloc_order = reclaim_order = pgdat->kswapd_order;
@@ -3523,6 +3650,23 @@ static int kswapd(void *p)
continue;
/*
+ * We want to know where we're adding pages so we can make
+ * smarter decisions about where we're going to put pressure
+ * when shrinking.
+ */
+ slab_diff = sum_zone_node_page_state(pgdat->node_id,
+ NR_SLAB_RECLAIMABLE);
+ inactive_diff = node_page_state(pgdat, NR_INACTIVE_FILE);
+ if (nr_slab > slab_diff)
+ slab_diff = 0;
+ else
+ slab_diff -= nr_slab;
+ if (inactive_diff < nr_inactive)
+ inactive_diff = 0;
+ else
+ inactive_diff -= nr_inactive;
+
+ /*
* Reclaim begins at the requested order but if a high-order
* reclaim fails then kswapd falls back to reclaiming for
* order-0. If that happens, kswapd will consider sleeping
@@ -3532,7 +3676,11 @@ static int kswapd(void *p)
*/
trace_mm_vmscan_kswapd_wake(pgdat->node_id, classzone_idx,
alloc_order);
- reclaim_order = balance_pgdat(pgdat, alloc_order, classzone_idx);
+ reclaim_order = balance_pgdat(pgdat, alloc_order, classzone_idx,
+ inactive_diff, slab_diff);
+ nr_inactive = node_page_state(pgdat, NR_INACTIVE_FILE);
+ nr_slab = sum_zone_node_page_state(pgdat->node_id,
+ NR_SLAB_RECLAIMABLE);
if (reclaim_order < alloc_order)
goto kswapd_try_sleep;
}
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [RFC PATCH] mm, oom: cgroup-aware OOM-killer
From: Johannes Weiner @ 2017-05-23 13:25 UTC (permalink / raw)
To: Michal Hocko
Cc: Roman Gushchin, Vladimir Davydov, Tejun Heo, Li Zefan,
Tetsuo Handa, kernel-team, cgroups, linux-doc, linux-kernel,
linux-mm
In-Reply-To: <20170523070747.GF12813@dhcp22.suse.cz>
On Tue, May 23, 2017 at 09:07:47AM +0200, Michal Hocko wrote:
> On Mon 22-05-17 18:01:16, Roman Gushchin wrote:
> > On Sat, May 20, 2017 at 09:37:29PM +0300, Vladimir Davydov wrote:
> > > On Thu, May 18, 2017 at 05:28:04PM +0100, Roman Gushchin wrote:
> > > ...
> > > > +5-2-4. Cgroup-aware OOM Killer
> > > > +
> > > > +Cgroup v2 memory controller implements a cgroup-aware OOM killer.
> > > > +It means that it treats memory cgroups as memory consumers
> > > > +rather then individual processes. Under the OOM conditions it tries
> > > > +to find an elegible leaf memory cgroup, and kill all processes
> > > > +in this cgroup. If it's not possible (e.g. all processes belong
> > > > +to the root cgroup), it falls back to the traditional per-process
> > > > +behaviour.
> > >
> > > I agree that the current OOM victim selection algorithm is totally
> > > unfair in a system using containers and it has been crying for rework
> > > for the last few years now, so it's great to see this finally coming.
> > >
> > > However, I don't reckon that killing a whole leaf cgroup is always the
> > > best practice. It does make sense when cgroups are used for
> > > containerizing services or applications, because a service is unlikely
> > > to remain operational after one of its processes is gone, but one can
> > > also use cgroups to containerize processes started by a user. Kicking a
> > > user out for one of her process has gone mad doesn't sound right to me.
> >
> > I agree, that it's not always a best practise, if you're not allowed
> > to change the cgroup configuration (e.g. create new cgroups).
> > IMHO, this case is mostly covered by using the v1 cgroup interface,
> > which remains unchanged.
>
> But there are features which are v2 only and users might really want to
> use it. So I really do not buy this v2-only argument.
I have to agree here. We won't get around making the leaf killing
opt-in or opt-out in some fashion.
> > > Another example when the policy you're suggesting fails in my opinion is
> > > in case a service (cgroup) consists of sub-services (sub-cgroups) that
> > > run processes. The main service may stop working normally if one of its
> > > sub-services is killed. So it might make sense to kill not just an
> > > individual process or a leaf cgroup, but the whole main service with all
> > > its sub-services.
> >
> > I agree, although I do not pretend for solving all possible
> > userspace problems caused by an OOM.
> >
> > How to react on an OOM - is definitely a policy, which depends
> > on the workload. Nothing is changing here from how it's working now,
> > except now kernel will choose a victim cgroup, and kill the victim cgroup
> > rather than a process.
>
> There is a _big_ difference. The current implementation just tries
> to recover from the OOM situation without carying much about the
> consequences on the workload. This is the last resort and a services for
> the _system_ to get back to sane state. You are trying to make it more
> clever and workload aware and that is inevitable going to depend on the
> specific workload. I really do think we cannot simply hardcode any
> policy into the kernel for this purpose and that is why I would like to
> see a discussion about how to do that in a more extensible way. This
> might be harder to implement now but it I believe it will turn out
> better longerm.
And that's where I still maintain that this isn't really a policy
change. Because what this code does ISN'T more clever, and the OOM
killer STILL IS a last-resort thing. We don't need any elaborate
just-in-time evaluation of what each entity is worth. We just want to
kill the biggest job, not the biggest MM. Just like you wouldn't want
just the biggest VMA unmapped and freed, since it leaves your process
incoherent, killing one process leaves a job incoherent.
I understand that making it fully configurable is a tempting thought,
because you'd offload all responsibility to userspace. But on the
other hand, this was brought up years ago and nothing has happened
since. And to me this is evidence that nobody really cares all that
much. Because it's still a rather rare event, and there isn't much you
cannot accomplish with periodic score adjustments.
> > > And both kinds of workloads (services/applications and individual
> > > processes run by users) can co-exist on the same host - consider the
> > > default systemd setup, for instance.
> > >
> > > IMHO it would be better to give users a choice regarding what they
> > > really want for a particular cgroup in case of OOM - killing the whole
> > > cgroup or one of its descendants. For example, we could introduce a
> > > per-cgroup flag that would tell the kernel whether the cgroup can
> > > tolerate killing a descendant or not. If it can, the kernel will pick
> > > the fattest sub-cgroup or process and check it. If it cannot, it will
> > > kill the whole cgroup and all its processes and sub-cgroups.
> >
> > The last thing we want to do, is to compare processes with cgroups.
> > I agree, that we can have some option to disable the cgroup-aware OOM at all,
> > mostly for backward-compatibility. But I don't think it should be a
> > per-cgroup configuration option, which we will support forever.
>
> I can clearly see a demand for "this is definitely more important
> container than others so do not kill" usecases. I can also see demand
> for "do not kill this container running for X days". And more are likely
> to pop out.
That can all be done with scoring.
In fact, we HAD the oom killer consider a target's cputime/runtime
before, and David replaced it all with simple scoring in a63d83f427fb
("oom: badness heuristic rewrite").
This was 10 years ago, and nobody has missed anything critical enough
to implement something beyond scoring. So I don't see why we'd need to
do it for cgroups all of a sudden.
They're nothing special, they just group together things we have been
OOM killing for ages. So why shouldn't we use the same config model?
It seems to me, what we need for this patch is 1) a way to toggle
whether the processes and subgroups of a group are interdependent or
independent and 2) configurable OOM scoring per cgroup analogous to
what we have per process already. If a group is marked interdependent
we stop descending into it and evaluate it as one entity. Otherwise,
we go look for victims in its subgroups and individual processes.
Are there real-life usecases that wouldn't be covered by this?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: Kirill A. Shutemov @ 2017-05-23 13:23 UTC (permalink / raw)
To: Jérôme Glisse
Cc: akpm, linux-kernel, linux-mm, John Hubbard, David Nellans,
Kirill A . Shutemov, Dan Williams, Ross Zwisler
In-Reply-To: <20170522165206.6284-9-jglisse@redhat.com>
On Mon, May 22, 2017 at 12:51:59PM -0400, Jerome Glisse wrote:
> A ZONE_DEVICE page that reach a refcount of 1 is free ie no longer
> have any user. For device private pages this is important to catch
> and thus we need to special case put_page() for this.
>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> ---
> include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> kernel/memremap.c | 1 -
> 2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a825dab..11f7bac 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -23,6 +23,7 @@
> #include <linux/page_ext.h>
> #include <linux/err.h>
> #include <linux/page_ref.h>
> +#include <linux/memremap.h>
>
> struct mempolicy;
> struct anon_vma;
> @@ -795,6 +796,20 @@ static inline bool is_device_private_page(const struct page *page)
> return ((page_zonenum(page) == ZONE_DEVICE) &&
> (page->pgmap->type == MEMORY_DEVICE_PRIVATE));
> }
> +
> +static inline void put_zone_device_private_page(struct page *page)
Could you measure how much bloat this change produce?
I would rather make put_zone_device_private_page() non-inline. put_page()
is inlined everewhere. It's beneficial to keep it skinny.
(And I guess it would help solving 0-day reporeted build issue).
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 2/6] mm, gup: Ensure real head page is ref-counted when using hugepages
From: Kirill A. Shutemov @ 2017-05-23 13:13 UTC (permalink / raw)
To: Punit Agrawal
Cc: akpm, linux-mm, linux-kernel, linux-arm-kernel, catalin.marinas,
will.deacon, n-horiguchi, kirill.shutemov, mike.kravetz,
steve.capper, mark.rutland, hillf.zj, linux-arch, aneesh.kumar,
Michal Hocko
In-Reply-To: <20170522133604.11392-3-punit.agrawal@arm.com>
On Mon, May 22, 2017 at 02:36:00PM +0100, Punit Agrawal wrote:
> When speculatively taking references to a hugepage using
> page_cache_add_speculative() in gup_huge_pmd(), it is assumed that the
> page returned by pmd_page() is the head page. Although normally true,
> this assumption doesn't hold when the hugepage comprises of successive
> page table entries such as when using contiguous bit on arm64 at PTE or
> PMD levels.
>
> This can be addressed by ensuring that the page passed to
> page_cache_add_speculative() is the real head or by de-referencing the
> head page within the function.
>
> We take the first approach to keep the usage pattern aligned with
> page_cache_get_speculative() where users already pass the appropriate
> page, i.e., the de-referenced head.
>
> Apply the same logic to fix gup_huge_[pud|pgd]() as well.
Hm. Okay. But I'm kinda surprise that this is the only place that need to
be adjusted.
Have you validated all other pmd_page() use-cases?
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 1/6] mm, gup: Remove broken VM_BUG_ON_PAGE compound check for hugepages
From: Kirill A. Shutemov @ 2017-05-23 13:09 UTC (permalink / raw)
To: Punit Agrawal
Cc: akpm, Will Deacon, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, n-horiguchi, kirill.shutemov, mike.kravetz,
steve.capper, mark.rutland, hillf.zj, linux-arch, aneesh.kumar
In-Reply-To: <20170522133604.11392-2-punit.agrawal@arm.com>
On Mon, May 22, 2017 at 02:35:59PM +0100, Punit Agrawal wrote:
> From: Will Deacon <will.deacon@arm.com>
>
> When operating on hugepages with DEBUG_VM enabled, the GUP code checks the
> compound head for each tail page prior to calling page_cache_add_speculative.
> This is broken, because on the fast-GUP path (where we don't hold any page
> table locks) we can be racing with a concurrent invocation of
> split_huge_page_to_list.
>
> split_huge_page_to_list deals with this race by using page_ref_freeze to
> freeze the page and force concurrent GUPs to fail whilst the component
> pages are modified. This modification includes clearing the compound_head
> field for the tail pages, so checking this prior to a successful call
> to page_cache_add_speculative can lead to false positives: In fact,
> page_cache_add_speculative *already* has this check once the page refcount
> has been successfully updated, so we can simply remove the broken calls
> to VM_BUG_ON_PAGE.
>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Acked-by: Steve Capper <steve.capper@arm.com>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Looks reasonable to me:
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] Patch for remapping pages around the fault page
From: Kirill A. Shutemov @ 2017-05-23 13:07 UTC (permalink / raw)
To: Sarunya Pumma
Cc: rppt, linux-mm, akpm, kirill.shutemov, jack, ross.zwisler, mhocko,
aneesh.kumar, lstoakes, dave.jiang
In-Reply-To: <1495379520-23752-1-git-send-email-sarunya@vt.edu>
On Sun, May 21, 2017 at 11:12:00AM -0400, Sarunya Pumma wrote:
> After the fault handler performs the __do_fault function to read a fault
> page when a page fault occurs, it does not map other pages that have been
> read together with the fault page. This can cause a number of minor page
> faults to be large. Therefore, this patch is developed to remap pages
> around the fault page by aiming to map the pages that have been read
> synchronously or asynchronously with the fault page.
>
> The major function of this patch is the redo_fault_around function. This
> function computes the start and end offsets of the pages to be mapped,
> determines whether to do the page remapping, remaps pages using the
> map_pages function, and returns. In the redo_fault_around function, the
> start and end offsets are computed the same way as the do_fault_around
> function. To determine whether to do the remapping, we determine if the
> pages around the fault page are already mapped. If they are, the remapping
> will not be performed.
>
> As checking every page can be inefficient if a number of pages to be mapped
> is large, we have added a threshold called "vm_nr_rempping" to consider
> whether to check the status of every page around the fault page or just
> some pages. Note that the vm_nr_rempping parameter can be adjusted via the
> Sysctl interface. In the case that a number of pages to be mapped is
> smaller than the vm_nr_rempping threshold, we check all pages around the
> fault page (within the start and end offsets). Otherwise, we check only the
> adjacent pages (left and right).
>
> The page remapping is beneficial when performing the "almost sequential"
> page accesses, where pages are accessed in order but some pages are
> skipped.
>
> The following is one example scenario that we can reduce one page fault
> every 16 page:
>
> Assume that we want to access pages sequentially and skip every page that
> marked as PG_readahead. Assume that the read-ahead size is 32 pages and the
> number of pages to be mapped each time (fault_around_pages) is 16.
>
> When accessing a page at offset 0, a major page fault occurs, so pages from
> page 0 to page 31 is read from the disk to the page cache. With this, page
> 24 is marked as a read-ahead page (PG_readahead). Then only page 0 is
> mapped to the virtual memory space.
>
> When accessing a page at offset 1, a minor page fault occurs, pages from
> page 0 to page 15 will be mapped.
>
> We keep accessing pages until page 31. Note that we skip page 24.
>
> When accessing a page at offset 32, a major page fault occurs. The same
> process will be repeated. The other 32 pages will be read from the disk.
> Only page 32 is mapped. Then a minor page fault at the next page (page
> 33) will occur.
>
> From this example, two page faults occur every 16 page. With this patch, we
> can eliminate the minor page fault in every 16 page.
>
> Thank you very much for your time for reviewing the patch.
>
> Signed-off-by: Sarunya Pumma <sarunya@vt.edu>
Still no performance numbers?
I doubt it's useful. You woundn't get "a number of minor page faults".
The first minor page fault would take faultaround path and map these
pages.
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 2/4] thp: fix MADV_DONTNEED vs. numa balancing race
From: Vlastimil Babka @ 2017-05-23 12:42 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Kirill A. Shutemov, Andrew Morton, linux-mm, linux-kernel
In-Reply-To: <20170516202919.GA2843@redhat.com>
On 05/16/2017 10:29 PM, Andrea Arcangeli wrote:
> On Wed, Apr 12, 2017 at 03:33:35PM +0200, Vlastimil Babka wrote:
>>
>> pmdp_invalidate() does:
>>
>> pmd_t entry = *pmdp;
>> set_pmd_at(vma->vm_mm, address, pmdp, pmd_mknotpresent(entry));
>>
>> so it's not atomic and if CPU sets dirty or accessed in the middle of
>> this, they will be lost?
>
> I agree it looks like the dirty bit can be lost. Furthermore this also
> loses a MMU notifier invalidate that will lead to corruption at the
> secondary MMU level (which will keep using the old protection
> permission, potentially keeping writing to a wrprotected page).
Oh, I didn't paste the whole function, just the pmd manipulation.
There's also a third line:
flush_pmd_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
so there's no missing invalidate, AFAICS? Sorry for the confusion.
>>
>> But I don't see how the other invalidate caller
>> __split_huge_pmd_locked() deals with this either. Andrea, any idea?
>
> The original code I wrote did this in __split_huge_page_map to create
> the "entry" to establish in the pte pagetables:
>
> entry = mk_pte(page + i, vma->vm_page_prot);
> entry = maybe_mkwrite(pte_mkdirty(entry),
> vma);
>
> For anonymous memory the dirty bit is only meaningful for swapping,
> and THP couldn't be swapped so setting it unconditional avoided any
> issue with the pmdp_invalidate; pmdp_establish.
Yeah, but now we are going to swap THP's, and we have shmem THP's...
> pmdp_invalidate is needed primarily to avoid aliasing of two different
> TLB translation pointing from the same virtual address to the the same
> physical address that triggered machine checks (while needing to keep
> the pmd huge at all times, back then it was also splitting huge,
> splitting is a software bit so userland could still access the data,
> splitting bit only blocked kernel code to manipulate on it similar to
> what migration entry does right now upstream, except those prevent
> userland to access the page during split which is less efficient than
> the splitting bit, but at least it's only used for the physical split,
> back then there was no difference between virtual and physical split
> and physical split is less frequent than the virtual one right now).
This took me a while to grasp, but I think I understand now :)
> It looks like this needs a pmdp_populate that atomically grabs the
> value of the pmd and returns it like pmdp_huge_get_and_clear_notify
> does
pmdp_huge_get_and_clear_notify() is now gone...
> and a _notify variant to use "freeze" is false (if freeze is true
> the MMU notifier invalidate must have happened when the pmd was set to
> a migration entry). If pmdp_populate_notify (freeze==true)
> /pmd_populate (freeze==false) would return the old pmd value
> atomically with xchg() (just instead of setting it to 0 we should set
> it to the mknotpresent one), then we can set the dirty bit on the ptes
> (__split_huge_pmd_locked) or in the pmd itself in the change_huge_pmd
> accordingly.
I think the confusion was partially caused by the comment at the
original caller of pmdp_invalidate():
we first mark the
* current pmd notpresent (atomically because here the pmd_trans_huge
* and pmd_trans_splitting must remain set at all times on the pmd
* until the split is complete for this pmd),
It says "atomically" but in fact that only means that the pmd_trans_huge
and pmd_trans_splitting flags are not temporarily cleared at any point
of time, right? It's not a true atomic swap.
> If the "dirty" flag information is obtained by the pmd read before
> calling pmdp_invalidate is not ok (losing _notify also not ok).
Right.
> Thanks!
> Andrea
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v2 1/6] mm, page_alloc: fix more premature OOM due to race with cpuset update
From: Vlastimil Babka @ 2017-05-23 11:32 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-mm, linux-api, linux-kernel, cgroups,
Li Zefan, Mel Gorman, David Rientjes, Christoph Lameter,
Hugh Dickins, Andrea Arcangeli, Anshuman Khandual,
Kirill A. Shutemov
In-Reply-To: <20170519115110.GB29839@dhcp22.suse.cz>
On 05/19/2017 01:51 PM, Michal Hocko wrote:
> On Wed 17-05-17 10:11:35, Vlastimil Babka wrote:
>> Commit e47483bca2cc ("mm, page_alloc: fix premature OOM when racing with cpuset
>> mems update") has fixed known recent regressions found by LTP's cpuset01
>> testcase. I have however found that by modifying the testcase to use per-vma
>> mempolicies via bind(2) instead of per-task mempolicies via set_mempolicy(2),
>> the premature OOM still happens and the issue is much older.
>>
>> The root of the problem is that the cpuset's mems_allowed and mempolicy's
>> nodemask can temporarily have no intersection, thus get_page_from_freelist()
>> cannot find any usable zone. The current semantic for empty intersection is to
>> ignore mempolicy's nodemask and honour cpuset restrictions. This is checked in
>> node_zonelist(), but the racy update can happen after we already passed the
>> check. Such races should be protected by the seqlock task->mems_allowed_seq,
>> but it doesn't work here, because 1) mpol_rebind_mm() does not happen under
>> seqlock for write, and doing so would lead to deadlock, as it takes mmap_sem
>> for write, while the allocation can have mmap_sem for read when it's taking the
>> seqlock for read. And 2) the seqlock cookie of callers of node_zonelist()
>> (alloc_pages_vma() and alloc_pages_current()) is different than the one of
>> __alloc_pages_slowpath(), so there's still a potential race window.
>>
>> This patch fixes the issue by having __alloc_pages_slowpath() check for empty
>> intersection of cpuset and ac->nodemask before OOM or allocation failure. If
>> it's indeed empty, the nodemask is ignored and allocation retried, which mimics
>> node_zonelist(). This works fine, because almost all callers of
>> __alloc_pages_nodemask are obtaining the nodemask via node_zonelist(). The only
>> exception is new_node_page() from hotplug, where the potential violation of
>> nodemask isn't an issue, as there's already a fallback allocation attempt
>> without any nodemask. If there's a future caller that needs to have its specific
>> nodemask honoured over task's cpuset restrictions, we'll have to e.g. add a gfp
>> flag for that.
>>
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>
> Do we want this backported to the stable tree?
I'm not aware of any external report and the problem is there for a long
time.
> OK I do agree this makes some sense as a quick and easy to backport
> workaround.
It might not be that straightforward, the __alloc_pages* stuff has been
through a lot of changes recently, and e.g. the handling of
cpuset_mems_cookie has moved to __alloc_pages_slowpath() in the last
version or two.
So I'm not very enthusiastic about stable here.
> Acked-by: Michal Hocko <mhocko@suse.com?
Thanks!
>
>> ---
>> mm/page_alloc.c | 51 ++++++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 38 insertions(+), 13 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index beb2827fd5de..43aa767c3188 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -3661,6 +3661,39 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
>> return false;
>> }
>>
>> +static inline bool
>> +check_retry_cpuset(int cpuset_mems_cookie, struct alloc_context *ac)
>> +{
>> + /*
>> + * It's possible that cpuset's mems_allowed and the nodemask from
>> + * mempolicy don't intersect. This should be normally dealt with by
>> + * policy_nodemask(), but it's possible to race with cpuset update in
>> + * such a way the check therein was true, and then it became false
>> + * before we got our cpuset_mems_cookie here.
>> + * This assumes that for all allocations, ac->nodemask can come only
>> + * from MPOL_BIND mempolicy (whose documented semantics is to be ignored
>> + * when it does not intersect with the cpuset restrictions) or the
>> + * caller can deal with a violated nodemask.
>> + */
>> + if (cpusets_enabled() && ac->nodemask &&
>> + !cpuset_nodemask_valid_mems_allowed(ac->nodemask)) {
>> + ac->nodemask = NULL;
>> + return true;
>> + }
>> +
>> + /*
>> + * When updating a task's mems_allowed or mempolicy nodemask, it is
>> + * possible to race with parallel threads in such a way that our
>> + * allocation can fail while the mask is being updated. If we are about
>> + * to fail, check if the cpuset changed during allocation and if so,
>> + * retry.
>> + */
>> + if (read_mems_allowed_retry(cpuset_mems_cookie))
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> static inline struct page *
>> __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>> struct alloc_context *ac)
>> @@ -3856,11 +3889,9 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>> &compaction_retries))
>> goto retry;
>>
>> - /*
>> - * It's possible we raced with cpuset update so the OOM would be
>> - * premature (see below the nopage: label for full explanation).
>> - */
>> - if (read_mems_allowed_retry(cpuset_mems_cookie))
>> +
>> + /* Deal with possible cpuset update races before we start OOM killing */
>> + if (check_retry_cpuset(cpuset_mems_cookie, ac))
>> goto retry_cpuset;
>>
>> /* Reclaim has failed us, start killing things */
>> @@ -3879,14 +3910,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>> }
>>
>> nopage:
>> - /*
>> - * When updating a task's mems_allowed or mempolicy nodemask, it is
>> - * possible to race with parallel threads in such a way that our
>> - * allocation can fail while the mask is being updated. If we are about
>> - * to fail, check if the cpuset changed during allocation and if so,
>> - * retry.
>> - */
>> - if (read_mems_allowed_retry(cpuset_mems_cookie))
>> + /* Deal with possible cpuset update races before we fail */
>> + if (check_retry_cpuset(cpuset_mems_cookie, ac))
>> goto retry_cpuset;
>>
>> /*
>> --
>> 2.12.2
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm/oom_kill: count global and memory cgroup oom kills
From: Tetsuo Handa @ 2017-05-23 11:29 UTC (permalink / raw)
To: Konstantin Khlebnikov, Michal Hocko
Cc: linux-mm, Andrew Morton, Tejun Heo, cgroups, linux-kernel,
Vlastimil Babka
In-Reply-To: <f05c5a6d-27df-4080-a0b5-68694d8e4165@yandex-team.ru>
On 2017/05/23 20:05, Konstantin Khlebnikov wrote:
> On 23.05.2017 10:27, Michal Hocko wrote:
>> On Fri 19-05-17 17:22:30, Konstantin Khlebnikov wrote:
>>> Show count of global oom killer invocations in /proc/vmstat and
>>> count of oom kills inside memory cgroup in knob "memory.events"
>>> (in memory.oom_control for v1 cgroup).
>>>
>>> Also describe difference between "oom" and "oom_kill" in memory
>>> cgroup documentation. Currently oom in memory cgroup kills tasks
>>> iff shortage has happened inside page fault.
>>>
>>> These counters helps in monitoring oom kills - for now
>>> the only way is grepping for magic words in kernel log.
>>
>> Have you considered adding memcg's oom alternative for the global case
>> as well. It would be useful to see how many times we hit the OOM
>> condition without killing anything. That could help debugging issues
>> when the OOM killer cannot be invoked (e.g. GFP_NO{FS,IO} contextx)
>> and the system cannot get out of the oom situation.
>
> I think present warn_alloc() should be enough for debugging,
> maybe it should taint kernel in some cases to give a hint for future warnings/bugs.
>
I don't think warn_alloc() is enough. We can fail to warn using warn_alloc(), see
http://lkml.kernel.org/r/1495331504-12480-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp .
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
From: Anshuman Khandual @ 2017-05-23 11:19 UTC (permalink / raw)
To: Vlastimil Babka, Christoph Hellwig, Andrew Morton
Cc: Anshuman Khandual, linux-mm, linux-kernel
In-Reply-To: <09a6bafa-5743-425e-8def-bd9219cd756c@suse.cz>
On 05/23/2017 02:08 PM, Vlastimil Babka wrote:
> On 05/23/2017 09:02 AM, Christoph Hellwig wrote:
>> On Mon, May 22, 2017 at 02:11:49PM -0700, Andrew Morton wrote:
>>> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
>>>
>>>> There are many places where we define size either left shifting integers
>>>> or multiplying 1024s without any generic definition to fall back on. But
>>>> there are couples of (powerpc and lz4) attempts to define these standard
>>>> memory sizes. Lets move these definitions to core VM to make sure that
>>>> all new usage come from these definitions eventually standardizing it
>>>> across all places.
>>> Grep further - there are many more definitions and some may now
>>> generate warnings.
>>>
>>> Newly including mm.h for these things seems a bit heavyweight. I can't
>>> immediately think of a more appropriate place. Maybe printk.h or
>>> kernel.h.
>> IFF we do these kernel.h is the right place. And please also add the
>> MiB & co variants for the binary versions right next to the decimal
>> ones.
> Those defined in the patch are binary, not decimal. Do we even need
> decimal ones?
>
I can define KiB, MiB, .... with the same values as binary.
Did not get about the decimal ones, we need different names
for them holding values which are multiple of 1024 ?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm: Define KB, MB, GB, TB in core VM
From: Anshuman Khandual @ 2017-05-23 11:13 UTC (permalink / raw)
To: Andrew Morton, Anshuman Khandual; +Cc: linux-mm, linux-kernel
In-Reply-To: <20170522141149.9ef84bb0713769f4af0383f0@linux-foundation.org>
On 05/23/2017 02:41 AM, Andrew Morton wrote:
> On Mon, 22 May 2017 16:47:42 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
>
>> There are many places where we define size either left shifting integers
>> or multiplying 1024s without any generic definition to fall back on. But
>> there are couples of (powerpc and lz4) attempts to define these standard
>> memory sizes. Lets move these definitions to core VM to make sure that
>> all new usage come from these definitions eventually standardizing it
>> across all places.
> Grep further - there are many more definitions and some may now
> generate warnings.
Yeah, warning reports started coming in. Will try to change
all of those to follow the new definitions added.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm/oom_kill: count global and memory cgroup oom kills
From: Konstantin Khlebnikov @ 2017-05-23 11:05 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, Tejun Heo, cgroups, linux-kernel,
Vlastimil Babka
In-Reply-To: <20170523072704.GJ12813@dhcp22.suse.cz>
On 23.05.2017 10:27, Michal Hocko wrote:
> On Fri 19-05-17 17:22:30, Konstantin Khlebnikov wrote:
>> Show count of global oom killer invocations in /proc/vmstat and
>> count of oom kills inside memory cgroup in knob "memory.events"
>> (in memory.oom_control for v1 cgroup).
>>
>> Also describe difference between "oom" and "oom_kill" in memory
>> cgroup documentation. Currently oom in memory cgroup kills tasks
>> iff shortage has happened inside page fault.
>>
>> These counters helps in monitoring oom kills - for now
>> the only way is grepping for magic words in kernel log.
>
> Have you considered adding memcg's oom alternative for the global case
> as well. It would be useful to see how many times we hit the OOM
> condition without killing anything. That could help debugging issues
> when the OOM killer cannot be invoked (e.g. GFP_NO{FS,IO} contextx)
> and the system cannot get out of the oom situation.
I think present warn_alloc() should be enough for debugging,
maybe it should taint kernel in some cases to give a hint for future warnings/bugs.
>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>
> Acked-by: Michal Hocko <mhocko@suse.com>
>
>> ---
>> Documentation/cgroup-v2.txt | 12 +++++++++++-
>> include/linux/memcontrol.h | 1 +
>> include/linux/vm_event_item.h | 1 +
>> mm/memcontrol.c | 2 ++
>> mm/oom_kill.c | 6 ++++++
>> mm/vmstat.c | 1 +
>> 6 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/cgroup-v2.txt b/Documentation/cgroup-v2.txt
>> index dc5e2dcdbef4..a742008d76aa 100644
>> --- a/Documentation/cgroup-v2.txt
>> +++ b/Documentation/cgroup-v2.txt
>> @@ -830,9 +830,19 @@ PAGE_SIZE multiple when read back.
>>
>> oom
>>
>> + The number of time the cgroup's memory usage was
>> + reached the limit and allocation was about to fail.
>> + Result could be oom kill, -ENOMEM from any syscall or
>> + completely ignored in cases like disk readahead.
>> + For now oom in memory cgroup kills tasks iff shortage
>> + has happened inside page fault.
>> +
>> + oom_kill
>> +
>> The number of times the OOM killer has been invoked in
>> the cgroup. This may not exactly match the number of
>> - processes killed but should generally be close.
>> + processes killed but should generally be close: each
>> + invocation could kill several processes at once.
>>
>> memory.stat
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index 899949bbb2f9..2cdcebb78b58 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -55,6 +55,7 @@ enum memcg_event_item {
>> MEMCG_HIGH,
>> MEMCG_MAX,
>> MEMCG_OOM,
>> + MEMCG_OOM_KILL,
>> MEMCG_NR_EVENTS,
>> };
>>
>> diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
>> index d84ae90ccd5c..1707e0a7d943 100644
>> --- a/include/linux/vm_event_item.h
>> +++ b/include/linux/vm_event_item.h
>> @@ -41,6 +41,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>> KSWAPD_LOW_WMARK_HIT_QUICKLY, KSWAPD_HIGH_WMARK_HIT_QUICKLY,
>> PAGEOUTRUN, PGROTATED,
>> DROP_PAGECACHE, DROP_SLAB,
>> + OOM_KILL,
>> #ifdef CONFIG_NUMA_BALANCING
>> NUMA_PTE_UPDATES,
>> NUMA_HUGE_PTE_UPDATES,
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 94172089f52f..416024837b81 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -3574,6 +3574,7 @@ static int mem_cgroup_oom_control_read(struct seq_file *sf, void *v)
>>
>> seq_printf(sf, "oom_kill_disable %d\n", memcg->oom_kill_disable);
>> seq_printf(sf, "under_oom %d\n", (bool)memcg->under_oom);
>> + seq_printf(sf, "oom_kill %lu\n", memcg_sum_events(memcg, MEMCG_OOM_KILL));
>> return 0;
>> }
>>
>> @@ -5165,6 +5166,7 @@ static int memory_events_show(struct seq_file *m, void *v)
>> seq_printf(m, "high %lu\n", memcg_sum_events(memcg, MEMCG_HIGH));
>> seq_printf(m, "max %lu\n", memcg_sum_events(memcg, MEMCG_MAX));
>> seq_printf(m, "oom %lu\n", memcg_sum_events(memcg, MEMCG_OOM));
>> + seq_printf(m, "oom_kill %lu\n", memcg_sum_events(memcg, MEMCG_OOM_KILL));
>>
>> return 0;
>> }
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 04c9143a8625..c50bff3c3409 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -873,6 +873,12 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
>> victim = p;
>> }
>>
>> + /* Raise event before sending signal: reaper must see this */
>> + if (!is_memcg_oom(oc))
>> + count_vm_event(OOM_KILL);
>> + else
>> + mem_cgroup_event(oc->memcg, MEMCG_OOM_KILL);
>> +
>> /* Get a reference to safely compare mm after task_unlock(victim) */
>> mm = victim->mm;
>> mmgrab(mm);
>> diff --git a/mm/vmstat.c b/mm/vmstat.c
>> index 76f73670200a..fe80b81a86e0 100644
>> --- a/mm/vmstat.c
>> +++ b/mm/vmstat.c
>> @@ -1018,6 +1018,7 @@ const char * const vmstat_text[] = {
>>
>> "drop_pagecache",
>> "drop_slab",
>> + "oom_kill",
>>
>> #ifdef CONFIG_NUMA_BALANCING
>> "numa_pte_updates",
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 4/6] powerpc/mm: Add devmap support for ppc64
From: Balbir Singh @ 2017-05-23 10:40 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-mm,
Aneesh Kumar K . V
In-Reply-To: <20170523040524.13717-4-oohall@gmail.com>
On Tue, May 23, 2017 at 2:05 PM, Oliver O'Halloran <oohall@gmail.com> wrote:
> Add support for the devmap bit on PTEs and PMDs for PPC64 Book3S. This
> is used to differentiate device backed memory from transparent huge
> pages since they are handled in more or less the same manner by the core
> mm code.
>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> v1 -> v2: Properly differentiate THP and PMD Devmap entries. The
> mm core assumes that pmd_trans_huge() and pmd_devmap() are mutually
> exclusive and v1 had pmd_trans_huge() being true on a devmap pmd.
>
> Aneesh, this has been fleshed out substantially since v1. Can you
> re-review it? Also no explicit gup support is required in this patch
> since devmap support was added generic GUP as a part of making x86 use
> the generic version.
> ---
> arch/powerpc/include/asm/book3s/64/hash-64k.h | 2 +-
> arch/powerpc/include/asm/book3s/64/pgtable.h | 37 ++++++++++++++++++++++++++-
> arch/powerpc/include/asm/book3s/64/radix.h | 2 +-
> arch/powerpc/mm/hugetlbpage.c | 2 +-
> arch/powerpc/mm/pgtable-book3s64.c | 4 +--
> arch/powerpc/mm/pgtable-hash64.c | 4 ++-
> arch/powerpc/mm/pgtable-radix.c | 3 ++-
> arch/powerpc/mm/pgtable_64.c | 2 +-
> 8 files changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> index 9732837aaae8..eaaf613c5347 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> @@ -180,7 +180,7 @@ static inline void mark_hpte_slot_valid(unsigned char *hpte_slot_array,
> */
> static inline int hash__pmd_trans_huge(pmd_t pmd)
> {
> - return !!((pmd_val(pmd) & (_PAGE_PTE | H_PAGE_THP_HUGE)) ==
> + return !!((pmd_val(pmd) & (_PAGE_PTE | H_PAGE_THP_HUGE | _PAGE_DEVMAP)) ==
> (_PAGE_PTE | H_PAGE_THP_HUGE));
> }
Like Aneesh suggested, I think we can probably skip this check here
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 85bc9875c3be..24634e92dd0b 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -79,6 +79,9 @@
>
> #define _PAGE_SOFT_DIRTY _RPAGE_SW3 /* software: software dirty tracking */
> #define _PAGE_SPECIAL _RPAGE_SW2 /* software: special page */
> +#define _PAGE_DEVMAP _RPAGE_SW1
> +#define __HAVE_ARCH_PTE_DEVMAP
> +
> /*
> * Drivers request for cache inhibited pte mapping using _PAGE_NO_CACHE
> * Instead of fixing all of them, add an alternate define which
> @@ -599,6 +602,16 @@ static inline pte_t pte_mkhuge(pte_t pte)
> return pte;
> }
>
> +static inline pte_t pte_mkdevmap(pte_t pte)
> +{
> + return __pte(pte_val(pte) | _PAGE_SPECIAL|_PAGE_DEVMAP);
> +}
> +
> +static inline int pte_devmap(pte_t pte)
> +{
> + return !!(pte_raw(pte) & cpu_to_be64(_PAGE_DEVMAP));
> +}
> +
> static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
> {
> /* FIXME!! check whether this need to be a conditional */
> @@ -963,6 +976,9 @@ static inline pte_t *pmdp_ptep(pmd_t *pmd)
> #define pmd_mk_savedwrite(pmd) pte_pmd(pte_mk_savedwrite(pmd_pte(pmd)))
> #define pmd_clear_savedwrite(pmd) pte_pmd(pte_clear_savedwrite(pmd_pte(pmd)))
>
> +#define pud_pfn(...) (0)
> +#define pgd_pfn(...) (0)
> +
Don't get these bits.. why are they zero?
> #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
> #define pmd_soft_dirty(pmd) pte_soft_dirty(pmd_pte(pmd))
> #define pmd_mksoft_dirty(pmd) pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)))
> @@ -1137,7 +1153,6 @@ static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
> return true;
> }
>
> -
> #define arch_needs_pgtable_deposit arch_needs_pgtable_deposit
> static inline bool arch_needs_pgtable_deposit(void)
> {
> @@ -1146,6 +1161,26 @@ static inline bool arch_needs_pgtable_deposit(void)
> return true;
> }
>
> +static inline pmd_t pmd_mkdevmap(pmd_t pmd)
> +{
> + return pte_pmd(pte_mkdevmap(pmd_pte(pmd)));
> +}
> +
> +static inline int pmd_devmap(pmd_t pmd)
> +{
> + return pte_devmap(pmd_pte(pmd));
> +}
This should be defined only if #ifdef __HAVE_ARCH_PTE_DEVMAP
The rest looks OK
Balbir Singh.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: mm, something wring in page_lock_anon_vma_read()?
From: zhong jiang @ 2017-05-23 10:32 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Hugh Dickins, Xishi Qiu, Andrew Morton, Tejun Heo, Michal Hocko,
Johannes Weiner, Mel Gorman, Michal Hocko, Minchan Kim,
David Rientjes, Joonsoo Kim, aarcange, sumeet.keswani,
Rik van Riel, Linux MM, LKML
In-Reply-To: <aea91199-2b40-85fd-8c93-2d807ed726bd@suse.cz>
On 2017/5/23 17:33, Vlastimil Babka wrote:
> On 05/23/2017 11:21 AM, zhong jiang wrote:
>> On 2017/5/23 0:51, Vlastimil Babka wrote:
>>> On 05/20/2017 05:01 AM, zhong jiang wrote:
>>>> On 2017/5/20 10:40, Hugh Dickins wrote:
>>>>> On Sat, 20 May 2017, Xishi Qiu wrote:
>>>>>> Here is a bug report form redhat: https://bugzilla.redhat.com/show_bug.cgi?id=1305620
>>>>>> And I meet the bug too. However it is hard to reproduce, and
>>>>>> 624483f3ea82598("mm: rmap: fix use-after-free in __put_anon_vma") is not help.
>>>>>>
>>>>>> From the vmcore, it seems that the page is still mapped(_mapcount=0 and _count=2),
>>>>>> and the value of mapping is a valid address(mapping = 0xffff8801b3e2a101),
>>>>>> but anon_vma has been corrupted.
>>>>>>
>>>>>> Any ideas?
>>>>> Sorry, no. I assume that _mapcount has been misaccounted, for example
>>>>> a pte mapped in on top of another pte; but cannot begin tell you where
>>>>> in Red Hat's kernel-3.10.0-229.4.2.el7 that might happen.
>>>>>
>>>>> Hugh
>>>>>
>>>>> .
>>>>>
>>>> Hi, Hugh
>>>>
>>>> I find the following message from the dmesg.
>>>>
>>>> [26068.316592] BUG: Bad rss-counter state mm:ffff8800a7de2d80 idx:1 val:1
>>>>
>>>> I can prove that the __mapcount is misaccount. when task is exited. the rmap
>>>> still exist.
>>> Check if the kernel in question contains this commit: ad33bb04b2a6 ("mm:
>>> thp: fix SMP race condition between THP page fault and MADV_DONTNEED")
>> HI, Vlastimil
>>
>> I miss the patch.
> Try applying it then, there's good chance the error and crash will go
> away. Even if your workload doesn't actually run any madvise(MADV_DONTNEED).
ok , I will try. Thanks
>> when I read the patch. I find the following issue. but I am sure it is right.
>>
>> if (unlikely(pmd_trans_unstable(pmd)))
>> return 0;
>> /*
>> * A regular pmd is established and it can't morph into a huge pmd
>> * from under us anymore at this point because we hold the mmap_sem
>> * read mode and khugepaged takes it in write mode. So now it's
>> * safe to run pte_offset_map().
>> */
>> pte = pte_offset_map(pmd, address);
>>
>> after pmd_trans_unstable call, without any protect method. by the comments,
>> it think the pte_offset_map is safe. before pte_offset_map call, it still may be
>> unstable. it is possible?
> IIRC it's "unstable" wrt possible none->huge->none transition. But once
> we've seen it's a regular pmd via pmd_trans_unstable(), we're safe as a
> transition from regular pmd can't happen.
Thank you for clarify.
Regards
zhongjiang
>> Thanks
>> zhongjiang
>>>> Thanks
>>>> zhongjiang
>>>>
>>>> --
>>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>>> the body to majordomo@kvack.org. For more info on Linux MM,
>>>> see: http://www.linux-mm.org/ .
>>>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>>>
>>> .
>>>
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
> .
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] mm/oom_kill: count global and memory cgroup oom kills
From: Konstantin Khlebnikov @ 2017-05-23 10:32 UTC (permalink / raw)
To: David Rientjes
Cc: Roman Guschin, linux-mm, Andrew Morton, Tejun Heo, cgroups,
linux-kernel, Vlastimil Babka, Michal Hocko, hannes
In-Reply-To: <alpine.DEB.2.10.1705230044590.50796@chino.kir.corp.google.com>
On 23.05.2017 10:49, David Rientjes wrote:
> On Mon, 22 May 2017, Konstantin Khlebnikov wrote:
>
>> Nope, they are different. I think we should rephase documentation somehow
>>
>> low - count of reclaims below low level
>> high - count of post-allocation reclaims above high level
>> max - count of direct reclaims
>> oom - count of failed direct reclaims
>> oom_kill - count of oom killer invocations and killed processes
>>
>
> In our kernel, we've maintained counts of oom kills per memcg for years as
> part of memory.oom_control for memcg v1, but we've also found it helpful
> to complement that with another count that specifies the number of
> processes oom killed that were attached to that exact memcg.
>
> In your patch, oom_kill in memory.oom_control specifies that number of oom
> events that resulted in an oom kill of a process from that hierarchy, but
> not the number of processes killed from a specific memcg (the difference
> between oc->memcg and mem_cgroup_from_task(victim)). Not sure if you
> would also find it helpful.
>
This is worth addition. Let's call it "oom_victim" for short.
It allows to locate leaky part if they are spread over sub-containers within common limit.
But doesn't tell which limit caused this kill. For hierarchical limits this might be not so easy.
I think oom_kill better suits for automatic actions - restart affected hierarchy, increase limits, e.t.c.
But oom_victim allows to determine container affected by global oom killer.
So, probably it's worth to merge them together and increment oom_kill by global killer for victim memcg:
if (!is_memcg_oom(oc)) {
count_vm_event(OOM_KILL);
mem_cgroup_count_vm_event(mm, OOM_KILL);
} else
mem_cgroup_event(oc->memcg, OOM_KILL);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 3/6] mm/hugetlb: add size parameter to huge_pte_offset()
From: kbuild test robot @ 2017-05-23 10:04 UTC (permalink / raw)
To: Punit Agrawal
Cc: kbuild-all, akpm, linux-mm, linux-kernel, linux-arm-kernel,
catalin.marinas, will.deacon, n-horiguchi, kirill.shutemov,
mike.kravetz, steve.capper, mark.rutland, hillf.zj, linux-arch,
aneesh.kumar, Tony Luck, Fenghua Yu, James Hogan, Ralf Baechle,
James E.J. Bottomley, Helge Deller, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
Heiko Carstens, Yoshinori Sato, Rich Felker, David S. Miller,
Chris Metcalf, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Alexander Viro, Michal Hocko
In-Reply-To: <20170522133604.11392-4-punit.agrawal@arm.com>
[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]
Hi Punit,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc2 next-20170523]
[cannot apply to mmotm/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Punit-Agrawal/Support-for-contiguous-pte-hugepages/20170523-142407
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_get_and_clear':
>> arch/arm64/mm/hugetlbpage.c:200:10: error: too few arguments to function 'huge_pte_offset'
cpte = huge_pte_offset(mm, addr);
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
pte_t *huge_pte_offset(struct mm_struct *mm,
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_set_access_flags':
arch/arm64/mm/hugetlbpage.c:238:10: error: too few arguments to function 'huge_pte_offset'
cpte = huge_pte_offset(vma->vm_mm, addr);
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
pte_t *huge_pte_offset(struct mm_struct *mm,
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_set_wrprotect':
arch/arm64/mm/hugetlbpage.c:263:10: error: too few arguments to function 'huge_pte_offset'
cpte = huge_pte_offset(mm, addr);
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
pte_t *huge_pte_offset(struct mm_struct *mm,
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c: In function 'huge_ptep_clear_flush':
arch/arm64/mm/hugetlbpage.c:280:10: error: too few arguments to function 'huge_pte_offset'
cpte = huge_pte_offset(vma->vm_mm, addr);
^~~~~~~~~~~~~~~
arch/arm64/mm/hugetlbpage.c:135:8: note: declared here
pte_t *huge_pte_offset(struct mm_struct *mm,
^~~~~~~~~~~~~~~
vim +/huge_pte_offset +200 arch/arm64/mm/hugetlbpage.c
66b3923a David Woods 2015-12-17 194 if (pte_cont(*ptep)) {
66b3923a David Woods 2015-12-17 195 int ncontig, i;
66b3923a David Woods 2015-12-17 196 size_t pgsize;
66b3923a David Woods 2015-12-17 197 pte_t *cpte;
66b3923a David Woods 2015-12-17 198 bool is_dirty = false;
66b3923a David Woods 2015-12-17 199
66b3923a David Woods 2015-12-17 @200 cpte = huge_pte_offset(mm, addr);
66b3923a David Woods 2015-12-17 201 ncontig = find_num_contig(mm, addr, cpte, *cpte, &pgsize);
66b3923a David Woods 2015-12-17 202 /* save the 1st pte to return */
66b3923a David Woods 2015-12-17 203 pte = ptep_get_and_clear(mm, addr, cpte);
:::::: The code at line 200 was first introduced by commit
:::::: 66b3923a1a0f77a563b43f43f6ad091354abbfe9 arm64: hugetlb: add support for PTE contiguous bit
:::::: TO: David Woods <dwoods@ezchip.com>
:::::: CC: Will Deacon <will.deacon@arm.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35202 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] dax: Fix race between colliding PMD & PTE entries
From: Jan Kara @ 2017-05-23 9:59 UTC (permalink / raw)
To: Ross Zwisler
Cc: Andrew Morton, linux-kernel, Darrick J. Wong, Alexander Viro,
Christoph Hellwig, Dan Williams, Dave Hansen, Jan Kara,
Matthew Wilcox, linux-fsdevel, linux-mm, linux-nvdimm,
Kirill A . Shutemov, Pawel Lebioda, Dave Jiang, Xiong Zhou,
Eryu Guan, stable
In-Reply-To: <20170522215749.23516-2-ross.zwisler@linux.intel.com>
On Mon 22-05-17 15:57:49, Ross Zwisler wrote:
> We currently have two related PMD vs PTE races in the DAX code. These can
> both be easily triggered by having two threads reading and writing
> simultaneously to the same private mapping, with the key being that private
> mapping reads can be handled with PMDs but private mapping writes are
> always handled with PTEs so that we can COW.
>
> Here is the first race:
>
> CPU 0 CPU 1
>
> (private mapping write)
> __handle_mm_fault()
> create_huge_pmd() - FALLBACK
> handle_pte_fault()
> passes check for pmd_devmap()
>
> (private mapping read)
> __handle_mm_fault()
> create_huge_pmd()
> dax_iomap_pmd_fault() inserts PMD
>
> dax_iomap_pte_fault() does a PTE fault, but we already have a DAX PMD
> installed in our page tables at this spot.
>
> Here's the second race:
>
> CPU 0 CPU 1
>
> (private mapping read)
> __handle_mm_fault()
> passes check for pmd_none()
> create_huge_pmd()
> dax_iomap_pmd_fault() inserts PMD
>
> (private mapping write)
> __handle_mm_fault()
> create_huge_pmd() - FALLBACK
> (private mapping read)
> __handle_mm_fault()
> passes check for pmd_none()
> create_huge_pmd()
>
> handle_pte_fault()
> dax_iomap_pte_fault() inserts PTE
> dax_iomap_pmd_fault() inserts PMD,
> but we already have a PTE at
> this spot.
>
> The core of the issue is that while there is isolation between faults to
> the same range in the DAX fault handlers via our DAX entry locking, there
> is no isolation between faults in the code in mm/memory.c. This means for
> instance that this code in __handle_mm_fault() can run:
>
> if (pmd_none(*vmf.pmd) && transparent_hugepage_enabled(vma)) {
> ret = create_huge_pmd(&vmf);
>
> But by the time we actually get to run the fault handler called by
> create_huge_pmd(), the PMD is no longer pmd_none() because a racing PTE
> fault has installed a normal PMD here as a parent. This is the cause of
> the 2nd race. The first race is similar - there is the following check in
> handle_pte_fault():
>
> } else {
> /* See comment in pte_alloc_one_map() */
> if (pmd_devmap(*vmf->pmd) || pmd_trans_unstable(vmf->pmd))
> return 0;
>
> So if a pmd_devmap() PMD (a DAX PMD) has been installed at vmf->pmd, we
> will bail and retry the fault. This is correct, but there is nothing
> preventing the PMD from being installed after this check but before we
> actually get to the DAX PTE fault handlers.
>
> In my testing these races result in the following types of errors:
>
> BUG: Bad rss-counter state mm:ffff8800a817d280 idx:1 val:1
> BUG: non-zero nr_ptes on freeing mm: 15
>
> Fix this issue by having the DAX fault handlers verify that it is safe to
> continue their fault after they have taken an entry lock to block other
> racing faults.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Pawel Lebioda <pawel.lebioda@intel.com>
> Cc: stable@vger.kernel.org
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
>
> Changes from v1:
> - Handle the failure case in dax_iomap_pte_fault() by retrying the fault
> (Jan).
>
> This series has survived my new xfstest (generic/437) and full xfstest
> regression testing runs.
> ---
> fs/dax.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index c22eaf1..fc62f36 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1155,6 +1155,17 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
> }
>
> /*
> + * It is possible, particularly with mixed reads & writes to private
> + * mappings, that we have raced with a PMD fault that overlaps with
> + * the PTE we need to set up. If so just return and the fault will be
> + * retried.
> + */
> + if (pmd_devmap(*vmf->pmd)) {
> + vmf_ret = VM_FAULT_NOPAGE;
> + goto unlock_entry;
> + }
> +
> + /*
> * Note that we don't bother to use iomap_apply here: DAX required
> * the file system block size to be equal the page size, which means
> * that we never have to deal with more than a single extent here.
> @@ -1398,6 +1409,15 @@ static int dax_iomap_pmd_fault(struct vm_fault *vmf,
> goto fallback;
>
> /*
> + * It is possible, particularly with mixed reads & writes to private
> + * mappings, that we have raced with a PTE fault that overlaps with
> + * the PMD we need to set up. If so we just fall back to a PTE fault
> + * ourselves.
> + */
> + if (!pmd_none(*vmf->pmd))
> + goto unlock_entry;
> +
> + /*
> * Note that we don't use iomap_apply here. We aren't doing I/O, only
> * setting up a mapping, so really we're using iomap_begin() as a way
> * to look up our filesystem block.
> --
> 2.9.4
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/1] Sealable memory support
From: Igor Stoppa @ 2017-05-23 9:43 UTC (permalink / raw)
To: Kees Cook, Casey Schaufler
Cc: Michal Hocko, Dave Hansen, Laura Abbott, Linux-MM,
kernel-hardening@lists.openwall.com, LKML, Daniel Micay
In-Reply-To: <CAGXu5j+3-CZpZ4Vj2fHH+0UPAa_jOdJQxHtrQ=F_FvvzWvE00Q@mail.gmail.com>
On 23/05/17 00:38, Kees Cook wrote:
> On Fri, May 19, 2017 at 3:38 AM, Igor Stoppa <igor.stoppa@huawei.com> wrote:
[...]
> For the first bit of bikeshedding, should this really be called
> seal/unseal? My mind is probably just broken from having read TPM
> documentation, but this isn't really "sealing" as I'd understand it
> (it's not tied to a credential, for example). It's "only" rw/ro.
> Perhaps "protect/unprotect" or just simply "readonly/writable", and
> call the base function "romalloc"?
I was not aware of the specific mean of "seal", in this context.
The term was implicitly proposed by Michal Hocko, while discussing about
the mechanism and I liked it more than what I was using initially:
"lockable".
tbh I like the sound of "smalloc" better than "romalloc"
But this is really the least of my worries :-P
> This is fundamentally a heap allocator, with linked lists, etc. I'd
> like to see as much attention as possible given to hardening it
> against attacks, especially adding redzoning around the metadata at
> least, and perhaps requiring that CONFIG_DEBUG_LIST be enabled.
My initial goal was to provide something that is useful without
affecting performance.
You seem to be pushing for a more extreme approach.
While I have nothing against it and I actually agree that it can be
useful, I would not make it mandatory.
More on this later.
> And as
> part of that, I'd like hardened usercopy to grow knowledge of these
> allocations so we can bounds-check objects. Right now, mm/usercopy.c
> just looks at PageSlab(page) to decide if it should do slab checks. I
> think adding a check for this type of object would be very important
> there.
I am not familiar with this and I need to study it, however I still
think that if there is a significant trade-off in terms of performance
vs resilience, it should be optional, for those who want it.
Maybe there could be a master toggle for these options, if it makes
sense to group them logically. How does this sound?
> The ro/rw granularity here is the _entire_ pool, not a specific
> allocation (or page containing the allocation). I'm concerned that
> makes this very open to race conditions where, especially in the
> global pool, one thread can be trying to write to ro data in a pool
> and another has made the pool writable.
I have the impression we are thinking to something different.
Close, but different enough.
First of all, how using a mutex can create races?
Do you mean with respect of other resources that might be held by
competing users of the pool?
That, imho, is a locking problem that cannot be solved here.
You can try to mitigate it, by reducing the chances it will happen, but
basically you are trying to make do with an user of the API that is not
implementing locking correctly.
I'd say that it's better to fix the user.
If you meant something else, I really didn't get it :-)
More about the frequency of the access: you seem to expect very often
seal/unseal - or lock/unlock, while I don't.
What I envision as primary case for unlocking is the tear down of the
entire pool, for example preceding the unloading of the module.
Think about __ro_after_init : once it is r/o, it stays r/o.
Which also means that there would be possibly a busy transient, when
allocations are made. That is true.
But only for shared pools. If a module uses one pool, I would expect the
initialization of the module to be mostly sequential.
So, no chance of races. Do you agree?
Furthermore, if a module _really_ wants to do parallel allocation, then
maybe it's simpler and cleaner to have one pool per "thread" or whatever
is the mechanism used.
The global pool is mostly there for completing the offering of
__ro_after_init. If one wants ot use it, it's available.
It saves the trouble of having own pool, it means competing with the
rest of the kernel for locking.
If it seems a bad idea, it can be easily removed.
[...]
>> +#include "smalloc.h"
>
> Shouldn't this just be <linux/smalloc.h> ?
yes, I have fixed it in the next revision
[...]
>> + /* If no pool specified, use the global one. */
>> + if (!pool)
>> + pool = global_pool;
>
> It should be impossible, but this should check for global_pool == NULL too, IMO.
I'm curious: why do you think it could happen?
I'd rather throw an exception, if I cannot initialize global_pool.
[...]
>> + if (pool->seal == seal) {
>> + mutex_unlock(&pool->lock);
>> + return;
>
> I actually think this should be a BUG condition, since this means a
> mismatched seal/unseal happened. The pool should never be left
> writable at rest, a user should create a pool, write to it, seal.
On this I agree.
> Any
> updates should unseal, write, seal. To attempt an unseal and find it
> already unsealed seems bad.
But what you are describing seems exactly the case where there could be
a race.
EX: 2 writers try to unseal and add/change something.
They could have a collision both when trying to seal and unseal.
One could argue that there should be an atomic unseal-update-seal and
maybe unseal-allocate-seal.
I would understand the need for that - and maybe that's really how it
should be implemented, for doing what you seem to envision.
But that's where I think we have different use-cases in mind.
I'd rather not add extra locking to something that doesn't need it:
Allocate - write - seal - read, read, read, ... - unseal - destroy.
What you describe is more complex and does need extra logging, but I
would advocate a different implementation, at least initially.
Later, if it makes sense, fold them into one.
> Finding a user for this would help clarify its protection properties,
> too. (The LSM example is likely not the best starting point for that,
> as it would depend on other changes that are under discussion.)
Well, I *have* to do the LSM - and SE Linux too.
That's where this started and why I'm here with this patch.
>From what I could understand about the discussion, the debate about the
changes is settled (is it?).
Casey seemed to be ok with this.
If something else is needed, ok, I can do that too, but I didn't see any
specific reason why LSM should not be good enough as first example.
If you can provide a specific reason why it's not suitable, I can
reconsider.
[...]
>> + if (!pool) {
>> + pr_err("No memory for allocating pool.");
>
> It might be handy to have pools named like they are for the slab allocator.
I considered this.
The only reason I could come up with why it might be desirable is if the
same pool needs to be accessed from two or more places that do not share
the pointer. It doesn't seem particularly useful.
The only upside I can think of is that it would save a memory page, vs
the case of creating 2 separate pools.
It would also introduce locking cross-dependency.
Did I overlook some reason why it would be desirable?
[...]
>> +int smalloc_destroy(struct smalloc_pool *pool)
>> +{
>> + struct list_head *pos, *q;
>> + struct smalloc_node *node;
>> +
>> + if (!pool)
>> + return -EINVAL;
locking was missing, I added it in the new version
also I moved to goto-like error handling, since there were several
similar exit paths.
>> + list_for_each_safe(pos, q, &pool->list) {
>> + node = list_entry(pos, struct smalloc_node, list);
btw, here and in other places I have switched to list_for_each_entry
[...]
> typo: space after "un"
ok
[...]
>> +typedef uint64_t align_t;
>> +
>> +enum seal_t {
>> + SMALLOC_UNSEALED,
>> + SMALLOC_SEALED,
>> +};
>> +
>> +#define __SMALLOC_ALIGNED__ __aligned(sizeof(align_t))
How about the alignment? Is it a desirable feature?
Did I overlook some reason why it would not work?
>> +#define NODE_HEADER \
>> + struct { \
>> + __SMALLOC_ALIGNED__ struct { \
>> + struct list_head list; \
>> + align_t *free; \
>> + unsigned long available_words; \
>> + }; \
>> + }
Does this look ok? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]
> I'd really like to see kernel-doc for the API functions (likely in the .c file).
Yes, I just see no point right now, since the API doesn't seem to be
agreed/finalized yet.
> Thanks again for working on this! If you can find examples of file
> operations living in the heap, those would be great examples for using
> this API (assuming the other properties can be improved).
As I explained above, I would prefer to continue with LSM, unless you
have some specific reason against it.
If you have some example in mind about file operations living on the
heap - which I suspect you do have :) - we could discuss also about
those and if the locking needs to be modified, but from my perspective
the use case is significantly different and I wouldn't pile it up with this.
Of course I might be missing the point you are trying to make.
In that case, I'm afraid that further explanation is needed from you, as
I do not get it :-)
Thanks a lot for the review, this is exactly the sort of early feedback
I was hoping to receive.
--
igor
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [HMM 08/15] mm/ZONE_DEVICE: special case put_page() for device private pages
From: kbuild test robot @ 2017-05-23 9:34 UTC (permalink / raw)
To: Jérôme Glisse
Cc: kbuild-all, akpm, linux-kernel, linux-mm, John Hubbard,
David Nellans, Kirill A . Shutemov, Dan Williams, Ross Zwisler
In-Reply-To: <20170522165206.6284-9-jglisse@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3880 bytes --]
Hi Jerome,
[auto build test ERROR on mmotm/master]
[also build test ERROR on next-20170523]
[cannot apply to v4.12-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/J-r-me-Glisse/HMM-Heterogeneous-Memory-Management-v22/20170523-153623
base: git://git.cmpxchg.org/linux-mmotm.git master
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
In file included from include/linux/mm.h:26:0,
from include/linux/pid_namespace.h:6,
from include/linux/ptrace.h:9,
from arch/xtensa/kernel/asm-offsets.c:21:
>> include/linux/memremap.h:97:5: error: unknown type name 'pmd_t'
pmd_t *pmdp);
^
>> include/linux/memremap.h:112:2: error: unknown type name 'dev_page_fault_t'
dev_page_fault_t page_fault;
^
make[2]: *** [arch/xtensa/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +/pmd_t +97 include/linux/memremap.h
637316dc Jerome Glisse 2017-05-22 91 * This allows the device driver to implement its own memory management.)
637316dc Jerome Glisse 2017-05-22 92 */
637316dc Jerome Glisse 2017-05-22 93 typedef int (*dev_page_fault_t)(struct vm_area_struct *vma,
637316dc Jerome Glisse 2017-05-22 94 unsigned long addr,
637316dc Jerome Glisse 2017-05-22 95 struct page *page,
637316dc Jerome Glisse 2017-05-22 96 unsigned int flags,
637316dc Jerome Glisse 2017-05-22 @97 pmd_t *pmdp);
637316dc Jerome Glisse 2017-05-22 98 typedef void (*dev_page_free_t)(struct page *page, void *data);
637316dc Jerome Glisse 2017-05-22 99
9476df7d Dan Williams 2016-01-15 100 /**
9476df7d Dan Williams 2016-01-15 101 * struct dev_pagemap - metadata for ZONE_DEVICE mappings
637316dc Jerome Glisse 2017-05-22 102 * @page_fault: callback when CPU fault on an unaddressable device page
637316dc Jerome Glisse 2017-05-22 103 * @page_free: free page callback when page refcount reaches 1
4b94ffdc Dan Williams 2016-01-15 104 * @altmap: pre-allocated/reserved memory for vmemmap allocations
5c2c2587 Dan Williams 2016-01-15 105 * @res: physical address range covered by @ref
5c2c2587 Dan Williams 2016-01-15 106 * @ref: reference count that pins the devm_memremap_pages() mapping
9476df7d Dan Williams 2016-01-15 107 * @dev: host device of the mapping for debug
637316dc Jerome Glisse 2017-05-22 108 * @data: private data pointer for page_free()
637316dc Jerome Glisse 2017-05-22 109 * @type: memory type: see MEMORY_* in memory_hotplug.h
9476df7d Dan Williams 2016-01-15 110 */
9476df7d Dan Williams 2016-01-15 111 struct dev_pagemap {
637316dc Jerome Glisse 2017-05-22 @112 dev_page_fault_t page_fault;
637316dc Jerome Glisse 2017-05-22 113 dev_page_free_t page_free;
4b94ffdc Dan Williams 2016-01-15 114 struct vmem_altmap *altmap;
4b94ffdc Dan Williams 2016-01-15 115 const struct resource *res;
:::::: The code at line 97 was first introduced by commit
:::::: 637316dc095146524a922f5429b0b78840bede2d mm/ZONE_DEVICE: new type of ZONE_DEVICE for unaddressable memory v2
:::::: TO: Jerome Glisse <jglisse@redhat.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50173 bytes --]
^ permalink raw reply
* Re: mm, something wring in page_lock_anon_vma_read()?
From: Vlastimil Babka @ 2017-05-23 9:33 UTC (permalink / raw)
To: zhong jiang
Cc: Hugh Dickins, Xishi Qiu, Andrew Morton, Tejun Heo, Michal Hocko,
Johannes Weiner, Mel Gorman, Michal Hocko, Minchan Kim,
David Rientjes, Joonsoo Kim, aarcange, sumeet.keswani,
Rik van Riel, Linux MM, LKML
In-Reply-To: <5923FF31.5020801@huawei.com>
On 05/23/2017 11:21 AM, zhong jiang wrote:
> On 2017/5/23 0:51, Vlastimil Babka wrote:
>> On 05/20/2017 05:01 AM, zhong jiang wrote:
>>> On 2017/5/20 10:40, Hugh Dickins wrote:
>>>> On Sat, 20 May 2017, Xishi Qiu wrote:
>>>>> Here is a bug report form redhat: https://bugzilla.redhat.com/show_bug.cgi?id=1305620
>>>>> And I meet the bug too. However it is hard to reproduce, and
>>>>> 624483f3ea82598("mm: rmap: fix use-after-free in __put_anon_vma") is not help.
>>>>>
>>>>> From the vmcore, it seems that the page is still mapped(_mapcount=0 and _count=2),
>>>>> and the value of mapping is a valid address(mapping = 0xffff8801b3e2a101),
>>>>> but anon_vma has been corrupted.
>>>>>
>>>>> Any ideas?
>>>> Sorry, no. I assume that _mapcount has been misaccounted, for example
>>>> a pte mapped in on top of another pte; but cannot begin tell you where
>>>> in Red Hat's kernel-3.10.0-229.4.2.el7 that might happen.
>>>>
>>>> Hugh
>>>>
>>>> .
>>>>
>>> Hi, Hugh
>>>
>>> I find the following message from the dmesg.
>>>
>>> [26068.316592] BUG: Bad rss-counter state mm:ffff8800a7de2d80 idx:1 val:1
>>>
>>> I can prove that the __mapcount is misaccount. when task is exited. the rmap
>>> still exist.
>> Check if the kernel in question contains this commit: ad33bb04b2a6 ("mm:
>> thp: fix SMP race condition between THP page fault and MADV_DONTNEED")
> HI, Vlastimil
>
> I miss the patch.
Try applying it then, there's good chance the error and crash will go
away. Even if your workload doesn't actually run any madvise(MADV_DONTNEED).
> when I read the patch. I find the following issue. but I am sure it is right.
>
> if (unlikely(pmd_trans_unstable(pmd)))
> return 0;
> /*
> * A regular pmd is established and it can't morph into a huge pmd
> * from under us anymore at this point because we hold the mmap_sem
> * read mode and khugepaged takes it in write mode. So now it's
> * safe to run pte_offset_map().
> */
> pte = pte_offset_map(pmd, address);
>
> after pmd_trans_unstable call, without any protect method. by the comments,
> it think the pte_offset_map is safe. before pte_offset_map call, it still may be
> unstable. it is possible?
IIRC it's "unstable" wrt possible none->huge->none transition. But once
we've seen it's a regular pmd via pmd_trans_unstable(), we're safe as a
transition from regular pmd can't happen.
> Thanks
> zhongjiang
>>> Thanks
>>> zhongjiang
>>>
>>> --
>>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>>> the body to majordomo@kvack.org. For more info on Linux MM,
>>> see: http://www.linux-mm.org/ .
>>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>>
>>
>> .
>>
>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/6] powerpc/mm: Wire up hpte_removebolted for powernv
From: Balbir Singh @ 2017-05-23 9:27 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-mm,
Anton Blanchard
In-Reply-To: <20170523040524.13717-1-oohall@gmail.com>
On Tue, May 23, 2017 at 2:05 PM, Oliver O'Halloran <oohall@gmail.com> wrote:
> From: Anton Blanchard <anton@samba.org>
>
> Adds support for removing bolted (i.e kernel linear mapping) mappings on
> powernv. This is needed to support memory hot unplug operations which
> are required for the teardown of DAX/PMEM devices.
>
> Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> v1 -> v2: Fixed the commit author
> Added VM_WARN_ON() if we attempt to remove an unbolted hpte
> ---
> arch/powerpc/mm/hash_native_64.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/arch/powerpc/mm/hash_native_64.c b/arch/powerpc/mm/hash_native_64.c
> index 65bb8f33b399..b534d041cfe8 100644
> --- a/arch/powerpc/mm/hash_native_64.c
> +++ b/arch/powerpc/mm/hash_native_64.c
> @@ -407,6 +407,38 @@ static void native_hpte_updateboltedpp(unsigned long newpp, unsigned long ea,
> tlbie(vpn, psize, psize, ssize, 0);
> }
>
> +/*
> + * Remove a bolted kernel entry. Memory hotplug uses this.
> + *
> + * No need to lock here because we should be the only user.
> + */
> +static int native_hpte_removebolted(unsigned long ea, int psize, int ssize)
> +{
> + unsigned long vpn;
> + unsigned long vsid;
> + long slot;
> + struct hash_pte *hptep;
> +
> + vsid = get_kernel_vsid(ea, ssize);
> + vpn = hpt_vpn(ea, vsid, ssize);
> +
> + slot = native_hpte_find(vpn, psize, ssize);
> + if (slot == -1)
> + return -ENOENT;
> +
> + hptep = htab_address + slot;
> +
> + VM_WARN_ON(!(be64_to_cpu(hptep->v) & HPTE_V_BOLTED));
> +
> + /* Invalidate the hpte */
> + hptep->v = 0;
> +
> + /* Invalidate the TLB */
> + tlbie(vpn, psize, psize, ssize, 0);
> + return 0;
> +}
> +
Reviewed-by: Balbir Singh <bsingharora@gmail.com>
Balbir Singh.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox