* Re: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Wei Wang @ 2017-04-27 6:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev@lists.oasis-open.org, linux-kernel@vger.kernel.org,
qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org,
kvm@vger.kernel.org, linux-mm@kvack.org, david@redhat.com,
Hansen, Dave, cornelia.huck@de.ibm.com, akpm@linux-foundation.org,
mgorman@techsingularity.net, aarcange@redhat.com,
amit.shah@redhat.com, pbonzini@redhat.com,
liliang.opensource@gmail.com
In-Reply-To: <20170426192753-mutt-send-email-mst@kernel.org>
On 04/27/2017 07:20 AM, Michael S. Tsirkin wrote:
> On Wed, Apr 26, 2017 at 11:03:34AM +0000, Wang, Wei W wrote:
>> Hi Michael, could you please give some feedback?
> I'm sorry, I'm not sure feedback on what you are requesting.
Oh, just some trivial things (e.g. use a field in the
header, hdr->chunks to indicate the number of chunks
in the payload) that wasn't confirmed.
I will prepare the new version with fixing the agreed issues,
and we can continue to discuss those parts if you still find
them improper.
>
> The interface looks reasonable now, even though there's
> a way to make it even simpler if we can limit chunk size
> to 2G (in fact 4G - 1). Do you think we can live with this
> limitation?
Yes, I think we can. So, is it good to change to use the
previous 64-bit chunk format (52-bit base + 12-bit size)?
>
> But the code still needs some cleanup.
>
OK. We'll also still to discuss your comments in the patch 05.
Best,
Wei
--
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: Freeing HugeTLB page into buddy allocator
From: Naoya Horiguchi @ 2017-04-27 5:54 UTC (permalink / raw)
To: Anshuman Khandual
Cc: wujianguo@huawei.com, Linux Memory Management List,
Linux Kernel Mailing List
In-Reply-To: <4f609205-fb69-4af5-3235-3abf05aa822a@linux.vnet.ibm.com>
On Tue, Apr 25, 2017 at 02:27:27PM +0530, Anshuman Khandual wrote:
> Hello Jianguo,
>
> In the commit a49ecbcd7b0d5a1cda, it talks about HugeTLB page being
> freed into buddy allocator instead of hugepage_freelists. But if
> I look the code closely for the function unmap_and_move_huge_page()
> it only calls putback_active_hugepage() which puts the page into the
> huge page active list to free up the source HugeTLB page after any
> successful migration. I might be missing something here, so can you
> please point me where we release the HugeTLB page into buddy allocator
> directly during migration ?
Hi Anshuman,
As stated in the patch description, source hugetlb page is freed after
successful migration if overcommit is configured.
The call chain is like below:
soft_offline_huge_page
migrate_pages
unmap_and_move_huge_page
putback_active_hugepage(hpage)
put_page // refcount is down to 0
__put_page
__put_compound_page
free_huge_page
if (h->surplus_huge_pages_node[nid])
update_and_free_page
__free_pages
So the inline comment
+ /* overcommit hugetlb page will be freed to buddy */
might be confusing because at this point the overcommit hugetlb page was
already freed to buddy.
I hope this will help you.
Thanks,
Naoya Horiguchi
>
>
> commit a49ecbcd7b0d5a1cda7d60e03df402dd0ef76ac8
> Author: Jianguo Wu <wujianguo@huawei.com>
> Date: Wed Dec 18 17:08:54 2013 -0800
>
> mm/memory-failure.c: recheck PageHuge() after hugetlb page migrate successfully
>
> After a successful hugetlb page migration by soft offline, the source
> page will either be freed into hugepage_freelists or buddy(over-commit
> page). If page is in buddy, page_hstate(page) will be NULL. It will
> hit a NULL pointer dereference in dequeue_hwpoisoned_huge_page().
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000058
> IP: [<ffffffff81163761>] dequeue_hwpoisoned_huge_page+0x131/0x1d0
> PGD c23762067 PUD c24be2067 PMD 0
> Oops: 0000 [#1] SMP
>
> So check PageHuge(page) after call migrate_pages() successfully.
>
> Signed-off-by: Jianguo Wu <wujianguo@huawei.com>
> Tested-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index b7c1716..db08af9 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1505,10 +1505,16 @@ static int soft_offline_huge_page(struct page *page, int flags)
> if (ret > 0)
> ret = -EIO;
> } else {
> - set_page_hwpoison_huge_page(hpage);
> - dequeue_hwpoisoned_huge_page(hpage);
> - atomic_long_add(1 << compound_order(hpage),
> - &num_poisoned_pages);
> + /* overcommit hugetlb page will be freed to buddy */
> + if (PageHuge(page)) {
> + set_page_hwpoison_huge_page(hpage);
> + dequeue_hwpoisoned_huge_page(hpage);
> + atomic_long_add(1 << compound_order(hpage),
> + &num_poisoned_pages);
> + } else {
> + SetPageHWPoison(page);
> + atomic_long_inc(&num_poisoned_pages);
> + }
> }
> return ret;
> }
>
> Regards
> Anshuman
>
>
--
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 -v10 1/3] mm, THP, swap: Delay splitting THP during swap out
From: Minchan Kim @ 2017-04-27 5:31 UTC (permalink / raw)
To: Huang, Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Andrea Arcangeli,
Ebru Akagunduz, Johannes Weiner, Michal Hocko, Tejun Heo,
Hugh Dickins, Shaohua Li, Rik van Riel, cgroups
In-Reply-To: <20170425125658.28684-2-ying.huang@intel.com>
On Tue, Apr 25, 2017 at 08:56:56PM +0800, Huang, Ying wrote:
> From: Huang Ying <ying.huang@intel.com>
>
> In this patch, splitting huge page is delayed from almost the first
> step of swapping out to after allocating the swap space for the
> THP (Transparent Huge Page) and adding the THP into the swap cache.
> This will batch the corresponding operation, thus improve THP swap out
> throughput.
>
> This is the first step for the THP swap optimization. The plan is to
> delay splitting the THP step by step and avoid splitting the THP
> finally.
>
> The advantages of the THP swap support include:
>
> - Batch the swap operations for the THP and reduce lock
> acquiring/releasing, including allocating/freeing the swap space,
> adding/deleting to/from the swap cache, and writing/reading the swap
> space, etc. This will help to improve the THP swap performance.
>
> - The THP swap space read/write will be 2M sequential IO. It is
> particularly helpful for the swap read, which usually are 4k random
> IO. This will help to improve the THP swap performance.
>
> - It will help the memory fragmentation, especially when the THP is
> heavily used by the applications. The 2M continuous pages will be
> free up after the THP swapping out.
>
> - It will improve the THP utilization on the system with the swap
> turned on. Because the speed for khugepaged to collapse the normal
> pages into the THP is quite slow. After the THP is split during the
> swapping out, it will take quite long time for the normal pages to
> collapse back into the THP after being swapped in. The high THP
> utilization helps the efficiency of the page based memory management
> too.
>
> There are some concerns regarding THP swap in, mainly because possible
> enlarged read/write IO size (for swap in/out) may put more overhead on
> the storage device. To deal with that, the THP swap in should be
> turned on only when necessary. For example, it can be selected via
> "always/never/madvise" logic, to be turned on globally, turned off
> globally, or turned on only for VMA with MADV_HUGEPAGE, etc.
>
> In this patch, one swap cluster is used to hold the contents of each
> THP swapped out. So, the size of the swap cluster is changed to that
> of the THP (Transparent Huge Page) on x86_64 architecture (512). For
> other architectures which want such THP swap optimization,
> ARCH_USES_THP_SWAP_CLUSTER needs to be selected in the Kconfig file
> for the architecture. In effect, this will enlarge swap cluster size
> by 2 times on x86_64. Which may make it harder to find a free cluster
> when the swap space becomes fragmented. So that, this may reduce the
> continuous swap space allocation and sequential write in theory. The
> performance test in 0day shows no regressions caused by this.
What about other architecures?
I mean THP page size on every architectures would be various.
If THP page size is much bigger than 2M, the architecture should
have big swap cluster size for supporting THP swap-out feature.
It means fast empty-swap cluster consumption so that it can suffer
from fragmentation easily which causes THP swap void and swap slot
allocations slow due to not being able to use per-cpu.
What I suggested was contiguous multiple swap cluster allocations
to meet THP page size. If some of architecure's THP size is 64M
and SWAP_CLUSTER_SIZE is 2M, it should allocate 32 contiguos
swap clusters. For that, swap layer need to manage clusters sort
in order which would be more overhead in CONFIG_THP_SWAP case
but I think it's tradeoff. With that, every architectures can
support THP swap easily without arch-specific something.
If (PAGE_SIZE * 512) swap cluster size were okay for most of
architecture, just increase it. It's orthogonal work regardless of
THP swapout. Then, we don't need to manage swap clusters sort
in order in x86_64 which SWAP_CLUSTER_SIZE is equal to
THP_PAGE_SIZE. It's just a bonus by side-effect.
AFAIR, I suggested it but cannot remember why we cannot go with
this way.
>
> In the future of THP swap optimization, some information of the
> swapped out THP (such as compound map count) will be recorded in the
> swap_cluster_info data structure.
>
> The mem cgroup swap accounting functions are enhanced to support
> charge or uncharge a swap cluster backing a THP as a whole.
>
> The swap cluster allocate/free functions are added to allocate/free a
> swap cluster for a THP. A fair simple algorithm is used for swap
> cluster allocation, that is, only the first swap device in priority
> list will be tried to allocate the swap cluster. The function will
> fail if the trying is not successful, and the caller will fallback to
> allocate a single swap slot instead. This works good enough for
> normal cases. If the difference of the number of the free swap
> clusters among multiple swap devices is significant, it is possible
> that some THPs are split earlier than necessary. For example, this
> could be caused by big size difference among multiple swap devices.
>
> The swap cache functions is enhanced to support add/delete THP to/from
> the swap cache as a set of (HPAGE_PMD_NR) sub-pages. This may be
> enhanced in the future with multi-order radix tree. But because we
> will split the THP soon during swapping out, that optimization doesn't
> make much sense for this first step.
>
> The THP splitting functions are enhanced to support to split THP in
> swap cache during swapping out. The page lock will be held during
> allocating the swap cluster, adding the THP into the swap cache and
> splitting the THP. So in the code path other than swapping out, if
> the THP need to be split, the PageSwapCache(THP) will be always false.
>
> The swap cluster is only available for SSD, so the THP swap
> optimization in this patchset has no effect for HDD.
>
> With the patch, the swap out throughput improves 11.5% (from about
> 3.73GB/s to about 4.16GB/s) in the vm-scalability swap-w-seq test case
> with 8 processes. The test is done on a Xeon E5 v3 system. The swap
> device used is a RAM simulated PMEM (persistent memory) device. To
> test the sequential swapping out, the test case creates 8 processes,
> which sequentially allocate and write to the anonymous pages until the
> RAM and part of the swap device is used up.
>
> [hannes@cmpxchg.org: extensive cleanups and simplifications, reduce code size]
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Ebru Akagunduz <ebru.akagunduz@gmail.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Shaohua Li <shli@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: cgroups@vger.kernel.org
> Suggested-by: Andrew Morton <akpm@linux-foundation.org> [for config option]
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> [for changes in huge_memory.c and huge_mm.h]
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> ---
> arch/x86/Kconfig | 1 +
> include/linux/page-flags.h | 7 +-
> include/linux/swap.h | 25 ++++-
> include/linux/swap_cgroup.h | 6 +-
> mm/Kconfig | 12 +++
> mm/huge_memory.c | 11 +-
> mm/memcontrol.c | 50 ++++-----
> mm/shmem.c | 2 +-
> mm/swap_cgroup.c | 40 +++++--
> mm/swap_slots.c | 16 ++-
> mm/swap_state.c | 114 ++++++++++++--------
> mm/swapfile.c | 256 ++++++++++++++++++++++++++++++++------------
> 12 files changed, 375 insertions(+), 165 deletions(-)
< snip >
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1290,7 +1290,7 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc)
> SetPageUptodate(page);
> }
>
> - swap = get_swap_page();
> + swap = get_swap_page(page);
> if (!swap.val)
> goto redirty;
>
If swap is non-ssd, swap.val could be zero. Right?
If so, could we retry like anonymous page swapout?
>
> -swp_entry_t get_swap_page(void)
> +swp_entry_t get_swap_page(struct page *page)
> {
> swp_entry_t entry, *pentry;
> struct swap_slots_cache *cache;
>
> + entry.val = 0;
> +
> + if (PageTransHuge(page)) {
> + if (hpage_nr_pages(page) == SWAPFILE_CLUSTER)
> + get_swap_pages(1, true, &entry);
> + return entry;
> + }
> +
< snip >
> /**
> @@ -178,20 +192,12 @@ int add_to_swap(struct page *page, struct list_head *list)
> VM_BUG_ON_PAGE(!PageLocked(page), page);
> VM_BUG_ON_PAGE(!PageUptodate(page), page);
>
> - entry = get_swap_page();
> +retry:
> + entry = get_swap_page(page);
> if (!entry.val)
> - return 0;
> -
> - if (mem_cgroup_try_charge_swap(page, entry)) {
> - swapcache_free(entry);
> - return 0;
> - }
> -
> - if (unlikely(PageTransHuge(page)))
> - if (unlikely(split_huge_page_to_list(page, list))) {
> - swapcache_free(entry);
> - return 0;
> - }
> + goto fail;
So, with non-SSD swap, THP page *always* get the fail to get swp_entry_t
and retry after split the page. However, it makes unncessary get_swap_pages
call which is not trivial. If there is no SSD swap, thp-swap out should
be void without adding any performance overhead.
Hmm, but I have no good idea to do it simple. :(
--
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 v5 08/11] mm: hwpoison: soft offline supports thp migration
From: Naoya Horiguchi @ 2017-04-27 4:41 UTC (permalink / raw)
To: Zi Yan
Cc: Anshuman Khandual, Zi Yan, kirill.shutemov@linux.intel.com,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
akpm@linux-foundation.org, minchan@kernel.org, vbabka@suse.cz,
mgorman@techsingularity.net, mhocko@kernel.org,
dnellans@nvidia.com
In-Reply-To: <58FA2B85.5040904@cs.rutgers.edu>
On Fri, Apr 21, 2017 at 10:55:49AM -0500, Zi Yan wrote:
>
>
> Anshuman Khandual wrote:
> > On 04/21/2017 02:17 AM, Zi Yan wrote:
> >> From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> >>
> >> This patch enables thp migration for soft offline.
> >>
> >> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> >>
> >> ChangeLog: v1 -> v5:
> >> - fix page isolation counting error
> >>
> >> Signed-off-by: Zi Yan <zi.yan@cs.rutgers.edu>
> >> ---
> >> mm/memory-failure.c | 35 ++++++++++++++---------------------
> >> 1 file changed, 14 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >> index 9b77476ef31f..23ff02eb3ed4 100644
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -1481,7 +1481,17 @@ static struct page *new_page(struct page *p, unsigned long private, int **x)
> >> if (PageHuge(p))
> >> return alloc_huge_page_node(page_hstate(compound_head(p)),
> >> nid);
> >> - else
> >> + else if (thp_migration_supported() && PageTransHuge(p)) {
> >> + struct page *thp;
> >> +
> >> + thp = alloc_pages_node(nid,
> >> + (GFP_TRANSHUGE | __GFP_THISNODE) & ~__GFP_RECLAIM,
> >
> > Why not __GFP_RECLAIM ? Its soft offline path we wait a bit before
> > declaring that THP page cannot be allocated and hence should invoke
> > reclaim methods as well.
>
> I am not sure how much effort the kernel wants to put here to soft
> offline a THP. Naoya knows more here.
What I thought at first was that soft offline is not an urgent user
and no need to reclaim (i.e. give a little impact on other thread.)
But that's not a strong opinion, so if you like __GFP_RECLAIM here,
I'm fine about that.
>
>
> >
> >> + HPAGE_PMD_ORDER);
> >> + if (!thp)
> >> + return NULL;
> >> + prep_transhuge_page(thp);
> >> + return thp;
> >> + } else
> >> return __alloc_pages_node(nid, GFP_HIGHUSER_MOVABLE, 0);
> >> }
> >>
> >> @@ -1665,8 +1675,8 @@ static int __soft_offline_page(struct page *page, int flags)
> >> * cannot have PAGE_MAPPING_MOVABLE.
> >> */
> >> if (!__PageMovable(page))
> >> - inc_node_page_state(page, NR_ISOLATED_ANON +
> >> - page_is_file_cache(page));
> >> + mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
> >> + page_is_file_cache(page), hpage_nr_pages(page));
> >> list_add(&page->lru, &pagelist);
> >> ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
> >> MIGRATE_SYNC, MR_MEMORY_FAILURE);
> >> @@ -1689,28 +1699,11 @@ static int __soft_offline_page(struct page *page, int flags)
> >> static int soft_offline_in_use_page(struct page *page, int flags)
> >> {
> >> int ret;
> >> - struct page *hpage = compound_head(page);
> >> -
> >> - if (!PageHuge(page) && PageTransHuge(hpage)) {
> >> - lock_page(hpage);
> >> - if (!PageAnon(hpage) || unlikely(split_huge_page(hpage))) {
> >> - unlock_page(hpage);
> >> - if (!PageAnon(hpage))
> >> - pr_info("soft offline: %#lx: non anonymous thp\n", page_to_pfn(page));
> >> - else
> >> - pr_info("soft offline: %#lx: thp split failed\n", page_to_pfn(page));
> >> - put_hwpoison_page(hpage);
> >> - return -EBUSY;
> >> - }
> >> - unlock_page(hpage);
> >> - get_hwpoison_page(page);
> >> - put_hwpoison_page(hpage);
> >> - }
> >>
> >> if (PageHuge(page))
> >> ret = soft_offline_huge_page(page, flags);
> >> else
> >> - ret = __soft_offline_page(page, flags);
> >> + ret = __soft_offline_page(compound_head(page), flags);
> >
> > Hmm, what if the THP allocation fails in the new_page() path and
> > we fallback for general page allocation. In that case we will
> > always be still calling with the head page ? Because we dont
> > split the huge page any more.
>
> This could be a problem if the user wants to offline a TailPage but due
> to THP allocation failure, the HeadPage is offlined.
Right, "retry with split" part is unfinished, so we need some improvement.
>
> It may be better to only soft offline THPs if page ==
> compound_head(page). If page != compound_head(page), we still split THPs
> like before.
>
> Because in migrate_pages(), we cannot guarantee any TailPages in that
> THP are migrated (1. THP allocation failure causes THP splitting, then
> only HeadPage is going to be migrated; 2. even if we change existing
> migrate_pages() implementation to add all TailPages to migration list
> instead of LRU list, we still cannot guarantee the TailPage we want to
> migrate is migrated.).
>
> Naoya, what do you think?
Maybe soft offline is a special caller of page migration because it
basically wants to migrate only one page, but thp migration still has
a benefit because we can avoid thp split.
So I like that we try thp migration at first, and if it fails we fall
back to split and migrate (only) a raw error page. This should be done
in caller side for soft offline, because it knows where the error page is.
As for generic case (for other migration callers which mainly want to
migrate multiple pages for their purpose,) thp split and retry can be
done in common migration code. After thp split, all subpages are linked
to migration list, then we retry without returning to the caller.
So I think that split_huge_page() can be moved to (for example) for-loop
in migrate_pages().
I tried to write a patch for it last year, but considering vm event
accounting, the patch might be large (~100 lines).
Thanks,
Naoya Horiguchi
--
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 -v3] mm, swap: Sort swap entries before free
From: Minchan Kim @ 2017-04-27 4:35 UTC (permalink / raw)
To: Huang, Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Hugh Dickins, Shaohua Li,
Rik van Riel
In-Reply-To: <87y3un2vdp.fsf@yhuang-dev.intel.com>
On Wed, Apr 26, 2017 at 08:42:10PM +0800, Huang, Ying wrote:
> Minchan Kim <minchan@kernel.org> writes:
>
> > On Fri, Apr 21, 2017 at 08:29:30PM +0800, Huang, Ying wrote:
> >> "Huang, Ying" <ying.huang@intel.com> writes:
> >>
> >> > Minchan Kim <minchan@kernel.org> writes:
> >> >
> >> >> On Wed, Apr 19, 2017 at 04:14:43PM +0800, Huang, Ying wrote:
> >> >>> Minchan Kim <minchan@kernel.org> writes:
> >> >>>
> >> >>> > Hi Huang,
> >> >>> >
> >> >>> > On Fri, Apr 07, 2017 at 02:49:01PM +0800, Huang, Ying wrote:
> >> >>> >> From: Huang Ying <ying.huang@intel.com>
> >> >>> >>
> >> >>> >> void swapcache_free_entries(swp_entry_t *entries, int n)
> >> >>> >> {
> >> >>> >> struct swap_info_struct *p, *prev;
> >> >>> >> @@ -1075,6 +1083,10 @@ void swapcache_free_entries(swp_entry_t *entries, int n)
> >> >>> >>
> >> >>> >> prev = NULL;
> >> >>> >> p = NULL;
> >> >>> >> +
> >> >>> >> + /* Sort swap entries by swap device, so each lock is only taken once. */
> >> >>> >> + if (nr_swapfiles > 1)
> >> >>> >> + sort(entries, n, sizeof(entries[0]), swp_entry_cmp, NULL);
> >> >>> >
> >> >>> > Let's think on other cases.
> >> >>> >
> >> >>> > There are two swaps and they are configured by priority so a swap's usage
> >> >>> > would be zero unless other swap used up. In case of that, this sorting
> >> >>> > is pointless.
> >> >>> >
> >> >>> > As well, nr_swapfiles is never decreased so if we enable multiple
> >> >>> > swaps and then disable until a swap is remained, this sorting is
> >> >>> > pointelss, too.
> >> >>> >
> >> >>> > How about lazy sorting approach? IOW, if we found prev != p and,
> >> >>> > then we can sort it.
> >> >>>
> >> >>> Yes. That should be better. I just don't know whether the added
> >> >>> complexity is necessary, given the array is short and sort is fast.
> >> >>
> >> >> Huh?
> >> >>
> >> >> 1. swapon /dev/XXX1
> >> >> 2. swapon /dev/XXX2
> >> >> 3. swapoff /dev/XXX2
> >> >> 4. use only one swap
> >> >> 5. then, always pointless sort.
> >> >
> >> > Yes. In this situation we will do unnecessary sorting. What I don't
> >> > know is whether the unnecessary sorting will hurt performance in real
> >> > life. I can do some measurement.
> >>
> >> I tested the patch with 1 swap device and 1 process to eat memory
> >> (remove the "if (nr_swapfiles > 1)" for test). I think this is the
> >> worse case because there is no lock contention. The memory freeing time
> >> increased from 1.94s to 2.12s (increase ~9.2%). So there is some
> >> overhead for some cases. I change the algorithm to something like
> >> below,
> >>
> >> void swapcache_free_entries(swp_entry_t *entries, int n)
> >> {
> >> struct swap_info_struct *p, *prev;
> >> int i;
> >> + swp_entry_t entry;
> >> + unsigned int prev_swp_type;
> >>
> >> if (n <= 0)
> >> return;
> >>
> >> + prev_swp_type = swp_type(entries[0]);
> >> + for (i = n - 1; i > 0; i--) {
> >> + if (swp_type(entries[i]) != prev_swp_type)
> >> + break;
> >> + }
> >
> > That's really what I want to avoid. For many swap usecases,
> > it adds unnecessary overhead.
> >
> >> +
> >> + /* Sort swap entries by swap device, so each lock is only taken once. */
> >> + if (i)
> >> + sort(entries, n, sizeof(entries[0]), swp_entry_cmp, NULL);
> >> prev = NULL;
> >> p = NULL;
> >> for (i = 0; i < n; ++i) {
> >> - p = swap_info_get_cont(entries[i], prev);
> >> + entry = entries[i];
> >> + p = swap_info_get_cont(entry, prev);
> >> if (p)
> >> - swap_entry_free(p, entries[i]);
> >> + swap_entry_free(p, entry);
> >> prev = p;
> >> }
> >> if (p)
> >>
> >> With this patch, the memory freeing time increased from 1.94s to 1.97s.
> >> I think this is good enough. Do you think so?
> >
> > What I mean is as follows(I didn't test it at all):
> >
> > With this, sort entries if we found multiple entries in current
> > entries. It adds some condition checks for non-multiple swap
> > usecase but it would be more cheaper than the sorting.
> > And it adds a [un]lock overhead for multiple swap usecase but
> > it should be a compromise for single-swap usecase which is more
> > popular.
> >
>
> How about the following solution? It can avoid [un]lock overhead and
> double lock issue for multiple swap user case and has good performance
> for one swap user case too.
How worse with approach I suggested compared to as-is?
Unless it's too bad, let's not add more complicated thing to just
enhance the minor usecase in such even *slow* path.
It adds code size/maintainance overead.
With your suggestion, it might enhance a bit with speicific benchmark
but not sure it's really worth for real practice.
>
> Best Regards,
> Huang, Ying
>
> From 7bd903c42749c448ef6acbbdee8dcbc1c5b498b9 Mon Sep 17 00:00:00 2001
> From: Huang Ying <ying.huang@intel.com>
> Date: Thu, 23 Feb 2017 13:05:20 +0800
> Subject: [PATCH -v5] mm, swap: Sort swap entries before free
>
> To reduce the lock contention of swap_info_struct->lock when freeing
> swap entry. The freed swap entries will be collected in a per-CPU
> buffer firstly, and be really freed later in batch. During the batch
> freeing, if the consecutive swap entries in the per-CPU buffer belongs
> to same swap device, the swap_info_struct->lock needs to be
> acquired/released only once, so that the lock contention could be
> reduced greatly. But if there are multiple swap devices, it is
> possible that the lock may be unnecessarily released/acquired because
> the swap entries belong to the same swap device are non-consecutive in
> the per-CPU buffer.
>
> To solve the issue, the per-CPU buffer is sorted according to the swap
> device before freeing the swap entries. Test shows that the time
> spent by swapcache_free_entries() could be reduced after the patch.
>
> With the patch, the memory (some swapped out) free time reduced
> 13.6% (from 2.59s to 2.28s) in the vm-scalability swap-w-rand test
> case with 16 processes. The test is done on a Xeon E5 v3 system. The
> swap device used is a RAM simulated PMEM (persistent memory) device.
> To test swapping, the test case creates 16 processes, which allocate
> and write to the anonymous pages until the RAM and part of the swap
> device is used up, finally the memory (some swapped out) is freed
> before exit.
>
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> Acked-by: Tim Chen <tim.c.chen@intel.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Shaohua Li <shli@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Rik van Riel <riel@redhat.com>
>
> v5:
>
> - Use a smarter way to determine whether sort is necessary.
>
> v4:
>
> - Avoid unnecessary sort if all entries are from one swap device.
>
> v3:
>
> - Add some comments in code per Rik's suggestion.
>
> v2:
>
> - Avoid sort swap entries if there is only one swap device.
> ---
> mm/swapfile.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 71890061f653..10e75f9e8ac1 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -37,6 +37,7 @@
> #include <linux/swapfile.h>
> #include <linux/export.h>
> #include <linux/swap_slots.h>
> +#include <linux/sort.h>
>
> #include <asm/pgtable.h>
> #include <asm/tlbflush.h>
> @@ -1065,20 +1066,52 @@ void swapcache_free(swp_entry_t entry)
> }
> }
>
> +static int swp_entry_cmp(const void *ent1, const void *ent2)
> +{
> + const swp_entry_t *e1 = ent1, *e2 = ent2;
> +
> + return (int)(swp_type(*e1) - swp_type(*e2));
> +}
> +
> void swapcache_free_entries(swp_entry_t *entries, int n)
> {
> struct swap_info_struct *p, *prev;
> - int i;
> + int i, m;
> + swp_entry_t entry;
> + unsigned int prev_swp_type;
>
> if (n <= 0)
> return;
>
> prev = NULL;
> p = NULL;
> - for (i = 0; i < n; ++i) {
> - p = swap_info_get_cont(entries[i], prev);
> - if (p)
> - swap_entry_free(p, entries[i]);
> + m = 0;
> + prev_swp_type = swp_type(entries[0]);
> + for (i = 0; i < n; i++) {
> + entry = entries[i];
> + if (likely(swp_type(entry) == prev_swp_type)) {
> + p = swap_info_get_cont(entry, prev);
> + if (likely(p))
> + swap_entry_free(p, entry);
> + prev = p;
> + } else if (!m)
> + m = i;
> + }
> + if (p)
> + spin_unlock(&p->lock);
> + if (likely(!m))
> + return;
> +
> + /* Sort swap entries by swap device, so each lock is only taken once. */
> + sort(entries + m, n - m, sizeof(entries[0]), swp_entry_cmp, NULL);
> + prev = NULL;
> + for (i = m; i < n; i++) {
> + entry = entries[i];
> + if (swp_type(entry) == prev_swp_type)
> + continue;
> + p = swap_info_get_cont(entry, prev);
> + if (likely(p))
> + swap_entry_free(p, entry);
> prev = p;
> }
> if (p)
> --
> 2.11.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/vmstat: Standardize file operations variable names
From: Anshuman Khandual @ 2017-04-27 3:06 UTC (permalink / raw)
To: linux-kernel, linux-mm; +Cc: akpm
Standardized the file operation variable names related to all
four memory management /proc interface files. Also changed all
the symbol permissions (S_IRUGO) into octal permissions (0444)
as it got complains from checkpatch.pl script. This does not
create any functional change to the interface.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
mm/vmstat.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 5a4f5c5..43b6087 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1318,7 +1318,7 @@ static int fragmentation_open(struct inode *inode, struct file *file)
return seq_open(file, &fragmentation_op);
}
-static const struct file_operations fragmentation_file_operations = {
+static const struct file_operations buddyinfo_file_operations = {
.open = fragmentation_open,
.read = seq_read,
.llseek = seq_lseek,
@@ -1337,7 +1337,7 @@ static int pagetypeinfo_open(struct inode *inode, struct file *file)
return seq_open(file, &pagetypeinfo_op);
}
-static const struct file_operations pagetypeinfo_file_ops = {
+static const struct file_operations pagetypeinfo_file_operations = {
.open = pagetypeinfo_open,
.read = seq_read,
.llseek = seq_lseek,
@@ -1454,7 +1454,7 @@ static int zoneinfo_open(struct inode *inode, struct file *file)
return seq_open(file, &zoneinfo_op);
}
-static const struct file_operations proc_zoneinfo_file_operations = {
+static const struct file_operations zoneinfo_file_operations = {
.open = zoneinfo_open,
.read = seq_read,
.llseek = seq_lseek,
@@ -1543,7 +1543,7 @@ static int vmstat_open(struct inode *inode, struct file *file)
return seq_open(file, &vmstat_op);
}
-static const struct file_operations proc_vmstat_file_operations = {
+static const struct file_operations vmstat_file_operations = {
.open = vmstat_open,
.read = seq_read,
.llseek = seq_lseek,
@@ -1789,10 +1789,10 @@ void __init init_mm_internals(void)
start_shepherd_timer();
#endif
#ifdef CONFIG_PROC_FS
- proc_create("buddyinfo", S_IRUGO, NULL, &fragmentation_file_operations);
- proc_create("pagetypeinfo", S_IRUGO, NULL, &pagetypeinfo_file_ops);
- proc_create("vmstat", S_IRUGO, NULL, &proc_vmstat_file_operations);
- proc_create("zoneinfo", S_IRUGO, NULL, &proc_zoneinfo_file_operations);
+ proc_create("buddyinfo", 0444, NULL, &buddyinfo_file_operations);
+ proc_create("pagetypeinfo", 0444, NULL, &pagetypeinfo_file_operations);
+ proc_create("vmstat", 0444, NULL, &vmstat_file_operations);
+ proc_create("zoneinfo", 0444, NULL, &zoneinfo_file_operations);
#endif
}
--
1.8.5.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 related
* Re: [PATCH] powerpc/mm/hugetlb: Add support for 1G huge pages
From: Aneesh Kumar K.V @ 2017-04-27 2:51 UTC (permalink / raw)
To: Anshuman Khandual, akpm, mpe, benh, paulus
Cc: linux-mm, linux-kernel, linuxppc-dev
In-Reply-To: <2ccc1911-ff33-d0fd-195d-44ed4b8d1fb3@linux.vnet.ibm.com>
Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
> On 04/17/2017 10:44 PM, Aneesh Kumar K.V wrote:
>> POWER9 supports hugepages of size 2M and 1G in radix MMU mode. This patch
>> enables the usage of 1G page size for hugetlbfs. This also update the helper
>> such we can do 1G page allocation at runtime.
>>
>> Since we can do this only when radix translation mode is enabled, we can't use
>> the generic gigantic_page_supported helper. Hence provide a way for architecture
>> to override gigantic_page_supported helper.
>>
>> We still don't enable 1G page size on DD1 version. This is to avoid doing
>> workaround mentioned in commit: 6d3a0379ebdc8 (powerpc/mm: Add
>> radix__tlb_flush_pte_p9_dd1()
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/book3s/64/hugetlb.h | 13 +++++++++++++
>> arch/powerpc/mm/hugetlbpage.c | 7 +++++--
>> arch/powerpc/platforms/Kconfig.cputype | 1 +
>> mm/hugetlb.c | 4 ++++
>> 4 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> index 6666cd366596..86f27cc8ec61 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> @@ -50,4 +50,17 @@ static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
>> else
>> return entry;
>> }
>> +
>> +#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
>> + ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
>> + defined(CONFIG_CMA))
>> +#define gigantic_page_supported gigantic_page_supported
>
> As I have mentioned in later part of the reply, it does not really
> make sense to have both arch call back as well as generic config
> option checking to decide on whether a feature is enabled or not.
>
>> +static inline bool gigantic_page_supported(void)
>> +{
>> + if (radix_enabled())
>> + return true;
>> + return false;
>> +}
>> +#endif
>> +
>> #endif
>> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
>> index a4f33de4008e..80f6d2ed551a 100644
>> --- a/arch/powerpc/mm/hugetlbpage.c
>> +++ b/arch/powerpc/mm/hugetlbpage.c
>> @@ -763,8 +763,11 @@ static int __init add_huge_page_size(unsigned long long size)
>> * Hash: 16M and 16G
>> */
>> if (radix_enabled()) {
>> - if (mmu_psize != MMU_PAGE_2M)
>> - return -EINVAL;
>> + if (mmu_psize != MMU_PAGE_2M) {
>> + if (cpu_has_feature(CPU_FTR_POWER9_DD1) ||
>> + (mmu_psize != MMU_PAGE_1G))
>> + return -EINVAL;
>> + }
>> } else {
>> if (mmu_psize != MMU_PAGE_16M && mmu_psize != MMU_PAGE_16G)
>> return -EINVAL;
>> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
>> index ef4c4b8fc547..f4ba4bf0d762 100644
>> --- a/arch/powerpc/platforms/Kconfig.cputype
>> +++ b/arch/powerpc/platforms/Kconfig.cputype
>> @@ -343,6 +343,7 @@ config PPC_STD_MMU_64
>> config PPC_RADIX_MMU
>> bool "Radix MMU Support"
>> depends on PPC_BOOK3S_64
>> + select ARCH_HAS_GIGANTIC_PAGE
>> default y
>> help
>
> As we are already checking for radix_enabled() test inside function
> gigantic_page_supported(), do we still need to conditionally enable
> this on Radix based platforms only ?
>
>
>> Enable support for the Power ISA 3.0 Radix style MMU. Currently this
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 3d0aab9ee80d..2c090189f314 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1158,7 +1158,11 @@ static int alloc_fresh_gigantic_page(struct hstate *h,
>> return 0;
>> }
>>
>> +#ifndef gigantic_page_supported
>> static inline bool gigantic_page_supported(void) { return true; }
>> +#define gigantic_page_supported gigantic_page_supported
>> +#endif
>
> As seen above, now that arch's decision to support this feature is not
> based solely on ARCH_HAS_GIGANTIC_PAGE config option but also on the
> availability of platform features like radix, is it a good time to have
> an arch call back deciding on gigantic_page_supported() test instead of
> just checking presence of config options like
>
> #if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
> ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> defined(CONFIG_CMA))
>
> We should not have both as proposed. I mean CONFIG_ARCH_HAS_GIGANTIC_PAGE
> should not be enabled unless we have MEMORY_ISOLATION && COMPACTION && CMA
> and once enabled we should have arch_gigantic_page_supported() deciding for
> gigantic_page_supported().
I will update the patch. I guess I can also fixup other arch that enable
GIGANTIC_PAGE accordingly.
-aneesh
--
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: your mail
From: Joonsoo Kim @ 2017-04-27 2:08 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm, Andrew Morton, Mel Gorman, Vlastimil Babka,
Andrea Arcangeli, Jerome Glisse, Reza Arbab, Yasuaki Ishimatsu,
qiuxishi, Kani Toshimitsu, slaoub, Andi Kleen, David Rientjes,
Daniel Kiper, Igor Mammedov, Vitaly Kuznetsov, LKML
In-Reply-To: <20170426091906.GB12504@dhcp22.suse.cz>
On Wed, Apr 26, 2017 at 11:19:06AM +0200, Michal Hocko wrote:
> > > [...]
> > >
> > > > > You are trying to change a semantic of something that has a well defined
> > > > > meaning. I disagree that we should change it. It might sound like a
> > > > > simpler thing to do because pfn walkers will have to be checked but what
> > > > > you are proposing is conflating two different things together.
> > > >
> > > > I don't think that *I* try to change the semantic of pfn_valid().
> > > > It would be original semantic of pfn_valid().
> > > >
> > > > "If pfn_valid() returns true, we can get proper struct page and the
> > > > zone information,"
> > >
> > > I do not see any guarantee about the zone information anywhere. In fact
> > > this is not true with the original implementation as I've tried to
> > > explain already. We do have new pages associated with a zone but that
> > > association might change during the online phase. So you cannot really
> > > rely on that information until the page is online. There is no real
> > > change in that regards after my rework.
> >
> > I know that what you did doesn't change thing much. What I try to say
> > is that previous implementation related to pfn_valid() in hotplug is
> > wrong. Please do not assume that hotplug implementation is correct and
> > other pfn_valid() users are incorrect. There is no design document so
> > I'm not sure which one is correct but assumption that pfn_valid() user
> > can access whole the struct page information makes much sense to me.
>
> Not really. E.g. ZONE_DEVICE pages are never online AFAIK. I believe we
> still need pfn_valid to work for those pfns. Really, pfn_valid has a
It's really contrary example to your insist. They requires not only
struct page but also other information, especially, the zone index.
They checks zone idx to know whether this page is for ZONE_DEVICE or not.
So, pfn_valid() for ZONE_DEVICE pages assume that struct page has all
the valid information. It's perfectly matched with my suggestion.
Online isn't important issue here. What the important point is the condition
that pfn_valid() return true. pfn_valid() for ZONE_DEVICE returns true after
arch_add_memory() since all the struct page information is fixed there.
If zone of hotplugged memory cannot be fixed at this moment, you can
defef it until all the information is fixed (onlining). That
seems to be better semantic of pfn_valid() to me.
> different meaning than you would like it to have. Who knows how many
> others like that are lurking there. I feel much more comfortable to go
> and hunt already broken code and fix it rathert than break something
> unexpectedly.
I think that I did my best to explain my reasoning. It seems that we
cannot agree with each other so it's better for some others to express
their opinion to this problem. I will stop this discussion from now
on.
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 -mm -v3] mm, swap: Sort swap entries before free
From: Huang, Ying @ 2017-04-27 1:21 UTC (permalink / raw)
To: Tim Chen
Cc: Huang, Ying, Minchan Kim, Andrew Morton, linux-mm, linux-kernel,
Hugh Dickins, Shaohua Li, Rik van Riel
In-Reply-To: <1493237623.3209.142.camel@linux.intel.com>
Tim Chen <tim.c.chen@linux.intel.com> writes:
>>
>> From 7bd903c42749c448ef6acbbdee8dcbc1c5b498b9 Mon Sep 17 00:00:00 2001
>> From: Huang Ying <ying.huang@intel.com>
>> Date: Thu, 23 Feb 2017 13:05:20 +0800
>> Subject: [PATCH -v5] mm, swap: Sort swap entries before free
>>
>>A
>> ---
>> A mm/swapfile.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>> A 1 file changed, 38 insertions(+), 5 deletions(-)
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 71890061f653..10e75f9e8ac1 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -37,6 +37,7 @@
>> A #include <linux/swapfile.h>
>> A #include <linux/export.h>
>> A #include <linux/swap_slots.h>
>> +#include <linux/sort.h>
>> A
>> A #include <asm/pgtable.h>
>> A #include <asm/tlbflush.h>
>> @@ -1065,20 +1066,52 @@ void swapcache_free(swp_entry_t entry)
>> A }
>> A }
>> A
>> +static int swp_entry_cmp(const void *ent1, const void *ent2)
>> +{
>> + const swp_entry_t *e1 = ent1, *e2 = ent2;
>> +
>> + return (int)(swp_type(*e1) - swp_type(*e2));
>> +}
>> +
>> A void swapcache_free_entries(swp_entry_t *entries, int n)
>> A {
>> A struct swap_info_struct *p, *prev;
>> - int i;
>> + int i, m;
>> + swp_entry_t entry;
>> + unsigned int prev_swp_type;
>
> I think it will be clearer to name prev_swp_type as first_swp_type
> as this is the swp type of the first entry.
Yes. That is better! Will do that.
>> A
>> A if (n <= 0)
>> A return;
>> A
>> A prev = NULL;
>> A p = NULL;
>> - for (i = 0; i < n; ++i) {
>> - p = swap_info_get_cont(entries[i], prev);
>> - if (p)
>> - swap_entry_free(p, entries[i]);
>> + m = 0;
>> + prev_swp_type = swp_type(entries[0]);
>> + for (i = 0; i < n; i++) {
>> + entry = entries[i];
>> + if (likely(swp_type(entry) == prev_swp_type)) {
>> + p = swap_info_get_cont(entry, prev);
>> + if (likely(p))
>> + swap_entry_free(p, entry);
>> + prev = p;
>> + } else if (!m)
>> + m = i;
>> + }
>> + if (p)
>> + spin_unlock(&p->lock);
>> + if (likely(!m))
>> + return;
>> +
>
> We could still have prev_swp_type at the first entry after sorting.
> and we can avoid an unlock/relock for this case if we do this:
>
> if (likely(!m)) {
> if (p)
> spin_unlock(&p->lock);
> return;
> }
>
>> + /* Sort swap entries by swap device, so each lock is only taken once. */
>> + sort(entries + m, n - m, sizeof(entries[0]), swp_entry_cmp, NULL);
>> + prev = NULL;
>
> Can eliminate prev=NULL if we adopt the above change.
>
>> + for (i = m; i < n; i++) {
>> + entry = entries[i];
>> + if (swp_type(entry) == prev_swp_type)
>> + continue;
>
> The if/continue statement seems incorrect. When swp_type(entry) == prev_swp_type
> we also need to free entry. A The if/continue statement should be deleted.
>
> Say we have 3 entries with swp_type
> 1,2,1
>
> We will get prev_swp_type as 1 and free the first entry
> and sort the remaining two. A The last entry with
> swp_type 1 will not be freed.
The first loop in the function will scan all elements of the array, so
the first and third entry will be freed in the first loop. Then the the
second and the third entry will be sorted. But all entries with the
same swap type (device) of the first entry needn't to be freed again.
The key point is that we will scan all elements of the array in the
first loop, record the first entry that has different swap type
(device).
Best Regards,
Huang, Ying
>> + p = swap_info_get_cont(entry, prev);
>> + if (likely(p))
>> + swap_entry_free(p, entry);
>> A prev = p;
>> A }
>> A if (p)
>
> Thanks.
>
> Tim
--
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, zone_device: replace {get, put}_zone_device_page() with a single reference
From: Dan Williams @ 2017-04-27 0:55 UTC (permalink / raw)
To: akpm
Cc: linux-mm, Jérôme Glisse, Logan Gunthorpe, linux-kernel,
Kirill Shutemov
In-Reply-To: <20170423233125.nehmgtzldgi25niy@node.shutemov.name>
Kirill points out that the calls to {get,put}_dev_pagemap() can be
removed from the mm fast path if we take a single get_dev_pagemap()
reference to signify that the page is alive and use the final put of the
page to drop that reference.
This does require some care to make sure that any waits for the
percpu_ref to drop to zero occur *after* devm_memremap_page_release(),
since it now maintains its own elevated reference.
Cc Ingo Molnar <mingo@redhat.com>
Cc: JA(C)rA'me Glisse <jglisse@redhat.com>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Suggested-by: Kirill Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
This patch might fix the regression that we found with the conversion to
generic get_user_pages_fast() in the x86/mm branch pending for 4.12
(commit 2947ba054a4d "x86/mm/gup: Switch GUP to the generic
get_user_page_fast() implementation"). I'll test tomorrow, but in case
someone can give it a try before I wake up, here's an early version.
drivers/dax/pmem.c | 2 +-
drivers/nvdimm/pmem.c | 13 +++++++++++--
include/linux/mm.h | 14 --------------
kernel/memremap.c | 22 +++++++++-------------
mm/swap.c | 10 ++++++++++
5 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/dax/pmem.c b/drivers/dax/pmem.c
index d4ca19bd74eb..9f2a0b4fd801 100644
--- a/drivers/dax/pmem.c
+++ b/drivers/dax/pmem.c
@@ -43,6 +43,7 @@ static void dax_pmem_percpu_exit(void *data)
struct dax_pmem *dax_pmem = to_dax_pmem(ref);
dev_dbg(dax_pmem->dev, "%s\n", __func__);
+ wait_for_completion(&dax_pmem->cmp);
percpu_ref_exit(ref);
}
@@ -53,7 +54,6 @@ static void dax_pmem_percpu_kill(void *data)
dev_dbg(dax_pmem->dev, "%s\n", __func__);
percpu_ref_kill(ref);
- wait_for_completion(&dax_pmem->cmp);
}
static int dax_pmem_probe(struct device *dev)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 3b3dab73d741..6be0c1253fcd 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -25,6 +25,7 @@
#include <linux/badblocks.h>
#include <linux/memremap.h>
#include <linux/vmalloc.h>
+#include <linux/blk-mq.h>
#include <linux/pfn_t.h>
#include <linux/slab.h>
#include <linux/pmem.h>
@@ -243,6 +244,11 @@ static void pmem_release_queue(void *q)
blk_cleanup_queue(q);
}
+static void pmem_freeze_queue(void *q)
+{
+ blk_mq_freeze_queue_start(q);
+}
+
static void pmem_release_disk(void *__pmem)
{
struct pmem_device *pmem = __pmem;
@@ -301,6 +307,9 @@ static int pmem_attach_disk(struct device *dev,
if (!q)
return -ENOMEM;
+ if (devm_add_action_or_reset(dev, pmem_release_queue, q))
+ return -ENOMEM;
+
pmem->pfn_flags = PFN_DEV;
if (is_nd_pfn(dev)) {
addr = devm_memremap_pages(dev, &pfn_res, &q->q_usage_counter,
@@ -320,10 +329,10 @@ static int pmem_attach_disk(struct device *dev,
pmem->size, ARCH_MEMREMAP_PMEM);
/*
- * At release time the queue must be dead before
+ * At release time the queue must be frozen before
* devm_memremap_pages is unwound
*/
- if (devm_add_action_or_reset(dev, pmem_release_queue, q))
+ if (devm_add_action_or_reset(dev, pmem_freeze_queue, q))
return -ENOMEM;
if (IS_ERR(addr))
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 00a8fa7e366a..ce17b35257ac 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -758,19 +758,11 @@ static inline enum zone_type page_zonenum(const struct page *page)
}
#ifdef CONFIG_ZONE_DEVICE
-void get_zone_device_page(struct page *page);
-void put_zone_device_page(struct page *page);
static inline bool is_zone_device_page(const struct page *page)
{
return page_zonenum(page) == ZONE_DEVICE;
}
#else
-static inline void get_zone_device_page(struct page *page)
-{
-}
-static inline void put_zone_device_page(struct page *page)
-{
-}
static inline bool is_zone_device_page(const struct page *page)
{
return false;
@@ -786,9 +778,6 @@ static inline void get_page(struct page *page)
*/
VM_BUG_ON_PAGE(page_ref_count(page) <= 0, page);
page_ref_inc(page);
-
- if (unlikely(is_zone_device_page(page)))
- get_zone_device_page(page);
}
static inline void put_page(struct page *page)
@@ -797,9 +786,6 @@ static inline void put_page(struct page *page)
if (put_page_testzero(page))
__put_page(page);
-
- if (unlikely(is_zone_device_page(page)))
- put_zone_device_page(page);
}
#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP)
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 07e85e5229da..5316efdde083 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -182,18 +182,6 @@ struct page_map {
struct vmem_altmap altmap;
};
-void get_zone_device_page(struct page *page)
-{
- percpu_ref_get(page->pgmap->ref);
-}
-EXPORT_SYMBOL(get_zone_device_page);
-
-void put_zone_device_page(struct page *page)
-{
- put_dev_pagemap(page->pgmap);
-}
-EXPORT_SYMBOL(put_zone_device_page);
-
static void pgmap_radix_release(struct resource *res)
{
resource_size_t key, align_start, align_size, align_end;
@@ -237,6 +225,10 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
struct resource *res = &page_map->res;
resource_size_t align_start, align_size;
struct dev_pagemap *pgmap = &page_map->pgmap;
+ unsigned long pfn;
+
+ for_each_device_pfn(pfn, page_map)
+ put_page(pfn_to_page(pfn));
if (percpu_ref_tryget_live(pgmap->ref)) {
dev_WARN(dev, "%s: page mapping is still live!\n", __func__);
@@ -277,7 +269,10 @@ struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
*
* Notes:
* 1/ @ref must be 'live' on entry and 'dead' before devm_memunmap_pages() time
- * (or devm release event).
+ * (or devm release event). The expected order of events is that @ref has
+ * been through percpu_ref_kill() before devm_memremap_pages_release(). The
+ * wait for the completion of kill and percpu_ref_exit() must occur after
+ * devm_memremap_pages_release().
*
* 2/ @res is expected to be a host memory range that could feasibly be
* treated as a "System RAM" range, i.e. not a device mmio range, but
@@ -379,6 +374,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
*/
list_del(&page->lru);
page->pgmap = pgmap;
+ percpu_ref_get(ref);
}
devres_add(dev, page_map);
return __va(res->start);
diff --git a/mm/swap.c b/mm/swap.c
index 5dabf444d724..01267dda6668 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -97,6 +97,16 @@ static void __put_compound_page(struct page *page)
void __put_page(struct page *page)
{
+ if (is_zone_device_page(page)) {
+ put_dev_pagemap(page->pgmap);
+
+ /*
+ * The page belong to device, do not return it to
+ * page allocator.
+ */
+ return;
+ }
+
if (unlikely(PageCompound(page)))
__put_compound_page(page);
else
--
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
* mmotm 2017-04-26-16-39 uploaded
From: akpm @ 2017-04-26 23:40 UTC (permalink / raw)
To: mm-commits, linux-kernel, linux-mm, linux-fsdevel, linux-next,
sfr, mhocko, broonie
The mm-of-the-moment snapshot 2017-04-26-16-39 has been uploaded to
http://www.ozlabs.org/~akpm/mmotm/
mmotm-readme.txt says
README for mm-of-the-moment:
http://www.ozlabs.org/~akpm/mmotm/
This is a snapshot of my -mm patch queue. Uploaded at random hopefully
more than once a week.
You will need quilt to apply these patches to the latest Linus release (4.x
or 4.x-rcY). The series file is in broken-out.tar.gz and is duplicated in
http://ozlabs.org/~akpm/mmotm/series
The file broken-out.tar.gz contains two datestamp files: .DATE and
.DATE-yyyy-mm-dd-hh-mm-ss. Both contain the string yyyy-mm-dd-hh-mm-ss,
followed by the base kernel version against which this patch series is to
be applied.
This tree is partially included in linux-next. To see which patches are
included in linux-next, consult the `series' file. Only the patches
within the #NEXT_PATCHES_START/#NEXT_PATCHES_END markers are included in
linux-next.
A git tree which contains the memory management portion of this tree is
maintained at git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
by Michal Hocko. It contains the patches which are between the
"#NEXT_PATCHES_START mm" and "#NEXT_PATCHES_END" markers, from the series
file, http://www.ozlabs.org/~akpm/mmotm/series.
A full copy of the full kernel tree with the linux-next and mmotm patches
already applied is available through git within an hour of the mmotm
release. Individual mmotm releases are tagged. The master branch always
points to the latest release, so it's constantly rebasing.
http://git.cmpxchg.org/cgit.cgi/linux-mmotm.git/
To develop on top of mmotm git:
$ git remote add mmotm git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
$ git remote update mmotm
$ git checkout -b topic mmotm/master
<make changes, commit>
$ git send-email mmotm/master.. [...]
To rebase a branch with older patches to a new mmotm release:
$ git remote update mmotm
$ git rebase --onto mmotm/master <topic base> topic
The directory http://www.ozlabs.org/~akpm/mmots/ (mm-of-the-second)
contains daily snapshots of the -mm tree. It is updated more frequently
than mmotm, and is untested.
A git copy of this tree is available at
http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/
and use of this tree is similar to
http://git.cmpxchg.org/cgit.cgi/linux-mmotm.git/, described above.
This mmotm tree contains the following patches against 4.11-rc8:
(patches marked "*" will be included in linux-next)
origin.patch
i-need-old-gcc.patch
* mm-uncharge-poisoned-pages.patch
* mm-skip-hwpoisoned-pages-when-onlining-pages.patch
* arm-arch-arm-include-asm-pageh-needs-personalityh.patch
* dma-debug-make-locking-to-work-for-rt.patch
* scripts-spellingtxt-add-several-more-common-spelling-mistakes.patch
* blackfin-bf609-let-clk_disable-return-immediately-if-clk-is-null.patch
* fs-ocfs2-cluster-use-setup_timer.patch
* ocfs2-o2hb-revert-hb-threshold-to-keep-compatible.patch
* fs-ocfs2-cluster-use-offset_in_page-macro.patch
* ocfs2-old-mle-put-and-release-after-the-function-dlm_add_migration_mle-called.patch
* ocfs2-old-mle-put-and-release-after-the-function-dlm_add_migration_mle-called-fix.patch
* ocfs2-dlm-optimization-of-code-while-free-dead-node-locks.patch
* ocfs2-dlm-optimization-of-code-while-free-dead-node-locks-checkpatch-fixes.patch
* block-restore-proc-partitions-to-not-display-non-partitionable-removable-devices.patch
mm.patch
* slab-avoid-ipis-when-creating-kmem-caches.patch
* mm-fix-100%-cpu-kswapd-busyloop-on-unreclaimable-nodes.patch
* mm-fix-100%-cpu-kswapd-busyloop-on-unreclaimable-nodes-fix.patch
* mm-fix-100%-cpu-kswapd-busyloop-on-unreclaimable-nodes-fix-2.patch
* mm-fix-check-for-reclaimable-pages-in-pf_memalloc-reclaim-throttling.patch
* mm-remove-seemingly-spurious-reclaimability-check-from-laptop_mode-gating.patch
* mm-remove-unnecessary-reclaimability-check-from-numa-balancing-target.patch
* mm-dont-avoid-high-priority-reclaim-on-unreclaimable-nodes.patch
* mm-dont-avoid-high-priority-reclaim-on-memcg-limit-reclaim.patch
* mm-delete-nr_pages_scanned-and-pgdat_reclaimable.patch
* revert-mm-vmscan-account-for-skipped-pages-as-a-partial-scan.patch
* mm-remove-unnecessary-back-off-function-when-retrying-page-reclaim.patch
* writeback-use-setup_deferrable_timer.patch
* mm-delete-unnecessary-ttu_-flags.patch
* mm-dont-assume-anonymous-pages-have-swapbacked-flag.patch
* mm-move-madv_free-pages-into-lru_inactive_file-list.patch
* mm-move-madv_free-pages-into-lru_inactive_file-list-checkpatch-fixes.patch
* mm-reclaim-madv_free-pages.patch
* mm-reclaim-madv_free-pages-fix.patch
* mm-fix-lazyfree-bug-on-check-in-try_to_unmap_one.patch
* mm-fix-lazyfree-bug-on-check-in-try_to_unmap_one-fix.patch
* mm-enable-madv_free-for-swapless-system.patch
* proc-show-madv_free-pages-info-in-smaps.patch
* proc-show-madv_free-pages-info-in-smaps-fix.patch
* mm-memcontrol-provide-shmem-statistics.patch
* mm-swap-fix-a-race-in-free_swap_and_cache.patch
* mm-use-is_migrate_highatomic-to-simplify-the-code.patch
* mm-use-is_migrate_highatomic-to-simplify-the-code-fix.patch
* mm-use-is_migrate_isolate_page-to-simplify-the-code.patch
* mm-vmstat-print-non-populated-zones-in-zoneinfo.patch
* mm-vmstat-suppress-pcp-stats-for-unpopulated-zones-in-zoneinfo.patch
* lockdep-teach-lockdep-about-memalloc_noio_save.patch
* lockdep-allow-to-disable-reclaim-lockup-detection.patch
* xfs-abstract-pf_fstrans-to-pf_memalloc_nofs.patch
* mm-introduce-memalloc_nofs_saverestore-api.patch
* mm-introduce-memalloc_nofs_saverestore-api-fix.patch
* xfs-use-memalloc_nofs_saverestore-instead-of-memalloc_noio.patch
* jbd2-mark-the-transaction-context-with-the-scope-gfp_nofs-context.patch
* jbd2-mark-the-transaction-context-with-the-scope-gfp_nofs-context-fix.patch
* jbd2-make-the-whole-kjournald2-kthread-nofs-safe.patch
* jbd2-make-the-whole-kjournald2-kthread-nofs-safe-checkpatch-fixes.patch
* mm-tighten-up-the-fault-path-a-little.patch
* mm-remove-rodata_test_data-export-add-pr_fmt.patch
* mm-compaction-reorder-fields-in-struct-compact_control.patch
* mm-compaction-remove-redundant-watermark-check-in-compact_finished.patch
* mm-page_alloc-split-smallest-stolen-page-in-fallback.patch
* mm-page_alloc-split-smallest-stolen-page-in-fallback-fix.patch
* mm-page_alloc-count-movable-pages-when-stealing-from-pageblock.patch
* mm-page_alloc-count-movable-pages-when-stealing-from-pageblock-fix.patch
* mm-compaction-change-migrate_async_suitable-to-suitable_migration_source.patch
* mm-compaction-add-migratetype-to-compact_control.patch
* mm-compaction-restrict-async-compaction-to-pageblocks-of-same-migratetype.patch
* mm-compaction-finish-whole-pageblock-to-reduce-fragmentation.patch
* mm-do-not-use-double-negation-for-testing-page-flags.patch
* mm-vmscan-fix-zone-balance-check-in-prepare_kswapd_sleep.patch
* mm-vmscan-only-clear-pgdat-congested-dirty-writeback-state-when-balanced.patch
* mm-vmscan-prevent-kswapd-sleeping-prematurely-due-to-mismatched-classzone_idx.patch
* mm-vmscan-prevent-kswapd-sleeping-prematurely-due-to-mismatched-classzone_idx-fix.patch
* mm-page_alloc-__gfp_nowarn-shouldnt-suppress-stall-warnings.patch
* mm-sparse-refine-usemap_size-a-little.patch
* mm-compaction-ignore-block-suitable-after-check-large-free-page.patch
* mm-vmscan-more-restrictive-condition-for-retry-in-do_try_to_free_pages.patch
* mm-vmscan-more-restrictive-condition-for-retry-in-do_try_to_free_pages-v5.patch
* mm-remove-unncessary-ret-in-page_referenced.patch
* mm-remove-swap_dirty-in-ttu.patch
* mm-remove-swap_mlock-check-for-swap_success-in-ttu.patch
* mm-make-the-try_to_munlock-void-function.patch
* mm-make-the-try_to_munlock-void-function-fix.patch
* mm-remove-swap_mlock-in-ttu.patch
* mm-remove-swap_again-in-ttu.patch
* mm-make-ttus-return-boolean.patch
* mm-make-rmap_walk-void-function.patch
* mm-make-rmap_one-boolean-function.patch
* mm-remove-swap_.patch
* mm-remove-swap_-fix.patch
* mm-swap-fix-comment-in-__read_swap_cache_async.patch
* mm-swap-improve-readability-via-make-spin_lock-unlock-balanced.patch
* mm-swap-avoid-lock-swap_avail_lock-when-held-cluster-lock.patch
* mm-enable-page-poisoning-early-at-boot.patch
* mm-enable-page-poisoning-early-at-boot-v2.patch
* mm-include-linux-migrateh-fixing-checkpatch-warning-regarding-function-definition.patch
* swap-add-warning-if-swap-slots-cache-failed-to-initialize.patch
* swap-add-warning-if-swap-slots-cache-failed-to-initialize-fix.patch
* mm-fix-spelling-error.patch
* userfaultfd-selftest-combine-all-cases-into-the-single-executable.patch
* mm-add-additional-consistency-check.patch
* oom-improve-oom-disable-handling.patch
* mm-mmap-replace-shm_huge_mask-with-map_huge_mask-inside-mmap_pgoff.patch
* mm-vmscan-fix-io-refault-regression-in-cache-workingset-transition.patch
* mm-memcontrol-clean-up-memoryevents-counting-function.patch
* mm-memcontrol-re-use-global-vm-event-enum.patch
* mm-memcontrol-re-use-node-vm-page-state-enum.patch
* mm-memcontrol-use-node-page-state-naming-scheme-for-memcg.patch
* mm-swap-remove-unused-function-prototype.patch
* documentation-vm-add-hugetlbfs-reservation-overview.patch
* mm-madvise-clean-up-madv_soft_offline-and-madv_hwpoison.patch
* mm-madvise-move-up-the-behavior-parameter-validation.patch
* mm-softoffline-add-page-flag-description-in-error-paths.patch
* mm-page_alloc-remove-debug_guardpage_minorder-test-in-warn_alloc.patch
* zram-handle-multiple-pages-attached-bios-bvec.patch
* zram-partial-io-refactoring.patch
* zram-use-zram_slot_lock-instead-of-raw-bit_spin_lock-op.patch
* zram-remove-zram_meta-structure.patch
* zram-introduce-zram-data-accessor.patch
* zram-use-zram_free_page-instead-of-open-coded.patch
* zram-reduce-load-operation-in-page_same_filled.patch
* fs-fix-data-invalidation-in-the-cleancache-during-direct-io.patch
* fs-block_dev-always-invalidate-cleancache-in-invalidate_bdev.patch
* mm-truncate-bail-out-early-from-invalidate_inode_pages2_range-if-mapping-is-empty.patch
* mm-truncate-avoid-pointless-cleancache_invalidate_inode-calls.patch
* mm-gup-fix-access_ok-argument-type.patch
* mm-swap-fix-swap-space-leak-in-error-path-of-swap_free_entries.patch
* mm-hwpoison-call-shake_page-unconditionally.patch
* mm-hwpoison-call-shake_page-after-try_to_unmap-for-mlocked-page.patch
* mm-page_alloc-return-0-in-case-this-node-has-no-page-within-the-zone.patch
* mm-vmscan-do-not-pass-reclaimed-slab-to-vmpressure.patch
* mm-page_owner-align-with-pageblock_nr-pages.patch
* mm-walk-the-zone-in-pageblock_nr_pages-steps.patch
* kasan-introduce-helper-functions-for-determining-bug-type.patch
* kasan-unify-report-headers.patch
* kasan-change-allocation-and-freeing-stack-traces-headers.patch
* kasan-simplify-address-description-logic.patch
* kasan-change-report-header.patch
* kasan-improve-slab-object-description.patch
* kasan-print-page-description-after-stacks.patch
* kasan-improve-double-free-report-format.patch
* kasan-separate-report-parts-by-empty-lines.patch
* proc-remove-cast-from-memory-allocation.patch
* proc-sysctl-fix-the-int-overflow-for-jiffies-conversion.patch
* drivers-virt-use-get_user_pages_unlocked.patch
* jiffiesh-declare-jiffies-and-jiffies_64-with-____cacheline_aligned_in_smp.patch
* make-help-add-tools-help-target.patch
* locking-hung_task-defer-showing-held-locks.patch
* vmci-fix-a-couple-integer-overflow-tests.patch
* c2port-checking-for-null-instead-of-is_err.patch
* revert-lib-test_sortc-make-it-explicitly-non-modular.patch
* lib-add-module-support-to-array-based-sort-tests.patch
* lib-add-module-support-to-linked-list-sorting-tests.patch
* firmware-makefile-force-recompilation-if-makefile-changes.patch
* checkpatch-remove-obsolete-config_experimental-checks.patch
* checkpatch-add-ability-to-find-bad-uses-of-vsprintf-%pfoo-extensions.patch
* checkpatch-add-ability-to-find-bad-uses-of-vsprintf-%pfoo-extensions-fix.patch
* checkpatch-add-ability-to-find-bad-uses-of-vsprintf-%pfoo-extensions-fix-fix.patch
* checkpatch-improve-embedded_function_name-test.patch
* checkpatch-allow-space-leading-blank-lines-in-email-headers.patch
* checkpatch-avoid-suggesting-struct-definitions-should-be-const.patch
* checkpatch-improve-multistatement_macro_use_do_while-test.patch
* checkpatch-clarify-the-embedded_function_name-message.patch
* checkpatch-special-audit-for-revert-commit-line.patch
* checkpatch-improve-kalloc-with-multiplication-and-sizeof-test.patch
* reiserfs-use-designated-initializers.patch
* fork-free-vmapped-stacks-in-cache-when-cpus-are-offline.patch
* cpumask-make-nr_cpumask_bits-unsigned.patch
* crash-move-crashkernel-parsing-and-vmcore-related-code-under-config_crash_core.patch
* ia64-reuse-append_elf_note-and-final_note-functions.patch
* powerpc-fadump-remove-dependency-with-config_kexec.patch
* powerpc-fadump-reuse-crashkernel-parameter-for-fadump-memory-reservation.patch
* powerpc-fadump-update-documentation-about-crashkernel-parameter-reuse.patch
* kdump-vmcoreinfo-report-actual-value-of-phys_base.patch
* uapi-fix-linux-sysctlh-userspace-compilation-errors.patch
* pidns-disable-pid-allocation-if-pid_ns_prepare_proc-is-failed-in-alloc_pid.patch
* ns-allow-ns_entries-to-have-custom-symlink-content.patch
* pidns-expose-task-pid_ns_for_children-to-userspace.patch
* taskstats-add-e-u-stime-for-tgid-command.patch
* taskstats-add-e-u-stime-for-tgid-command-fix.patch
* taskstats-add-e-u-stime-for-tgid-command-fix-fix.patch
* fs-affs-bugfix-make-symbolic-links-work-again.patch
* fs-affs-bugfix-enable-writes-on-ofs-disks.patch
* fs-affs-bugfix-write-files-greater-than-page-size-on-ofs.patch
* kcov-simplify-interrupt-check.patch
* scripts-gdb-add-lx-fdtdump-command.patch
* kernel-reboot-add-devm_register_reboot_notifier.patch
* kernel-reboot-add-devm_register_reboot_notifier-fix.patch
* fault-inject-use-correct-check-for-interrupts.patch
* fault-inject-support-systematic-fault-injection.patch
* fault-inject-support-systematic-fault-injection-fix.patch
* fault-inject-automatically-detect-the-number-base-for-fail-nth-write-interface.patch
* fault-inject-parse-as-natural-1-based-value-for-fail-nth-write-interface.patch
* fault-inject-make-fail-nth-read-write-interface-symmetric.patch
* fault-inject-simplify-access-check-for-fail-nth.patch
* fault-inject-simplify-access-check-for-fail-nth-fix.patch
* fault-inject-add-proc-pid-fail-nth.patch
* zlib-inflate-fix-potential-buffer-overflow.patch
* initramfs-provide-a-way-to-ignore-image-provided-by-bootloader.patch
* initramfs-use-vfs_stat-lstat-directly.patch
* ipc-shm-some-shmat-cleanups.patch
* sysvipc-cacheline-align-kern_ipc_perm.patch
linux-next.patch
linux-next-git-rejects.patch
* imx7-fix-kconfig-warning-and-build-errors.patch
* sparc64-ng4-memset-32-bits-overflow.patch
* mm-zeroing-hash-tables-in-allocator.patch
* mm-updated-callers-to-use-hash_zero-flag.patch
* mm-adaptive-hash-table-scaling.patch
* mm-introduce-kvalloc-helpers.patch
* mm-introduce-kvalloc-helpers-fix.patch
* mm-support-__gfp_repeat-in-kvmalloc_node-for-32kb.patch
* rhashtable-simplify-a-strange-allocation-pattern.patch
* ila-simplify-a-strange-allocation-pattern.patch
* xattr-zero-out-memory-copied-to-userspace-in-getxattr.patch
* treewide-use-kvalloc-rather-than-opencoded-variants.patch
* net-use-kvmalloc-with-__gfp_repeat-rather-than-open-coded-variant.patch
* md-use-kvmalloc-rather-than-opencoded-variant.patch
* bcache-use-kvmalloc.patch
* mm-swap-use-kvzalloc-to-allocate-some-swap-data-structure.patch
* mm-vmalloc-use-__gfp_highmem-implicitly.patch
* scripts-spellingtxt-add-memory-pattern-and-fix-typos.patch
* scripts-spellingtxt-add-regsiter-register-spelling-mistake.patch
* scripts-spellingtxt-add-intialised-pattern-and-fix-typo-instances.patch
* treewide-correct-diffrent-and-banlance-typos.patch
* treewide-move-set_memory_-functions-away-from-cacheflushh.patch
* arm-use-set_memoryh-header.patch
* arm64-use-set_memoryh-header.patch
* s390-use-set_memoryh-header.patch
* x86-use-set_memoryh-header.patch
* agp-use-set_memoryh-header.patch
* drm-use-set_memoryh-header.patch
* drm-use-set_memoryh-header-fix.patch
* intel_th-use-set_memoryh-header.patch
* watchdog-hpwdt-use-set_memoryh-header.patch
* bpf-use-set_memoryh-header.patch
* module-use-set_memoryh-header.patch
* pm-hibernate-use-set_memoryh-header.patch
* alsa-use-set_memoryh-header.patch
* misc-sram-use-set_memoryh-header.patch
* video-vermilion-use-set_memoryh-header.patch
* drivers-staging-media-atomisp-pci-atomisp2-use-set_memoryh.patch
* treewide-decouple-cacheflushh-and-set_memoryh.patch
* treewide-decouple-cacheflushh-and-set_memoryh-fix.patch
* kref-remove-warn_on-for-null-release-functions.patch
* megasas-remove-expensive-inline-from-megasas_return_cmd.patch
* remove-expensive-warn_on-in-pagefault_disabled_dec.patch
* fs-remove-set-but-not-checked-aop_flag_uninterruptible-flag.patch
* docs-vm-transhuge-fix-few-trivial-typos.patch
* docs-vm-transhuge-fix-few-trivial-typos-fix.patch
* format-security-move-static-strings-to-const.patch
* fs-f2fs-use-ktime_get_real_seconds-for-sit_info-times.patch
* trace-make-trace_hwlat-timestamp-y2038-safe.patch
* fs-cifs-replace-current_time-by-other-appropriate-apis.patch
* fs-ceph-current_time-with-ktime_get_real_ts.patch
* fs-ufs-use-ktime_get_real_ts64-for-birthtime.patch
* fs-ubifs-replace-current_time_sec-with-current_time.patch
* lustre-replace-current_time-macro.patch
* apparmorfs-replace-current_time-with-current_time.patch
* gfs2-replace-current_time-with-current_time.patch
* time-delete-current_time_sec-and-current_time.patch
* time-delete-current_fs_time-function.patch
* mm-huge_memory-use-zap_deposited_table-more.patch
* mm-huge_memory-deposit-a-pgtable-for-dax-pmd-faults-when-required.patch
* mm-prevent-potential-recursive-reclaim-due-to-clearing-pf_memalloc.patch
* mm-introduce-memalloc_noreclaim_saverestore.patch
* treewide-convert-pf_memalloc-manipulations-to-new-helpers.patch
* treewide-convert-pf_memalloc-manipulations-to-new-helpers-fix.patch
* mtd-nand-nandsim-convert-to-memalloc_noreclaim_.patch
* dax-add-tracepoints-to-dax_iomap_pte_fault.patch
* dax-add-tracepoints-to-dax_pfn_mkwrite.patch
* dax-add-tracepoints-to-dax_load_hole.patch
* dax-add-tracepoints-to-dax_writeback_mapping_range.patch
* dax-add-tracepoints-to-dax_writeback_mapping_range-fix.patch
* dax-add-tracepoint-to-dax_writeback_one.patch
* dax-add-tracepoint-to-dax_writeback_one-fix.patch
* dax-add-tracepoint-to-dax_insert_mapping.patch
* selftests-vm-add-a-test-for-virtual-address-range-mapping.patch
* lib-crc-ccitt-add-ccitt-false-crc16-variant.patch
mm-add-strictlimit-knob-v2.patch
make-sure-nobodys-leaking-resources.patch
releasing-resources-with-children.patch
make-frame_pointer-default=y.patch
kernel-forkc-export-kernel_thread-to-modules.patch
mutex-subsystem-synchro-test-module.patch
slab-leaks3-default-y.patch
workaround-for-a-pci-restoring-bug.patch
--
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: [virtio-dev] Re: [PATCH v9 2/5] virtio-balloon: VIRTIO_BALLOON_F_BALLOON_CHUNKS
From: Michael S. Tsirkin @ 2017-04-26 23:20 UTC (permalink / raw)
To: Wang, Wei W
Cc: virtio-dev@lists.oasis-open.org, linux-kernel@vger.kernel.org,
qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org,
kvm@vger.kernel.org, linux-mm@kvack.org, david@redhat.com,
Hansen, Dave, cornelia.huck@de.ibm.com, akpm@linux-foundation.org,
mgorman@techsingularity.net, aarcange@redhat.com,
amit.shah@redhat.com, pbonzini@redhat.com,
liliang.opensource@gmail.com
In-Reply-To: <286AC319A985734F985F78AFA26841F7391F6DCD@shsmsx102.ccr.corp.intel.com>
On Wed, Apr 26, 2017 at 11:03:34AM +0000, Wang, Wei W wrote:
> Hi Michael, could you please give some feedback?
I'm sorry, I'm not sure feedback on what you are requesting.
The interface looks reasonable now, even though there's
a way to make it even simpler if we can limit chunk size
to 2G (in fact 4G - 1). Do you think we can live with this
limitation?
But the code still needs some cleanup.
--
MST
--
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/2] dax: fix data corruption due to stale mmap reads
From: Ross Zwisler @ 2017-04-26 22:52 UTC (permalink / raw)
To: Jan Kara
Cc: Ross Zwisler, Andrew Morton, linux-kernel, Alexander Viro,
Alexey Kuznetsov, Andrey Ryabinin, Anna Schumaker,
Christoph Hellwig, Dan Williams, Darrick J. Wong,
Eric Van Hensbergen, Jens Axboe, Johannes Weiner,
Konrad Rzeszutek Wilk, Latchesar Ionkov, linux-cifs,
linux-fsdevel, linux-mm, linux-nfs, linux-nvdimm, Matthew Wilcox,
Ron Minnich, samba-technical, Steve French, Trond Myklebust,
v9fs-developer
In-Reply-To: <20170426085235.GA21738@quack2.suse.cz>
On Wed, Apr 26, 2017 at 10:52:35AM +0200, Jan Kara wrote:
> On Tue 25-04-17 16:59:36, Ross Zwisler wrote:
> > On Tue, Apr 25, 2017 at 01:10:43PM +0200, Jan Kara wrote:
> > <>
> > > Hum, but now thinking more about it I have hard time figuring out why write
> > > vs fault cannot actually still race:
> > >
> > > CPU1 - write(2) CPU2 - read fault
> > >
> > > dax_iomap_pte_fault()
> > > ->iomap_begin() - sees hole
> > > dax_iomap_rw()
> > > iomap_apply()
> > > ->iomap_begin - allocates blocks
> > > dax_iomap_actor()
> > > invalidate_inode_pages2_range()
> > > - there's nothing to invalidate
> > > grab_mapping_entry()
> > > - we add zero page in the radix
> > > tree & map it to page tables
> > >
> > > Similarly read vs write fault may end up racing in a wrong way and try to
> > > replace already existing exceptional entry with a hole page?
> >
> > Yep, this race seems real to me, too. This seems very much like the issues
> > that exist when a thread is doing direct I/O. One thread is doing I/O to an
> > intermediate buffer (page cache for direct I/O case, zero page for us), and
> > the other is going around it directly to media, and they can get out of sync.
> >
> > IIRC the direct I/O code looked something like:
> >
> > 1/ invalidate existing mappings
> > 2/ do direct I/O to media
> > 3/ invalidate mappings again, just in case. Should be cheap if there weren't
> > any conflicting faults. This makes sure any new allocations we made are
> > faulted in.
>
> Yeah, the problem is people generally expect weird behavior when they mix
> direct and buffered IO (or let alone mmap) however everyone expects
> standard read(2) and write(2) to be completely coherent with mmap(2).
Yep, fair enough.
> > I guess one option would be to replicate that logic in the DAX I/O path, or we
> > could try and enhance our locking so page faults can't race with I/O since
> > both can allocate blocks.
>
> In the abstract way, the problem is that we have radix tree (and page
> tables) cache block mapping information and the operation: "read block
> mapping information, store it in the radix tree" is not serialized in any
> way against other block allocations so the information we store can be out
> of date by the time we store it.
>
> One way to solve this would be to move ->iomap_begin call in the fault
> paths under entry lock although that would mean I have to redo how ext4
> handles DAX faults because with current code it would create lock inversion
> wrt transaction start.
I don't think this alone is enough to save us. The I/O path doesn't currently
take any DAX radix tree entry locks, so our race would just become:
CPU1 - write(2) CPU2 - read fault
dax_iomap_pte_fault()
grab_mapping_entry() // newly moved
->iomap_begin() - sees hole
dax_iomap_rw()
iomap_apply()
->iomap_begin - allocates blocks
dax_iomap_actor()
invalidate_inode_pages2_range()
- there's nothing to invalidate
- we add zero page in the radix
tree & map it to page tables
In their current form I don't think we want to take DAX radix tree entry locks
in the I/O path because that would effectively serialize I/O over a given
radix tree entry. For a 2MiB entry, for example, all I/O to that 2MiB range
would be serialized.
> Another solution would be to grab i_mmap_sem for write when doing write
> fault of a page and similarly have it grabbed for writing when doing
> write(2). This would scale rather poorly but if we later replaced it with a
> range lock (Davidlohr has already posted a nice implementation of it) it
> won't be as bad. But I guess option 1) is better...
The best idea I had for handling this sounds similar, which would be to
convert the radix tree locks to essentially be reader/writer locks. I/O and
faults that don't modify the block mapping could just take read-level locks,
and could all run concurrently. I/O or faults that modify a block mapping
would take a write lock, and serialize with other writers and readers.
You could know if you needed a write lock without asking the filesystem - if
you're a write and the radix tree entry is empty or is for a zero page, you
grab the write lock.
This dovetails nicely with the idea of having the radix tree act as a cache
for block mappings. You take the appropriate lock on the radix tree entry,
and it has the block mapping info for your I/O or fault so you don't have to
call into the FS. I/O would also participate so we would keep info about
block mappings that we gather from I/O to help shortcut our page faults.
How does this sound vs the range lock idea? How hard do you think it would be
to convert our current wait queue system to reader/writer style locking?
Also, how do you think we should deal with the current PMD corruption? Should
we go with the current fix (I can augment the comments as you suggested), and
then handle optimizations to that approach and the solution to this larger
race as a follow-on?
--
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: [RFC PATCH 00/14] cgroup: Implement cgroup v2 thread mode & CPU controller
From: Tejun Heo @ 2017-04-26 22:30 UTC (permalink / raw)
To: Waiman Long
Cc: Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar, cgroups,
linux-kernel, linux-doc, linux-mm, kernel-team, pjt, luto, efault
In-Reply-To: <fa35c889-85a8-8b85-c836-4c5070cd7cdc@redhat.com>
Hello, Waiman.
On Wed, Apr 26, 2017 at 12:05:27PM -0400, Waiman Long wrote:
> Does anyone has time to take a look at these patches?
>
> As the merge window is going to open up next week, I am not going to
> bother you guys when the merge window opens.
Will get to it next week. Sorry about the delay. We're deploying
cgroup2 across the fleet and seeing a lot of interesting issues and I
was chasing down CPU controller performance issues for the last month
or so, which is now getting wrapped up.
Thanks.
--
tejun
--
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 -v3] mm, swap: Sort swap entries before free
From: Tim Chen @ 2017-04-26 20:13 UTC (permalink / raw)
To: Huang, Ying, Minchan Kim
Cc: Andrew Morton, linux-mm, linux-kernel, Hugh Dickins, Shaohua Li,
Rik van Riel
In-Reply-To: <87y3un2vdp.fsf@yhuang-dev.intel.com>
>
> From 7bd903c42749c448ef6acbbdee8dcbc1c5b498b9 Mon Sep 17 00:00:00 2001
> From: Huang Ying <ying.huang@intel.com>
> Date: Thu, 23 Feb 2017 13:05:20 +0800
> Subject: [PATCH -v5] mm, swap: Sort swap entries before free
>
>A
> ---
> A mm/swapfile.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> A 1 file changed, 38 insertions(+), 5 deletions(-)
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 71890061f653..10e75f9e8ac1 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -37,6 +37,7 @@
> A #include <linux/swapfile.h>
> A #include <linux/export.h>
> A #include <linux/swap_slots.h>
> +#include <linux/sort.h>
> A
> A #include <asm/pgtable.h>
> A #include <asm/tlbflush.h>
> @@ -1065,20 +1066,52 @@ void swapcache_free(swp_entry_t entry)
> A }
> A }
> A
> +static int swp_entry_cmp(const void *ent1, const void *ent2)
> +{
> + const swp_entry_t *e1 = ent1, *e2 = ent2;
> +
> + return (int)(swp_type(*e1) - swp_type(*e2));
> +}
> +
> A void swapcache_free_entries(swp_entry_t *entries, int n)
> A {
> A struct swap_info_struct *p, *prev;
> - int i;
> + int i, m;
> + swp_entry_t entry;
> + unsigned int prev_swp_type;
I think it will be clearer to name prev_swp_type as first_swp_type
as this is the swp type of the first entry.
> A
> A if (n <= 0)
> A return;
> A
> A prev = NULL;
> A p = NULL;
> - for (i = 0; i < n; ++i) {
> - p = swap_info_get_cont(entries[i], prev);
> - if (p)
> - swap_entry_free(p, entries[i]);
> + m = 0;
> + prev_swp_type = swp_type(entries[0]);
> + for (i = 0; i < n; i++) {
> + entry = entries[i];
> + if (likely(swp_type(entry) == prev_swp_type)) {
> + p = swap_info_get_cont(entry, prev);
> + if (likely(p))
> + swap_entry_free(p, entry);
> + prev = p;
> + } else if (!m)
> + m = i;
> + }
> + if (p)
> + spin_unlock(&p->lock);
> + if (likely(!m))
> + return;
> +
We could still have prev_swp_type at the first entry after sorting.
and we can avoid an unlock/relock for this case if we do this:
if (likely(!m)) {
if (p)
spin_unlock(&p->lock);
return;
}
> + /* Sort swap entries by swap device, so each lock is only taken once. */
> + sort(entries + m, n - m, sizeof(entries[0]), swp_entry_cmp, NULL);
> + prev = NULL;
Can eliminate prev=NULL if we adopt the above change.
> + for (i = m; i < n; i++) {
> + entry = entries[i];
> + if (swp_type(entry) == prev_swp_type)
> + continue;
The if/continue statement seems incorrect. When swp_type(entry) == prev_swp_type
we also need to free entry. A The if/continue statement should be deleted.
Say we have 3 entries with swp_type
1,2,1
We will get prev_swp_type as 1 and free the first entry
and sort the remaining two. A The last entry with
swp_type 1 will not be freed.
> + p = swap_info_get_cont(entry, prev);
> + if (likely(p))
> + swap_entry_free(p, entry);
> A prev = p;
> A }
> A if (p)
Thanks.
Tim
--
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 4/4] mm: Adaptive hash table scaling
From: Michal Hocko @ 2017-04-26 20:11 UTC (permalink / raw)
To: Andrew Morton
Cc: Pavel Tatashin, linux-mm, sparclinux, linux-fsdevel, Al Viro
In-Reply-To: <20170303153247.f16a31c95404c02a8f3e2c5f@linux-foundation.org>
On Fri 03-03-17 15:32:47, Andrew Morton wrote:
> On Thu, 2 Mar 2017 00:33:45 -0500 Pavel Tatashin <pasha.tatashin@oracle.com> wrote:
>
> > Allow hash tables to scale with memory but at slower pace, when HASH_ADAPT
> > is provided every time memory quadruples the sizes of hash tables will only
> > double instead of quadrupling as well. This algorithm starts working only
> > when memory size reaches a certain point, currently set to 64G.
> >
> > This is example of dentry hash table size, before and after four various
> > memory configurations:
> >
> > MEMORY SCALE HASH_SIZE
> > old new old new
> > 8G 13 13 8M 8M
> > 16G 13 13 16M 16M
> > 32G 13 13 32M 32M
> > 64G 13 13 64M 64M
> > 128G 13 14 128M 64M
> > 256G 13 14 256M 128M
> > 512G 13 15 512M 128M
> > 1024G 13 15 1024M 256M
> > 2048G 13 16 2048M 256M
> > 4096G 13 16 4096M 512M
> > 8192G 13 17 8192M 512M
> > 16384G 13 17 16384M 1024M
> > 32768G 13 18 32768M 1024M
> > 65536G 13 18 65536M 2048M
>
> OK, but what are the runtime effects? Presumably some workloads will
> slow down a bit. How much? How do we know that this is a worthwhile
> tradeoff?
>
> If the effect of this change is "undetectable" then those hash tables
> are simply too large, and additional tuning is needed, yes?
I am playing with a 3TB and have hit the following
[ 0.961309] Dentry cache hash table entries: 536870912 (order: 20, 4294967296 bytes)
[ 2.300012] vmalloc: allocation failure, allocated 1383612416 of 2147487744 bytes
[ 2.307473] swapper/0: page allocation failure: order:0, mode:0x2080020(GFP_ATOMIC)
[ 2.315101] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 4.4.49-hotplug19-default #1
[ 2.324017] Hardware name: Huawei 9008/IT91SMUB, BIOS BLXSV607 04/17/2017
[ 2.330775] ffffffff8101aba5 ffffffff8130efa0 ffffffff81863f48 ffffffff81c03e40
[ 2.338201] ffffffff8118c9a2 02080020fff00300 ffffffff81863f48 ffffffff81c03de0
[ 2.345628] 0000000000000018 ffffffff81c03e50 ffffffff81c03df8 ffffffff811d28e6
[ 2.353056] Call Trace:
[ 2.355507] [<ffffffff81019a99>] dump_trace+0x59/0x310
[ 2.360710] [<ffffffff81019e3a>] show_stack_log_lvl+0xea/0x170
[ 2.366605] [<ffffffff8101abc1>] show_stack+0x21/0x40
[ 2.371723] [<ffffffff8130efa0>] dump_stack+0x5c/0x7c
[ 2.376842] [<ffffffff8118c9a2>] warn_alloc_failed+0xe2/0x150
[ 2.382655] [<ffffffff811c2a10>] __vmalloc_node_range+0x240/0x280
[ 2.388814] [<ffffffff811c2a97>] __vmalloc+0x47/0x50
[ 2.393851] [<ffffffff81da02ae>] alloc_large_system_hash+0x189/0x25d
[ 2.400264] [<ffffffff81da7625>] inode_init+0x74/0xa3
[ 2.405381] [<ffffffff81da7483>] vfs_caches_init+0x59/0xe1
[ 2.410930] [<ffffffff81d6f070>] start_kernel+0x474/0x4d0
[ 2.416392] [<ffffffff81d6e719>] x86_64_start_kernel+0x147/0x156
Allocating 4G for a hash table is just ridiculous. 512MB which this
patch should give looks much reasonable, although I would argue it is
still a _lot_.
I cannot say I would be really happy about the chosen approach,
though. Why HASH_ADAPT is not implicit? Which hash table would need
gigabytes of memory and still benefit from it? Even if there is such an
example then it should use the explicit high_limit. I do not like this
opt-in because it is just too easy to miss that and hit the same issue
again. And in fact only few users of alloc_large_system_hash are using
the flag. E.g. why {dcache,inode}_init_early do not have the flag? I
am pretty sure that having a physically contiguous hash table would be
better over vmalloc from the TLB point of view.
mount_hashtable resp. mountpoint_hashtable are another example. Other
users just have a reasonable max value. So can we do the following
on top of your commit? I think that we should rethink the scaling as
well but I do not have a good answer for the maximum size so let's just
start with a more reasonable API first.
---
diff --git a/fs/dcache.c b/fs/dcache.c
index 808ea99062c2..363502faa328 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3585,7 +3585,7 @@ static void __init dcache_init(void)
sizeof(struct hlist_bl_head),
dhash_entries,
13,
- HASH_ZERO | HASH_ADAPT,
+ HASH_ZERO,
&d_hash_shift,
&d_hash_mask,
0,
diff --git a/fs/inode.c b/fs/inode.c
index a9caf53df446..b3c0731ec1fe 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1950,7 +1950,7 @@ void __init inode_init(void)
sizeof(struct hlist_head),
ihash_entries,
14,
- HASH_ZERO | HASH_ADAPT,
+ HASH_ZERO,
&i_hash_shift,
&i_hash_mask,
0,
diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
index dbaf312b3317..e223d91b6439 100644
--- a/include/linux/bootmem.h
+++ b/include/linux/bootmem.h
@@ -359,7 +359,6 @@ extern void *alloc_large_system_hash(const char *tablename,
#define HASH_SMALL 0x00000002 /* sub-page allocation allowed, min
* shift passed via *_hash_shift */
#define HASH_ZERO 0x00000004 /* Zero allocated hash table */
-#define HASH_ADAPT 0x00000008 /* Adaptive scale for large memory */
/* Only NUMA needs hash distribution. 64bit NUMA architectures have
* sufficient vmalloc space.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fa752de84eef..3bf60669d200 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7226,7 +7226,7 @@ void *__init alloc_large_system_hash(const char *tablename,
if (PAGE_SHIFT < 20)
numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
- if (flags & HASH_ADAPT) {
+ if (!high_limit) {
unsigned long adapt;
for (adapt = ADAPT_SCALE_NPAGES; adapt < numentries;
--
Michal Hocko
SUSE Labs
--
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
* [PATCH v3 2/2] dax: add regression test for stale mmap reads
From: Ross Zwisler @ 2017-04-26 18:05 UTC (permalink / raw)
To: fstests, Xiong Zhou, jmoyer, eguan
Cc: Ross Zwisler, Christoph Hellwig, Dan Williams, Darrick J. Wong,
Jan Kara, linux-fsdevel, linux-mm, linux-nvdimm, Andrew Morton
In-Reply-To: <20170426180531.26291-1-ross.zwisler@linux.intel.com>
This adds a regression test for the following kernel patch:
dax: fix data corruption due to stale mmap reads
The above patch fixes an issue where users of DAX can suffer data
corruption from stale mmap reads via the following sequence:
- open an mmap over a 2MiB hole
- read from a 2MiB hole, faulting in a 2MiB zero page
- write to the hole with write(3p). The write succeeds but we incorrectly
leave the 2MiB zero page mapping intact.
- via the mmap, read the data that was just written. Since the zero page
mapping is still intact we read back zeroes instead of the new data.
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
.gitignore | 1 +
src/Makefile | 2 +-
src/t_mmap_stale_pmd.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/generic/427 | 59 ++++++++++++++++++++++++++++++++++++++++
tests/generic/427.out | 2 ++
tests/generic/group | 1 +
6 files changed, 137 insertions(+), 1 deletion(-)
create mode 100644 src/t_mmap_stale_pmd.c
create mode 100755 tests/generic/427
create mode 100644 tests/generic/427.out
diff --git a/.gitignore b/.gitignore
index ded4a61..4d64414 100644
--- a/.gitignore
+++ b/.gitignore
@@ -134,6 +134,7 @@
/src/renameat2
/src/t_rename_overwrite
/src/t_mmap_dio
+/src/t_mmap_stale_pmd
# dmapi/ binaries
/dmapi/src/common/cmd/read_invis
diff --git a/src/Makefile b/src/Makefile
index abfd873..f1338ca 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -12,7 +12,7 @@ TARGETS = dirstress fill fill2 getpagesize holes lstat64 \
godown resvtest writemod makeextents itrash rename \
multi_open_unlink dmiperf unwritten_sync genhashnames t_holes \
t_mmap_writev t_truncate_cmtime dirhash_collide t_rename_overwrite \
- holetest t_truncate_self t_mmap_dio af_unix
+ holetest t_truncate_self t_mmap_dio af_unix t_mmap_stale_pmd
LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
diff --git a/src/t_mmap_stale_pmd.c b/src/t_mmap_stale_pmd.c
new file mode 100644
index 0000000..b447222
--- /dev/null
+++ b/src/t_mmap_stale_pmd.c
@@ -0,0 +1,73 @@
+#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define MiB(a) ((a)*1024*1024)
+
+void err_exit(char *op)
+{
+ fprintf(stderr, "%s: %s\n", op, strerror(errno));
+ exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+ volatile int a __attribute__((__unused__));
+ char *buffer = "HELLO WORLD!";
+ char *data;
+ int fd, err, ret = 0;
+
+ if (argc < 2) {
+ printf("Usage: %s <pmem file>\n", basename(argv[0]));
+ exit(0);
+ }
+
+ fd = open(argv[1], O_RDWR|O_CREAT, S_IRUSR|S_IWUSR);
+ if (fd < 0)
+ err_exit("fd");
+
+ /*
+ * This allows us to map a huge zero page, and we do it at a non-zero
+ * offset for a little additional testing.
+ */
+ ftruncate(fd, 0);
+ ftruncate(fd, MiB(4));
+
+ data = mmap(NULL, MiB(2), PROT_READ, MAP_SHARED, fd, MiB(2));
+
+ /*
+ * This faults in a 2MiB zero page to satisfy the read.
+ * 'a' is volatile so this read doesn't get optimized out.
+ */
+ a = data[0];
+
+ pwrite(fd, buffer, strlen(buffer), MiB(2));
+
+ /*
+ * Try and use the mmap to read back the data we just wrote with
+ * pwrite(). If the kernel bug is present the mapping from the 2MiB
+ * zero page will still be intact, and we'll read back zeros instead.
+ */
+ if (strncmp(buffer, data, strlen(buffer))) {
+ fprintf(stderr, "strncmp mismatch: '%s' vs '%s'\n", buffer,
+ data);
+ ret = 1;
+ }
+
+ err = munmap(data, MiB(2));
+ if (err < 0)
+ err_exit("munmap");
+
+ err = close(fd);
+ if (err < 0)
+ err_exit("close");
+
+ return ret;
+}
diff --git a/tests/generic/427 b/tests/generic/427
new file mode 100755
index 0000000..3677fce
--- /dev/null
+++ b/tests/generic/427
@@ -0,0 +1,59 @@
+#! /bin/bash
+# FS QA Test 427
+#
+# This is a regression test for kernel patch:
+# dax: fix data corruption due to stale mmap reads
+# created by Ross Zwisler <ross.zwisler@linux.intel.com>
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Intel Corporation. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+ cd /
+ rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+_require_test_program "t_mmap_stale_pmd"
+
+# real QA test starts here
+src/t_mmap_stale_pmd $TEST_DIR/testfile
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/427.out b/tests/generic/427.out
new file mode 100644
index 0000000..61295e5
--- /dev/null
+++ b/tests/generic/427.out
@@ -0,0 +1,2 @@
+QA output created by 427
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index f29009c..06f6e9d 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -429,3 +429,4 @@
424 auto quick
425 auto quick attr
426 auto quick exportfs
+427 auto quick
--
2.9.3
--
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
* [PATCH v3 1/2] xfs: fix incorrect argument count check
From: Ross Zwisler @ 2017-04-26 18:05 UTC (permalink / raw)
To: fstests, Xiong Zhou, jmoyer, eguan
Cc: Ross Zwisler, Christoph Hellwig, Dan Williams, Darrick J. Wong,
Jan Kara, linux-fsdevel, linux-mm, linux-nvdimm, Andrew Morton
t_mmap_dio.c actually requires 4 arguments, not 3 as the current check
enforces:
# ./src/t_mmap_dio
usage: t_mmap_dio <src file> <dest file> <size> <msg>
# ./src/t_mmap_dio one two three
open src(No such file or directory) len 0 (null)
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Fixes: 456581661b4d ("xfs: test per-inode DAX flag by IO")
---
src/t_mmap_dio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/t_mmap_dio.c b/src/t_mmap_dio.c
index 69b9ca8..6c8ca1a 100644
--- a/src/t_mmap_dio.c
+++ b/src/t_mmap_dio.c
@@ -39,7 +39,7 @@ int main(int argc, char **argv)
char *dfile;
unsigned long len, opt;
- if (argc < 4)
+ if (argc < 5)
usage(basename(argv[0]));
while ((opt = getopt(argc, argv, "b")) != -1)
--
2.9.3
--
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: [PATCH v2 2/2] dax: add regression test for stale mmap reads
From: Ross Zwisler @ 2017-04-26 18:00 UTC (permalink / raw)
To: Xiong Zhou
Cc: Ross Zwisler, fstests, jmoyer, eguan, Christoph Hellwig,
Dan Williams, Darrick J. Wong, Jan Kara, linux-fsdevel, linux-mm,
linux-nvdimm, Andrew Morton
In-Reply-To: <20170426090907.q5jj3ywsvldsbq7n@XZHOUW.usersys.redhat.com>
On Wed, Apr 26, 2017 at 05:09:07PM +0800, Xiong Zhou wrote:
> On Tue, Apr 25, 2017 at 02:51:06PM -0600, Ross Zwisler wrote:
<>
> > + /*
> > + * Try and use the mmap to read back the data we just wrote with
> > + * pwrite(). If the kernel bug is present the mapping from the 2MiB
> > + * zero page will still be intact, and we'll read back zeros instead.
> > + */
> > + if (strncmp(buffer, data, strlen(buffer))) {
> > + fprintf(stderr, "strncmp mismatch: '%s' vs '%s'\n", buffer,
> > + data);
> munmap
> close(fd);
> > + exit(1);
> > + }
> > +
> munmap
Yep, thanks, fixed in v3.
--
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 2/2] dax: add regression test for stale mmap reads
From: Ross Zwisler @ 2017-04-26 17:50 UTC (permalink / raw)
To: Eryu Guan
Cc: Ross Zwisler, fstests, Xiong Zhou, jmoyer, Christoph Hellwig,
Dan Williams, Darrick J. Wong, Jan Kara, linux-fsdevel, linux-mm,
linux-nvdimm, Andrew Morton
In-Reply-To: <20170426074727.GG26397@eguan.usersys.redhat.com>
On Wed, Apr 26, 2017 at 03:47:27PM +0800, Eryu Guan wrote:
> On Tue, Apr 25, 2017 at 02:51:06PM -0600, Ross Zwisler wrote:
<>
> > diff --git a/tests/generic/427 b/tests/generic/427
> > new file mode 100755
> > index 0000000..6e265a1
> > --- /dev/null
> > +++ b/tests/generic/427
> > @@ -0,0 +1,67 @@
> > +#! /bin/bash
> > +# FS QA Test 427
> > +#
> > +# This is a regression test for kernel patch:
> > +# dax: fix data corruption due to stale mmap reads
> > +# created by Ross Zwisler <ross.zwisler@linux.intel.com>
> > +#
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2017 Intel Corporation. All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> > +#-----------------------------------------------------------------------
> > +#
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + cd /
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_supported_os Linux
> > +_require_test_program "t_dax_stale_pmd"
> > +_require_xfs_io_command "falloc"
>
> I'm wondering if falloc is really needed? If not, this test could be run
> with ext2/3 too. See below.
>
> > +_require_user
>
> This is not needed anymore.
Fixed in v3.
> > +
> > +# real QA test starts here
> > +
> > +# ensure we have no pre-existing block allocations, so we get a hole
> > +rm -f $TEST_DIR/testfile
> > +$XFS_IO_PROG -f -c "falloc 0 4M" $TEST_DIR/testfile >> $seqres.full 2>&1
>
> I found that 'xfs_io -fc "truncate 4M" $TEST_DIR/testfile' works too,
> from the comments in test and kernel patch, if I understand correctly,
> we only need to mmap un-allocated blocks, right?
>
> If truncate(2) works too, I think we can move truncate operation to the
> t_dax_stale_pmd program too, because the whole truncate && mmap && read
> sequence are logically together, this also avoids the confusion on why
> testfile is in 4M size.
Yep, that works. In v3 I've moved to using only ftruncate so we can enable
ext2/3, and I've moved those calls into the C file with comments explaining
why we're doing things.
--
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: [RFC PATCH 00/14] cgroup: Implement cgroup v2 thread mode & CPU controller
From: Waiman Long @ 2017-04-26 16:05 UTC (permalink / raw)
To: Tejun Heo, Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar
Cc: cgroups, linux-kernel, linux-doc, linux-mm, kernel-team, pjt,
luto, efault
In-Reply-To: <1492783452-12267-1-git-send-email-longman@redhat.com>
On 04/21/2017 10:03 AM, Waiman Long wrote:
> This patchset incorporates the following 2 patchsets from Tejun Heo:
>
> 1) cgroup v2 thread mode patchset (5 patches)
> https://lkml.org/lkml/2017/2/2/592
> 2) CPU Controller on Control Group v2 (2 patches)
> https://lkml.org/lkml/2016/8/5/368
>
> Additional patches are then layered on top to implement the following
> new features:
>
> 1) An enhanced v2 thread mode where a thread root (root of a threaded
> subtree) can have non-threaded children with non-threaded
> controllers enabled and no internal process constraint does
> not apply.
> 2) An enhanced debug controller which dumps out more information
> relevant to the debugging and testing of cgroup v2 in general.
> 3) Separate control knobs for resource domain controllers can be
> enabled in a thread root to manage all the internal processes in
> the threaded subtree.
>
> Patches 1-5 are Tejun's cgroup v2 thread mode patchset.
>
> Patch 6 fixes a task_struct reference counting bug introduced in
> patch 1.
>
> Patch 7 moves the debug cgroup out from cgroup_v1.c into its own
> file.
>
> Patch 8 keeps more accurate counts of the number of tasks associated
> with each css_set.
>
> Patch 9 enhances the debug controller to provide more information
> relevant to the cgroup v2 thread mode to ease debugging effort.
>
> Patch 10 implements the enhanced cgroup v2 thread mode with the
> following enhancements:
>
> 1) Thread roots are treated differently from threaded cgroups.
> 2) Thread root can now have non-threaded controllers enabled as well
> as non-threaded children.
>
> Patches 11-12 are Tejun's CPU controller on control group v2 patchset.
>
> Patch 13 makes both cpu and cpuacct controllers threaded.
>
> Patch 14 enables the creation of a special "cgroup.self" directory
> under the thread root to hold resource control knobs for controllers
> that don't want resource competiton between internal processes and
> non-threaded child cgroups.
>
> Preliminary testing was done with the debug controller enabled. Things
> seemed to work fine so far. More rigorous testing will be needed, and
> any suggestions are welcome.
>
> Tejun Heo (7):
> cgroup: reorganize cgroup.procs / task write path
> cgroup: add @flags to css_task_iter_start() and implement
> CSS_TASK_ITER_PROCS
> cgroup: introduce cgroup->proc_cgrp and threaded css_set handling
> cgroup: implement CSS_TASK_ITER_THREADED
> cgroup: implement cgroup v2 thread support
> sched: Misc preps for cgroup unified hierarchy interface
> sched: Implement interface for cgroup unified hierarchy
>
> Waiman Long (7):
> cgroup: Fix reference counting bug in cgroup_procs_write()
> cgroup: Move debug cgroup to its own file
> cgroup: Keep accurate count of tasks in each css_set
> cgroup: Make debug cgroup support v2 and thread mode
> cgroup: Implement new thread mode semantics
> sched: Make cpu/cpuacct threaded controllers
> cgroup: Enable separate control knobs for thread root internal
> processes
>
> Documentation/cgroup-v2.txt | 114 +++++-
> include/linux/cgroup-defs.h | 56 +++
> include/linux/cgroup.h | 12 +-
> kernel/cgroup/Makefile | 1 +
> kernel/cgroup/cgroup-internal.h | 18 +-
> kernel/cgroup/cgroup-v1.c | 217 +++-------
> kernel/cgroup/cgroup.c | 862 ++++++++++++++++++++++++++++++++++------
> kernel/cgroup/cpuset.c | 6 +-
> kernel/cgroup/debug.c | 284 +++++++++++++
> kernel/cgroup/freezer.c | 6 +-
> kernel/cgroup/pids.c | 1 +
> kernel/events/core.c | 1 +
> kernel/sched/core.c | 150 ++++++-
> kernel/sched/cpuacct.c | 55 ++-
> kernel/sched/cpuacct.h | 5 +
> mm/memcontrol.c | 3 +-
> net/core/netclassid_cgroup.c | 2 +-
> 17 files changed, 1478 insertions(+), 315 deletions(-)
> create mode 100644 kernel/cgroup/debug.c
>
Does anyone has time to take a look at these patches?
As the merge window is going to open up next week, I am not going to
bother you guys when the merge window opens.
Cheers,
Longman
--
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] Remove hardcoding of ___GFP_xxx bitmasks
From: Igor Stoppa @ 2017-04-26 15:29 UTC (permalink / raw)
To: Michal Hocko; +Cc: namhyung, linux-mm, linux-kernel
In-Reply-To: <20170426144750.GH12504@dhcp22.suse.cz>
On 26/04/17 17:47, Michal Hocko wrote:
> On Wed 26-04-17 16:35:49, Igor Stoppa wrote:
>> The bitmasks used for ___GFP_xxx can be defined in terms of an enum,
>> which doesn't require manual updates to its values.
>
> GFP masks are rarely updated so why is this worth doing?
I have plans for that [1] - yeah, I should have not written only to ml -
but I thought there was sufficient value in this patch to be sent alone.
I got into this part of the code because (if I understood correctly)
there are no spare bits available from the 32bits mask that is currently
in use.
Adding a new zone, therefore, would cause the bumping to a 64bits type.
If the zone is not strictly needed, some people might prefer to have the
option to stick to 32 bits.
Which in turn would mean more #ifdefs.
Using the enum should provide the same flexibility with a more limited
number of #ifdefs in the code.
But I really thought that there is a value in the change per-se.
Regardless of what other patches might follow.
>> As bonus, __GFP_BITS_SHIFT is automatically kept consistent.
>
> this alone doesn't sound like a huge win to me, to be honest. We already
> have ___GFP_$FOO and __GFP_FOO you are adding __GFP_$FOO_SHIFT. This is
> too much IMHO.
I do not like the #defines being floating and potentially inconsistent
with the rest, when they are supposed to represent all the individual
bits in a bitmask.
One might argue that an error will be detected fairly soon, but to me
using an enum to automatically manage the values and counter of items
seems preferable.
> Also the current mm tree has ___GFP_NOLOCKDEP which is not addressed
> here so I suspect you have based your change on the Linus tree.
I used your tree from kernel.org - I asked yesterday on #mm if it was a
good idea and was told that it should be ok, so I did it, but I can redo
the patch with mm.
If you prefer to have this patch only as part of the larger patchset,
I'm also fine with it.
Also, if you could reply to [1], that would be greatly appreciated.
Maybe I'm starting from some wrong assumption or there is a better way
to achieve what I want.
thanks, igor
[1] http://marc.info/?l=linux-mm&m=149276346722464&w=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 1/1] Remove hardcoding of ___GFP_xxx bitmasks
From: Michal Hocko @ 2017-04-26 14:47 UTC (permalink / raw)
To: Igor Stoppa; +Cc: namhyung, linux-mm, linux-kernel
In-Reply-To: <20170426133549.22603-2-igor.stoppa@huawei.com>
On Wed 26-04-17 16:35:49, Igor Stoppa wrote:
> The bitmasks used for ___GFP_xxx can be defined in terms of an enum,
> which doesn't require manual updates to its values.
GFP masks are rarely updated so why is this worth doing?
> As bonus, __GFP_BITS_SHIFT is automatically kept consistent.
this alone doesn't sound like a huge win to me, to be honest. We already
have ___GFP_$FOO and __GFP_FOO you are adding __GFP_$FOO_SHIFT. This is
too much IMHO.
Also the current mm tree has ___GFP_NOLOCKDEP which is not addressed
here so I suspect you have based your change on the Linus tree.
> Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
> ---
> include/linux/gfp.h | 82 +++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 55 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 0fe0b62..2f894c5 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -14,33 +14,62 @@ struct vm_area_struct;
> * include/trace/events/mmflags.h and tools/perf/builtin-kmem.c
> */
>
> +enum gfp_bitmask_shift {
> + __GFP_DMA_SHIFT = 0,
> + __GFP_HIGHMEM_SHIFT,
> + __GFP_DMA32_SHIFT,
> + __GFP_MOVABLE_SHIFT,
> + __GFP_RECLAIMABLE_SHIFT,
> + __GFP_HIGH_SHIFT,
> + __GFP_IO_SHIFT,
> + __GFP_FS_SHIFT,
> + __GFP_COLD_SHIFT,
> + __GFP_NOWARN_SHIFT,
> + __GFP_REPEAT_SHIFT,
> + __GFP_NOFAIL_SHIFT,
> + __GFP_NORETRY_SHIFT,
> + __GFP_MEMALLOC_SHIFT,
> + __GFP_COMP_SHIFT,
> + __GFP_ZERO_SHIFT,
> + __GFP_NOMEMALLOC_SHIFT,
> + __GFP_HARDWALL_SHIFT,
> + __GFP_THISNODE_SHIFT,
> + __GFP_ATOMIC_SHIFT,
> + __GFP_ACCOUNT_SHIFT,
> + __GFP_NOTRACK_SHIFT,
> + __GFP_DIRECT_RECLAIM_SHIFT,
> + __GFP_WRITE_SHIFT,
> + __GFP_KSWAPD_RECLAIM_SHIFT,
> + __GFP_BITS_SHIFT
> +};
> +
> +
> /* Plain integer GFP bitmasks. Do not use this directly. */
> -#define ___GFP_DMA 0x01u
> -#define ___GFP_HIGHMEM 0x02u
> -#define ___GFP_DMA32 0x04u
> -#define ___GFP_MOVABLE 0x08u
> -#define ___GFP_RECLAIMABLE 0x10u
> -#define ___GFP_HIGH 0x20u
> -#define ___GFP_IO 0x40u
> -#define ___GFP_FS 0x80u
> -#define ___GFP_COLD 0x100u
> -#define ___GFP_NOWARN 0x200u
> -#define ___GFP_REPEAT 0x400u
> -#define ___GFP_NOFAIL 0x800u
> -#define ___GFP_NORETRY 0x1000u
> -#define ___GFP_MEMALLOC 0x2000u
> -#define ___GFP_COMP 0x4000u
> -#define ___GFP_ZERO 0x8000u
> -#define ___GFP_NOMEMALLOC 0x10000u
> -#define ___GFP_HARDWALL 0x20000u
> -#define ___GFP_THISNODE 0x40000u
> -#define ___GFP_ATOMIC 0x80000u
> -#define ___GFP_ACCOUNT 0x100000u
> -#define ___GFP_NOTRACK 0x200000u
> -#define ___GFP_DIRECT_RECLAIM 0x400000u
> -#define ___GFP_WRITE 0x800000u
> -#define ___GFP_KSWAPD_RECLAIM 0x1000000u
> -/* If the above are modified, __GFP_BITS_SHIFT may need updating */
> +#define ___GFP_DMA (1u << __GFP_DMA_SHIFT)
> +#define ___GFP_HIGHMEM (1u << __GFP_HIGHMEM_SHIFT)
> +#define ___GFP_DMA32 (1u << __GFP_DMA32_SHIFT)
> +#define ___GFP_MOVABLE (1u << __GFP_MOVABLE_SHIFT)
> +#define ___GFP_RECLAIMABLE (1u << __GFP_RECLAIMABLE_SHIFT)
> +#define ___GFP_HIGH (1u << __GFP_HIGH_SHIFT)
> +#define ___GFP_IO (1u << __GFP_IO_SHIFT)
> +#define ___GFP_FS (1u << __GFP_FS_SHIFT)
> +#define ___GFP_COLD (1u << __GFP_COLD_SHIFT)
> +#define ___GFP_NOWARN (1u << __GFP_NOWARN_SHIFT)
> +#define ___GFP_REPEAT (1u << __GFP_REPEAT_SHIFT)
> +#define ___GFP_NOFAIL (1u << __GFP_NOFAIL_SHIFT)
> +#define ___GFP_NORETRY (1u << __GFP_NORETRY_SHIFT)
> +#define ___GFP_MEMALLOC (1u << __GFP_MEMALLOC_SHIFT)
> +#define ___GFP_COMP (1u << __GFP_COMP_SHIFT)
> +#define ___GFP_ZERO (1u << __GFP_ZERO_SHIFT)
> +#define ___GFP_NOMEMALLOC (1u << __GFP_NOMEMALLOC_SHIFT)
> +#define ___GFP_HARDWALL (1u << __GFP_HARDWALL_SHIFT)
> +#define ___GFP_THISNODE (1u << __GFP_THISNODE_SHIFT)
> +#define ___GFP_ATOMIC (1u << __GFP_ATOMIC_SHIFT)
> +#define ___GFP_ACCOUNT (1u << __GFP_ACCOUNT_SHIFT)
> +#define ___GFP_NOTRACK (1u << __GFP_NOTRACK_SHIFT)
> +#define ___GFP_DIRECT_RECLAIM (1u << __GFP_DIRECT_RECLAIM_SHIFT)
> +#define ___GFP_WRITE (1u << __GFP_WRITE_SHIFT)
> +#define ___GFP_KSWAPD_RECLAIM (1u << __GFP_KSWAPD_RECLAIM_SHIFT)
>
> /*
> * Physical address zone modifiers (see linux/mmzone.h - low four bits)
> @@ -180,7 +209,6 @@ struct vm_area_struct;
> #define __GFP_NOTRACK_FALSE_POSITIVE (__GFP_NOTRACK)
>
> /* Room for N __GFP_FOO bits */
> -#define __GFP_BITS_SHIFT 25
> #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
>
> /*
> --
> 2.9.3
>
--
Michal Hocko
SUSE Labs
--
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] powerpc/mm/hugetlb: Add support for 1G huge pages
From: Anshuman Khandual @ 2017-04-26 13:48 UTC (permalink / raw)
To: Aneesh Kumar K.V, akpm, mpe, benh, paulus
Cc: linux-mm, linux-kernel, linuxppc-dev
In-Reply-To: <1492449255-29062-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On 04/17/2017 10:44 PM, Aneesh Kumar K.V wrote:
> POWER9 supports hugepages of size 2M and 1G in radix MMU mode. This patch
> enables the usage of 1G page size for hugetlbfs. This also update the helper
> such we can do 1G page allocation at runtime.
>
> Since we can do this only when radix translation mode is enabled, we can't use
> the generic gigantic_page_supported helper. Hence provide a way for architecture
> to override gigantic_page_supported helper.
>
> We still don't enable 1G page size on DD1 version. This is to avoid doing
> workaround mentioned in commit: 6d3a0379ebdc8 (powerpc/mm: Add
> radix__tlb_flush_pte_p9_dd1()
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/book3s/64/hugetlb.h | 13 +++++++++++++
> arch/powerpc/mm/hugetlbpage.c | 7 +++++--
> arch/powerpc/platforms/Kconfig.cputype | 1 +
> mm/hugetlb.c | 4 ++++
> 4 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> index 6666cd366596..86f27cc8ec61 100644
> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> @@ -50,4 +50,17 @@ static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
> else
> return entry;
> }
> +
> +#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
> + ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> + defined(CONFIG_CMA))
> +#define gigantic_page_supported gigantic_page_supported
As I have mentioned in later part of the reply, it does not really
make sense to have both arch call back as well as generic config
option checking to decide on whether a feature is enabled or not.
> +static inline bool gigantic_page_supported(void)
> +{
> + if (radix_enabled())
> + return true;
> + return false;
> +}
> +#endif
> +
> #endif
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index a4f33de4008e..80f6d2ed551a 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -763,8 +763,11 @@ static int __init add_huge_page_size(unsigned long long size)
> * Hash: 16M and 16G
> */
> if (radix_enabled()) {
> - if (mmu_psize != MMU_PAGE_2M)
> - return -EINVAL;
> + if (mmu_psize != MMU_PAGE_2M) {
> + if (cpu_has_feature(CPU_FTR_POWER9_DD1) ||
> + (mmu_psize != MMU_PAGE_1G))
> + return -EINVAL;
> + }
> } else {
> if (mmu_psize != MMU_PAGE_16M && mmu_psize != MMU_PAGE_16G)
> return -EINVAL;
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index ef4c4b8fc547..f4ba4bf0d762 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -343,6 +343,7 @@ config PPC_STD_MMU_64
> config PPC_RADIX_MMU
> bool "Radix MMU Support"
> depends on PPC_BOOK3S_64
> + select ARCH_HAS_GIGANTIC_PAGE
> default y
> help
As we are already checking for radix_enabled() test inside function
gigantic_page_supported(), do we still need to conditionally enable
this on Radix based platforms only ?
> Enable support for the Power ISA 3.0 Radix style MMU. Currently this
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 3d0aab9ee80d..2c090189f314 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1158,7 +1158,11 @@ static int alloc_fresh_gigantic_page(struct hstate *h,
> return 0;
> }
>
> +#ifndef gigantic_page_supported
> static inline bool gigantic_page_supported(void) { return true; }
> +#define gigantic_page_supported gigantic_page_supported
> +#endif
As seen above, now that arch's decision to support this feature is not
based solely on ARCH_HAS_GIGANTIC_PAGE config option but also on the
availability of platform features like radix, is it a good time to have
an arch call back deciding on gigantic_page_supported() test instead of
just checking presence of config options like
#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
defined(CONFIG_CMA))
We should not have both as proposed. I mean CONFIG_ARCH_HAS_GIGANTIC_PAGE
should not be enabled unless we have MEMORY_ISOLATION && COMPACTION && CMA
and once enabled we should have arch_gigantic_page_supported() deciding for
gigantic_page_supported().
--
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 1/1] Remove hardcoding of ___GFP_xxx bitmasks
From: Igor Stoppa @ 2017-04-26 13:35 UTC (permalink / raw)
To: mhocko, namhyung; +Cc: linux-mm, linux-kernel, Igor Stoppa
In-Reply-To: <20170426133549.22603-1-igor.stoppa@huawei.com>
The bitmasks used for ___GFP_xxx can be defined in terms of an enum,
which doesn't require manual updates to its values.
As bonus, __GFP_BITS_SHIFT is automatically kept consistent.
Signed-off-by: Igor Stoppa <igor.stoppa@huawei.com>
---
include/linux/gfp.h | 82 +++++++++++++++++++++++++++++++++++------------------
1 file changed, 55 insertions(+), 27 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 0fe0b62..2f894c5 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -14,33 +14,62 @@ struct vm_area_struct;
* include/trace/events/mmflags.h and tools/perf/builtin-kmem.c
*/
+enum gfp_bitmask_shift {
+ __GFP_DMA_SHIFT = 0,
+ __GFP_HIGHMEM_SHIFT,
+ __GFP_DMA32_SHIFT,
+ __GFP_MOVABLE_SHIFT,
+ __GFP_RECLAIMABLE_SHIFT,
+ __GFP_HIGH_SHIFT,
+ __GFP_IO_SHIFT,
+ __GFP_FS_SHIFT,
+ __GFP_COLD_SHIFT,
+ __GFP_NOWARN_SHIFT,
+ __GFP_REPEAT_SHIFT,
+ __GFP_NOFAIL_SHIFT,
+ __GFP_NORETRY_SHIFT,
+ __GFP_MEMALLOC_SHIFT,
+ __GFP_COMP_SHIFT,
+ __GFP_ZERO_SHIFT,
+ __GFP_NOMEMALLOC_SHIFT,
+ __GFP_HARDWALL_SHIFT,
+ __GFP_THISNODE_SHIFT,
+ __GFP_ATOMIC_SHIFT,
+ __GFP_ACCOUNT_SHIFT,
+ __GFP_NOTRACK_SHIFT,
+ __GFP_DIRECT_RECLAIM_SHIFT,
+ __GFP_WRITE_SHIFT,
+ __GFP_KSWAPD_RECLAIM_SHIFT,
+ __GFP_BITS_SHIFT
+};
+
+
/* Plain integer GFP bitmasks. Do not use this directly. */
-#define ___GFP_DMA 0x01u
-#define ___GFP_HIGHMEM 0x02u
-#define ___GFP_DMA32 0x04u
-#define ___GFP_MOVABLE 0x08u
-#define ___GFP_RECLAIMABLE 0x10u
-#define ___GFP_HIGH 0x20u
-#define ___GFP_IO 0x40u
-#define ___GFP_FS 0x80u
-#define ___GFP_COLD 0x100u
-#define ___GFP_NOWARN 0x200u
-#define ___GFP_REPEAT 0x400u
-#define ___GFP_NOFAIL 0x800u
-#define ___GFP_NORETRY 0x1000u
-#define ___GFP_MEMALLOC 0x2000u
-#define ___GFP_COMP 0x4000u
-#define ___GFP_ZERO 0x8000u
-#define ___GFP_NOMEMALLOC 0x10000u
-#define ___GFP_HARDWALL 0x20000u
-#define ___GFP_THISNODE 0x40000u
-#define ___GFP_ATOMIC 0x80000u
-#define ___GFP_ACCOUNT 0x100000u
-#define ___GFP_NOTRACK 0x200000u
-#define ___GFP_DIRECT_RECLAIM 0x400000u
-#define ___GFP_WRITE 0x800000u
-#define ___GFP_KSWAPD_RECLAIM 0x1000000u
-/* If the above are modified, __GFP_BITS_SHIFT may need updating */
+#define ___GFP_DMA (1u << __GFP_DMA_SHIFT)
+#define ___GFP_HIGHMEM (1u << __GFP_HIGHMEM_SHIFT)
+#define ___GFP_DMA32 (1u << __GFP_DMA32_SHIFT)
+#define ___GFP_MOVABLE (1u << __GFP_MOVABLE_SHIFT)
+#define ___GFP_RECLAIMABLE (1u << __GFP_RECLAIMABLE_SHIFT)
+#define ___GFP_HIGH (1u << __GFP_HIGH_SHIFT)
+#define ___GFP_IO (1u << __GFP_IO_SHIFT)
+#define ___GFP_FS (1u << __GFP_FS_SHIFT)
+#define ___GFP_COLD (1u << __GFP_COLD_SHIFT)
+#define ___GFP_NOWARN (1u << __GFP_NOWARN_SHIFT)
+#define ___GFP_REPEAT (1u << __GFP_REPEAT_SHIFT)
+#define ___GFP_NOFAIL (1u << __GFP_NOFAIL_SHIFT)
+#define ___GFP_NORETRY (1u << __GFP_NORETRY_SHIFT)
+#define ___GFP_MEMALLOC (1u << __GFP_MEMALLOC_SHIFT)
+#define ___GFP_COMP (1u << __GFP_COMP_SHIFT)
+#define ___GFP_ZERO (1u << __GFP_ZERO_SHIFT)
+#define ___GFP_NOMEMALLOC (1u << __GFP_NOMEMALLOC_SHIFT)
+#define ___GFP_HARDWALL (1u << __GFP_HARDWALL_SHIFT)
+#define ___GFP_THISNODE (1u << __GFP_THISNODE_SHIFT)
+#define ___GFP_ATOMIC (1u << __GFP_ATOMIC_SHIFT)
+#define ___GFP_ACCOUNT (1u << __GFP_ACCOUNT_SHIFT)
+#define ___GFP_NOTRACK (1u << __GFP_NOTRACK_SHIFT)
+#define ___GFP_DIRECT_RECLAIM (1u << __GFP_DIRECT_RECLAIM_SHIFT)
+#define ___GFP_WRITE (1u << __GFP_WRITE_SHIFT)
+#define ___GFP_KSWAPD_RECLAIM (1u << __GFP_KSWAPD_RECLAIM_SHIFT)
/*
* Physical address zone modifiers (see linux/mmzone.h - low four bits)
@@ -180,7 +209,6 @@ struct vm_area_struct;
#define __GFP_NOTRACK_FALSE_POSITIVE (__GFP_NOTRACK)
/* Room for N __GFP_FOO bits */
-#define __GFP_BITS_SHIFT 25
#define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
/*
--
2.9.3
--
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
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