* [PATCH V2 4/7] mm/gup: Add FOLL_LONGTERM capability to GUP fast
From: ira.weiny @ 2019-02-13 23:04 UTC (permalink / raw)
To: linux-mips, linux-kernel, kvm-ppc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, kvm, linux-fpga, dri-devel, linux-rdma,
linux-media, linux-scsi, devel, virtualization, netdev,
linux-fbdev, xen-devel, devel, linux-mm, ceph-devel, rds-devel
Cc: Martin Brandenburg, Rich Felker, David Hildenbrand, David Airlie,
Jason Wang, Kai Mäkisara, Michal Hocko, Ira Weiny, Ben Chan,
Rob Springer, Todd Poynor, Yoshinori Sato, Joerg Roedel,
Michael S. Tsirkin, Jason Gunthorpe, Ingo Molnar, Wu Hao,
Alan Tull, John Hubbard, James E.J. Bottomley, Alex Williamson,
Stefano Stabellini, Borislav Petkov, Alexander Viro,
Thomas Gleixner, Mauro Carvalho Chehab, Peter Zijlstra,
Martin K. Petersen, Dennis Dalessandro, Cornelia Huck,
Jens Wiklander, Alexandre Bounine, Bartlomiej Zolnierkiewicz,
Daniel Vetter, Moritz Fischer, Andrew Morton, David S. Miller,
Kirill A. Shutemov, Christian Benvenuti
In-Reply-To: <20190213230455.5605-1-ira.weiny@intel.com>
From: Ira Weiny <ira.weiny@intel.com>
DAX pages were previously unprotected from longterm pins when users
called get_user_pages_fast().
Use the new FOLL_LONGTERM flag to check for DEVMAP pages and fall
back to regular GUP processing if a DEVMAP page is encountered.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
mm/gup.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index 6f32d36b3c5b..f7e759c523bb 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1439,6 +1439,9 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
goto pte_unmap;
if (pte_devmap(pte)) {
+ if (unlikely(flags & FOLL_LONGTERM))
+ goto pte_unmap;
+
pgmap = get_dev_pagemap(pte_pfn(pte), pgmap);
if (unlikely(!pgmap)) {
undo_dev_pagemap(nr, nr_start, pages);
@@ -1578,8 +1581,11 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
if (!pmd_access_permitted(orig, flags & FOLL_WRITE))
return 0;
- if (pmd_devmap(orig))
+ if (pmd_devmap(orig)) {
+ if (unlikely(flags & FOLL_LONGTERM))
+ return 0;
return __gup_device_huge_pmd(orig, pmdp, addr, end, pages, nr);
+ }
refs = 0;
page = pmd_page(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
@@ -1904,8 +1910,20 @@ int get_user_pages_fast(unsigned long start, int nr_pages,
start += nr << PAGE_SHIFT;
pages += nr;
- ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
- gup_flags);
+ if (gup_flags & FOLL_LONGTERM) {
+ down_read(¤t->mm->mmap_sem);
+ ret = __gup_longterm_locked(current, current->mm,
+ start, nr_pages - nr,
+ pages, NULL, gup_flags);
+ up_read(¤t->mm->mmap_sem);
+ } else {
+ /*
+ * retain FAULT_FOLL_ALLOW_RETRY optimization if
+ * possible
+ */
+ ret = get_user_pages_unlocked(start, nr_pages - nr,
+ pages, gup_flags);
+ }
/* Have to be a bit careful with return values */
if (nr > 0) {
--
2.20.1
^ permalink raw reply related
* [PATCH V2 5/7] IB/hfi1: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
From: ira.weiny @ 2019-02-13 23:04 UTC (permalink / raw)
To: linux-mips, linux-kernel, kvm-ppc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, kvm, linux-fpga, dri-devel, linux-rdma,
linux-media, linux-scsi, devel, virtualization, netdev,
linux-fbdev, xen-devel, devel, linux-mm, ceph-devel, rds-devel
Cc: Martin Brandenburg, Rich Felker, David Hildenbrand, David Airlie,
Jason Wang, Kai Mäkisara, Michal Hocko, Ira Weiny, Ben Chan,
Rob Springer, Todd Poynor, Yoshinori Sato, Joerg Roedel,
Michael S. Tsirkin, Jason Gunthorpe, Ingo Molnar, Wu Hao,
Alan Tull, John Hubbard, James E.J. Bottomley, Alex Williamson,
Stefano Stabellini, Borislav Petkov, Alexander Viro,
Thomas Gleixner, Mauro Carvalho Chehab, Peter Zijlstra,
Martin K. Petersen, Dennis Dalessandro, Cornelia Huck,
Jens Wiklander, Alexandre Bounine, Bartlomiej Zolnierkiewicz,
Daniel Vetter, Moritz Fischer, Andrew Morton, David S. Miller,
Kirill A. Shutemov, Christian Benvenuti
In-Reply-To: <20190213230455.5605-1-ira.weiny@intel.com>
From: Ira Weiny <ira.weiny@intel.com>
Use the new FOLL_LONGTERM to get_user_pages_fast() to protect against
FS DAX pages being mapped.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/infiniband/hw/hfi1/user_pages.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/user_pages.c b/drivers/infiniband/hw/hfi1/user_pages.c
index 78ccacaf97d0..6a7f9cd5a94e 100644
--- a/drivers/infiniband/hw/hfi1/user_pages.c
+++ b/drivers/infiniband/hw/hfi1/user_pages.c
@@ -104,9 +104,11 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np
bool writable, struct page **pages)
{
int ret;
+ unsigned int gup_flags = writable ? FOLL_WRITE : 0;
- ret = get_user_pages_fast(vaddr, npages, writable ? FOLL_WRITE : 0,
- pages);
+ gup_flags |= FOLL_LONGTERM;
+
+ ret = get_user_pages_fast(vaddr, npages, gup_flags, pages);
if (ret < 0)
return ret;
--
2.20.1
^ permalink raw reply related
* [PATCH V2 6/7] IB/qib: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
From: ira.weiny @ 2019-02-13 23:04 UTC (permalink / raw)
To: linux-mips, linux-kernel, kvm-ppc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, kvm, linux-fpga, dri-devel, linux-rdma,
linux-media, linux-scsi, devel, virtualization, netdev,
linux-fbdev, xen-devel, devel, linux-mm, ceph-devel, rds-devel
Cc: Martin Brandenburg, Rich Felker, David Hildenbrand, David Airlie,
Jason Wang, Kai Mäkisara, Michal Hocko, Ira Weiny, Ben Chan,
Rob Springer, Todd Poynor, Yoshinori Sato, Joerg Roedel,
Michael S. Tsirkin, Jason Gunthorpe, Ingo Molnar, Wu Hao,
Alan Tull, John Hubbard, James E.J. Bottomley, Alex Williamson,
Stefano Stabellini, Borislav Petkov, Alexander Viro,
Thomas Gleixner, Mauro Carvalho Chehab, Peter Zijlstra,
Martin K. Petersen, Dennis Dalessandro, Cornelia Huck,
Jens Wiklander, Alexandre Bounine, Bartlomiej Zolnierkiewicz,
Daniel Vetter, Moritz Fischer, Andrew Morton, David S. Miller,
Kirill A. Shutemov, Christian Benvenuti
In-Reply-To: <20190213230455.5605-1-ira.weiny@intel.com>
From: Ira Weiny <ira.weiny@intel.com>
Use the new FOLL_LONGTERM to get_user_pages_fast() to protect against
FS DAX pages being mapped.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/infiniband/hw/qib/qib_user_sdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/qib/qib_user_sdma.c b/drivers/infiniband/hw/qib/qib_user_sdma.c
index 31c523b2a9f5..b53cc0240e02 100644
--- a/drivers/infiniband/hw/qib/qib_user_sdma.c
+++ b/drivers/infiniband/hw/qib/qib_user_sdma.c
@@ -673,7 +673,7 @@ static int qib_user_sdma_pin_pages(const struct qib_devdata *dd,
else
j = npages;
- ret = get_user_pages_fast(addr, j, 0, pages);
+ ret = get_user_pages_fast(addr, j, FOLL_LONGTERM, pages);
if (ret != j) {
i = 0;
j = ret;
--
2.20.1
^ permalink raw reply related
* [PATCH V2 7/7] IB/mthca: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
From: ira.weiny @ 2019-02-13 23:04 UTC (permalink / raw)
To: linux-mips, linux-kernel, kvm-ppc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, kvm, linux-fpga, dri-devel, linux-rdma,
linux-media, linux-scsi, devel, virtualization, netdev,
linux-fbdev, xen-devel, devel, linux-mm, ceph-devel, rds-devel
Cc: Martin Brandenburg, Rich Felker, David Hildenbrand, David Airlie,
Jason Wang, Kai Mäkisara, Michal Hocko, Ira Weiny, Ben Chan,
Rob Springer, Todd Poynor, Yoshinori Sato, Joerg Roedel,
Michael S. Tsirkin, Jason Gunthorpe, Ingo Molnar, Wu Hao,
Alan Tull, John Hubbard, James E.J. Bottomley, Alex Williamson,
Stefano Stabellini, Borislav Petkov, Alexander Viro,
Thomas Gleixner, Mauro Carvalho Chehab, Peter Zijlstra,
Martin K. Petersen, Dennis Dalessandro, Cornelia Huck,
Jens Wiklander, Alexandre Bounine, Bartlomiej Zolnierkiewicz,
Daniel Vetter, Moritz Fischer, Andrew Morton, David S. Miller,
Kirill A. Shutemov, Christian Benvenuti
In-Reply-To: <20190213230455.5605-1-ira.weiny@intel.com>
From: Ira Weiny <ira.weiny@intel.com>
Use the new FOLL_LONGTERM to get_user_pages_fast() to protect against
FS DAX pages being mapped.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
drivers/infiniband/hw/mthca/mthca_memfree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_memfree.c b/drivers/infiniband/hw/mthca/mthca_memfree.c
index 112d2f38e0de..8ff0e90d7564 100644
--- a/drivers/infiniband/hw/mthca/mthca_memfree.c
+++ b/drivers/infiniband/hw/mthca/mthca_memfree.c
@@ -472,7 +472,8 @@ int mthca_map_user_db(struct mthca_dev *dev, struct mthca_uar *uar,
goto out;
}
- ret = get_user_pages_fast(uaddr & PAGE_MASK, 1, FOLL_WRITE, pages);
+ ret = get_user_pages_fast(uaddr & PAGE_MASK, 1,
+ FOLL_WRITE | FOLL_LONGTERM, pages);
if (ret < 0)
goto out;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH V2 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
From: Jason Gunthorpe @ 2019-02-13 23:11 UTC (permalink / raw)
To: ira.weiny
Cc: Martin Brandenburg, linux-fbdev, Rich Felker, kvm, linux-sh,
David Airlie, linux-fpga, dri-devel, linux-kernel, linux-mm,
Michal Hocko, sparclinux, Thomas Gleixner, devel, linux-s390,
rds-devel, Mauro Carvalho Chehab, linux-scsi, linux-rdma,
Joerg Roedel, David Hildenbrand, Rob Springer, Peter Zijlstra,
Ingo Molnar, xen-devel, Jason Wang, devel, linux-media, Ben Chan,
Alan Tull, Bartlomiej Zolnierkiewicz, John Hubbard,
James E.J. Bottomley, kvm-ppc, Alex Williamson,
Stefano Stabellini, Borislav Petkov, Alexander Viro, ceph-devel,
virtualization, Kai Mäkisara, Yoshinori Sato,
Martin K. Petersen, netdev, Dennis Dalessandro, Cornelia Huck,
linux-mips, Jens Wiklander, Alexandre Bounine, Todd Poynor,
Michael S. Tsirkin, Daniel Vetter, Moritz Fischer, Andrew Morton,
Wu Hao, linuxppc-dev, David S. Miller, Kirill A. Shutemov,
Christian Benvenuti
In-Reply-To: <20190213230455.5605-4-ira.weiny@intel.com>
On Wed, Feb 13, 2019 at 03:04:51PM -0800, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
>
> To facilitate additional options to get_user_pages_fast() change the
> singular write parameter to be gup_flags.
So now we have:
long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
struct page **pages, unsigned int gup_flags);
and
int get_user_pages_fast(unsigned long start, int nr_pages,
unsigned int gup_flags, struct page **pages)
Does this make any sense? At least the arguments should be in the same
order, I think.
Also this comment:
/*
* get_user_pages_unlocked() is suitable to replace the form:
*
* down_read(&mm->mmap_sem);
* get_user_pages(tsk, mm, ..., pages, NULL);
* up_read(&mm->mmap_sem);
*
* with:
*
* get_user_pages_unlocked(tsk, mm, ..., pages);
*
* It is functionally equivalent to get_user_pages_fast so
* get_user_pages_fast should be used instead if specific gup_flags
* (e.g. FOLL_FORCE) are not required.
*/
Needs some attention as the recommendation is now nonsense.
Honestly a proper explanation of why two functions exist would be
great at this point :)
Jason
^ permalink raw reply
* Re: [PATCH V2 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
From: Ira Weiny @ 2019-02-13 23:52 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Martin Brandenburg, linux-fbdev, Rich Felker, kvm, linux-sh,
David Airlie, linux-fpga, dri-devel, linux-kernel, linux-mm,
Michal Hocko, sparclinux, Thomas Gleixner, devel, linux-s390,
rds-devel, Mauro Carvalho Chehab, linux-scsi, linux-rdma,
Joerg Roedel, David Hildenbrand, Rob Springer, Peter Zijlstra,
Ingo Molnar, xen-devel, Jason Wang, devel, linux-media, Ben Chan,
Alan Tull, Bartlomiej Zolnierkiewicz, John Hubbard,
James E.J. Bottomley, kvm-ppc, Alex Williamson,
Stefano Stabellini, Borislav Petkov, Alexander Viro, ceph-devel,
virtualization, Kai Mäkisara, Yoshinori Sato,
Martin K. Petersen, netdev, Dennis Dalessandro, Cornelia Huck,
linux-mips, Jens Wiklander, Alexandre Bounine, Todd Poynor,
Michael S. Tsirkin, Daniel Vetter, Moritz Fischer, Andrew Morton,
Wu Hao, linuxppc-dev, David S. Miller, Kirill A. Shutemov,
Christian Benvenuti
In-Reply-To: <20190213231110.GD24692@ziepe.ca>
On Wed, Feb 13, 2019 at 04:11:10PM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 13, 2019 at 03:04:51PM -0800, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > To facilitate additional options to get_user_pages_fast() change the
> > singular write parameter to be gup_flags.
>
> So now we have:
>
> long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> struct page **pages, unsigned int gup_flags);
>
> and
>
> int get_user_pages_fast(unsigned long start, int nr_pages,
> unsigned int gup_flags, struct page **pages)
>
> Does this make any sense? At least the arguments should be in the same
> order, I think.
Yes... and no. see below.
>
> Also this comment:
> /*
> * get_user_pages_unlocked() is suitable to replace the form:
> *
> * down_read(&mm->mmap_sem);
> * get_user_pages(tsk, mm, ..., pages, NULL);
> * up_read(&mm->mmap_sem);
> *
> * with:
> *
> * get_user_pages_unlocked(tsk, mm, ..., pages);
> *
> * It is functionally equivalent to get_user_pages_fast so
> * get_user_pages_fast should be used instead if specific gup_flags
> * (e.g. FOLL_FORCE) are not required.
> */
>
> Needs some attention as the recommendation is now nonsense.
IMO they are not functionally equivalent.
We can't remove *_unlocked() as it is used as both a helper for the arch
specific *_fast() calls, _and_ in drivers. Again I don't know the history here
but it could be that the drivers should never have used the call in the first
place??? Or been converted at some point?
I could change the comment to be something like
/*
* get_user_pages_unlocked() is only to be used by arch specific
* get_user_pages_fast() calls. Drivers should be calling
* get_user_pages_fast()
*/
Instead of the current comment.
And change the drivers to get_user_pages_fast().
However, I'm not sure if these drivers need the FOLL_TOUCH flag which
*_unlocked() adds for them. And adding FOLL_TOUCH to *_fast() is not going to
give the same functionality.
It _looks_ like we can add FOLL_TOUCH functionality to the fast path in the
generic code. I'm not sure about the arch's.
If we did that then we can have those drivers use FOLL_TOUCH or not in *_fast()
if they want/need.
>
> Honestly a proper explanation of why two functions exist would be
> great at this point :)
I've not researched it. I do agree that there seems to be a lot of calls in
this file and the differences are subtle.
Ira
>
> Jason
^ permalink raw reply
* Re: [PATCH 1/5] vfio/type1: use pinned_vm instead of locked_vm to account pinned pages
From: Daniel Jordan @ 2019-02-14 1:46 UTC (permalink / raw)
To: Alex Williamson
Cc: dave, jack, kvm, linux-mm, aik, linux-fpga, atull, linux-kernel,
kvm-ppc, Daniel Jordan, Jason Gunthorpe, peterz, mdf, akpm,
linuxppc-dev, cl, hao.wu
In-Reply-To: <20190213130330.76ef1987@w520.home>
On Wed, Feb 13, 2019 at 01:03:30PM -0700, Alex Williamson wrote:
> Daniel Jordan <daniel.m.jordan@oracle.com> wrote:
> > On Tue, Feb 12, 2019 at 11:41:10AM -0700, Alex Williamson wrote:
> > > This still makes me nervous because we have userspace dependencies on
> > > setting process locked memory.
> >
> > Could you please expand on this? Trying to get more context.
>
> VFIO is a userspace driver interface and the pinned/locked page
> accounting we're doing here is trying to prevent a user from exceeding
> their locked memory limits. Thus a VM management tool or unprivileged
> userspace driver needs to have appropriate locked memory limits
> configured for their use case. Currently we do not have a unified
> accounting scheme, so if a page is mlock'd by the user and also mapped
> through VFIO for DMA, it's accounted twice, these both increment
> locked_vm and userspace needs to manage that. If pinned memory
> and locked memory are now two separate buckets and we're only comparing
> one of them against the locked memory limit, then it seems we have
> effectively doubled the user's locked memory for this use case, as
> Jason questioned. The user could mlock one page and DMA map another,
> they're both "locked", but now they only take one slot in each bucket.
Right, yes. Should have been more specific. I was after a concrete use case
where this would happen (sounded like you may have had a specific tool in
mind).
But it doesn't matter. I understand your concern and agree that, given the
possibility that accounting in _some_ tool can be affected, we should fix
accounting before changing user visible behavior. I can start a separate
discussion, having opened the can of worms again :)
> If we continue forward with using a separate bucket here, userspace
> could infer that accounting is unified and lower the user's locked
> memory limit, or exploit the gap that their effective limit might
> actually exceed system memory. In the former case, if we do eventually
> correct to compare the total of the combined buckets against the user's
> locked memory limits, we'll break users that have adapted their locked
> memory limits to meet the apparent needs. In the latter case, the
> inconsistent accounting is potentially an attack vector.
Makes sense.
> > > There's a user visible difference if we
> > > account for them in the same bucket vs separate. Perhaps we're
> > > counting in the wrong bucket now, but if we "fix" that and userspace
> > > adapts, how do we ever go back to accounting both mlocked and pinned
> > > memory combined against rlimit? Thanks,
> >
> > PeterZ posted an RFC that addresses this point[1]. It kept pinned_vm and
> > locked_vm accounting separate, but allowed the two to be added safely to be
> > compared against RLIMIT_MEMLOCK.
>
> Unless I'm incorrect in the concerns above, I don't see how we can
> convert vfio before this occurs.
>
> > Anyway, until some solution is agreed on, are there objections to converting
> > locked_vm to an atomic, to avoid user-visible changes, instead of switching
> > locked_vm users to pinned_vm?
>
> Seems that as long as we have separate buckets that are compared
> individually to rlimit that we've got problems, it's just a matter of
> where they're exposed based on which bucket is used for which
> interface. Thanks,
Indeed. But for now, any concern with simply changing the type of the
currently used counter to an atomic, to reduce mmap_sem usage? This is just an
implementation detail, invisible to userspace.
^ permalink raw reply
* Re: [PATCH 0/5] use pinned_vm instead of locked_vm to account pinned pages
From: Ira Weiny @ 2019-02-14 1:53 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: dave, jack, kvm, atull, aik, linux-fpga, linux-kernel, kvm-ppc,
Daniel Jordan, linux-mm, alex.williamson, mdf, akpm, linuxppc-dev,
cl, hao.wu
In-Reply-To: <20190211225447.GN24692@ziepe.ca>
On Mon, Feb 11, 2019 at 03:54:47PM -0700, Jason Gunthorpe wrote:
> On Mon, Feb 11, 2019 at 05:44:32PM -0500, Daniel Jordan wrote:
>
> > All five of these places, and probably some of Davidlohr's conversions,
> > probably want to be collapsed into a common helper in the core mm for
> > accounting pinned pages. I tried, and there are several details that
> > likely need discussion, so this can be done as a follow-on.
>
> I've wondered the same..
I'm really thinking this would be a nice way to ensure it gets cleaned up and
does not happen again.
Also, by moving it to the core we could better manage any user visible changes.
From a high level, pinned is a subset of locked so it seems like we need a 2
sets of helpers.
try_increment_locked_vm(...)
decrement_locked_vm(...)
try_increment_pinned_vm(...)
decrement_pinned_vm(...)
Where try_increment_pinned_vm() also increments locked_vm... Of course this
may end up reverting the improvement of Davidlohr Bueso's atomic work... :-(
Furthermore it would seem better (although I don't know if at all possible) if
this were accounted for in core calls which tracked them based on how the pages
are being used so that drivers can't call try_increment_locked_vm() and then
pin the pages... Thus getting the account wrong vs what actually happened.
And then in the end we can go back to locked_vm being the value checked against
RLIMIT_MEMLOCK.
Ira
^ permalink raw reply
* Re: [PATCH v3 2/2] drivers/mtd: Fix device registration error
From: Aneesh Kumar K.V @ 2019-02-14 4:23 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linuxppc-dev, Alban Bedel, linux-mtd
In-Reply-To: <20190213142833.2b2999fd@collabora.com>
On 2/13/19 6:58 PM, Boris Brezillon wrote:
> Subject prefix should be "mtd: powernv_flash: "
>
> On Mon, 11 Feb 2019 19:03:38 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
>
>> This change helps me to get multiple mtd device registered. Without this
>> I get
>>
>> sysfs: cannot create duplicate filename '/bus/nvmem/devices/flash0'
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.0.0-rc2-00557-g1ef20ef21f22 #13
>> Call Trace:
>> [c0000000b38e3220] [c000000000b58fe4] dump_stack+0xe8/0x164 (unreliable)
>> [c0000000b38e3270] [c0000000004cf074] sysfs_warn_dup+0x84/0xb0
>> [c0000000b38e32f0] [c0000000004cf6c4] sysfs_do_create_link_sd.isra.0+0x114/0x150
>> [c0000000b38e3340] [c000000000726a84] bus_add_device+0x94/0x1e0
>> [c0000000b38e33c0] [c0000000007218f0] device_add+0x4d0/0x830
>> [c0000000b38e3480] [c0000000009d54a8] nvmem_register.part.2+0x1c8/0xb30
>> [c0000000b38e3560] [c000000000834530] mtd_nvmem_add+0x90/0x120
>> [c0000000b38e3650] [c000000000835bc8] add_mtd_device+0x198/0x4e0
>> [c0000000b38e36f0] [c00000000083619c] mtd_device_parse_register+0x11c/0x280
>> [c0000000b38e3780] [c000000000840830] powernv_flash_probe+0x180/0x250
>> [c0000000b38e3820] [c00000000072c120] platform_drv_probe+0x60/0xf0
>> [c0000000b38e38a0] [c0000000007283c8] really_probe+0x138/0x4d0
>> [c0000000b38e3930] [c000000000728acc] driver_probe_device+0x13c/0x1b0
>> [c0000000b38e39b0] [c000000000728c7c] __driver_attach+0x13c/0x1c0
>> [c0000000b38e3a30] [c000000000725130] bus_for_each_dev+0xa0/0x120
>> [c0000000b38e3a90] [c000000000727b2c] driver_attach+0x2c/0x40
>> [c0000000b38e3ab0] [c0000000007270f8] bus_add_driver+0x228/0x360
>> [c0000000b38e3b40] [c00000000072a2e0] driver_register+0x90/0x1a0
>> [c0000000b38e3bb0] [c00000000072c020] __platform_driver_register+0x50/0x70
>> [c0000000b38e3bd0] [c00000000105c984] powernv_flash_driver_init+0x24/0x38
>> [c0000000b38e3bf0] [c000000000010904] do_one_initcall+0x84/0x464
>> [c0000000b38e3cd0] [c000000001004548] kernel_init_freeable+0x530/0x634
>> [c0000000b38e3db0] [c000000000011154] kernel_init+0x1c/0x168
>> [c0000000b38e3e20] [c00000000000bed4] ret_from_kernel_thread+0x5c/0x68
>> mtd mtd1: Failed to register NVMEM device
>>
>> With the change we now have
>>
>> root@(none):/sys/bus/nvmem/devices# ls -al
>> total 0
>> drwxr-xr-x 2 root root 0 Feb 6 20:49 .
>> drwxr-xr-x 4 root root 0 Feb 6 20:49 ..
>> lrwxrwxrwx 1 root root 0 Feb 6 20:49 flash@0 -> ../../../devices/platform/ibm,opal:flash@0/mtd/mtd0/flash@0
>> lrwxrwxrwx 1 root root 0 Feb 6 20:49 flash@1 -> ../../../devices/platform/ibm,opal:flash@1/mtd/mtd1/flash@1
>>
>> Fixes: acfe63ec1c59 ("mtd: Convert to using %pOFn instead of device_node.name")
>
> Actually it's not this commit that is at fault as mtd->name was already
> given the value of device_node->name before that. I think you're
> actually fixing 1cbb4a1c433a ("mtd: powernv: Add powernv flash MTD
> abstraction driver").
>
> No need to send a new version, I can fix that when applying, just let
> me know if you're okay with the changes I suggested.
>
The suggested changes looks good.
Thanks
-aneesh
^ permalink raw reply
* [PATCH 1/1] powerpc/64: Adjust order in pcibios_init()
From: Sam Bobroff @ 2019-02-14 5:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: oohall
The pcibios_init() function for 64 bit PowerPC currently calls
pci_bus_add_devices() before pcibios_resource_survey(), which seems
incorrect because it adds devices and attempts to bind their drivers
before allocating their resources (although no problems seem to be
apparent).
So move the call to pci_bus_add_devices() to after
pcibios_resource_survey().
This will also allow the ppc_md.pcibios_bus_add_device() hooks to
perform actions that depend on PCI resources, both during rescanning
(where this is already the case) and at boot time, which should
support improvements and refactoring.
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
Hi everyone,
I've tested this on a P9 for both the host and a KVM guest, and the change
hasn't caused any differences in PCI resource assignments or the general boot
messages.
I've also had a go at inspecting most of the code used by pci_bus_add_devices()
and pcibios_resource_survey() and it doesn't look like there are going to be
any changes in behaviour caused by reordering. It might be worth mentioning
that the hotplug path (see pcibios_finish_adding_to_bus()) already does
resource allocation before calling pci_bus_add_devices().
However, it would be great if someone could test this change on some older
hardware or comment on wether we should make the same change on 32 bit machines.
Cheers,
Sam.
arch/powerpc/kernel/pci_64.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 9d8c10d55407..1ce28888dbdb 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -58,14 +58,16 @@ static int __init pcibios_init(void)
pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
/* Scan all of the recorded PCI controllers. */
- list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
+ list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
pcibios_scan_phb(hose);
- pci_bus_add_devices(hose->bus);
- }
/* Call common code to handle resource allocation */
pcibios_resource_survey();
+ /* Add devices. */
+ list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
+ pci_bus_add_devices(hose->bus);
+
printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
return 0;
--
2.19.0.2.gcad72f5712
^ permalink raw reply related
* Re: [PATCH 0/5] use pinned_vm instead of locked_vm to account pinned pages
From: Jason Gunthorpe @ 2019-02-14 6:00 UTC (permalink / raw)
To: Ira Weiny
Cc: dave, jack, kvm, atull, aik, linux-fpga, linux-kernel, kvm-ppc,
Daniel Jordan, linux-mm, alex.williamson, mdf, akpm, linuxppc-dev,
cl, hao.wu
In-Reply-To: <20190214015314.GB1151@iweiny-DESK2.sc.intel.com>
On Wed, Feb 13, 2019 at 05:53:14PM -0800, Ira Weiny wrote:
> On Mon, Feb 11, 2019 at 03:54:47PM -0700, Jason Gunthorpe wrote:
> > On Mon, Feb 11, 2019 at 05:44:32PM -0500, Daniel Jordan wrote:
> >
> > > All five of these places, and probably some of Davidlohr's conversions,
> > > probably want to be collapsed into a common helper in the core mm for
> > > accounting pinned pages. I tried, and there are several details that
> > > likely need discussion, so this can be done as a follow-on.
> >
> > I've wondered the same..
>
> I'm really thinking this would be a nice way to ensure it gets cleaned up and
> does not happen again.
>
> Also, by moving it to the core we could better manage any user visible changes.
>
> From a high level, pinned is a subset of locked so it seems like we need a 2
> sets of helpers.
>
> try_increment_locked_vm(...)
> decrement_locked_vm(...)
>
> try_increment_pinned_vm(...)
> decrement_pinned_vm(...)
>
> Where try_increment_pinned_vm() also increments locked_vm... Of course this
> may end up reverting the improvement of Davidlohr Bueso's atomic work... :-(
>
> Furthermore it would seem better (although I don't know if at all possible) if
> this were accounted for in core calls which tracked them based on how the pages
> are being used so that drivers can't call try_increment_locked_vm() and then
> pin the pages... Thus getting the account wrong vs what actually happened.
>
> And then in the end we can go back to locked_vm being the value checked against
> RLIMIT_MEMLOCK.
Someone would need to understand the bug that was fixed by splitting
them.
I think it had to do with double accounting pinned and mlocked pages
and thus delivering a lower than expected limit to userspace.
vfio has this bug, RDMA does not. RDMA has a bug where it can
overallocate locked memory, vfio doesn't.
Really unclear how to fix this. The pinned/locked split with two
buckets may be the right way.
Jason
^ permalink raw reply
* [PATCH] powerpc/64s: Fix possible corruption on big endian due to pgd/pud_present()
From: Michael Ellerman @ 2019-02-14 6:23 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-mm, erhard_f, jack, aneesh.kumar, linux-kernel
In v4.20 we changed our pgd/pud_present() to check for _PAGE_PRESENT
rather than just checking that the value is non-zero, e.g.:
static inline int pgd_present(pgd_t pgd)
{
- return !pgd_none(pgd);
+ return (pgd_raw(pgd) & cpu_to_be64(_PAGE_PRESENT));
}
Unfortunately this is broken on big endian, as the result of the
bitwise && is truncated to int, which is always zero because
_PAGE_PRESENT is 0x8000000000000000ul. This means pgd_present() and
pud_present() are always false at compile time, and the compiler
elides the subsequent code.
Remarkably with that bug present we are still able to boot and run
with few noticeable effects. However under some work loads we are able
to trigger a warning in the ext4 code:
WARNING: CPU: 11 PID: 29593 at fs/ext4/inode.c:3927 .ext4_set_page_dirty+0x70/0xb0
CPU: 11 PID: 29593 Comm: debugedit Not tainted 4.20.0-rc1 #1
...
NIP .ext4_set_page_dirty+0x70/0xb0
LR .set_page_dirty+0xa0/0x150
Call Trace:
.set_page_dirty+0xa0/0x150
.unmap_page_range+0xbf0/0xe10
.unmap_vmas+0x84/0x130
.unmap_region+0xe8/0x190
.__do_munmap+0x2f0/0x510
.__vm_munmap+0x80/0x110
.__se_sys_munmap+0x14/0x30
system_call+0x5c/0x70
The fix is simple, we need to convert the result of the bitwise && to
an int before returning it.
Thanks to Jan Kara and Aneesh for help with debugging.
Fixes: da7ad366b497 ("powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit")
Cc: stable@vger.kernel.org # v4.20+
Reported-by: Erhard F. <erhard_f@mailbox.org>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index c9bfe526ca9d..d8c8d7c9df15 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -904,7 +904,7 @@ static inline int pud_none(pud_t pud)
static inline int pud_present(pud_t pud)
{
- return (pud_raw(pud) & cpu_to_be64(_PAGE_PRESENT));
+ return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PRESENT));
}
extern struct page *pud_page(pud_t pud);
@@ -951,7 +951,7 @@ static inline int pgd_none(pgd_t pgd)
static inline int pgd_present(pgd_t pgd)
{
- return (pgd_raw(pgd) & cpu_to_be64(_PAGE_PRESENT));
+ return !!(pgd_raw(pgd) & cpu_to_be64(_PAGE_PRESENT));
}
static inline pte_t pgd_pte(pgd_t pgd)
--
2.20.1
^ permalink raw reply related
* [PATCH] powerpc/book3s: Remove a few page table update interfaces.
From: Aneesh Kumar K.V @ 2019-02-14 6:45 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: Aneesh Kumar K.V, linuxppc-dev
When updating page tables, we need to make sure we fill the page table
entry valid bit. We should be using page table populate interface for
updating the table entries. The page table 'set' interface allows
updating the raw value of page table entry. This can result in
updating the entry wrongly. Remove the 'set' interface so that we avoid
its future usage.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgalloc.h | 8 ++++----
arch/powerpc/include/asm/book3s/64/pgtable.h | 14 --------------
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index 9c1173283b96..138bc2ecc0c4 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -111,7 +111,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)
{
- pgd_set(pgd, __pgtable_ptr_val(pud) | PGD_VAL_BITS);
+ *pgd = __pgd(__pgtable_ptr_val(pud) | PGD_VAL_BITS);
}
static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
@@ -138,7 +138,7 @@ static inline void pud_free(struct mm_struct *mm, pud_t *pud)
static inline void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd)
{
- pud_set(pud, __pgtable_ptr_val(pmd) | PUD_VAL_BITS);
+ *pud = __pud(__pgtable_ptr_val(pmd) | PUD_VAL_BITS);
}
static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
@@ -176,13 +176,13 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmd,
static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmd,
pte_t *pte)
{
- pmd_set(pmd, __pgtable_ptr_val(pte) | PMD_VAL_BITS);
+ *pmd = __pmd(__pgtable_ptr_val(pte) | PMD_VAL_BITS);
}
static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd,
pgtable_t pte_page)
{
- pmd_set(pmd, __pgtable_ptr_val(pte_page) | PMD_VAL_BITS);
+ *pmd = __pmd(__pgtable_ptr_val(pte_page) | PMD_VAL_BITS);
}
static inline pgtable_t pmd_pgtable(pmd_t pmd)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index dc71e2b92003..a24e00fb7fa7 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -853,11 +853,6 @@ static inline bool pte_ci(pte_t pte)
return false;
}
-static inline void pmd_set(pmd_t *pmdp, unsigned long val)
-{
- *pmdp = __pmd(val);
-}
-
static inline void pmd_clear(pmd_t *pmdp)
{
*pmdp = __pmd(0);
@@ -889,11 +884,6 @@ static inline int pmd_bad(pmd_t pmd)
return hash__pmd_bad(pmd);
}
-static inline void pud_set(pud_t *pudp, unsigned long val)
-{
- *pudp = __pud(val);
-}
-
static inline void pud_clear(pud_t *pudp)
{
*pudp = __pud(0);
@@ -936,10 +926,6 @@ static inline bool pud_access_permitted(pud_t pud, bool write)
}
#define pgd_write(pgd) pte_write(pgd_pte(pgd))
-static inline void pgd_set(pgd_t *pgdp, unsigned long val)
-{
- *pgdp = __pgd(val);
-}
static inline void pgd_clear(pgd_t *pgdp)
{
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 03/11] kernel/locks: consolidate RWSEM_GENERIC_* options
From: Geert Uytterhoeven @ 2019-02-14 8:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, linux-s390, linux-kbuild, Linux Kernel Mailing List,
Masahiro Yamada, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-4-hch@lst.de>
On Thu, Feb 14, 2019 at 12:08 AM Christoph Hellwig <hch@lst.de> wrote:
> Introduce one central definition of RWSEM_XCHGADD_ALGORITHM and
> RWSEM_GENERIC_SPINLOCK in kernel/Kconfig.locks and let architectures
> select RWSEM_XCHGADD_ALGORITHM if they want it, otherwise default to
> the spinlock version.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> arch/m68k/Kconfig | 7 -------
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 07/11] init: consolidate the GENERIC_CALIBRATE_DELAY symbol
From: Geert Uytterhoeven @ 2019-02-14 8:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, linux-s390, linux-kbuild, Linux Kernel Mailing List,
Masahiro Yamada, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-8-hch@lst.de>
On Thu, Feb 14, 2019 at 3:25 AM Christoph Hellwig <hch@lst.de> wrote:
> Add one definition to init/Kconfig and let the architectures select it
> if supported. The only complication is xtensa, where it is a user
> visible option - we introduce a xtensa-specific symbol instead to work
> around this.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> arch/m68k/Kconfig | 5 +----
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 09/11] lib: consolidate the GENERIC_CSUM symbol
From: Geert Uytterhoeven @ 2019-02-14 8:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, linux-s390, linux-kbuild, Linux Kernel Mailing List,
Masahiro Yamada, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-10-hch@lst.de>
On Thu, Feb 14, 2019 at 3:25 AM Christoph Hellwig <hch@lst.de> wrote:
> Add one definition to lib/Kconfig and let the architectures
> select if it supported.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> arch/m68k/Kconfig | 3 ---
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 10/11] lib: consolidate the GENERIC_HWEIGHT symbol
From: Geert Uytterhoeven @ 2019-02-14 8:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xtensa, linux-s390, linux-kbuild, Linux Kernel Mailing List,
Masahiro Yamada, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-11-hch@lst.de>
On Thu, Feb 14, 2019 at 3:11 AM Christoph Hellwig <hch@lst.de> wrote:
> Introduce a new ARCH_HAS_HWEIGHT symbol for alpha and ia64, and just
> default to the generic version otherwise.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> arch/m68k/Kconfig | 4 ----
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Kernel panic when loading the IDE controller driver
From: sgosavi1 @ 2019-02-14 8:17 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <173559f3-7b2b-735b-acc7-66ccbdc7905d@c-s.fr>
> Maybe look around
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?
> id=9a0e77f28b50128df0c9e26ae489e44e29a7270a
>Also look at ide_platform.c. I imagine there must be some way to set it
>up in your device tree.
I have gone through this before and also tried the pata_platform driver but
no success yet. I haven't found any example that passes the IO ports and IRQ
information through the device tree to the driver code.
Thanks,
Sachin.
--
Sent from: http://linuxppc.10917.n7.nabble.com/linuxppc-dev-f3.html
^ permalink raw reply
* Re: [PATCH 01/11] powerpc: remove dead ifdefs in <asm/checksum.h>
From: Christophe Leroy @ 2019-02-14 8:26 UTC (permalink / raw)
To: Christoph Hellwig, Masahiro Yamada
Cc: linux-xtensa, linux-kbuild, linux-s390, linux-kernel, linux-riscv,
linuxppc-dev
In-Reply-To: <20190213174005.28785-2-hch@lst.de>
Le 13/02/2019 à 18:39, Christoph Hellwig a écrit :
> __KERNEL__ is never not defined for non-uapi headers, and GENERIC_CSUM
> isn't ever set for powerpc either.
GENERIC_CSUM was abandonned by commit d4fde568a34a ("powerpc/64: Use
optimized checksum routines on little-endian")
Could you also remove the 'config GENERIC_CSUM' item in
arch/powerpc/Kconfig ?
Christophe
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/powerpc/include/asm/checksum.h | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h
> index a78a57e5058d..37c309500260 100644
> --- a/arch/powerpc/include/asm/checksum.h
> +++ b/arch/powerpc/include/asm/checksum.h
> @@ -1,6 +1,5 @@
> #ifndef _ASM_POWERPC_CHECKSUM_H
> #define _ASM_POWERPC_CHECKSUM_H
> -#ifdef __KERNEL__
>
> /*
> * This program is free software; you can redistribute it and/or
> @@ -9,9 +8,6 @@
> * 2 of the License, or (at your option) any later version.
> */
>
> -#ifdef CONFIG_GENERIC_CSUM
> -#include <asm-generic/checksum.h>
> -#else
> #include <linux/bitops.h>
> #include <linux/in6.h>
> /*
> @@ -217,6 +213,4 @@ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
> const struct in6_addr *daddr,
> __u32 len, __u8 proto, __wsum sum);
>
> -#endif
> -#endif /* __KERNEL__ */
> -#endif
> +#endif /* _ASM_POWERPC_CHECKSUM_H */
>
^ permalink raw reply
* Re: [PATCH v2] hugetlb: allow to free gigantic pages regardless of the configuration
From: Vlastimil Babka @ 2019-02-14 9:52 UTC (permalink / raw)
To: Dave Hansen, Alexandre Ghiti, Catalin Marinas, Will Deacon,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, x86, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Alexander Viro, Mike Kravetz,
linux-arm-kernel, linux-kernel, linuxppc-dev, linux-s390,
linux-fsdevel, linux-mm
In-Reply-To: <d367b5c7-eb05-6d0b-f9bf-5b3fc3f392a9@intel.com>
On 2/13/19 8:30 PM, Dave Hansen wrote:
>> -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
>> +#ifdef CONFIG_COMPACTION_CORE
>> static __init int gigantic_pages_init(void)
>> {
>> /* With compaction or CMA we can allocate gigantic pages at runtime */
>> diff --git a/fs/Kconfig b/fs/Kconfig
>> index ac474a61be37..8fecd3ea5563 100644
>> --- a/fs/Kconfig
>> +++ b/fs/Kconfig
>> @@ -207,8 +207,9 @@ config HUGETLB_PAGE
>> config MEMFD_CREATE
>> def_bool TMPFS || HUGETLBFS
>>
>> -config ARCH_HAS_GIGANTIC_PAGE
>> +config COMPACTION_CORE
>> bool
>> + default y if (MEMORY_ISOLATION && MIGRATION) || CMA
>
> This takes a hard dependency (#if) and turns it into a Kconfig *default*
> that can be overridden. That seems like trouble.
>
> Shouldn't it be:
>
> config COMPACTION_CORE
> def_bool y
> depends on (MEMORY_ISOLATION && MIGRATION) || CMA
Agreed. Also I noticed that it now depends on MIGRATION instead of
COMPACTION. That intention is correct IMHO, but will fail to
compile/link when both COMPACTION and CMA are disabled, and would need
more changes in mm/internal.h and mm/compaction.c (possibly just
replacing CMA in all "if defined CONFIG_COMPACTION || defined
CONFIG_CMA" instances with COMPACTION_CORE, but there might be more
problems, wanna try? :)
Also, I realized that COMPACTION_CORE is a wrong name, sorry about that.
What the config really provides is alloc_contig_range(), so it should be
named either CONFIG_CMA_CORE (as it provides contiguous memory
allocation, but not the related reservation and accounting), or
something like CONFIG_CONTIG_ALLOC. I would also move it from fs/Kconfig
to mm/Kconfig.
Thanks!
^ permalink raw reply
* Re: [PATCH 01/11] powerpc: remove dead ifdefs in <asm/checksum.h>
From: Andreas Schwab @ 2019-02-14 8:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, linux-kbuild, linux-xtensa, linux-kernel,
Masahiro Yamada, linux-riscv, linuxppc-dev
In-Reply-To: <20190213174005.28785-2-hch@lst.de>
On Feb 13 2019, Christoph Hellwig <hch@lst.de> wrote:
> __KERNEL__ is never not defined for non-uapi headers, and GENERIC_CSUM
"... never not ... non-..." That's a bit too negative, I think. :-)
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v3 2/2] locking/rwsem: Optimize down_read_trylock()
From: Peter Zijlstra @ 2019-02-14 10:33 UTC (permalink / raw)
To: Waiman Long
Cc: linux-arch, linux-xtensa, Davidlohr Bueso, linux-ia64, Tim Chen,
Arnd Bergmann, linux-sh, linux-hexagon, x86, Will Deacon,
linux-kernel, Linus Torvalds, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-alpha, sparclinux, Thomas Gleixner,
linuxppc-dev, Andrew Morton, linux-arm-kernel
In-Reply-To: <1550089932-6888-3-git-send-email-longman@redhat.com>
On Wed, Feb 13, 2019 at 03:32:12PM -0500, Waiman Long wrote:
> Modify __down_read_trylock() to optimize for an unlocked rwsem and make
> it generate slightly better code.
>
> Before this patch, down_read_trylock:
>
> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> 0x0000000000000005 <+5>: jmp 0x18 <down_read_trylock+24>
> 0x0000000000000007 <+7>: lea 0x1(%rdx),%rcx
> 0x000000000000000b <+11>: mov %rdx,%rax
> 0x000000000000000e <+14>: lock cmpxchg %rcx,(%rdi)
> 0x0000000000000013 <+19>: cmp %rax,%rdx
> 0x0000000000000016 <+22>: je 0x23 <down_read_trylock+35>
> 0x0000000000000018 <+24>: mov (%rdi),%rdx
> 0x000000000000001b <+27>: test %rdx,%rdx
> 0x000000000000001e <+30>: jns 0x7 <down_read_trylock+7>
> 0x0000000000000020 <+32>: xor %eax,%eax
> 0x0000000000000022 <+34>: retq
> 0x0000000000000023 <+35>: mov %gs:0x0,%rax
> 0x000000000000002c <+44>: or $0x3,%rax
> 0x0000000000000030 <+48>: mov %rax,0x20(%rdi)
> 0x0000000000000034 <+52>: mov $0x1,%eax
> 0x0000000000000039 <+57>: retq
>
> After patch, down_read_trylock:
>
> 0x0000000000000000 <+0>: callq 0x5 <down_read_trylock+5>
> 0x0000000000000005 <+5>: xor %eax,%eax
> 0x0000000000000007 <+7>: lea 0x1(%rax),%rdx
> 0x000000000000000b <+11>: lock cmpxchg %rdx,(%rdi)
> 0x0000000000000010 <+16>: jne 0x29 <down_read_trylock+41>
> 0x0000000000000012 <+18>: mov %gs:0x0,%rax
> 0x000000000000001b <+27>: or $0x3,%rax
> 0x000000000000001f <+31>: mov %rax,0x20(%rdi)
> 0x0000000000000023 <+35>: mov $0x1,%eax
> 0x0000000000000028 <+40>: retq
> 0x0000000000000029 <+41>: test %rax,%rax
> 0x000000000000002c <+44>: jns 0x7 <down_read_trylock+7>
> 0x000000000000002e <+46>: xor %eax,%eax
> 0x0000000000000030 <+48>: retq
>
> By using a rwsem microbenchmark, the down_read_trylock() rate (with a
> load of 10 to lengthen the lock critical section) on a x86-64 system
> before and after the patch were:
>
> Before Patch After Patch
> # of Threads rlock rlock
> ------------ ----- -----
> 1 14,496 14,716
> 2 8,644 8,453
> 4 6,799 6,983
> 8 5,664 7,190
>
> On a ARM64 system, the performance results were:
>
> Before Patch After Patch
> # of Threads rlock rlock
> ------------ ----- -----
> 1 23,676 24,488
> 2 7,697 9,502
> 4 4,945 3,440
> 8 2,641 1,603
Urgh, yes LL/SC is the obvious exception that can actually do better
here :/
Will, what say you?
^ permalink raw reply
* Re: [PATCH v4 0/3] locking/rwsem: Rwsem rearchitecture part 0
From: Peter Zijlstra @ 2019-02-14 10:37 UTC (permalink / raw)
To: Waiman Long
Cc: linux-ia64, linux-sh, Will Deacon, linux-mips, H. Peter Anvin,
sparclinux, linux-riscv, linux-arch, linux-s390, Davidlohr Bueso,
linux-c6x-dev, linux-hexagon, x86, Ingo Molnar, uclinux-h8-devel,
linux-xtensa, Arnd Bergmann, linux-um, linux-m68k, openrisc,
Borislav Petkov, Thomas Gleixner, linux-arm-kernel, Tim Chen,
linux-parisc, Linus Torvalds, linux-kernel, linux-alpha,
nios2-dev, Andrew Morton, linuxppc-dev
In-Reply-To: <1550095217-12047-1-git-send-email-longman@redhat.com>
On Wed, Feb 13, 2019 at 05:00:14PM -0500, Waiman Long wrote:
> v4:
> - Remove rwsem-spinlock.c and make all archs use rwsem-xadd.c.
>
> v3:
> - Optimize __down_read_trylock() for the uncontended case as suggested
> by Linus.
>
> v2:
> - Add patch 2 to optimize __down_read_trylock() as suggested by PeterZ.
> - Update performance test data in patch 1.
>
> The goal of this patchset is to remove the architecture specific files
> for rwsem-xadd to make it easer to add enhancements in the later rwsem
> patches. It also removes the legacy rwsem-spinlock.c file and make all
> the architectures use one single implementation of rwsem - rwsem-xadd.c.
>
> Waiman Long (3):
> locking/rwsem: Remove arch specific rwsem files
> locking/rwsem: Remove rwsem-spinlock.c & use rwsem-xadd.c for all
> archs
> locking/rwsem: Optimize down_read_trylock()
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
with the caveat that I'm happy to exchange patch 3 back to my earlier
suggestion in case Will expesses concerns wrt the ARM64 performance of
Linus' suggestion.
^ permalink raw reply
* Re: [PATCH v2] hugetlb: allow to free gigantic pages regardless of the configuration
From: Alexandre Ghiti @ 2019-02-14 10:49 UTC (permalink / raw)
To: Dave Hansen, Vlastimil Babka, Catalin Marinas, Will Deacon,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, x86, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Alexander Viro, Mike Kravetz,
linux-arm-kernel, linux-kernel, linuxppc-dev, linux-s390,
linux-fsdevel, linux-mm
In-Reply-To: <d367b5c7-eb05-6d0b-f9bf-5b3fc3f392a9@intel.com>
On 02/13/2019 08:30 PM, Dave Hansen wrote:
>> -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA)
>> +#ifdef CONFIG_COMPACTION_CORE
>> static __init int gigantic_pages_init(void)
>> {
>> /* With compaction or CMA we can allocate gigantic pages at runtime */
>> diff --git a/fs/Kconfig b/fs/Kconfig
>> index ac474a61be37..8fecd3ea5563 100644
>> --- a/fs/Kconfig
>> +++ b/fs/Kconfig
>> @@ -207,8 +207,9 @@ config HUGETLB_PAGE
>> config MEMFD_CREATE
>> def_bool TMPFS || HUGETLBFS
>>
>> -config ARCH_HAS_GIGANTIC_PAGE
>> +config COMPACTION_CORE
>> bool
>> + default y if (MEMORY_ISOLATION && MIGRATION) || CMA
> This takes a hard dependency (#if) and turns it into a Kconfig *default*
> that can be overridden. That seems like trouble.
>
> Shouldn't it be:
>
> config COMPACTION_CORE
> def_bool y
> depends on (MEMORY_ISOLATION && MIGRATION) || CMA
>
> ?
Thanks for that,
Alex
^ permalink raw reply
* Re: [PATCH 03/11] kernel/locks: consolidate RWSEM_GENERIC_* options
From: Geert Uytterhoeven @ 2019-02-14 10:52 UTC (permalink / raw)
To: Christoph Hellwig, Waiman Long
Cc: Linux-Arch, linux-xtensa, linux-s390, linux-kbuild,
Linux Kernel Mailing List, Masahiro Yamada, linux-riscv,
linuxppc-dev
In-Reply-To: <20190213174005.28785-4-hch@lst.de>
On Thu, Feb 14, 2019 at 12:08 AM Christoph Hellwig <hch@lst.de> wrote:
> Introduce one central definition of RWSEM_XCHGADD_ALGORITHM and
> RWSEM_GENERIC_SPINLOCK in kernel/Kconfig.locks and let architectures
> select RWSEM_XCHGADD_ALGORITHM if they want it, otherwise default to
> the spinlock version.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Note that this conflicts with "[PATCH v4 2/3] locking/rwsem: Remove
rwsem-spinlock.c & use rwsem-xadd.c for all archs"
https://lore.kernel.org/lkml/1550095217-12047-3-git-send-email-longman@redhat.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ 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