* Re: [PATCH V3 1/2] mm: Add get_user_pages_cma_migrate
From: Aneesh Kumar K.V @ 2018-10-16 7:16 UTC (permalink / raw)
To: Alexey Kardashevskiy, akpm, Michal Hocko, mpe
Cc: linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <6112386d-65cd-fc1f-b012-e33da2c3b8fe@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 18/09/2018 21:58, Aneesh Kumar K.V wrote:
>> This helper does a get_user_pages_fast and if it find pages in the CMA area
>> it will try to migrate them before taking page reference. This makes sure that
>> we don't keep non-movable pages (due to page reference count) in the CMA area.
>> Not able to move pages out of CMA area result in CMA allocation failures.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>> include/linux/hugetlb.h | 2 +
>> include/linux/migrate.h | 3 +
>> mm/hugetlb.c | 4 +-
>> mm/migrate.c | 132 ++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 139 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index 6b68e345f0ca..1abccb1a1ecc 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -357,6 +357,8 @@ struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
>> nodemask_t *nmask);
>> struct page *alloc_huge_page_vma(struct hstate *h, struct vm_area_struct *vma,
>> unsigned long address);
>> +struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask,
>> + int nid, nodemask_t *nmask);
>> int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
>> pgoff_t idx);
>>
>> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
>> index f2b4abbca55e..d82b35afd2eb 100644
>> --- a/include/linux/migrate.h
>> +++ b/include/linux/migrate.h
>> @@ -286,6 +286,9 @@ static inline int migrate_vma(const struct migrate_vma_ops *ops,
>> }
>> #endif /* IS_ENABLED(CONFIG_MIGRATE_VMA_HELPER) */
>>
>> +extern int get_user_pages_cma_migrate(unsigned long start, int nr_pages, int write,
>> + struct page **pages);
>> +
>> #endif /* CONFIG_MIGRATION */
>>
>> #endif /* _LINUX_MIGRATE_H */
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 3c21775f196b..1abbfcb84f66 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1585,8 +1585,8 @@ static struct page *alloc_surplus_huge_page(struct hstate *h, gfp_t gfp_mask,
>> return page;
>> }
>>
>> -static struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask,
>> - int nid, nodemask_t *nmask)
>> +struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask,
>> + int nid, nodemask_t *nmask)
>> {
>> struct page *page;
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index d6a2e89b086a..2f92534ea7a1 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -3006,3 +3006,135 @@ int migrate_vma(const struct migrate_vma_ops *ops,
>> }
>> EXPORT_SYMBOL(migrate_vma);
>> #endif /* defined(MIGRATE_VMA_HELPER) */
>> +
>> +static struct page *new_non_cma_page(struct page *page, unsigned long private)
>> +{
>> + /*
>> + * We want to make sure we allocate the new page from the same node
>> + * as the source page.
>> + */
>> + int nid = page_to_nid(page);
>> + gfp_t gfp_mask = GFP_USER | __GFP_THISNODE;
>> +
>> + if (PageHighMem(page))
>> + gfp_mask |= __GFP_HIGHMEM;
>> +
>> + if (PageTransHuge(page)) {
>> + struct page *thp;
>> + gfp_t thp_gfpmask = GFP_TRANSHUGE | __GFP_THISNODE;
>> +
>> + /*
>> + * Remove the movable mask so that we don't allocate from
>> + * CMA area again.
>> + */
>> + thp_gfpmask &= ~__GFP_MOVABLE;
>> + thp = __alloc_pages_node(nid, thp_gfpmask, HPAGE_PMD_ORDER);
>
>
> HPAGE_PMD_ORDER is 2MB or 1GB? THP are always that PMD order?
2M or 16M. THP is at PMD level.
>
>
>> + if (!thp)
>> + return NULL;
>> + prep_transhuge_page(thp);
>> + return thp;
>> +
>> +#ifdef CONFIG_HUGETLB_PAGE
>> + } else if (PageHuge(page)) {
>> +
>> + struct hstate *h = page_hstate(page);
>> + /*
>> + * We don't want to dequeue from the pool because pool pages will
>> + * mostly be from the CMA region.
>> + */
>> + return alloc_migrate_huge_page(h, gfp_mask, nid, NULL);
>> +#endif
>> + }
>> +
>> + return __alloc_pages_node(nid, gfp_mask, 0);
>> +}
>> +
>> +/**
>> + * get_user_pages_cma_migrate() - pin user pages in memory by migrating pages in CMA region
>> + * @start: starting user address
>> + * @nr_pages: number of pages from start to pin
>> + * @write: whether pages will be written to
>> + * @pages: array that receives pointers to the pages pinned.
>> + * Should be at least nr_pages long.
>> + *
>> + * Attempt to pin user pages in memory without taking mm->mmap_sem.
>> + * If not successful, it will fall back to taking the lock and
>> + * calling get_user_pages().
>
>
> I do not see any locking or get_user_pages(), hidden somewhere?
>
The rules are same as get_user_pages_fast, which does that pin attempt
without taking mm->mmap_sem. If it fail get_user_pages_fast will take
the mmap_sem and try to pin the pages. The details are in
get_user_pages_fast. You can look at get_user_pages_unlocked
>> + *
>> + * If the pinned pages are backed by CMA region, we migrate those pages out,
>> + * allocating new pages from non-CMA region. This helps in avoiding keeping
>> + * pages pinned in the CMA region for a long time thereby resulting in
>> + * CMA allocation failures.
>> + *
>> + * Returns number of pages pinned. This may be fewer than the number
>> + * requested. If nr_pages is 0 or negative, returns 0. If no pages
>> + * were pinned, returns -errno.
>> + */
>> +
>> +int get_user_pages_cma_migrate(unsigned long start, int nr_pages, int write,
>> + struct page **pages)
>> +{
>> + int i, ret;
>> + bool drain_allow = true;
>> + bool migrate_allow = true;
>> + LIST_HEAD(cma_page_list);
>> +
>> +get_user_again:
>> + ret = get_user_pages_fast(start, nr_pages, write, pages);
>> + if (ret <= 0)
>> + return ret;
>> +
>> + for (i = 0; i < ret; ++i) {
>> + /*
>> + * If we get a page from the CMA zone, since we are going to
>> + * be pinning these entries, we might as well move them out
>> + * of the CMA zone if possible.
>> + */
>> + if (is_migrate_cma_page(pages[i]) && migrate_allow) {
>> + if (PageHuge(pages[i]))
>> + isolate_huge_page(pages[i], &cma_page_list);
>> + else {
>> + struct page *head = compound_head(pages[i]);
>> +
>> + if (!PageLRU(head) && drain_allow) {
>> + lru_add_drain_all();
>> + drain_allow = false;
>> + }
>> +
>> + if (!isolate_lru_page(head)) {
>> + list_add_tail(&head->lru, &cma_page_list);
>> + mod_node_page_state(page_pgdat(head),
>> + NR_ISOLATED_ANON +
>> + page_is_file_cache(head),
>> + hpage_nr_pages(head));
>
>
> Above 10 lines I cannot really comment due to my massive ignorance in
> this area, especially about what lru_add_drain_all() and
> mod_node_page_state() :(
That makes sure we move the pages from per cpu lru vec and add them to
the right lru list so that we can isolate the pages correctly.
>
>
>> + }
>> + }
>> + }
>> + }
>> + if (!list_empty(&cma_page_list)) {
>> + /*
>> + * drop the above get_user_pages reference.
>> + */
>> + for (i = 0; i < ret; ++i)
>> + put_page(pages[i]);
>> +
>> + if (migrate_pages(&cma_page_list, new_non_cma_page,
>> + NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE)) {
>> + /*
>> + * some of the pages failed migration. Do get_user_pages
>> + * without migration.
>> + */
>> + migrate_allow = false;
>
>
> migrate_allow seems useless, simply calling get_user_pages_fast() should
> make the code easier to read imho. And the comment says
> get_user_pages(), where does this guy hide?
I didn't get that suggestion. What we want to do here is try to migrate pages as
long as we find CMA pages in the result of get_user_pages_fast. If we
failed any migration attempt, don't try to migrate again.
>
>> +
>> + if (!list_empty(&cma_page_list))
>> + putback_movable_pages(&cma_page_list);
>> + }
>> + /*
>> + * We did migrate all the pages, Try to get the page references again
>> + * migrating any new CMA pages which we failed to isolate earlier.
>> + */
>> + drain_allow = true;
>
> Move this "drain_allow = true" right after "get_user_again:"? 1
>
>
>> + goto get_user_again;
>> + }
>> + return ret;
>> +}
>>
>
> --
> Alexey
-aneesh
^ permalink raw reply
* Re: [PATCH kernel v2] powerpc/ioda/npu: Call skiboot's hot reset hook when disabling NPU2
From: Alistair Popple @ 2018-10-16 7:32 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Reza Arbab, linuxppc-dev, David Gibson
In-Reply-To: <374a32c1-8787-2b82-af5d-163d9a496a25@ozlabs.ru>
On Tuesday, 16 October 2018 1:22:53 PM AEDT Alexey Kardashevskiy wrote:
>
> On 16/10/2018 13:19, Alistair Popple wrote:
> >> reset_ntl() does what npu2_dev_procedure_reset() does plus more stuff,
> >> there nothing really in npu2_dev_procedure_reset() which reset_ntl()
> >> does not do already from the hardware standpoint. And it did stop HMIs
> >> for me though.
> >>
> >> but ok, what will be sufficient then if not reset_ntl()?
> >
> > Argh, yes you are correct. Specifically both npu2_dev_procedure_reset() and
> > reset_ntl() contain:
> >
> > /* NTL Reset */
> > val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
> > val |= PPC_BIT(8) | PPC_BIT(9);
> > npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
> >
> > Which should fence the brick. However from what I recall there was more to
> > reliably preventing HMIs than merely fencing the brick. It invovled a sequence
> > of fencing and flushing the cache with dcbf instructions at the right time which
> > is why we also have the FLR. Unfortunately I don't know the precise details,
> > perhaps if we send enough coffee Balbir's way he might be able remind us?
>
>
> He suggested and ack'ed that skiboot patch, I can repeat beers^wcoffee
> but it won't change much ;)
Ha. Couldn't hurt ;)
I was pretty sure flushing the caches was an important part of the sequence to
avoid HMI's. I believe you are trying to deal with unexpected guest terminations
which means the driver won't have a chance to flush the caches prior to
termination so wouldn't you also need to do that somewhere? Unless the driver
does it at startup?
- Alistair
> >
> > - Alistair
> >
> >>
> >>
> >>>
> >>> - Alistair
> >>>
> >>>>
> >>>>
> >>>>
> >>>>>
> >>>>> - Alistair
> >>>>>
> >>>>> On Monday, 15 October 2018 6:17:51 PM AEDT Alexey Kardashevskiy wrote:
> >>>>>> Ping?
> >>>>>>
> >>>>>>
> >>>>>> On 02/10/2018 13:20, Alexey Kardashevskiy wrote:
> >>>>>>> The skiboot firmware has a hot reset handler which fences the NVIDIA V100
> >>>>>>> GPU RAM on Witherspoons and makes accesses no-op instead of throwing HMIs:
> >>>>>>> https://github.com/open-power/skiboot/commit/fca2b2b839a67
> >>>>>>>
> >>>>>>> Now we are going to pass V100 via VFIO which most certainly involves
> >>>>>>> KVM guests which are often terminated without getting a chance to offline
> >>>>>>> GPU RAM so we end up with a running machine with misconfigured memory.
> >>>>>>> Accessing this memory produces hardware management interrupts (HMI)
> >>>>>>> which bring the host down.
> >>>>>>>
> >>>>>>> To suppress HMIs, this wires up this hot reset hook to vfio_pci_disable()
> >>>>>>> via pci_disable_device() which switches NPU2 to a safe mode and prevents
> >>>>>>> HMIs.
> >>>>>>>
> >>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>>>>>> ---
> >>>>>>> Changes:
> >>>>>>> v2:
> >>>>>>> * updated the commit log
> >>>>>>> ---
> >>>>>>> arch/powerpc/platforms/powernv/pci-ioda.c | 10 ++++++++++
> >>>>>>> 1 file changed, 10 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> >>>>>>> index cde7102..e37b9cc 100644
> >>>>>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> >>>>>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> >>>>>>> @@ -3688,6 +3688,15 @@ static void pnv_pci_release_device(struct pci_dev *pdev)
> >>>>>>> pnv_ioda_release_pe(pe);
> >>>>>>> }
> >>>>>>>
> >>>>>>> +static void pnv_npu_disable_device(struct pci_dev *pdev)
> >>>>>>> +{
> >>>>>>> + struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
> >>>>>>> + struct eeh_pe *eehpe = edev ? edev->pe : NULL;
> >>>>>>> +
> >>>>>>> + if (eehpe && eeh_ops && eeh_ops->reset)
> >>>>>>> + eeh_ops->reset(eehpe, EEH_RESET_HOT);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> static void pnv_pci_ioda_shutdown(struct pci_controller *hose)
> >>>>>>> {
> >>>>>>> struct pnv_phb *phb = hose->private_data;
> >>>>>>> @@ -3732,6 +3741,7 @@ static const struct pci_controller_ops pnv_npu_ioda_controller_ops = {
> >>>>>>> .reset_secondary_bus = pnv_pci_reset_secondary_bus,
> >>>>>>> .dma_set_mask = pnv_npu_dma_set_mask,
> >>>>>>> .shutdown = pnv_pci_ioda_shutdown,
> >>>>>>> + .disable_device = pnv_npu_disable_device,
> >>>>>>> };
> >>>>>>>
> >>>>>>> static const struct pci_controller_ops pnv_npu_ocapi_ioda_controller_ops = {
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>>
> >>
> >>
> >
> >
>
>
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Export maximum memory value
From: Aravinda Prasad @ 2018-10-16 7:33 UTC (permalink / raw)
To: Naveen N. Rao, linuxppc-dev, mpe, Nathan Fontenot
In-Reply-To: <1539190032.dzxxjmgaua.naveen@linux.ibm.com>
On Wednesday 10 October 2018 10:19 PM, Naveen N. Rao wrote:
> Nathan Fontenot wrote:
>> On 10/10/2018 05:22 AM, Aravinda Prasad wrote:
>>> This patch exports the maximum possible amount of memory
>>> configured on the system via /proc/powerpc/lparcfg.
>>>
>>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/platforms/pseries/lparcfg.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c
>>> b/arch/powerpc/platforms/pseries/lparcfg.c
>>> index 7c872dc..aa82f55 100644
>>> --- a/arch/powerpc/platforms/pseries/lparcfg.c
>>> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
>>> @@ -26,6 +26,7 @@
>>> #include <linux/seq_file.h>
>>> #include <linux/slab.h>
>>> #include <linux/uaccess.h>
>>> +#include <linux/hugetlb.h>
>>> #include <asm/lppaca.h>
>>> #include <asm/hvcall.h>
>>> #include <asm/firmware.h>
>>> @@ -36,6 +37,7 @@
>>> #include <asm/vio.h>
>>> #include <asm/mmu.h>
>>> #include <asm/machdep.h>
>>> +#include <asm/drmem.h>
>>>
>>> #include "pseries.h"
>>>
>>> @@ -433,6 +435,16 @@ static void parse_em_data(struct seq_file *m)
>>> seq_printf(m, "power_mode_data=%016lx\n", retbuf[0]);
>>> }
>>>
>>> +static void maxmem_data(struct seq_file *m)
>>> +{
>>> + unsigned long maxmem = 0;
>>> +
>>> + maxmem += drmem_info->n_lmbs * drmem_info->lmb_size;
>>> + maxmem += hugetlb_total_pages() * PAGE_SIZE;
>>> +
>>> + seq_printf(m, "MaxMem=%ld\n", maxmem);
>>
>> Should this be MaxPossibleMem?
>>
>> At least for the drmem memory the value calculated is the maximum
>> possible
>> memory. I wonder if calling it MaxMem would lead users to think they have
>> that much memory available to them.
>
> That's a good point. This seems to be referred to as just 'maximum
> memory' in the LPAR configuration as well as in the lparstat
> documentation, but it shouldn't hurt to rename it here.
As Naveen mentioned, I am using the term referred in the LPAR
configuration. But, I can rename it.
Regards,
Aravinda
>
> - Naveen
>
--
Regards,
Aravinda
^ permalink raw reply
* Re: [PATCH V3 1/2] mm: Add get_user_pages_cma_migrate
From: Alexey Kardashevskiy @ 2018-10-16 7:52 UTC (permalink / raw)
To: Aneesh Kumar K.V, akpm, Michal Hocko, mpe
Cc: linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <87murewecs.fsf@linux.ibm.com>
On 16/10/2018 18:16, Aneesh Kumar K.V wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>
>> On 18/09/2018 21:58, Aneesh Kumar K.V wrote:
>>> This helper does a get_user_pages_fast and if it find pages in the CMA area
>>> it will try to migrate them before taking page reference. This makes sure that
>>> we don't keep non-movable pages (due to page reference count) in the CMA area.
>>> Not able to move pages out of CMA area result in CMA allocation failures.
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> ---
>>> include/linux/hugetlb.h | 2 +
>>> include/linux/migrate.h | 3 +
>>> mm/hugetlb.c | 4 +-
>>> mm/migrate.c | 132 ++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 139 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>>> index 6b68e345f0ca..1abccb1a1ecc 100644
>>> --- a/include/linux/hugetlb.h
>>> +++ b/include/linux/hugetlb.h
>>> @@ -357,6 +357,8 @@ struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
>>> nodemask_t *nmask);
>>> struct page *alloc_huge_page_vma(struct hstate *h, struct vm_area_struct *vma,
>>> unsigned long address);
>>> +struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask,
>>> + int nid, nodemask_t *nmask);
>>> int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
>>> pgoff_t idx);
>>>
>>> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
>>> index f2b4abbca55e..d82b35afd2eb 100644
>>> --- a/include/linux/migrate.h
>>> +++ b/include/linux/migrate.h
>>> @@ -286,6 +286,9 @@ static inline int migrate_vma(const struct migrate_vma_ops *ops,
>>> }
>>> #endif /* IS_ENABLED(CONFIG_MIGRATE_VMA_HELPER) */
>>>
>>> +extern int get_user_pages_cma_migrate(unsigned long start, int nr_pages, int write,
>>> + struct page **pages);
>>> +
>>> #endif /* CONFIG_MIGRATION */
>>>
>>> #endif /* _LINUX_MIGRATE_H */
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index 3c21775f196b..1abbfcb84f66 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -1585,8 +1585,8 @@ static struct page *alloc_surplus_huge_page(struct hstate *h, gfp_t gfp_mask,
>>> return page;
>>> }
>>>
>>> -static struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask,
>>> - int nid, nodemask_t *nmask)
>>> +struct page *alloc_migrate_huge_page(struct hstate *h, gfp_t gfp_mask,
>>> + int nid, nodemask_t *nmask)
>>> {
>>> struct page *page;
>>>
>>> diff --git a/mm/migrate.c b/mm/migrate.c
>>> index d6a2e89b086a..2f92534ea7a1 100644
>>> --- a/mm/migrate.c
>>> +++ b/mm/migrate.c
>>> @@ -3006,3 +3006,135 @@ int migrate_vma(const struct migrate_vma_ops *ops,
>>> }
>>> EXPORT_SYMBOL(migrate_vma);
>>> #endif /* defined(MIGRATE_VMA_HELPER) */
>>> +
>>> +static struct page *new_non_cma_page(struct page *page, unsigned long private)
>>> +{
>>> + /*
>>> + * We want to make sure we allocate the new page from the same node
>>> + * as the source page.
>>> + */
>>> + int nid = page_to_nid(page);
>>> + gfp_t gfp_mask = GFP_USER | __GFP_THISNODE;
>>> +
>>> + if (PageHighMem(page))
>>> + gfp_mask |= __GFP_HIGHMEM;
>>> +
>>> + if (PageTransHuge(page)) {
>>> + struct page *thp;
>>> + gfp_t thp_gfpmask = GFP_TRANSHUGE | __GFP_THISNODE;
>>> +
>>> + /*
>>> + * Remove the movable mask so that we don't allocate from
>>> + * CMA area again.
>>> + */
>>> + thp_gfpmask &= ~__GFP_MOVABLE;
>>> + thp = __alloc_pages_node(nid, thp_gfpmask, HPAGE_PMD_ORDER);
>>
>>
>> HPAGE_PMD_ORDER is 2MB or 1GB? THP are always that PMD order?
>
> 2M or 16M. THP is at PMD level.
>
>>
>>
>>> + if (!thp)
>>> + return NULL;
>>> + prep_transhuge_page(thp);
>>> + return thp;
>>> +
>>> +#ifdef CONFIG_HUGETLB_PAGE
>>> + } else if (PageHuge(page)) {
>>> +
>>> + struct hstate *h = page_hstate(page);
>>> + /*
>>> + * We don't want to dequeue from the pool because pool pages will
>>> + * mostly be from the CMA region.
>>> + */
>>> + return alloc_migrate_huge_page(h, gfp_mask, nid, NULL);
>>> +#endif
>>> + }
>>> +
>>> + return __alloc_pages_node(nid, gfp_mask, 0);
>>> +}
>>> +
>>> +/**
>>> + * get_user_pages_cma_migrate() - pin user pages in memory by migrating pages in CMA region
>>> + * @start: starting user address
>>> + * @nr_pages: number of pages from start to pin
>>> + * @write: whether pages will be written to
>>> + * @pages: array that receives pointers to the pages pinned.
>>> + * Should be at least nr_pages long.
>>> + *
>>> + * Attempt to pin user pages in memory without taking mm->mmap_sem.
>>> + * If not successful, it will fall back to taking the lock and
>>> + * calling get_user_pages().
>>
>>
>> I do not see any locking or get_user_pages(), hidden somewhere?
>>
>
> The rules are same as get_user_pages_fast, which does that pin attempt
> without taking mm->mmap_sem. If it fail get_user_pages_fast will take
> the mmap_sem and try to pin the pages. The details are in
> get_user_pages_fast. You can look at get_user_pages_unlocked
Ah, right.
>>> + *
>>> + * If the pinned pages are backed by CMA region, we migrate those pages out,
>>> + * allocating new pages from non-CMA region. This helps in avoiding keeping
>>> + * pages pinned in the CMA region for a long time thereby resulting in
>>> + * CMA allocation failures.
>>> + *
>>> + * Returns number of pages pinned. This may be fewer than the number
>>> + * requested. If nr_pages is 0 or negative, returns 0. If no pages
>>> + * were pinned, returns -errno.
>>> + */
>>> +
>>> +int get_user_pages_cma_migrate(unsigned long start, int nr_pages, int write,
>>> + struct page **pages)
>>> +{
>>> + int i, ret;
>>> + bool drain_allow = true;
>>> + bool migrate_allow = true;
>>> + LIST_HEAD(cma_page_list);
>>> +
>>> +get_user_again:
>>> + ret = get_user_pages_fast(start, nr_pages, write, pages);
>>> + if (ret <= 0)
>>> + return ret;
>>> +
>>> + for (i = 0; i < ret; ++i) {
>>> + /*
>>> + * If we get a page from the CMA zone, since we are going to
>>> + * be pinning these entries, we might as well move them out
>>> + * of the CMA zone if possible.
>>> + */
>>> + if (is_migrate_cma_page(pages[i]) && migrate_allow) {
>>> + if (PageHuge(pages[i]))
>>> + isolate_huge_page(pages[i], &cma_page_list);
>>> + else {
>>> + struct page *head = compound_head(pages[i]);
>>> +
>>> + if (!PageLRU(head) && drain_allow) {
>>> + lru_add_drain_all();
>>> + drain_allow = false;
>>> + }
>>> +
>>> + if (!isolate_lru_page(head)) {
>>> + list_add_tail(&head->lru, &cma_page_list);
>>> + mod_node_page_state(page_pgdat(head),
>>> + NR_ISOLATED_ANON +
>>> + page_is_file_cache(head),
>>> + hpage_nr_pages(head));
>>
>>
>> Above 10 lines I cannot really comment due to my massive ignorance in
>> this area, especially about what lru_add_drain_all() and
>> mod_node_page_state() :(
>
> That makes sure we move the pages from per cpu lru vec and add them to
> the right lru list so that we can isolate the pages correctly.
I understand the idea but cannot confirm the correctness :-/
>
>>
>>
>>> + }
>>> + }
>>> + }
>>> + }
>>> + if (!list_empty(&cma_page_list)) {
>>> + /*
>>> + * drop the above get_user_pages reference.
>>> + */
btw, can these pages be used by somebody else in this short window
before we migrated and pinned them?
>>> + for (i = 0; i < ret; ++i)
>>> + put_page(pages[i]);
>>> +
>>> + if (migrate_pages(&cma_page_list, new_non_cma_page,
>>> + NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE)) {
>>> + /*
>>> + * some of the pages failed migration. Do get_user_pages
>>> + * without migration.
>>> + */
>>> + migrate_allow = false;
>>
>>
>> migrate_allow seems useless, simply calling get_user_pages_fast() should
>> make the code easier to read imho. And the comment says
>> get_user_pages(), where does this guy hide?
>
> I didn't get that suggestion. What we want to do here is try to migrate pages as
> long as we find CMA pages in the result of get_user_pages_fast. If we
> failed any migration attempt, don't try to migrate again.
Setting migrate_allow to false here means you jump up, call
get_user_pages_fast() and then run the loop which will do nothing just
because if(...migrate_allow) is false. Instead of jumping up you could
just call get_user_pages_fast().
btw what is migrate_pages() leaves something in cma_page_list (I cannot
see it removing pages)? Won't it loop indefinitely?
>
>>
>>> +
>>> + if (!list_empty(&cma_page_list))
>>> + putback_movable_pages(&cma_page_list);
>>> + }
>>> + /*
>>> + * We did migrate all the pages, Try to get the page references again
>>> + * migrating any new CMA pages which we failed to isolate earlier.
>>> + */
>>> + drain_allow = true;
>>
>> Move this "drain_allow = true" right after "get_user_again:"? 1
>
>>
>>
>>> + goto get_user_again;
>>> + }
>>> + return ret;
>>> +}
>>>
>>
>> --
>> Alexey
>
> -aneesh
>
--
Alexey
^ permalink raw reply
* Re: [PATCH kernel v2] powerpc/ioda/npu: Call skiboot's hot reset hook when disabling NPU2
From: Alexey Kardashevskiy @ 2018-10-16 7:55 UTC (permalink / raw)
To: Alistair Popple; +Cc: Reza Arbab, linuxppc-dev, David Gibson
In-Reply-To: <2086775.d8i5QI7OoB@new-mexico>
On 16/10/2018 18:32, Alistair Popple wrote:
> On Tuesday, 16 October 2018 1:22:53 PM AEDT Alexey Kardashevskiy wrote:
>>
>> On 16/10/2018 13:19, Alistair Popple wrote:
>>>> reset_ntl() does what npu2_dev_procedure_reset() does plus more stuff,
>>>> there nothing really in npu2_dev_procedure_reset() which reset_ntl()
>>>> does not do already from the hardware standpoint. And it did stop HMIs
>>>> for me though.
>>>>
>>>> but ok, what will be sufficient then if not reset_ntl()?
>>>
>>> Argh, yes you are correct. Specifically both npu2_dev_procedure_reset() and
>>> reset_ntl() contain:
>>>
>>> /* NTL Reset */
>>> val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
>>> val |= PPC_BIT(8) | PPC_BIT(9);
>>> npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
>>>
>>> Which should fence the brick. However from what I recall there was more to
>>> reliably preventing HMIs than merely fencing the brick. It invovled a sequence
>>> of fencing and flushing the cache with dcbf instructions at the right time which
>>> is why we also have the FLR. Unfortunately I don't know the precise details,
>>> perhaps if we send enough coffee Balbir's way he might be able remind us?
>>
>>
>> He suggested and ack'ed that skiboot patch, I can repeat beers^wcoffee
>> but it won't change much ;)
>
> Ha. Couldn't hurt ;)
>
> I was pretty sure flushing the caches was an important part of the sequence to
> avoid HMI's. I believe you are trying to deal with unexpected guest terminations
> which means the driver won't have a chance to flush the caches prior to
> termination so
Correct.
> wouldn't you also need to do that somewhere? Unless the driver
> does it at startup?
VFIO performs GPU reset so I'd expect the GPUs to flush its caches
without any software interactions. Am I hoping for too much here?
>
> - Alistair
>
>>>
>>> - Alistair
>>>
>>>>
>>>>
>>>>>
>>>>> - Alistair
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> - Alistair
>>>>>>>
>>>>>>> On Monday, 15 October 2018 6:17:51 PM AEDT Alexey Kardashevskiy wrote:
>>>>>>>> Ping?
>>>>>>>>
>>>>>>>>
>>>>>>>> On 02/10/2018 13:20, Alexey Kardashevskiy wrote:
>>>>>>>>> The skiboot firmware has a hot reset handler which fences the NVIDIA V100
>>>>>>>>> GPU RAM on Witherspoons and makes accesses no-op instead of throwing HMIs:
>>>>>>>>> https://github.com/open-power/skiboot/commit/fca2b2b839a67
>>>>>>>>>
>>>>>>>>> Now we are going to pass V100 via VFIO which most certainly involves
>>>>>>>>> KVM guests which are often terminated without getting a chance to offline
>>>>>>>>> GPU RAM so we end up with a running machine with misconfigured memory.
>>>>>>>>> Accessing this memory produces hardware management interrupts (HMI)
>>>>>>>>> which bring the host down.
>>>>>>>>>
>>>>>>>>> To suppress HMIs, this wires up this hot reset hook to vfio_pci_disable()
>>>>>>>>> via pci_disable_device() which switches NPU2 to a safe mode and prevents
>>>>>>>>> HMIs.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>>>>>> ---
>>>>>>>>> Changes:
>>>>>>>>> v2:
>>>>>>>>> * updated the commit log
>>>>>>>>> ---
>>>>>>>>> arch/powerpc/platforms/powernv/pci-ioda.c | 10 ++++++++++
>>>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>>>>>>>> index cde7102..e37b9cc 100644
>>>>>>>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>>>>>>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>>>>>>>> @@ -3688,6 +3688,15 @@ static void pnv_pci_release_device(struct pci_dev *pdev)
>>>>>>>>> pnv_ioda_release_pe(pe);
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> +static void pnv_npu_disable_device(struct pci_dev *pdev)
>>>>>>>>> +{
>>>>>>>>> + struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
>>>>>>>>> + struct eeh_pe *eehpe = edev ? edev->pe : NULL;
>>>>>>>>> +
>>>>>>>>> + if (eehpe && eeh_ops && eeh_ops->reset)
>>>>>>>>> + eeh_ops->reset(eehpe, EEH_RESET_HOT);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> static void pnv_pci_ioda_shutdown(struct pci_controller *hose)
>>>>>>>>> {
>>>>>>>>> struct pnv_phb *phb = hose->private_data;
>>>>>>>>> @@ -3732,6 +3741,7 @@ static const struct pci_controller_ops pnv_npu_ioda_controller_ops = {
>>>>>>>>> .reset_secondary_bus = pnv_pci_reset_secondary_bus,
>>>>>>>>> .dma_set_mask = pnv_npu_dma_set_mask,
>>>>>>>>> .shutdown = pnv_pci_ioda_shutdown,
>>>>>>>>> + .disable_device = pnv_npu_disable_device,
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> static const struct pci_controller_ops pnv_npu_ocapi_ioda_controller_ops = {
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH V3 1/2] mm: Add get_user_pages_cma_migrate
From: Aneesh Kumar K.V @ 2018-10-16 8:35 UTC (permalink / raw)
To: Alexey Kardashevskiy, akpm, Michal Hocko, mpe
Cc: linux-mm, linuxppc-dev, linux-kernel
In-Reply-To: <485adcad-4996-ae2c-c098-9dc7bcd2d29a@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 16/10/2018 18:16, Aneesh Kumar K.V wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>
>>> + }
>>>> + }
>>>> + }
>>>> + if (!list_empty(&cma_page_list)) {
>>>> + /*
>>>> + * drop the above get_user_pages reference.
>>>> + */
>
>
> btw, can these pages be used by somebody else in this short window
> before we migrated and pinned them?
isolate lru page make sure that we remove them from lru list. So lru
walkers won't find the page. If somebody happen to increment the page
reference count in that window, the migrate_pages will fail. That is
handled via migrate_page_move_mapping returning EAGAIN
>
>
>>>> + for (i = 0; i < ret; ++i)
>>>> + put_page(pages[i]);
>>>> +
>>>> + if (migrate_pages(&cma_page_list, new_non_cma_page,
>>>> + NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE)) {
>>>> + /*
>>>> + * some of the pages failed migration. Do get_user_pages
>>>> + * without migration.
>>>> + */
>>>> + migrate_allow = false;
>>>
>>>
>>> migrate_allow seems useless, simply calling get_user_pages_fast() should
>>> make the code easier to read imho. And the comment says
>>> get_user_pages(), where does this guy hide?
>>
>> I didn't get that suggestion. What we want to do here is try to migrate pages as
>> long as we find CMA pages in the result of get_user_pages_fast. If we
>> failed any migration attempt, don't try to migrate again.
>
>
> Setting migrate_allow to false here means you jump up, call
> get_user_pages_fast() and then run the loop which will do nothing just
> because if(...migrate_allow) is false. Instead of jumping up you could
> just call get_user_pages_fast().
ok, that is coding preference I guess, I prefer to avoid multiple
get_user_pages_fast there. Since we droped the page reference, we need
to _go back_ and get the page reference without attempting to migrate. That
is the way I was looking at this.
>
> btw what is migrate_pages() leaves something in cma_page_list (I cannot
> see it removing pages)? Won't it loop indefinitely?
>
putback_movable_pages take care of that. The below hunk.
if (!list_empty(&cma_page_list))
putback_movable_pages(&cma_page_list);
-aneesh
^ permalink raw reply
* Patch "KVM: PPC: Book3S HV: Don't use compound_order to determine host mapping size" has been added to the 4.18-stable tree
From: gregkh @ 2018-10-16 9:10 UTC (permalink / raw)
To: alexander.levin, aneesh.kumar, david, gregkh, linuxppc-dev,
npiggin, paulus
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
KVM: PPC: Book3S HV: Don't use compound_order to determine host mapping size
to the 4.18-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kvm-ppc-book3s-hv-don-t-use-compound_order-to-determine-host-mapping-size.patch
and it can be found in the queue-4.18 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From foo@baz Tue Oct 16 11:10:21 CEST 2018
From: Nicholas Piggin <npiggin@gmail.com>
Date: Tue, 11 Sep 2018 20:48:34 +1000
Subject: KVM: PPC: Book3S HV: Don't use compound_order to determine host mapping size
From: Nicholas Piggin <npiggin@gmail.com>
[ Upstream commit 71d29f43b6332badc5598c656616a62575e83342 ]
THP paths can defer splitting compound pages until after the actual
remap and TLB flushes to split a huge PMD/PUD. This causes radix
partition scope page table mappings to get out of synch with the host
qemu page table mappings.
This results in random memory corruption in the guest when running
with THP. The easiest way to reproduce is use KVM balloon to free up
a lot of memory in the guest and then shrink the balloon to give the
memory back, while some work is being done in the guest.
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/kvm/book3s_64_mmu_radix.c | 91 +++++++++++++--------------------
1 file changed, 37 insertions(+), 54 deletions(-)
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -538,8 +538,8 @@ int kvmppc_book3s_radix_page_fault(struc
unsigned long ea, unsigned long dsisr)
{
struct kvm *kvm = vcpu->kvm;
- unsigned long mmu_seq, pte_size;
- unsigned long gpa, gfn, hva, pfn;
+ unsigned long mmu_seq;
+ unsigned long gpa, gfn, hva;
struct kvm_memory_slot *memslot;
struct page *page = NULL;
long ret;
@@ -636,9 +636,10 @@ int kvmppc_book3s_radix_page_fault(struc
*/
hva = gfn_to_hva_memslot(memslot, gfn);
if (upgrade_p && __get_user_pages_fast(hva, 1, 1, &page) == 1) {
- pfn = page_to_pfn(page);
upgrade_write = true;
} else {
+ unsigned long pfn;
+
/* Call KVM generic code to do the slow-path check */
pfn = __gfn_to_pfn_memslot(memslot, gfn, false, NULL,
writing, upgrade_p);
@@ -652,63 +653,45 @@ int kvmppc_book3s_radix_page_fault(struc
}
}
- /* See if we can insert a 1GB or 2MB large PTE here */
- level = 0;
- if (page && PageCompound(page)) {
- pte_size = PAGE_SIZE << compound_order(compound_head(page));
- if (pte_size >= PUD_SIZE &&
- (gpa & (PUD_SIZE - PAGE_SIZE)) ==
- (hva & (PUD_SIZE - PAGE_SIZE))) {
- level = 2;
- pfn &= ~((PUD_SIZE >> PAGE_SHIFT) - 1);
- } else if (pte_size >= PMD_SIZE &&
- (gpa & (PMD_SIZE - PAGE_SIZE)) ==
- (hva & (PMD_SIZE - PAGE_SIZE))) {
- level = 1;
- pfn &= ~((PMD_SIZE >> PAGE_SHIFT) - 1);
- }
- }
-
/*
- * Compute the PTE value that we need to insert.
+ * Read the PTE from the process' radix tree and use that
+ * so we get the shift and attribute bits.
*/
- if (page) {
- pgflags = _PAGE_READ | _PAGE_EXEC | _PAGE_PRESENT | _PAGE_PTE |
- _PAGE_ACCESSED;
- if (writing || upgrade_write)
- pgflags |= _PAGE_WRITE | _PAGE_DIRTY;
- pte = pfn_pte(pfn, __pgprot(pgflags));
+ local_irq_disable();
+ ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL, &shift);
+ pte = *ptep;
+ local_irq_enable();
+
+ /* Get pte level from shift/size */
+ if (shift == PUD_SHIFT &&
+ (gpa & (PUD_SIZE - PAGE_SIZE)) ==
+ (hva & (PUD_SIZE - PAGE_SIZE))) {
+ level = 2;
+ } else if (shift == PMD_SHIFT &&
+ (gpa & (PMD_SIZE - PAGE_SIZE)) ==
+ (hva & (PMD_SIZE - PAGE_SIZE))) {
+ level = 1;
} else {
- /*
- * Read the PTE from the process' radix tree and use that
- * so we get the attribute bits.
- */
- local_irq_disable();
- ptep = __find_linux_pte(vcpu->arch.pgdir, hva, NULL, &shift);
- pte = *ptep;
- local_irq_enable();
- if (shift == PUD_SHIFT &&
- (gpa & (PUD_SIZE - PAGE_SIZE)) ==
- (hva & (PUD_SIZE - PAGE_SIZE))) {
- level = 2;
- } else if (shift == PMD_SHIFT &&
- (gpa & (PMD_SIZE - PAGE_SIZE)) ==
- (hva & (PMD_SIZE - PAGE_SIZE))) {
- level = 1;
- } else if (shift && shift != PAGE_SHIFT) {
- /* Adjust PFN */
- unsigned long mask = (1ul << shift) - PAGE_SIZE;
- pte = __pte(pte_val(pte) | (hva & mask));
- }
- pte = __pte(pte_val(pte) | _PAGE_EXEC | _PAGE_ACCESSED);
- if (writing || upgrade_write) {
- if (pte_val(pte) & _PAGE_WRITE)
- pte = __pte(pte_val(pte) | _PAGE_DIRTY);
- } else {
- pte = __pte(pte_val(pte) & ~(_PAGE_WRITE | _PAGE_DIRTY));
+ level = 0;
+ if (shift > PAGE_SHIFT) {
+ /*
+ * If the pte maps more than one page, bring over
+ * bits from the virtual address to get the real
+ * address of the specific single page we want.
+ */
+ unsigned long rpnmask = (1ul << shift) - PAGE_SIZE;
+ pte = __pte(pte_val(pte) | (hva & rpnmask));
}
}
+ pte = __pte(pte_val(pte) | _PAGE_EXEC | _PAGE_ACCESSED);
+ if (writing || upgrade_write) {
+ if (pte_val(pte) & _PAGE_WRITE)
+ pte = __pte(pte_val(pte) | _PAGE_DIRTY);
+ } else {
+ pte = __pte(pte_val(pte) & ~(_PAGE_WRITE | _PAGE_DIRTY));
+ }
+
/* Allocate space in the tree and write the PTE */
ret = kvmppc_create_pte(kvm, pte, gpa, level, mmu_seq);
Patches currently in stable-queue which might be from npiggin@gmail.com are
queue-4.18/kvm-ppc-book3s-hv-don-t-use-compound_order-to-determine-host-mapping-size.patch
^ permalink raw reply
* Re: linux-next: qemu boot failures with today's linux-next
From: Michael Ellerman @ 2018-10-16 9:36 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Linux-Next Mailing List, PowerPC, Linux Kernel Mailing List
In-Reply-To: <20181016105240.70369b98@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
>> > Booting Linux via __start() @ 0x0000000000400000 ...
>>
>> If you git Ctrl-a-c you should get the qemu prompt. Then you can run
>> 'info registers' to print the regs and maybe see where it's stuck.
>>
>> And/or build with EARLY_DEBUG_LPAR to get early console output.
>
> That gave one more line:
>
> [ 0.000000] printk: bootconsole [udbg0] enabled
Yeah I tried it too, it must have crashed *really* early :)
cheers
^ permalink raw reply
* Re: [PATCH v4 00/18] of: overlay: validation checks, subsequent fixes
From: Michael Ellerman @ 2018-10-16 9:47 UTC (permalink / raw)
To: frowand.list, Rob Herring, Pantelis Antoniou,
Benjamin Herrenschmidt, Paul Mackerras, Alan Tull, Moritz Fischer
Cc: devicetree, linux-fpga, linuxppc-dev, linux-kernel
In-Reply-To: <1539657458-24401-1-git-send-email-frowand.list@gmail.com>
frowand.list@gmail.com writes:
> From: Frank Rowand <frank.rowand@sony.com>
>
> Add checks to (1) overlay apply process and (2) memory freeing
> triggered by overlay release. The checks are intended to detect
> possible memory leaks and invalid overlays.
>
> The checks revealed bugs in existing code. Fixed the bugs.
>
> While fixing bugs, noted other issues, which are fixed in
> separate patches.
>
> ***** Powerpc folks: I was not able to test the patches that
> ***** directly impact Powerpc systems that use dynamic
> ***** devicetree. Please review that code carefully and
> ***** test. The specific patches are: 03/16, 04/16, 07/16
Hi Frank,
Do you have this series in a git tree somewhere?
I tried applying it on top of linux-next but hit some conflicts which I
couldn't easily resolve.
cheers
^ permalink raw reply
* Re: [PATCH] driver core: device: add BUS_ATTR_WO macro
From: Greg KH @ 2018-10-16 10:48 UTC (permalink / raw)
To: Ioana Ciornei
Cc: elder@kernel.org, linux-scsi@vger.kernel.org, sage@redhat.com,
linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
idryomov@gmail.com, jth@kernel.org, linuxppc-dev@lists.ozlabs.org,
ceph-devel@vger.kernel.org
In-Reply-To: <HE1PR0401MB236354F325B7A56355068E35E0E80@HE1PR0401MB2363.eurprd04.prod.outlook.com>
On Tue, Oct 02, 2018 at 09:43:35AM +0000, Ioana Ciornei wrote:
> > > Add BUS_ATTR_WO macro to make it easier to add attributes without
> > > auditing the mode settings. Also, use the newly added macro where
> > > appropriate.
> > >
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > > ---
> > > arch/powerpc/platforms/pseries/ibmebus.c | 12 ++++----
> > > drivers/block/rbd.c | 48 ++++++++++++++++----------------
> > > drivers/scsi/fcoe/fcoe_sysfs.c | 4 +--
> > > drivers/scsi/fcoe/fcoe_transport.c | 10 +++----
> > > include/linux/device.h | 2 ++
> > > include/scsi/libfcoe.h | 8 +++---
> > > 6 files changed, 43 insertions(+), 41 deletions(-)
> >
> > Nice! This duplicates a lot of the work I did back in July but have not pushed out
> > very far due to the other things that ended up happening around that time:
> > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=bus_cleanup
> >
> > As the patch series seen at that link shows, you can do this in more places than
> > just what you did here.
> >
> > Either way, you should break this up into the individual patches, like I did or you
> > can take my patches if you want. Getting the BUS_ATTR_WO() macro added is
> > good to do now, and then you can go and hit up all of the different subsystems
> > that should be converted over to it.
>
> I can of course split my patch into individual ones and resubmit them, but as you already have the entire patch set ready, I feel like we can just push those. I looked through your changes and it seems like you covered all the subsystems. Please let me know if there is something else I should do.
>
> My intention here was to first add the _WO attribute so that afterwards I can add a new bus attribute in the fsl-mc bus.
Ok, I've queued up the patch that adds the _WO attribute now, so that
you should be able to use this after 4.20-rc1. I'll work on getting my
other patches merged in that series at that time as well.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: Split user/kernel definitions of struct pt_regs
From: Michael Ellerman @ 2018-10-16 10:50 UTC (permalink / raw)
To: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <14b76e3e-2497-4ac2-c0b5-ce801275ac64@linux.vnet.ibm.com>
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> On Monday 15 October 2018 04:38 PM, Michael Ellerman wrote:
>> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
>>
>>> On Saturday 13 October 2018 04:26 PM, Michael Ellerman wrote:
...
>>>> At the moment they're still identical, and we check that at build
>>>> time. That's because we have code (in ptrace etc.) that assumes that
>>>> they are the same. We will fix that code in future patches, and then
>>>> we can break the strict symmetry between the two structs.
>>> Nice and awesome. But just trying to understand. What will
>>> *regs will point to in the "struct sigcontext".
>>
>> It should always point to a user_pt_regs.
...
>>
>> I think it's not actually broken at the moment, because it's just a
>> pointer, and we don't do anything based on the sizeof() the type.
>
> yes. This clarifies. But still perf/perf_regs.c needs changes.
> Because perf support dumping user_space regs and interrupt regs.
> Once again, we dont use any sizeof(), but need to handle the
> user_pt_regs changes.
>
> I will have a look at that in the morning.
I did look at that and convinced myself that it was OK, but maybe I'm
wrong :D
My reasoning was that the regs we're using there are always the
in-kernel regs for the process at the point it took the PMU interrupt.
And the regs values aren't exported directly as a struct but rather via
regs_get_register().
But we may still want to change it to make things clearer.
cheers
^ permalink raw reply
* [PATCH v3] powerpc/pseries: Export raw per-CPU VPA data via debugfs
From: Aravinda Prasad @ 2018-10-16 11:50 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: nfont, naveen.n.rao, aravinda
This patch exports the raw per-CPU VPA data via debugfs.
A per-CPU file is created which exports the VPA data of
that CPU to help debug some of the VPA related issues or
to analyze the per-CPU VPA related statistics.
v3: Removed offline CPU check.
v2: Included offline CPU check and other review comments.
Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/lpar.c | 54 +++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index d3992ce..1c757a4 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -48,6 +48,7 @@
#include <asm/kexec.h>
#include <asm/fadump.h>
#include <asm/asm-prototypes.h>
+#include <asm/debugfs.h>
#include "pseries.h"
@@ -1028,3 +1029,56 @@ static int __init reserve_vrma_context_id(void)
return 0;
}
machine_device_initcall(pseries, reserve_vrma_context_id);
+
+#ifdef CONFIG_DEBUG_FS
+/* debugfs file interface for vpa data */
+static ssize_t vpa_file_read(struct file *filp, char __user *buf, size_t len,
+ loff_t *pos)
+{
+ int cpu = (long)filp->private_data;
+ struct lppaca *lppaca = &lppaca_of(cpu);
+
+ return simple_read_from_buffer(buf, len, pos, lppaca,
+ sizeof(struct lppaca));
+}
+
+static const struct file_operations vpa_fops = {
+ .open = simple_open,
+ .read = vpa_file_read,
+ .llseek = default_llseek,
+};
+
+static int __init vpa_debugfs_init(void)
+{
+ char name[16];
+ long i;
+ static struct dentry *vpa_dir;
+
+ if (!firmware_has_feature(FW_FEATURE_SPLPAR))
+ return 0;
+
+ vpa_dir = debugfs_create_dir("vpa", powerpc_debugfs_root);
+ if (!vpa_dir) {
+ pr_warn("%s: can't create vpa root dir\n", __func__);
+ return -ENOMEM;
+ }
+
+ /* set up the per-cpu vpa file*/
+ for_each_possible_cpu(i) {
+ struct dentry *d;
+
+ sprintf(name, "cpu-%ld", i);
+
+ d = debugfs_create_file(name, 0400, vpa_dir, (void *)i,
+ &vpa_fops);
+ if (!d) {
+ pr_warn("%s: can't create per-cpu vpa file\n",
+ __func__);
+ return -ENOMEM;
+ }
+ }
+
+ return 0;
+}
+machine_arch_initcall(pseries, vpa_debugfs_init);
+#endif /* CONFIG_DEBUG_FS */
^ permalink raw reply related
* Re: [PATCH 2/4] mm: speed up mremap by 500x on large regions (v2)
From: Vlastimil Babka @ 2018-10-16 11:29 UTC (permalink / raw)
To: Joel Fernandes, Christoph Hellwig
Cc: linux-mips, Rich Felker, linux-ia64, linux-sh, Peter Zijlstra,
Catalin Marinas, Dave Hansen, Will Deacon, mhocko, linux-mm,
lokeshgidra, sparclinux, linux-riscv, kvmarm, Jonas Bonn,
linux-s390, dancol, Yoshinori Sato, Max Filippov, linux-hexagon,
Helge Deller, maintainer:X86 ARCHITECTURE 32-BIT AND 64-BIT,
hughd, James E.J. Bottomley, kasan-dev, elfring, Ingo Molnar,
Geert Uytterhoeven, Andrey Ryabinin, linux-snps-arc, kernel-team,
Sam Creasey, linux-xtensa, Jeff Dike, linux-alpha, linux-um,
Stefan Kristiansson, Julia Lawall, linux-m68k, Borislav Petkov,
Andy Lutomirski, Ley Foon Tan, kirill, Stafford Horne,
Guan Xuetao, Chris Zankel, Tony Luck, linux-parisc, pantin,
linux-kernel, Fenghua Yu, minchan, Thomas Gleixner,
Richard Weinberger, anton.ivanov, nios2-dev, akpm, linuxppc-dev,
David S. Miller
In-Reply-To: <20181015223303.GA164293@joelaf.mtv.corp.google.com>
On 10/16/18 12:33 AM, Joel Fernandes wrote:
> On Mon, Oct 15, 2018 at 02:42:09AM -0700, Christoph Hellwig wrote:
>> On Fri, Oct 12, 2018 at 06:31:58PM -0700, Joel Fernandes (Google) wrote:
>>> Android needs to mremap large regions of memory during memory management
>>> related operations.
>>
>> Just curious: why?
>
> In Android we have a requirement of moving a large (up to a GB now, but may
> grow bigger in future) memory range from one location to another.
I think Christoph's "why?" was about the requirement, not why it hurts
applications. I admit I'm now also curious :)
> This move
> operation has to happen when the application threads are paused for this
> operation. Therefore, an inefficient move like it is now (for example 250ms
> on arm64) will cause response time issues for applications, which is not
> acceptable. Huge pages cannot be used in such memory ranges to avoid this
> inefficiency as (when the application threads are running) our fault handlers
> are designed to process 4KB pages at a time, to keep response times low. So
> using huge pages in this context can, again, cause response time issues.
>
> Also, the mremap syscall waiting for quarter of a second for a large mremap
> is quite weird and we ought to improve it where possible.
^ permalink raw reply
* [PATCH] powerpc/io: remove old GCC version implementation
From: Christophe Leroy @ 2018-10-16 12:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
GCC 4.6 is the minimum supported now.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/io.h | 20 --------------------
1 file changed, 20 deletions(-)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 0a034519957d..3ef40b703c4a 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -111,25 +111,6 @@ extern bool isa_io_special;
#define IO_SET_SYNC_FLAG()
#endif
-/* gcc 4.0 and older doesn't have 'Z' constraint */
-#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ == 0)
-#define DEF_MMIO_IN_X(name, size, insn) \
-static inline u##size name(const volatile u##size __iomem *addr) \
-{ \
- u##size ret; \
- __asm__ __volatile__("sync;"#insn" %0,0,%1;twi 0,%0,0;isync" \
- : "=r" (ret) : "r" (addr), "m" (*addr) : "memory"); \
- return ret; \
-}
-
-#define DEF_MMIO_OUT_X(name, size, insn) \
-static inline void name(volatile u##size __iomem *addr, u##size val) \
-{ \
- __asm__ __volatile__("sync;"#insn" %1,0,%2" \
- : "=m" (*addr) : "r" (val), "r" (addr) : "memory"); \
- IO_SET_SYNC_FLAG(); \
-}
-#else /* newer gcc */
#define DEF_MMIO_IN_X(name, size, insn) \
static inline u##size name(const volatile u##size __iomem *addr) \
{ \
@@ -146,7 +127,6 @@ static inline void name(volatile u##size __iomem *addr, u##size val) \
: "=Z" (*addr) : "r" (val) : "memory"); \
IO_SET_SYNC_FLAG(); \
}
-#endif
#define DEF_MMIO_IN_D(name, size, insn) \
static inline u##size name(const volatile u##size __iomem *addr) \
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] powerpc/io: remove old GCC version implementation
From: Joakim Tjernlund @ 2018-10-16 12:49 UTC (permalink / raw)
To: christophe.leroy@c-s.fr, paulus@samba.org, mpe@ellerman.id.au,
benh@kernel.crashing.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <199ae2a28f14f944ee9ae2770a3f1dd030e762bc.1539693039.git.christophe.leroy@c-s.fr>
On Tue, 2018-10-16 at 12:33 +0000, Christophe Leroy wrote:
>
>
> GCC 4.6 is the minimum supported now.
Ouch, from kernel 4.19 or earlier even ?
Jocke
^ permalink raw reply
* [PATCH v2 0/5] mm: dirty/accessed pte optimisations
From: Nicholas Piggin @ 2018-10-16 13:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List,
Nicholas Piggin, linux-mm, Ley Foon Tan, ppc-dev
Since v1 I fixed the hang in nios2, split the fork patch into two
as Linus asked, and added hugetlb code for the "don't bother write
protecting already writeprotected" patch.
Please consider this for more cooking in -mm.
Thanks,
Nick
Nicholas Piggin (5):
nios2: update_mmu_cache clear the old entry from the TLB
mm/cow: don't bother write protecting already write-protected huge
pages
mm/cow: optimise pte accessed bit handling in fork
mm/cow: optimise pte dirty bit handling in fork
mm: optimise pte dirty/accessed bit setting by demand based pte
insertion
arch/nios2/mm/cacheflush.c | 2 ++
mm/huge_memory.c | 24 ++++++++++++++++--------
mm/hugetlb.c | 2 +-
mm/memory.c | 19 +++++++++++--------
mm/vmscan.c | 8 ++++++++
5 files changed, 38 insertions(+), 17 deletions(-)
--
2.18.0
^ permalink raw reply
* [PATCH v2 1/5] nios2: update_mmu_cache clear the old entry from the TLB
From: Nicholas Piggin @ 2018-10-16 13:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List,
Nicholas Piggin, linux-mm, Ley Foon Tan, ppc-dev
In-Reply-To: <20181016131343.20556-1-npiggin@gmail.com>
Fault paths like do_read_fault will install a Linux pte with the young
bit clear. The CPU will fault again because the TLB has not been
updated, this time a valid pte exists so handle_pte_fault will just
set the young bit with ptep_set_access_flags, which flushes the TLB.
The TLB is flushed so the next attempt will go to the fast TLB handler
which loads the TLB with the new Linux pte. The access then proceeds.
This design is fragile to depend on the young bit being clear after
the initial Linux fault. A proposed core mm change to immediately set
the young bit upon such a fault, results in ptep_set_access_flags not
flushing the TLB because it finds no change to the pte. The spurious
fault fix path only flushes the TLB if the access was a store. If it
was a load, then this results in an infinite loop of page faults.
This change adds a TLB flush in update_mmu_cache, which removes that
TLB entry upon the first fault. This will cause the fast TLB handler
to load the new pte and avoid the Linux page fault entirely.
Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/nios2/mm/cacheflush.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/nios2/mm/cacheflush.c b/arch/nios2/mm/cacheflush.c
index 506f6e1c86d5..d58e7e80dc0d 100644
--- a/arch/nios2/mm/cacheflush.c
+++ b/arch/nios2/mm/cacheflush.c
@@ -204,6 +204,8 @@ void update_mmu_cache(struct vm_area_struct *vma,
struct page *page;
struct address_space *mapping;
+ flush_tlb_page(vma, address);
+
if (!pfn_valid(pfn))
return;
--
2.18.0
^ permalink raw reply related
* [PATCH v2 2/5] mm/cow: don't bother write protecting already write-protected huge pages
From: Nicholas Piggin @ 2018-10-16 13:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List,
Nicholas Piggin, linux-mm, Ley Foon Tan, ppc-dev
In-Reply-To: <20181016131343.20556-1-npiggin@gmail.com>
This is the HugePage / THP equivalent for 1b2de5d039c8 ("mm/cow: don't
bother write protecting already write-protected pages").
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
mm/huge_memory.c | 14 ++++++++++----
mm/hugetlb.c | 2 +-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 58269f8ba7c4..0fb0e3025f98 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -973,8 +973,11 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
mm_inc_nr_ptes(dst_mm);
pgtable_trans_huge_deposit(dst_mm, dst_pmd, pgtable);
- pmdp_set_wrprotect(src_mm, addr, src_pmd);
- pmd = pmd_mkold(pmd_wrprotect(pmd));
+ if (pmd_write(pmd)) {
+ pmdp_set_wrprotect(src_mm, addr, src_pmd);
+ pmd = pmd_wrprotect(pmd);
+ }
+ pmd = pmd_mkold(pmd);
set_pmd_at(dst_mm, addr, dst_pmd, pmd);
ret = 0;
@@ -1064,8 +1067,11 @@ int copy_huge_pud(struct mm_struct *dst_mm, struct mm_struct *src_mm,
/* No huge zero pud yet */
}
- pudp_set_wrprotect(src_mm, addr, src_pud);
- pud = pud_mkold(pud_wrprotect(pud));
+ if (pud_write(pud)) {
+ pudp_set_wrprotect(src_mm, addr, src_pud);
+ pud = pud_wrprotect(pud);
+ }
+ pud = pud_mkold(pud);
set_pud_at(dst_mm, addr, dst_pud, pud);
ret = 0;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 5c390f5a5207..54a4dcb6ee21 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3287,7 +3287,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
}
set_huge_swap_pte_at(dst, addr, dst_pte, entry, sz);
} else {
- if (cow) {
+ if (cow && huge_pte_write(entry)) {
/*
* No need to notify as we are downgrading page
* table protection not changing it to point
--
2.18.0
^ permalink raw reply related
* [PATCH v2 3/5] mm/cow: optimise pte accessed bit handling in fork
From: Nicholas Piggin @ 2018-10-16 13:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List,
Nicholas Piggin, linux-mm, Ley Foon Tan, ppc-dev
In-Reply-To: <20181016131343.20556-1-npiggin@gmail.com>
fork clears dirty/accessed bits from new ptes in the child. This logic
has existed since mapped page reclaim was done by scanning ptes when
it may have been quite important. Today with physical based pte
scanning, there is less reason to clear these bits, so this patch
avoids clearing the accessed bit in the child.
Any accessed bit is treated similarly to many, with the difference
today with > 1 referenced bit causing the page to be activated, while
1 bit causes it to be kept. This patch causes pages shared by fork(2)
to be more readily activated, but this heuristic is very fuzzy anyway
-- a page can be accessed by multiple threads via a single pte and be
just as important as one that is accessed via multiple ptes, for
example. In the end I don't believe fork(2) is a significant driver of
page reclaim behaviour that this should matter too much.
This and the following change eliminate a major source of faults that
powerpc/radix requires to set dirty/accessed bits in ptes, speeding
up a fork/exit microbenchmark by about 5% on POWER9 (16600 -> 17500
fork/execs per second).
Skylake appears to have a micro-fault overhead too -- a test which
allocates 4GB anonymous memory, reads each page, then forks, and times
the child reading a byte from each page. The first pass over the pages
takes about 1000 cycles per page, the second pass takes about 27
cycles (TLB miss). With no additional minor faults measured due to
either child pass, and the page array well exceeding TLB capacity, the
large cost must be caused by micro faults caused by setting accessed
bit.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
mm/huge_memory.c | 2 --
mm/memory.c | 1 -
mm/vmscan.c | 8 ++++++++
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 0fb0e3025f98..1f43265204d4 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -977,7 +977,6 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
pmdp_set_wrprotect(src_mm, addr, src_pmd);
pmd = pmd_wrprotect(pmd);
}
- pmd = pmd_mkold(pmd);
set_pmd_at(dst_mm, addr, dst_pmd, pmd);
ret = 0;
@@ -1071,7 +1070,6 @@ int copy_huge_pud(struct mm_struct *dst_mm, struct mm_struct *src_mm,
pudp_set_wrprotect(src_mm, addr, src_pud);
pud = pud_wrprotect(pud);
}
- pud = pud_mkold(pud);
set_pud_at(dst_mm, addr, dst_pud, pud);
ret = 0;
diff --git a/mm/memory.c b/mm/memory.c
index c467102a5cbc..0387ee1e3582 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1033,7 +1033,6 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
*/
if (vm_flags & VM_SHARED)
pte = pte_mkclean(pte);
- pte = pte_mkold(pte);
page = vm_normal_page(vma, addr, pte);
if (page) {
diff --git a/mm/vmscan.c b/mm/vmscan.c
index c5ef7240cbcb..e72d5b3336a0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1031,6 +1031,14 @@ static enum page_references page_check_references(struct page *page,
* to look twice if a mapped file page is used more
* than once.
*
+ * fork() will set referenced bits in child ptes despite
+ * not having been accessed, to avoid micro-faults of
+ * setting accessed bits. This heuristic is not perfectly
+ * accurate in other ways -- multiple map/unmap in the
+ * same time window would be treated as multiple references
+ * despite same number of actual memory accesses made by
+ * the program.
+ *
* Mark it and spare it for another trip around the
* inactive list. Another page table reference will
* lead to its activation.
--
2.18.0
^ permalink raw reply related
* [PATCH v2 4/5] mm/cow: optimise pte dirty bit handling in fork
From: Nicholas Piggin @ 2018-10-16 13:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List,
Nicholas Piggin, linux-mm, Ley Foon Tan, ppc-dev
In-Reply-To: <20181016131343.20556-1-npiggin@gmail.com>
fork clears dirty/accessed bits from new ptes in the child. This logic
has existed since mapped page reclaim was done by scanning ptes when
it may have been quite important. Today with physical based pte
scanning, there is less reason to clear these bits, so this patch
avoids clearing the dirty bit in the child.
Dirty bits are all tested and cleared together, and any dirty bit is
the same as many dirty bits, so from a correctness and writeback
bandwidth point-of-view it does not matter if the child gets a dirty
bit.
Dirty ptes are more costly to unmap because they require flushing
under the page table lock, but it is pretty rare to have a shared
dirty mapping that is copied on fork, so just simplify the code and
avoid this dirty clearing logic.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
mm/memory.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 0387ee1e3582..9e314339a0bd 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1028,11 +1028,12 @@ copy_one_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
}
/*
- * If it's a shared mapping, mark it clean in
- * the child
+ * Child inherits dirty and young bits from parent. There is no
+ * point clearing them because any cleaning or aging has to walk
+ * all ptes anyway, and it will notice the bits set in the parent.
+ * Leaving them set avoids stalls and even page faults on CPUs that
+ * handle these bits in software.
*/
- if (vm_flags & VM_SHARED)
- pte = pte_mkclean(pte);
page = vm_normal_page(vma, addr, pte);
if (page) {
--
2.18.0
^ permalink raw reply related
* [PATCH v2 5/5] mm: optimise pte dirty/accessed bit setting by demand based pte insertion
From: Nicholas Piggin @ 2018-10-16 13:13 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List,
Nicholas Piggin, linux-mm, Ley Foon Tan, ppc-dev
In-Reply-To: <20181016131343.20556-1-npiggin@gmail.com>
Similarly to the previous patch, this tries to optimise dirty/accessed
bits in ptes to avoid access costs of hardware setting them.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
mm/huge_memory.c | 12 ++++++++----
mm/memory.c | 9 ++++++---
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 1f43265204d4..38c2cd3b4879 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1197,6 +1197,7 @@ static vm_fault_t do_huge_pmd_wp_page_fallback(struct vm_fault *vmf,
for (i = 0; i < HPAGE_PMD_NR; i++, haddr += PAGE_SIZE) {
pte_t entry;
entry = mk_pte(pages[i], vma->vm_page_prot);
+ entry = pte_mkyoung(entry);
entry = maybe_mkwrite(pte_mkdirty(entry), vma);
memcg = (void *)page_private(pages[i]);
set_page_private(pages[i], 0);
@@ -2067,7 +2068,7 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
struct page *page;
pgtable_t pgtable;
pmd_t old_pmd, _pmd;
- bool young, write, soft_dirty, pmd_migration = false;
+ bool young, write, dirty, soft_dirty, pmd_migration = false;
unsigned long addr;
int i;
@@ -2145,7 +2146,8 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
page = pmd_page(old_pmd);
VM_BUG_ON_PAGE(!page_count(page), page);
page_ref_add(page, HPAGE_PMD_NR - 1);
- if (pmd_dirty(old_pmd))
+ dirty = pmd_dirty(old_pmd);
+ if (dirty)
SetPageDirty(page);
write = pmd_write(old_pmd);
young = pmd_young(old_pmd);
@@ -2176,8 +2178,10 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd,
entry = maybe_mkwrite(entry, vma);
if (!write)
entry = pte_wrprotect(entry);
- if (!young)
- entry = pte_mkold(entry);
+ if (young)
+ entry = pte_mkyoung(entry);
+ if (dirty)
+ entry = pte_mkdirty(entry);
if (soft_dirty)
entry = pte_mksoft_dirty(entry);
}
diff --git a/mm/memory.c b/mm/memory.c
index 9e314339a0bd..f907ea7a6303 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1804,10 +1804,9 @@ static int insert_pfn(struct vm_area_struct *vma, unsigned long addr,
entry = pte_mkspecial(pfn_t_pte(pfn, prot));
out_mkwrite:
- if (mkwrite) {
- entry = pte_mkyoung(entry);
+ entry = pte_mkyoung(entry);
+ if (mkwrite)
entry = maybe_mkwrite(pte_mkdirty(entry), vma);
- }
set_pte_at(mm, addr, pte, entry);
update_mmu_cache(vma, addr, pte); /* XXX: why not for insert_page? */
@@ -2534,6 +2533,7 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
}
flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
entry = mk_pte(new_page, vma->vm_page_prot);
+ entry = pte_mkyoung(entry);
entry = maybe_mkwrite(pte_mkdirty(entry), vma);
/*
* Clear the pte entry and flush it first, before updating the
@@ -3043,6 +3043,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
inc_mm_counter_fast(vma->vm_mm, MM_ANONPAGES);
dec_mm_counter_fast(vma->vm_mm, MM_SWAPENTS);
pte = mk_pte(page, vma->vm_page_prot);
+ pte = pte_mkyoung(pte);
if ((vmf->flags & FAULT_FLAG_WRITE) && reuse_swap_page(page, NULL)) {
pte = maybe_mkwrite(pte_mkdirty(pte), vma);
vmf->flags &= ~FAULT_FLAG_WRITE;
@@ -3185,6 +3186,7 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
__SetPageUptodate(page);
entry = mk_pte(page, vma->vm_page_prot);
+ entry = pte_mkyoung(entry);
if (vma->vm_flags & VM_WRITE)
entry = pte_mkwrite(pte_mkdirty(entry));
@@ -3453,6 +3455,7 @@ vm_fault_t alloc_set_pte(struct vm_fault *vmf, struct mem_cgroup *memcg,
flush_icache_page(vma, page);
entry = mk_pte(page, vma->vm_page_prot);
+ entry = pte_mkyoung(entry);
if (write)
entry = maybe_mkwrite(pte_mkdirty(entry), vma);
/* copy-on-write page */
--
2.18.0
^ permalink raw reply related
* Re: [PATCH] powerpc/io: remove old GCC version implementation
From: Christophe LEROY @ 2018-10-16 13:17 UTC (permalink / raw)
To: Joakim Tjernlund, paulus@samba.org, mpe@ellerman.id.au,
benh@kernel.crashing.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <8eb877e31130aee531409d18e19894d64690163d.camel@infinera.com>
Le 16/10/2018 à 14:49, Joakim Tjernlund a écrit :
> On Tue, 2018-10-16 at 12:33 +0000, Christophe Leroy wrote:
>>
>>
>> GCC 4.6 is the minimum supported now.
>
> Ouch, from kernel 4.19 or earlier even ?
>
Don't know, see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cafa0010
Christophe
^ permalink raw reply
* Re: Stack protector crash in pnv_smp_cpu_kill_self()
From: Michael Ellerman @ 2018-10-16 13:21 UTC (permalink / raw)
To: Christophe LEROY, linuxppc-dev@ozlabs.org
Cc: Abdul Haleem, Nicholas Piggin, Gautham R Shenoy
In-Reply-To: <a23f6298-f860-190c-4b45-333198fdc160@c-s.fr>
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Looks like a lack of initialisation of the canary for the non-boot CPUs
> on SMP, you applied this morning the patch I sent you for that.
>
> Is the patch in ?
Yeah it is.
$ git log --oneline 4ffe713b7587 arch/powerpc/kernel/smp.c
8e8a31d7fd54 powerpc: Use cpu_smallcore_sibling_mask at SMT level on bigcores
425752c63b6f powerpc: Detect the presence of big-cores via "ibm, thread-groups"
7241d26e8175 powerpc/64: properly initialise the stackprotector canary on SMP.
It only happens on a specific Power9 machine, not in sim, but it's 100%
reproducible on that hardware.
The canary value has changed (?!).
The value in paca->canary and current->canary agree, but they don't
match what's in the stack.
Clearly the idle code is doing something I don't understand :)
cheers
Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: pnv_smp_cpu_kill_self+0x2a0/0x2b0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc3-gcc-7.3.1-00190-g98c847323b3a-dirty #103
Call Trace:
[c000000007967b00] [c000000000ae864c] dump_stack+0xb0/0xf4 (unreliable)
[c000000007967b40] [c0000000000e64ac] panic+0x144/0x328
[c000000007967be0] [c0000000000e5f2c] __stack_chk_fail+0x2c/0x30
[c000000007967c40] [c00000000009f720] pnv_smp_cpu_kill_self+0x2a0/0x2b0
[c000000007967e10] [c0000000000475d8] cpu_die+0x48/0x70
[c000000007967e30] [c000000000020620] arch_cpu_idle_dead+0x20/0x40
[c000000007967e50] [c00000000012e574] do_idle+0x274/0x390
[c000000007967ec0] [c00000000012e8e8] cpu_startup_entry+0x38/0x50
[c000000007967ef0] [c000000000047314] start_secondary+0x5e4/0x600
[c000000007967f90] [c00000000000ac70] start_secondary_prolog+0x10/0x14
c00000000009f480 <pnv_smp_cpu_kill_self>:
c00000000009f480: f9 00 4c 3c addis r2,r12,249
c00000000009f484: 80 77 42 38 addi r2,r2,30592
c00000000009f488: a6 02 08 7c mflr r0
c00000000009f48c: 2d 48 fc 4b bl c000000000063cb8 <_mcount>
c00000000009f490: a6 02 08 7c mflr r0
c00000000009f494: c0 ff 01 fb std r24,-64(r1)
c00000000009f498: c8 ff 21 fb std r25,-56(r1)
c00000000009f49c: d0 ff 41 fb std r26,-48(r1)
c00000000009f4a0: d8 ff 61 fb std r27,-40(r1)
c00000000009f4a4: e0 ff 81 fb std r28,-32(r1)
c00000000009f4a8: e8 ff a1 fb std r29,-24(r1)
c00000000009f4ac: f0 ff c1 fb std r30,-16(r1)
c00000000009f4b0: f8 ff e1 fb std r31,-8(r1)
c00000000009f4b4: 10 00 01 f8 std r0,16(r1)
c00000000009f4b8: 31 fe 21 f8 stdu r1,-464(r1) -> c000000007967e10 - 464 = c000000007967c40
c00000000009f4bc: e8 0c 2d e9 ld r9,3304(r13)
c00000000009f4c0: 88 01 21 f9 std r9,392(r1) c000000007967c40 + 392 = c000000007967dc8
paca->canary = 31fc80016f07fb00 (0xce8)
current->canary =
1:mon> d8 c0000000077ef150
c0000000077ef150 31fc80016f07fb00 c000200006600080
1:mon> d8 %r1
c000000007967a90 c000000007967af0 0000000000000001
c000000007967aa0 c0000000000c2c00 c000000001036c00
c000000007967ab0 fffffffffffffffe c0000000010ff9b0
c000000007967ac0 c0000000010ff9b0 ffffffffffffffff
1:mon>
c000000007967ad0 0000000000000000 0000000000000000
c000000007967ae0 c000000000f05330 c0000000000c2bd0
c000000007967af0 c000000007967b40 31fc80016f07fb00
^^^^^^^^^^^^^^^^
pnv_smp_cpu_kill_self frame:
1:mon> d8 c000000007967c40
c000000007967c40 c000000007967e10 0000000000063fec
c000000007967c50 c00000000009f720 f6251c2ce0f21b00
c000000007967c60 c000001fec9f0280 c000000007bc3680
c000000007967c70 c000000007967ca0 c0000000077eeb80
c000000007967c80 c00000000012de20 c00000000001ee7c
c000000007967c90 c000000007967cb0 c000000000ef2b80
c000000007967ca0 c000000007967d50 0000000000000001
c000000007967cb0 c000000007967d90 0000000024028222
c000000007967cc0 c00000000017b0a0 0000000000000001
c000000007967cd0 c000000007967d50 0000000000000001
c000000007967ce0 c000000007967d20 c000000001036c00
c000000007967cf0 c000000007967d30 c000001ffe84d980
c000000007967d00 c00000000001ee7c 0000001ffd970000
c000000007967d10 c000000007967d30 c000001ffe862b80
c000000007967d20 0000000000000000 c000000000044c50
c000000007967d30 c000001ffe8451e0 c000000007bc3680
c000000007967d40 c000000007967d60 0000000000000000
c000000007967d50 c000000007967d90 c000000001036c00
c000000007967d60 c000000007967d90 c000000000f3bc80
c000000007967d70 c0000000001ae0f8 c000000000f12880
c000000007967d80 0000001ffd970000 c000000007967dc0
c000000007967d90 c000000007967e10 0000000000000004
c000000007967da0 c0000000001ae2b8 c000001ffe84e050
c000000007967db0 c000000007967e50 c000000001070174
c000000007967dc0 0000000000000000 f6251c2ce0f21b00
^^^^^^^^^^^^^^^^
canary
c000000007967dd0 0000000000000000 0000000000000004
c000000007967de0 0000000000080000 0000000000000002
c000000007967df0 c0000000010700b8 0000000000000001
c000000007967e00 0000000000000002 c00000000106fc58
cpu_die frame:
c000000007967e10 c000000007967e30 c000001ffe84e050
c000000007967e20 c0000000000475d8 c000000001036c00
c000000007967e30 c000000007967e50 0000000000000001
c000000007967e40 c000000000020620 c00000000106fc58
c000000007967e50 c000000007967ec0 c000000000004000
c000000007967e60 c00000000012e574 0000000000004000
c000000007967e70 010000000014c0d4 f6251c2ce0f21b00
^^^^^^^^^^^^^^^^
canary
> Le 15/10/2018 à 15:26, Michael Ellerman a écrit :
>> Hi all,
>>
>> Spotted this today, haven't had time to debug it further, just FYI in
>> case anyone else sees it.
>>
>> Running tests in cpufreq
>> ========================================
>> selftests: cpufreq: main.sh
>> pid 9727's current affinity mask: ffffffffffffffffffffffffffffffffffffffffffff
>> pid 9727's new affinity mask: 1
>> Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: pnv_smp_cpu_kill_self+0x2a0/0x2b0
>>
>> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc3-gcc-7.3.1-00168-g4ffe713b7587 #94
>> Call Trace:
>> [c000000007a1fb00] [c000000000ae7b4c] dump_stack+0xb0/0xf4 (unreliable)
>> [c000000007a1fb40] [c0000000000e59cc] panic+0x144/0x328
>> [c000000007a1fbe0] [c0000000000e544c] __stack_chk_fail+0x2c/0x30
>> [c000000007a1fc40] [c00000000009eca0] pnv_smp_cpu_kill_self+0x2a0/0x2b0
>> [c000000007a1fe10] [c0000000000475f8] cpu_die+0x48/0x70
>> [c000000007a1fe30] [c000000000020620] arch_cpu_idle_dead+0x20/0x40
>> [c000000007a1fe50] [c00000000012da94] do_idle+0x274/0x390
>> [c000000007a1fec0] [c00000000012de08] cpu_startup_entry+0x38/0x50
>> [c000000007a1fef0] [c000000000047334] start_secondary+0x5e4/0x600
>> [c000000007a1ff90] [c00000000000ac70] start_secondary_prolog+0x10/0x14
>> Rebooting in 10 seconds..
>> [39378.502863506,5] OPAL: Reboot request
>>
>>
>>
>> cheers
>>
^ permalink raw reply
* Re: [PATCH v06 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Michael Bringmann @ 2018-10-16 14:07 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: Nathan Fontenot, Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <87sh16lo72.fsf@concordia.ellerman.id.au>
On 10/15/2018 07:39 PM, Michael Ellerman wrote:
> Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index 2b796da..9c76345 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -541,6 +549,23 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>> return rc;
>> }
>>
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> + struct drmem_lmb *lmb;
>> + int rc;
>> +
>> + pr_info("Attempting to update multiple LMBs\n");
>> +
>> + for_each_drmem_lmb(lmb) {
>> + if (drmem_lmb_update(lmb)) {
>> + rc = dlpar_memory_readd_helper(lmb);
>> + drmem_remove_lmb_update(lmb);
>> + }
>> + }
>> +
>> + return rc;
>> +}
>
> This leaves rc potentially uninitialised.
>
> What should the result be in that case, -EINVAL ?
I will force it to be zero (0). Failure to find anything
to update is not an error.
>
> cheers
Thanks.
--
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line 363-5196
External: (512) 286-5196
Cell: (512) 466-0650
mwb@linux.vnet.ibm.com
^ permalink raw reply
* Re: Stack protector crash in pnv_smp_cpu_kill_self()
From: Segher Boessenkool @ 2018-10-16 14:23 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev@ozlabs.org, Gautham R Shenoy, Nicholas Piggin,
Abdul Haleem
In-Reply-To: <87ftx6kowh.fsf@concordia.ellerman.id.au>
On Wed, Oct 17, 2018 at 12:21:50AM +1100, Michael Ellerman wrote:
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>
> > Looks like a lack of initialisation of the canary for the non-boot CPUs
> > on SMP, you applied this morning the patch I sent you for that.
> >
> > Is the patch in ?
>
> Yeah it is.
>
> $ git log --oneline 4ffe713b7587 arch/powerpc/kernel/smp.c
> 8e8a31d7fd54 powerpc: Use cpu_smallcore_sibling_mask at SMT level on bigcores
> 425752c63b6f powerpc: Detect the presence of big-cores via "ibm, thread-groups"
> 7241d26e8175 powerpc/64: properly initialise the stackprotector canary on SMP.
>
>
> It only happens on a specific Power9 machine, not in sim, but it's 100%
> reproducible on that hardware.
>
> The canary value has changed (?!).
>
> The value in paca->canary and current->canary agree, but they don't
> match what's in the stack.
>
> Clearly the idle code is doing something I don't understand :)
Did something actually corrupt the stack?
Segher
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox