* [PATCH 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Mike Kravetz @ 2020-03-18 22:06 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller, Mike Kravetz
In-Reply-To: <20200318220634.32100-1-mike.kravetz@oracle.com>
Now that architectures provide arch_hugetlb_valid_size(), parsing
of "hugepagesz=" can be done in architecture independent code.
Create a single routine to handle hugepagesz= parsing and remove
all arch specific routines. We can also remove the interface
hugetlb_bad_size() as this is no longer used outside arch independent
code.
This also provides consistent behavior of hugetlbfs command line
options. The hugepagesz= option should only be specified once for
a specific size, but some architectures allow multiple instances.
This appears to be more of an oversight when code was added by some
architectures to set up ALL huge pages sizes.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
arch/arm64/mm/hugetlbpage.c | 15 ---------------
arch/powerpc/mm/hugetlbpage.c | 15 ---------------
arch/riscv/mm/hugetlbpage.c | 16 ----------------
arch/s390/mm/hugetlbpage.c | 18 ------------------
arch/sparc/mm/init_64.c | 22 ----------------------
arch/x86/mm/hugetlbpage.c | 16 ----------------
include/linux/hugetlb.h | 1 -
mm/hugetlb.c | 24 ++++++++++++++++++------
8 files changed, 18 insertions(+), 109 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index da30127086d0..4aa9534a45d7 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -476,18 +476,3 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long ps = memparse(opt, &opt);
-
- if arch_hugetlb_valid_size(ps)) {
- add_huge_page_size(ps);
- return 1;
- }
-
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %llu K\n", ps >> 10);
- return 0;
-}
-__setup("hugepagesz=", setup_hugepagesz);
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index b78f660252f3..166960ba1236 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -589,21 +589,6 @@ static int __init add_huge_page_size(unsigned long long size)
return 0;
}
-static int __init hugepage_setup_sz(char *str)
-{
- unsigned long long size;
-
- size = memparse(str, &str);
-
- if (add_huge_page_size(size) != 0) {
- hugetlb_bad_size();
- pr_err("Invalid huge page size specified(%llu)\n", size);
- }
-
- return 1;
-}
-__setup("hugepagesz=", hugepage_setup_sz);
-
static int __init hugetlbpage_init(void)
{
bool configured = false;
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index f1990882f16c..bdf89d7eb714 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -22,22 +22,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
- return 1;
- }
-
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
- return 0;
-
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
#ifdef CONFIG_CONTIG_ALLOC
static __init int gigantic_pages_init(void)
{
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index d92e8c5c3e71..b809762f206e 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -261,24 +261,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long size;
- char *string = opt;
-
- size = memparse(opt, &opt);
- if (arch_hugetlb_valid_size(size)) {
- hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- pr_err("hugepagesz= specifies an unsupported page size %s\n",
- string);
- return 0;
- }
- return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags)
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 4cc248817b19..5c29203fd460 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -398,28 +398,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return true;
}
-
-static int __init setup_hugepagesz(char *string)
-{
- unsigned long long hugepage_size;
- int rc = 0;
-
- hugepage_size = memparse(string, &string);
-
- if (!arch_hugetlb_valid_size(hugepage_size)) {
- hugetlb_bad_size();
- pr_err("hugepagesz=%llu not supported by MMU.\n",
- hugepage_size);
- goto out;
- }
-
- add_huge_page_size(hugepage_size);
- rc = 1;
-
-out:
- return rc;
-}
-__setup("hugepagesz=", setup_hugepagesz);
#endif /* CONFIG_HUGETLB_PAGE */
void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 51e6208fdeec..dd3ed09f6c23 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -191,22 +191,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n",
- ps >> 20);
- return 0;
- }
- return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
#ifdef CONFIG_CONTIG_ALLOC
static __init int gigantic_pages_init(void)
{
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 33343eb980d0..47244853ceb4 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -504,7 +504,6 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
int __init __alloc_bootmem_huge_page(struct hstate *h);
int __init alloc_bootmem_huge_page(struct hstate *h);
-void __init hugetlb_bad_size(void);
void __init hugetlb_add_hstate(unsigned order);
struct hstate *size_to_hstate(unsigned long size);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2f99359b93af..cd4ec07080fb 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void)
}
subsys_initcall(hugetlb_init);
-/* Should be called on processing a hugepagesz=... option */
-void __init hugetlb_bad_size(void)
-{
- parsed_valid_hugepagesz = false;
-}
-
void __init hugetlb_add_hstate(unsigned int order)
{
struct hstate *h;
@@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s)
}
__setup("hugepages=", hugetlb_nrpages_setup);
+static int __init hugepagesz_setup(char *s)
+{
+ unsigned long long size;
+ char *saved_s = s;
+
+ size = memparse(s, &s);
+
+ if (!arch_hugetlb_valid_size(size)) {
+ parsed_valid_hugepagesz = false;
+ pr_err("HugeTLB: unsupported hugepagesz %s\n", saved_s);
+ return 0;
+ }
+
+ hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+ return 1;
+}
+__setup("hugepagesz=", hugepagesz_setup);
+
static int __init default_hugepagesz_setup(char *s)
{
unsigned long long size;
--
2.24.1
^ permalink raw reply related
* [PATCH 3/4] hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
From: Mike Kravetz @ 2020-03-18 22:06 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller, Mike Kravetz
In-Reply-To: <20200318220634.32100-1-mike.kravetz@oracle.com>
The routine hugetlb_add_hstate prints a warning if the hstate already
exists. This was originally done as part of kernel command line
parsing. If 'hugepagesz=' was specified more than once, the warning
pr_warn("hugepagesz= specified twice, ignoring\n");
would be printed.
Some architectures want to enable all huge page sizes. They would
call hugetlb_add_hstate for all supported sizes. However, this was
done after command line processing and as a result hstates could have
already been created for some sizes. To make sure no warning were
printed, there would often be code like:
if (!size_to_hstate(size)
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT)
The only time we want to print the warning is as the result of command
line processing. So, remove the warning from hugetlb_add_hstate and
add it to the single arch independent routine processing "hugepagesz=".
After this, calls to size_to_hstate() in arch specific code can be
removed and hugetlb_add_hstate can be called without worrying about
warning messages.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
arch/arm64/mm/hugetlbpage.c | 16 ++++------------
arch/powerpc/mm/hugetlbpage.c | 3 +--
arch/riscv/mm/hugetlbpage.c | 2 +-
arch/sparc/mm/init_64.c | 19 ++++---------------
arch/x86/mm/hugetlbpage.c | 2 +-
mm/hugetlb.c | 10 +++++++---
6 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index 4aa9534a45d7..050809e6f0a9 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -441,22 +441,14 @@ void huge_ptep_clear_flush(struct vm_area_struct *vma,
clear_flush(vma->vm_mm, addr, ptep, pgsize, ncontig);
}
-static void __init add_huge_page_size(unsigned long size)
-{
- if (size_to_hstate(size))
- return;
-
- hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
-}
-
static int __init hugetlbpage_init(void)
{
#ifdef CONFIG_ARM64_4K_PAGES
- add_huge_page_size(PUD_SIZE);
+ hugetlb_add_hstate(ilog2(PUD_SIZE) - PAGE_SHIFT);
#endif
- add_huge_page_size(CONT_PMD_SIZE);
- add_huge_page_size(PMD_SIZE);
- add_huge_page_size(CONT_PTE_SIZE);
+ hugetlb_add_hstate(ilog2(CONT_PMD_SIZE) - PAGE_SHIFT);
+ hugetlb_add_hstate(ilog2(PMD_SIZE) - PAGE_SHIFT);
+ hugetlb_add_hstate(ilog2(CONT_PTE_SIZE) - PAGE_SHIFT);
return 0;
}
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 166960ba1236..f46464ba6fb4 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -584,8 +584,7 @@ static int __init add_huge_page_size(unsigned long long size)
if (!arch_hugetlb_valid_size(size))
return -EINVAL;
- if (!size_to_hstate(size))
- hugetlb_add_hstate(shift - PAGE_SHIFT);
+ hugetlb_add_hstate(shift - PAGE_SHIFT);
return 0;
}
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index bdf89d7eb714..beaa91941db8 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -26,7 +26,7 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
static __init int gigantic_pages_init(void)
{
/* With CONTIG_ALLOC, we can allocate gigantic pages at runtime */
- if (IS_ENABLED(CONFIG_64BIT) && !size_to_hstate(1UL << PUD_SHIFT))
+ if (IS_ENABLED(CONFIG_64BIT))
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
return 0;
}
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 5c29203fd460..8f619edc8f8c 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -325,23 +325,12 @@ static void __update_mmu_tsb_insert(struct mm_struct *mm, unsigned long tsb_inde
}
#ifdef CONFIG_HUGETLB_PAGE
-static void __init add_huge_page_size(unsigned long size)
-{
- unsigned int order;
-
- if (size_to_hstate(size))
- return;
-
- order = ilog2(size) - PAGE_SHIFT;
- hugetlb_add_hstate(order);
-}
-
static int __init hugetlbpage_init(void)
{
- add_huge_page_size(1UL << HPAGE_64K_SHIFT);
- add_huge_page_size(1UL << HPAGE_SHIFT);
- add_huge_page_size(1UL << HPAGE_256MB_SHIFT);
- add_huge_page_size(1UL << HPAGE_2GB_SHIFT);
+ hugetlb_add_hstate(HPAGE_64K_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate(HPAGE_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate(HPAGE_256MB_SHIFT - PAGE_SHIFT);
+ hugetlb_add_hstate(HPAGE_2GB_SHIFT - PAGE_SHIFT);
return 0;
}
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index dd3ed09f6c23..8a3f586e1217 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -195,7 +195,7 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
static __init int gigantic_pages_init(void)
{
/* With compaction or CMA we can allocate gigantic pages at runtime */
- if (boot_cpu_has(X86_FEATURE_GBPAGES) && !size_to_hstate(1UL << PUD_SHIFT))
+ if (boot_cpu_has(X86_FEATURE_GBPAGES))
hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
return 0;
}
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index cd4ec07080fb..cc85b4f156ca 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3116,8 +3116,7 @@ static int __init hugetlb_init(void)
}
default_hstate_size = HPAGE_SIZE;
- if (!size_to_hstate(default_hstate_size))
- hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+ hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
}
default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
if (default_hstate_max_huge_pages) {
@@ -3155,7 +3154,6 @@ void __init hugetlb_add_hstate(unsigned int order)
unsigned long i;
if (size_to_hstate(PAGE_SIZE << order)) {
- pr_warn("hugepagesz= specified twice, ignoring\n");
return;
}
BUG_ON(hugetlb_max_hstate >= HUGE_MAX_HSTATE);
@@ -3231,6 +3229,12 @@ static int __init hugepagesz_setup(char *s)
return 0;
}
+ if (size_to_hstate(size)) {
+ pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n",
+ saved_s);
+ return 0;
+ }
+
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
return 1;
}
--
2.24.1
^ permalink raw reply related
* Re: [PATCH 12/15] powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint
From: Segher Boessenkool @ 2020-03-18 21:27 UTC (permalink / raw)
To: Christophe Leroy
Cc: apopple, mikey, Ravi Bangoria, peterz, oleg, npiggin,
linux-kernel, paulus, jolsa, fweisbec, naveen.n.rao, linuxppc-dev,
mingo
In-Reply-To: <a7829a23-9b4d-0248-415e-85409f17dd77@c-s.fr>
On Wed, Mar 18, 2020 at 12:44:52PM +0100, Christophe Leroy wrote:
> Le 18/03/2020 à 12:35, Michael Ellerman a écrit :
> >Christophe Leroy <christophe.leroy@c-s.fr> writes:
> >>Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
> >>>Currently we assume that we have only one watchpoint supported by hw.
> >>>Get rid of that assumption and use dynamic loop instead. This should
> >>>make supporting more watchpoints very easy.
> >>
> >>I think using 'we' is to be avoided in commit message.
> >
> >Hmm, is it?
> >
> >I use 'we' all the time. Which doesn't mean it's correct, but I think it
> >reads OK.
> >
> >cheers
>
> From
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html :
>
> Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
> instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy
> to do frotz”, as if you are giving orders to the codebase to change its
> behaviour.
That is what is there already? "Get rid of ...".
You cannot describe the current situation with an imperative.
Segher
^ permalink raw reply
* Re: [patch V2 02/15] pci/switchtec: Replace completion wait queue usage for poll
From: Bjorn Helgaas @ 2020-03-18 21:26 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Arnd Bergmann,
Logan Gunthorpe, Paul E . McKenney, linuxppc-dev, Steven Rostedt,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204407.607241357@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:04PM +0100, Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> The poll callback is using the completion wait queue and sticks it into
> poll_wait() to wake up pollers after a command has completed.
>
> This works to some extent, but cannot provide EPOLLEXCLUSIVE support
> because the waker side uses complete_all() which unconditionally wakes up
> all waiters. complete_all() is required because completions internally use
> exclusive wait and complete() only wakes up one waiter by default.
>
> This mixes conceptually different mechanisms and relies on internal
> implementation details of completions, which in turn puts contraints on
> changing the internal implementation of completions.
>
> Replace it with a regular wait queue and store the state in struct
> switchtec_user.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
But please tweak the subject so it matches the other:
- pci/switchtec: Replace completion wait queue usage for poll
+ PCI/switchtec: Replace completion wait queue usage for poll
> ---
> V2: Reworded changelog.
> ---
> drivers/pci/switch/switchtec.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -52,10 +52,11 @@ struct switchtec_user {
>
> enum mrpc_state state;
>
> - struct completion comp;
> + wait_queue_head_t cmd_comp;
> struct kref kref;
> struct list_head list;
>
> + bool cmd_done;
> u32 cmd;
> u32 status;
> u32 return_code;
> @@ -77,7 +78,7 @@ static struct switchtec_user *stuser_cre
> stuser->stdev = stdev;
> kref_init(&stuser->kref);
> INIT_LIST_HEAD(&stuser->list);
> - init_completion(&stuser->comp);
> + init_waitqueue_head(&stuser->cmd_comp);
> stuser->event_cnt = atomic_read(&stdev->event_cnt);
>
> dev_dbg(&stdev->dev, "%s: %p\n", __func__, stuser);
> @@ -175,7 +176,7 @@ static int mrpc_queue_cmd(struct switcht
> kref_get(&stuser->kref);
> stuser->read_len = sizeof(stuser->data);
> stuser_set_state(stuser, MRPC_QUEUED);
> - reinit_completion(&stuser->comp);
> + stuser->cmd_done = false;
> list_add_tail(&stuser->list, &stdev->mrpc_queue);
>
> mrpc_cmd_submit(stdev);
> @@ -222,7 +223,8 @@ static void mrpc_complete_cmd(struct swi
> memcpy_fromio(stuser->data, &stdev->mmio_mrpc->output_data,
> stuser->read_len);
> out:
> - complete_all(&stuser->comp);
> + stuser->cmd_done = true;
> + wake_up_interruptible(&stuser->cmd_comp);
> list_del_init(&stuser->list);
> stuser_put(stuser);
> stdev->mrpc_busy = 0;
> @@ -529,10 +531,11 @@ static ssize_t switchtec_dev_read(struct
> mutex_unlock(&stdev->mrpc_mutex);
>
> if (filp->f_flags & O_NONBLOCK) {
> - if (!try_wait_for_completion(&stuser->comp))
> + if (!stuser->cmd_done)
> return -EAGAIN;
> } else {
> - rc = wait_for_completion_interruptible(&stuser->comp);
> + rc = wait_event_interruptible(stuser->cmd_comp,
> + stuser->cmd_done);
> if (rc < 0)
> return rc;
> }
> @@ -580,7 +583,7 @@ static __poll_t switchtec_dev_poll(struc
> struct switchtec_dev *stdev = stuser->stdev;
> __poll_t ret = 0;
>
> - poll_wait(filp, &stuser->comp.wait, wait);
> + poll_wait(filp, &stuser->cmd_comp, wait);
> poll_wait(filp, &stdev->event_wq, wait);
>
> if (lock_mutex_and_test_alive(stdev))
> @@ -588,7 +591,7 @@ static __poll_t switchtec_dev_poll(struc
>
> mutex_unlock(&stdev->mrpc_mutex);
>
> - if (try_wait_for_completion(&stuser->comp))
> + if (stuser->cmd_done)
> ret |= EPOLLIN | EPOLLRDNORM;
>
> if (stuser->event_cnt != atomic_read(&stdev->event_cnt))
> @@ -1272,7 +1275,8 @@ static void stdev_kill(struct switchtec_
>
> /* Wake up and kill any users waiting on an MRPC request */
> list_for_each_entry_safe(stuser, tmpuser, &stdev->mrpc_queue, list) {
> - complete_all(&stuser->comp);
> + stuser->cmd_done = true;
> + wake_up_interruptible(&stuser->cmd_comp);
> list_del_init(&stuser->list);
> stuser_put(stuser);
> }
>
^ permalink raw reply
* Re: [patch V2 01/15] PCI/switchtec: Fix init_completion race condition with poll_wait()
From: Bjorn Helgaas @ 2020-03-18 21:25 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Arnd Bergmann,
Logan Gunthorpe, Paul E . McKenney, linuxppc-dev, Steven Rostedt,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204407.497942274@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:03PM +0100, Thomas Gleixner wrote:
> From: Logan Gunthorpe <logang@deltatee.com>
>
> The call to init_completion() in mrpc_queue_cmd() can theoretically
> race with the call to poll_wait() in switchtec_dev_poll().
>
> poll() write()
> switchtec_dev_poll() switchtec_dev_write()
> poll_wait(&s->comp.wait); mrpc_queue_cmd()
> init_completion(&s->comp)
> init_waitqueue_head(&s->comp.wait)
>
> To my knowledge, no one has hit this bug.
>
> Fix this by using reinit_completion() instead of init_completion() in
> mrpc_queue_cmd().
>
> Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Link: https://lkml.kernel.org/r/20200313183608.2646-1-logang@deltatee.com
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Not because I understand and have reviewed this, but because I trust
you to do the right thing and it belongs with the rest of the series.
> ---
> drivers/pci/switch/switchtec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index a823b4b8ef8a..81dc7ac01381 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -175,7 +175,7 @@ static int mrpc_queue_cmd(struct switchtec_user *stuser)
> kref_get(&stuser->kref);
> stuser->read_len = sizeof(stuser->data);
> stuser_set_state(stuser, MRPC_QUEUED);
> - init_completion(&stuser->comp);
> + reinit_completion(&stuser->comp);
> list_add_tail(&stuser->list, &stdev->mrpc_queue);
>
> mrpc_cmd_submit(stdev);
> --
> 2.20.1
>
>
^ permalink raw reply
* [PATCH AUTOSEL 4.4 07/12] dt-bindings: net: FMan erratum A050385
From: Sasha Levin @ 2020-03-18 20:56 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, linuxppc-dev, David S . Miller,
Madalin Bucur
In-Reply-To: <20200318205648.17937-1-sashal@kernel.org>
From: Madalin Bucur <madalin.bucur@nxp.com>
[ Upstream commit 26d5bb9e4c4b541c475751e015072eb2cbf70d15 ]
FMAN DMA read or writes under heavy traffic load may cause FMAN
internal resource leak; thus stopping further packet processing.
The FMAN internal queue can overflow when FMAN splits single
read or write transactions into multiple smaller transactions
such that more than 17 AXI transactions are in flight from FMAN
to interconnect. When the FMAN internal queue overflows, it can
stall further packet processing. The issue can occur with any one
of the following three conditions:
1. FMAN AXI transaction crosses 4K address boundary (Errata
A010022)
2. FMAN DMA address for an AXI transaction is not 16 byte
aligned, i.e. the last 4 bits of an address are non-zero
3. Scatter Gather (SG) frames have more than one SG buffer in
the SG list and any one of the buffers, except the last
buffer in the SG list has data size that is not a multiple
of 16 bytes, i.e., other than 16, 32, 48, 64, etc.
With any one of the above three conditions present, there is
likelihood of stalled FMAN packet processing, especially under
stress with multiple ports injecting line-rate traffic.
To avoid situations that stall FMAN packet processing, all of the
above three conditions must be avoided; therefore, configure the
system with the following rules:
1. Frame buffers must not span a 4KB address boundary, unless
the frame start address is 256 byte aligned
2. All FMAN DMA start addresses (for example, BMAN buffer
address, FD[address] + FD[offset]) are 16B aligned
3. SG table and buffer addresses are 16B aligned and the size
of SG buffers are multiple of 16 bytes, except for the last
SG buffer that can be of any size.
Additional workaround notes:
- Address alignment of 64 bytes is recommended for maximally
efficient system bus transactions (although 16 byte alignment is
sufficient to avoid the stall condition)
- To support frame sizes that are larger than 4K bytes, there are
two options:
1. Large single buffer frames that span a 4KB page boundary can
be converted into SG frames to avoid transaction splits at
the 4KB boundary,
2. Align the large single buffer to 256B address boundaries,
ensure that the frame address plus offset is 256B aligned.
- If software generated SG frames have buffers that are unaligned
and with random non-multiple of 16 byte lengths, before
transmitting such frames via FMAN, frames will need to be copied
into a new single buffer or multiple buffer SG frame that is
compliant with the three rules listed above.
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Documentation/devicetree/bindings/powerpc/fsl/fman.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
index 1fc5328c0651b..23708f3f4e58a 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
@@ -110,6 +110,13 @@ PROPERTIES
Usage: required
Definition: See soc/fsl/qman.txt and soc/fsl/bman.txt
+- fsl,erratum-a050385
+ Usage: optional
+ Value type: boolean
+ Definition: A boolean property. Indicates the presence of the
+ erratum A050385 which indicates that DMA transactions that are
+ split can result in a FMan lock.
+
=============================================================================
FMan MURAM Node
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 09/15] dt-bindings: net: FMan erratum A050385
From: Sasha Levin @ 2020-03-18 20:56 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, devicetree, linuxppc-dev, David S . Miller,
Madalin Bucur
In-Reply-To: <20200318205629.17750-1-sashal@kernel.org>
From: Madalin Bucur <madalin.bucur@nxp.com>
[ Upstream commit 26d5bb9e4c4b541c475751e015072eb2cbf70d15 ]
FMAN DMA read or writes under heavy traffic load may cause FMAN
internal resource leak; thus stopping further packet processing.
The FMAN internal queue can overflow when FMAN splits single
read or write transactions into multiple smaller transactions
such that more than 17 AXI transactions are in flight from FMAN
to interconnect. When the FMAN internal queue overflows, it can
stall further packet processing. The issue can occur with any one
of the following three conditions:
1. FMAN AXI transaction crosses 4K address boundary (Errata
A010022)
2. FMAN DMA address for an AXI transaction is not 16 byte
aligned, i.e. the last 4 bits of an address are non-zero
3. Scatter Gather (SG) frames have more than one SG buffer in
the SG list and any one of the buffers, except the last
buffer in the SG list has data size that is not a multiple
of 16 bytes, i.e., other than 16, 32, 48, 64, etc.
With any one of the above three conditions present, there is
likelihood of stalled FMAN packet processing, especially under
stress with multiple ports injecting line-rate traffic.
To avoid situations that stall FMAN packet processing, all of the
above three conditions must be avoided; therefore, configure the
system with the following rules:
1. Frame buffers must not span a 4KB address boundary, unless
the frame start address is 256 byte aligned
2. All FMAN DMA start addresses (for example, BMAN buffer
address, FD[address] + FD[offset]) are 16B aligned
3. SG table and buffer addresses are 16B aligned and the size
of SG buffers are multiple of 16 bytes, except for the last
SG buffer that can be of any size.
Additional workaround notes:
- Address alignment of 64 bytes is recommended for maximally
efficient system bus transactions (although 16 byte alignment is
sufficient to avoid the stall condition)
- To support frame sizes that are larger than 4K bytes, there are
two options:
1. Large single buffer frames that span a 4KB page boundary can
be converted into SG frames to avoid transaction splits at
the 4KB boundary,
2. Align the large single buffer to 256B address boundaries,
ensure that the frame address plus offset is 256B aligned.
- If software generated SG frames have buffers that are unaligned
and with random non-multiple of 16 byte lengths, before
transmitting such frames via FMAN, frames will need to be copied
into a new single buffer or multiple buffer SG frame that is
compliant with the three rules listed above.
Signed-off-by: Madalin Bucur <madalin.bucur@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Documentation/devicetree/bindings/powerpc/fsl/fman.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
index df873d1f3b7c5..2aaae210317bb 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
@@ -110,6 +110,13 @@ PROPERTIES
Usage: required
Definition: See soc/fsl/qman.txt and soc/fsl/bman.txt
+- fsl,erratum-a050385
+ Usage: optional
+ Value type: boolean
+ Definition: A boolean property. Indicates the presence of the
+ erratum A050385 which indicates that DMA transactions that are
+ split can result in a FMan lock.
+
=============================================================================
FMan MURAM Node
--
2.20.1
^ permalink raw reply related
* [patch V2 11/15] completion: Use simple wait queues
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Paul E . McKenney, Logan Gunthorpe,
Arnd Bergmann, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Thomas Gleixner <tglx@linutronix.de>
completion uses a wait_queue_head_t to enqueue waiters.
wait_queue_head_t contains a spinlock_t to protect the list of waiters
which excludes it from being used in truly atomic context on a PREEMPT_RT
enabled kernel.
The spinlock in the wait queue head cannot be replaced by a raw_spinlock
because:
- wait queues can have custom wakeup callbacks, which acquire other
spinlock_t locks and have potentially long execution times
- wake_up() walks an unbounded number of list entries during the wake up
and may wake an unbounded number of waiters.
For simplicity and performance reasons complete() should be usable on
PREEMPT_RT enabled kernels.
completions do not use custom wakeup callbacks and are usually single
waiter, except for a few corner cases.
Replace the wait queue in the completion with a simple wait queue (swait),
which uses a raw_spinlock_t for protecting the waiter list and therefore is
safe to use inside truly atomic regions on PREEMPT_RT.
There is no semantical or functional change:
- completions use the exclusive wait mode which is what swait provides
- complete() wakes one exclusive waiter
- complete_all() wakes all waiters while holding the lock which protects
the wait queue against newly incoming waiters. The conversion to swait
preserves this behaviour.
complete_all() might cause unbound latencies with a large number of waiters
being woken at once, but most complete_all() usage sites are either in
testing or initialization code or have only a really small number of
concurrent waiters which for now does not cause a latency problem. Keep it
simple for now.
The fixup of the warning check in the USB gadget driver is just a straight
forward conversion of the lockless waiter check from one waitqueue type to
the other.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
---
V2: Split out the orinoco and usb gadget parts and amended change log
---
drivers/usb/gadget/function/f_fs.c | 2 +-
include/linux/completion.h | 8 ++++----
kernel/sched/completion.c | 36 +++++++++++++++++++-----------------
3 files changed, 24 insertions(+), 22 deletions(-)
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1703,7 +1703,7 @@ static void ffs_data_put(struct ffs_data
pr_info("%s(): freeing\n", __func__);
ffs_data_clear(ffs);
BUG_ON(waitqueue_active(&ffs->ev.waitq) ||
- waitqueue_active(&ffs->ep0req_completion.wait) ||
+ swait_active(&ffs->ep0req_completion.wait) ||
waitqueue_active(&ffs->wait));
destroy_workqueue(ffs->io_completion_wq);
kfree(ffs->dev_name);
--- a/include/linux/completion.h
+++ b/include/linux/completion.h
@@ -9,7 +9,7 @@
* See kernel/sched/completion.c for details.
*/
-#include <linux/wait.h>
+#include <linux/swait.h>
/*
* struct completion - structure used to maintain state for a "completion"
@@ -25,7 +25,7 @@
*/
struct completion {
unsigned int done;
- wait_queue_head_t wait;
+ struct swait_queue_head wait;
};
#define init_completion_map(x, m) __init_completion(x)
@@ -34,7 +34,7 @@ static inline void complete_acquire(stru
static inline void complete_release(struct completion *x) {}
#define COMPLETION_INITIALIZER(work) \
- { 0, __WAIT_QUEUE_HEAD_INITIALIZER((work).wait) }
+ { 0, __SWAIT_QUEUE_HEAD_INITIALIZER((work).wait) }
#define COMPLETION_INITIALIZER_ONSTACK_MAP(work, map) \
(*({ init_completion_map(&(work), &(map)); &(work); }))
@@ -85,7 +85,7 @@ static inline void complete_release(stru
static inline void __init_completion(struct completion *x)
{
x->done = 0;
- init_waitqueue_head(&x->wait);
+ init_swait_queue_head(&x->wait);
}
/**
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -29,12 +29,12 @@ void complete(struct completion *x)
{
unsigned long flags;
- spin_lock_irqsave(&x->wait.lock, flags);
+ raw_spin_lock_irqsave(&x->wait.lock, flags);
if (x->done != UINT_MAX)
x->done++;
- __wake_up_locked(&x->wait, TASK_NORMAL, 1);
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ swake_up_locked(&x->wait);
+ raw_spin_unlock_irqrestore(&x->wait.lock, flags);
}
EXPORT_SYMBOL(complete);
@@ -58,10 +58,12 @@ void complete_all(struct completion *x)
{
unsigned long flags;
- spin_lock_irqsave(&x->wait.lock, flags);
+ WARN_ON(irqs_disabled());
+
+ raw_spin_lock_irqsave(&x->wait.lock, flags);
x->done = UINT_MAX;
- __wake_up_locked(&x->wait, TASK_NORMAL, 0);
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ swake_up_all_locked(&x->wait);
+ raw_spin_unlock_irqrestore(&x->wait.lock, flags);
}
EXPORT_SYMBOL(complete_all);
@@ -70,20 +72,20 @@ do_wait_for_common(struct completion *x,
long (*action)(long), long timeout, int state)
{
if (!x->done) {
- DECLARE_WAITQUEUE(wait, current);
+ DECLARE_SWAITQUEUE(wait);
- __add_wait_queue_entry_tail_exclusive(&x->wait, &wait);
do {
if (signal_pending_state(state, current)) {
timeout = -ERESTARTSYS;
break;
}
+ __prepare_to_swait(&x->wait, &wait);
__set_current_state(state);
- spin_unlock_irq(&x->wait.lock);
+ raw_spin_unlock_irq(&x->wait.lock);
timeout = action(timeout);
- spin_lock_irq(&x->wait.lock);
+ raw_spin_lock_irq(&x->wait.lock);
} while (!x->done && timeout);
- __remove_wait_queue(&x->wait, &wait);
+ __finish_swait(&x->wait, &wait);
if (!x->done)
return timeout;
}
@@ -100,9 +102,9 @@ static inline long __sched
complete_acquire(x);
- spin_lock_irq(&x->wait.lock);
+ raw_spin_lock_irq(&x->wait.lock);
timeout = do_wait_for_common(x, action, timeout, state);
- spin_unlock_irq(&x->wait.lock);
+ raw_spin_unlock_irq(&x->wait.lock);
complete_release(x);
@@ -291,12 +293,12 @@ bool try_wait_for_completion(struct comp
if (!READ_ONCE(x->done))
return false;
- spin_lock_irqsave(&x->wait.lock, flags);
+ raw_spin_lock_irqsave(&x->wait.lock, flags);
if (!x->done)
ret = false;
else if (x->done != UINT_MAX)
x->done--;
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ raw_spin_unlock_irqrestore(&x->wait.lock, flags);
return ret;
}
EXPORT_SYMBOL(try_wait_for_completion);
@@ -322,8 +324,8 @@ bool completion_done(struct completion *
* otherwise we can end up freeing the completion before complete()
* is done referencing it.
*/
- spin_lock_irqsave(&x->wait.lock, flags);
- spin_unlock_irqrestore(&x->wait.lock, flags);
+ raw_spin_lock_irqsave(&x->wait.lock, flags);
+ raw_spin_unlock_irqrestore(&x->wait.lock, flags);
return true;
}
EXPORT_SYMBOL(completion_done);
^ permalink raw reply
* [patch V2 10/15] sched/swait: Prepare usage in completions
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Thomas Gleixner <tglx@linutronix.de>
As a preparation to use simple wait queues for completions:
- Provide swake_up_all_locked() to support complete_all()
- Make __prepare_to_swait() public available
This is done to enable the usage of complete() within truly atomic contexts
on a PREEMPT_RT enabled kernel.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
V2: Add comment to swake_up_all_locked()
---
kernel/sched/sched.h | 3 +++
kernel/sched/swait.c | 15 ++++++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2492,3 +2492,6 @@ static inline bool is_per_cpu_kthread(st
return true;
}
#endif
+
+void swake_up_all_locked(struct swait_queue_head *q);
+void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -32,6 +32,19 @@ void swake_up_locked(struct swait_queue_
}
EXPORT_SYMBOL(swake_up_locked);
+/*
+ * Wake up all waiters. This is an interface which is solely exposed for
+ * completions and not for general usage.
+ *
+ * It is intentionally different from swake_up_all() to allow usage from
+ * hard interrupt context and interrupt disabled regions.
+ */
+void swake_up_all_locked(struct swait_queue_head *q)
+{
+ while (!list_empty(&q->task_list))
+ swake_up_locked(q);
+}
+
void swake_up_one(struct swait_queue_head *q)
{
unsigned long flags;
@@ -69,7 +82,7 @@ void swake_up_all(struct swait_queue_hea
}
EXPORT_SYMBOL(swake_up_all);
-static void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
{
wait->task = current;
if (list_empty(&wait->task_list))
^ permalink raw reply
* [patch V2 08/15] Documentation: Add lock ordering and nesting documentation
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Thomas Gleixner <tglx@linutronix.de>
The kernel provides a variety of locking primitives. The nesting of these
lock types and the implications of them on RT enabled kernels is nowhere
documented.
Add initial documentation.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
V2: Addressed review comments from Randy
---
Documentation/locking/index.rst | 1
Documentation/locking/locktypes.rst | 298 ++++++++++++++++++++++++++++++++++++
2 files changed, 299 insertions(+)
create mode 100644 Documentation/locking/locktypes.rst
--- a/Documentation/locking/index.rst
+++ b/Documentation/locking/index.rst
@@ -7,6 +7,7 @@ locking
.. toctree::
:maxdepth: 1
+ locktypes
lockdep-design
lockstat
locktorture
--- /dev/null
+++ b/Documentation/locking/locktypes.rst
@@ -0,0 +1,298 @@
+.. _kernel_hacking_locktypes:
+
+==========================
+Lock types and their rules
+==========================
+
+Introduction
+============
+
+The kernel provides a variety of locking primitives which can be divided
+into two categories:
+
+ - Sleeping locks
+ - Spinning locks
+
+This document describes the lock types at least at the conceptual level and
+provides rules for nesting of lock types also under the aspect of PREEMPT_RT.
+
+Lock categories
+===============
+
+Sleeping locks
+--------------
+
+Sleeping locks can only be acquired in preemptible task context.
+
+Some of the implementations allow try_lock() attempts from other contexts,
+but that has to be really evaluated carefully including the question
+whether the unlock can be done from that context safely as well.
+
+Note that some lock types change their implementation details when
+debugging is enabled, so this should be really only considered if there is
+no other option.
+
+Sleeping lock types:
+
+ - mutex
+ - rt_mutex
+ - semaphore
+ - rw_semaphore
+ - ww_mutex
+ - percpu_rw_semaphore
+
+On a PREEMPT_RT enabled kernel the following lock types are converted to
+sleeping locks:
+
+ - spinlock_t
+ - rwlock_t
+
+Spinning locks
+--------------
+
+ - raw_spinlock_t
+ - bit spinlocks
+
+On a non PREEMPT_RT enabled kernel the following lock types are spinning
+locks as well:
+
+ - spinlock_t
+ - rwlock_t
+
+Spinning locks implicitly disable preemption and the lock / unlock functions
+can have suffixes which apply further protections:
+
+ =================== ====================================================
+ _bh() Disable / enable bottom halves (soft interrupts)
+ _irq() Disable / enable interrupts
+ _irqsave/restore() Save and disable / restore interrupt disabled state
+ =================== ====================================================
+
+
+rtmutex
+=======
+
+RT-mutexes are mutexes with support for priority inheritance (PI).
+
+PI has limitations on non PREEMPT_RT enabled kernels due to preemption and
+interrupt disabled sections.
+
+On a PREEMPT_RT enabled kernel most of these sections are fully
+preemptible. This is possible because PREEMPT_RT forces most executions
+into task context, especially interrupt handlers and soft interrupts, which
+allows to substitute spinlock_t and rwlock_t with RT-mutex based
+implementations.
+
+
+raw_spinlock_t and spinlock_t
+=============================
+
+raw_spinlock_t
+--------------
+
+raw_spinlock_t is a strict spinning lock implementation regardless of the
+kernel configuration including PREEMPT_RT enabled kernels.
+
+raw_spinlock_t is to be used only in real critical core code, low level
+interrupt handling and places where protecting (hardware) state is required
+to be safe against preemption and eventually interrupts.
+
+Another reason to use raw_spinlock_t is when the critical section is tiny
+to avoid the overhead of spinlock_t on a PREEMPT_RT enabled kernel in the
+contended case.
+
+spinlock_t
+----------
+
+The semantics of spinlock_t change with the state of CONFIG_PREEMPT_RT.
+
+On a non PREEMPT_RT enabled kernel spinlock_t is mapped to raw_spinlock_t
+and has exactly the same semantics.
+
+spinlock_t and PREEMPT_RT
+-------------------------
+
+On a PREEMPT_RT enabled kernel spinlock_t is mapped to a separate
+implementation based on rt_mutex which changes the semantics:
+
+ - Preemption is not disabled
+
+ - The hard interrupt related suffixes for spin_lock / spin_unlock
+ operations (_irq, _irqsave / _irqrestore) do not affect the CPUs
+ interrupt disabled state
+
+ - The soft interrupt related suffix (_bh()) is still disabling the
+ execution of soft interrupts, but contrary to a non PREEMPT_RT enabled
+ kernel, which utilizes the preemption count, this is achieved by a per
+ CPU bottom half locking mechanism.
+
+All other semantics of spinlock_t are preserved:
+
+ - Migration of tasks which hold a spinlock_t is prevented. On a non
+ PREEMPT_RT enabled kernel this is implicit due to preemption disable.
+ PREEMPT_RT has a separate mechanism to achieve this. This ensures that
+ pointers to per CPU variables stay valid even if the task is preempted.
+
+ - Task state preservation. The task state is not affected when a lock is
+ contended and the task has to schedule out and wait for the lock to
+ become available. The lock wake up restores the task state unless there
+ was a regular (not lock related) wake up on the task. This ensures that
+ the task state rules are always correct independent of the kernel
+ configuration.
+
+rwlock_t
+========
+
+rwlock_t is a multiple readers and single writer lock mechanism.
+
+On a non PREEMPT_RT enabled kernel rwlock_t is implemented as a spinning
+lock and the suffix rules of spinlock_t apply accordingly. The
+implementation is fair and prevents writer starvation.
+
+rwlock_t and PREEMPT_RT
+-----------------------
+
+On a PREEMPT_RT enabled kernel rwlock_t is mapped to a separate
+implementation based on rt_mutex which changes the semantics:
+
+ - Same changes as for spinlock_t
+
+ - The implementation is not fair and can cause writer starvation under
+ certain circumstances. The reason for this is that a writer cannot grant
+ its priority to multiple readers. Readers which are blocked on a writer
+ fully support the priority inheritance protocol.
+
+
+PREEMPT_RT caveats
+==================
+
+spinlock_t and rwlock_t
+-----------------------
+
+The substitution of spinlock_t and rwlock_t on PREEMPT_RT enabled kernels
+with RT-mutex based implementations has a few implications.
+
+On a non PREEMPT_RT enabled kernel the following code construct is
+perfectly fine::
+
+ local_irq_disable();
+ spin_lock(&lock);
+
+and fully equivalent to::
+
+ spin_lock_irq(&lock);
+
+Same applies to rwlock_t and the _irqsave() suffix variant.
+
+On a PREEMPT_RT enabled kernel this breaks because the RT-mutex
+substitution expects a fully preemptible context.
+
+The preferred solution is to use :c:func:`spin_lock_irq()` or
+:c:func:`spin_lock_irqsave()` and their unlock counterparts.
+
+PREEMPT_RT also offers a local_lock mechanism to substitute the
+local_irq_disable/save() constructs in cases where a separation of the
+interrupt disabling and the locking is really unavoidable. This should be
+restricted to very rare cases.
+
+
+raw_spinlock_t
+--------------
+
+Locking of a raw_spinlock_t disables preemption and eventually interrupts.
+Therefore code inside the critical region has to be careful to avoid calls
+into code which takes a regular spinlock_t or rwlock_t. A prime example is
+memory allocation.
+
+On a non PREEMPT_RT enabled kernel the following code construct is
+perfectly fine code::
+
+ raw_spin_lock(&lock);
+ p = kmalloc(sizeof(*p), GFP_ATOMIC);
+
+On a PREEMPT_RT enabled kernel this breaks because the memory allocator is
+fully preemptible and therefore does not support allocations from truly
+atomic contexts.
+
+Contrary to that the following code construct is perfectly fine on
+PREEMPT_RT as spin_lock() does not disable preemption::
+
+ spin_lock(&lock);
+ p = kmalloc(sizeof(*p), GFP_ATOMIC);
+
+Most places which use GFP_ATOMIC allocations are safe on PREEMPT_RT as the
+execution is forced into thread context and the lock substitution is
+ensuring preemptibility.
+
+
+bit spinlocks
+-------------
+
+Bit spinlocks are problematic for PREEMPT_RT as they cannot be easily
+substituted by an RT-mutex based implementation for obvious reasons.
+
+The semantics of bit spinlocks are preserved on a PREEMPT_RT enabled kernel
+and the caveats vs. raw_spinlock_t apply.
+
+Some bit spinlocks are substituted by regular spinlock_t for PREEMPT_RT but
+this requires conditional (#ifdef'ed) code changes at the usage side while
+the spinlock_t substitution is simply done by the compiler and the
+conditionals are restricted to header files and core implementation of the
+locking primitives and the usage sites do not require any changes.
+
+
+Lock type nesting rules
+=======================
+
+The most basic rules are:
+
+ - Lock types of the same lock category (sleeping, spinning) can nest
+ arbitrarily as long as they respect the general lock ordering rules to
+ prevent deadlocks.
+
+ - Sleeping lock types cannot nest inside spinning lock types.
+
+ - Spinning lock types can nest inside sleeping lock types.
+
+These rules apply in general independent of CONFIG_PREEMPT_RT.
+
+As PREEMPT_RT changes the lock category of spinlock_t and rwlock_t from
+spinning to sleeping this has obviously restrictions how they can nest with
+raw_spinlock_t.
+
+This results in the following nest ordering:
+
+ 1) Sleeping locks
+ 2) spinlock_t and rwlock_t
+ 3) raw_spinlock_t and bit spinlocks
+
+Lockdep is aware of these constraints to ensure that they are respected.
+
+
+Owner semantics
+===============
+
+Most lock types in the Linux kernel have strict owner semantics, i.e. the
+context (task) which acquires a lock has to release it.
+
+There are two exceptions:
+
+ - semaphores
+ - rwsems
+
+semaphores have no strict owner semantics for historical reasons. They are
+often used for both serialization and waiting purposes. That's generally
+discouraged and should be replaced by separate serialization and wait
+mechanisms.
+
+rwsems have grown interfaces which allow non owner release for special
+purposes. This usage is problematic on PREEMPT_RT because PREEMPT_RT
+substitutes all locking primitives except semaphores with RT-mutex based
+implementations to provide priority inheritance for all lock types except
+the truly spinning ones. Priority inheritance on ownerless locks is
+obviously impossible.
+
+For now the rwsem non-owner release excludes code which utilizes it from
+being used on PREEMPT_RT enabled kernels. In same cases this can be
+mitigated by disabling portions of the code, in other cases the complete
+functionality has to be disabled until a workable solution has been found.
^ permalink raw reply
* [patch V2 03/15] usb: gadget: Use completion interface instead of open coding it
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Greg Kroah-Hartman, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Paul E . McKenney,
linuxppc-dev, Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer,
Kalle Valo, Felipe Balbi, Logan Gunthorpe, linux-usb,
linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
ep_io() uses a completion on stack and open codes the waiting with:
wait_event_interruptible (done.wait, done.done);
and
wait_event (done.wait, done.done);
This waits in non-exclusive mode for complete(), but there is no reason to
do so because the completion can only be waited for by the task itself and
complete() wakes exactly one exlusive waiter.
Replace the open coded implementation with the corresponding
wait_for_completion*() functions.
No functional change.
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
---
V2: New patch to avoid the conversion to swait interfaces later
---
drivers/usb/gadget/legacy/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -344,7 +344,7 @@ ep_io (struct ep_data *epdata, void *buf
spin_unlock_irq (&epdata->dev->lock);
if (likely (value == 0)) {
- value = wait_event_interruptible (done.wait, done.done);
+ value = wait_for_completion_interruptible(&done);
if (value != 0) {
spin_lock_irq (&epdata->dev->lock);
if (likely (epdata->ep != NULL)) {
@@ -353,7 +353,7 @@ ep_io (struct ep_data *epdata, void *buf
usb_ep_dequeue (epdata->ep, epdata->req);
spin_unlock_irq (&epdata->dev->lock);
- wait_event (done.wait, done.done);
+ wait_for_completion(&done);
if (epdata->status == -ECONNRESET)
epdata->status = -EINTR;
} else {
^ permalink raw reply
* [patch V2 05/15] acpi: Remove header dependency
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
In order to avoid future header hell, remove the inclusion of
proc_fs.h from acpi_bus.h. All it needs is a forward declaration of a
struct.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/platform/x86/dell-smo8800.c | 1 +
drivers/platform/x86/wmi.c | 1 +
drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c | 1 +
include/acpi/acpi_bus.h | 2 +-
4 files changed, 4 insertions(+), 1 deletion(-)
--- a/drivers/platform/x86/dell-smo8800.c
+++ b/drivers/platform/x86/dell-smo8800.c
@@ -16,6 +16,7 @@
#include <linux/interrupt.h>
#include <linux/miscdevice.h>
#include <linux/uaccess.h>
+#include <linux/fs.h>
struct smo8800_device {
u32 irq; /* acpi device irq */
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -29,6 +29,7 @@
#include <linux/uaccess.h>
#include <linux/uuid.h>
#include <linux/wmi.h>
+#include <linux/fs.h>
#include <uapi/linux/wmi.h>
ACPI_MODULE_NAME("wmi");
--- a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c
+++ b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c
@@ -19,6 +19,7 @@
#include <linux/acpi.h>
#include <linux/uaccess.h>
#include <linux/miscdevice.h>
+#include <linux/fs.h>
#include "acpi_thermal_rel.h"
static acpi_handle acpi_thermal_rel_handle;
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -80,7 +80,7 @@ bool acpi_dev_present(const char *hid, c
#ifdef CONFIG_ACPI
-#include <linux/proc_fs.h>
+struct proc_dir_entry;
#define ACPI_BUS_FILE_ROOT "acpi"
extern struct proc_dir_entry *acpi_root_dir;
^ permalink raw reply
* [patch V2 02/15] pci/switchtec: Replace completion wait queue usage for poll
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
The poll callback is using the completion wait queue and sticks it into
poll_wait() to wake up pollers after a command has completed.
This works to some extent, but cannot provide EPOLLEXCLUSIVE support
because the waker side uses complete_all() which unconditionally wakes up
all waiters. complete_all() is required because completions internally use
exclusive wait and complete() only wakes up one waiter by default.
This mixes conceptually different mechanisms and relies on internal
implementation details of completions, which in turn puts contraints on
changing the internal implementation of completions.
Replace it with a regular wait queue and store the state in struct
switchtec_user.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
---
V2: Reworded changelog.
---
drivers/pci/switch/switchtec.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -52,10 +52,11 @@ struct switchtec_user {
enum mrpc_state state;
- struct completion comp;
+ wait_queue_head_t cmd_comp;
struct kref kref;
struct list_head list;
+ bool cmd_done;
u32 cmd;
u32 status;
u32 return_code;
@@ -77,7 +78,7 @@ static struct switchtec_user *stuser_cre
stuser->stdev = stdev;
kref_init(&stuser->kref);
INIT_LIST_HEAD(&stuser->list);
- init_completion(&stuser->comp);
+ init_waitqueue_head(&stuser->cmd_comp);
stuser->event_cnt = atomic_read(&stdev->event_cnt);
dev_dbg(&stdev->dev, "%s: %p\n", __func__, stuser);
@@ -175,7 +176,7 @@ static int mrpc_queue_cmd(struct switcht
kref_get(&stuser->kref);
stuser->read_len = sizeof(stuser->data);
stuser_set_state(stuser, MRPC_QUEUED);
- reinit_completion(&stuser->comp);
+ stuser->cmd_done = false;
list_add_tail(&stuser->list, &stdev->mrpc_queue);
mrpc_cmd_submit(stdev);
@@ -222,7 +223,8 @@ static void mrpc_complete_cmd(struct swi
memcpy_fromio(stuser->data, &stdev->mmio_mrpc->output_data,
stuser->read_len);
out:
- complete_all(&stuser->comp);
+ stuser->cmd_done = true;
+ wake_up_interruptible(&stuser->cmd_comp);
list_del_init(&stuser->list);
stuser_put(stuser);
stdev->mrpc_busy = 0;
@@ -529,10 +531,11 @@ static ssize_t switchtec_dev_read(struct
mutex_unlock(&stdev->mrpc_mutex);
if (filp->f_flags & O_NONBLOCK) {
- if (!try_wait_for_completion(&stuser->comp))
+ if (!stuser->cmd_done)
return -EAGAIN;
} else {
- rc = wait_for_completion_interruptible(&stuser->comp);
+ rc = wait_event_interruptible(stuser->cmd_comp,
+ stuser->cmd_done);
if (rc < 0)
return rc;
}
@@ -580,7 +583,7 @@ static __poll_t switchtec_dev_poll(struc
struct switchtec_dev *stdev = stuser->stdev;
__poll_t ret = 0;
- poll_wait(filp, &stuser->comp.wait, wait);
+ poll_wait(filp, &stuser->cmd_comp, wait);
poll_wait(filp, &stdev->event_wq, wait);
if (lock_mutex_and_test_alive(stdev))
@@ -588,7 +591,7 @@ static __poll_t switchtec_dev_poll(struc
mutex_unlock(&stdev->mrpc_mutex);
- if (try_wait_for_completion(&stuser->comp))
+ if (stuser->cmd_done)
ret |= EPOLLIN | EPOLLRDNORM;
if (stuser->event_cnt != atomic_read(&stdev->event_cnt))
@@ -1272,7 +1275,8 @@ static void stdev_kill(struct switchtec_
/* Wake up and kill any users waiting on an MRPC request */
list_for_each_entry_safe(stuser, tmpuser, &stdev->mrpc_queue, list) {
- complete_all(&stuser->comp);
+ stuser->cmd_done = true;
+ wake_up_interruptible(&stuser->cmd_comp);
list_del_init(&stuser->list);
stuser_put(stuser);
}
^ permalink raw reply
* [patch V2 04/15] orinoco_usb: Use the regular completion interfaces
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Thomas Gleixner <tglx@linutronix.de>
The completion usage in this driver is interesting:
- it uses a magic complete function which according to the comment was
implemented by invoking complete() four times in a row because
complete_all() was not exported at that time.
- it uses an open coded wait/poll which checks completion:done. Only one wait
side (device removal) uses the regular wait_for_completion() interface.
The rationale behind this is to prevent that wait_for_completion() consumes
completion::done which would prevent that all waiters are woken. This is not
necessary with complete_all() as that sets completion::done to UINT_MAX which
is left unmodified by the woken waiters.
Replace the magic complete function with complete_all() and convert the
open coded wait/poll to regular completion interfaces.
This changes the wait to exclusive wait mode. But that does not make any
difference because the wakers use complete_all() which ignores the
exclusive mode.
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
V2: New patch to avoid conversion to swait functions later.
---
drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 21 ++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -365,17 +365,6 @@ static struct request_context *ezusb_all
return ctx;
}
-
-/* Hopefully the real complete_all will soon be exported, in the mean
- * while this should work. */
-static inline void ezusb_complete_all(struct completion *comp)
-{
- complete(comp);
- complete(comp);
- complete(comp);
- complete(comp);
-}
-
static void ezusb_ctx_complete(struct request_context *ctx)
{
struct ezusb_priv *upriv = ctx->upriv;
@@ -409,7 +398,7 @@ static void ezusb_ctx_complete(struct re
netif_wake_queue(dev);
}
- ezusb_complete_all(&ctx->done);
+ complete_all(&ctx->done);
ezusb_request_context_put(ctx);
break;
@@ -419,7 +408,7 @@ static void ezusb_ctx_complete(struct re
/* This is normal, as all request contexts get flushed
* when the device is disconnected */
err("Called, CTX not terminating, but device gone");
- ezusb_complete_all(&ctx->done);
+ complete_all(&ctx->done);
ezusb_request_context_put(ctx);
break;
}
@@ -690,11 +679,11 @@ static void ezusb_req_ctx_wait(struct ez
* get the chance to run themselves. So we make sure
* that we don't sleep for ever */
int msecs = DEF_TIMEOUT * (1000 / HZ);
- while (!ctx->done.done && msecs--)
+
+ while (!try_wait_for_completion(&ctx->done) && msecs--)
udelay(1000);
} else {
- wait_event_interruptible(ctx->done.wait,
- ctx->done.done);
+ wait_for_completion(&ctx->done);
}
break;
default:
^ permalink raw reply
* [patch V2 09/15] timekeeping: Split jiffies seqlock
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Thomas Gleixner <tglx@linutronix.de>
seqlock consists of a sequence counter and a spinlock_t which is used to
serialize the writers. spinlock_t is substituted by a "sleeping" spinlock
on PREEMPT_RT enabled kernels which breaks the usage in the timekeeping
code as the writers are executed in hard interrupt and therefore
non-preemptible context even on PREEMPT_RT.
The spinlock in seqlock cannot be unconditionally replaced by a
raw_spinlock_t as many seqlock users have nesting spinlock sections or
other code which is not suitable to run in truly atomic context on RT.
Instead of providing a raw_seqlock API for a single use case, open code the
seqlock for the jiffies use case and implement it with a raw_spinlock_t and
a sequence counter.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/time/jiffies.c | 7 ++++---
kernel/time/tick-common.c | 10 ++++++----
kernel/time/tick-sched.c | 19 ++++++++++++-------
kernel/time/timekeeping.c | 6 ++++--
kernel/time/timekeeping.h | 3 ++-
5 files changed, 28 insertions(+), 17 deletions(-)
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -58,7 +58,8 @@ static struct clocksource clocksource_ji
.max_cycles = 10,
};
-__cacheline_aligned_in_smp DEFINE_SEQLOCK(jiffies_lock);
+__cacheline_aligned_in_smp DEFINE_RAW_SPINLOCK(jiffies_lock);
+__cacheline_aligned_in_smp seqcount_t jiffies_seq;
#if (BITS_PER_LONG < 64)
u64 get_jiffies_64(void)
@@ -67,9 +68,9 @@ u64 get_jiffies_64(void)
u64 ret;
do {
- seq = read_seqbegin(&jiffies_lock);
+ seq = read_seqcount_begin(&jiffies_seq);
ret = jiffies_64;
- } while (read_seqretry(&jiffies_lock, seq));
+ } while (read_seqcount_retry(&jiffies_seq, seq));
return ret;
}
EXPORT_SYMBOL(get_jiffies_64);
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -84,13 +84,15 @@ int tick_is_oneshot_available(void)
static void tick_periodic(int cpu)
{
if (tick_do_timer_cpu == cpu) {
- write_seqlock(&jiffies_lock);
+ raw_spin_lock(&jiffies_lock);
+ write_seqcount_begin(&jiffies_seq);
/* Keep track of the next tick event */
tick_next_period = ktime_add(tick_next_period, tick_period);
do_timer(1);
- write_sequnlock(&jiffies_lock);
+ write_seqcount_end(&jiffies_seq);
+ raw_spin_unlock(&jiffies_lock);
update_wall_time();
}
@@ -162,9 +164,9 @@ void tick_setup_periodic(struct clock_ev
ktime_t next;
do {
- seq = read_seqbegin(&jiffies_lock);
+ seq = read_seqcount_begin(&jiffies_seq);
next = tick_next_period;
- } while (read_seqretry(&jiffies_lock, seq));
+ } while (read_seqcount_retry(&jiffies_seq, seq));
clockevents_switch_state(dev, CLOCK_EVT_STATE_ONESHOT);
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -65,7 +65,8 @@ static void tick_do_update_jiffies64(kti
return;
/* Reevaluate with jiffies_lock held */
- write_seqlock(&jiffies_lock);
+ raw_spin_lock(&jiffies_lock);
+ write_seqcount_begin(&jiffies_seq);
delta = ktime_sub(now, last_jiffies_update);
if (delta >= tick_period) {
@@ -91,10 +92,12 @@ static void tick_do_update_jiffies64(kti
/* Keep the tick_next_period variable up to date */
tick_next_period = ktime_add(last_jiffies_update, tick_period);
} else {
- write_sequnlock(&jiffies_lock);
+ write_seqcount_end(&jiffies_seq);
+ raw_spin_unlock(&jiffies_lock);
return;
}
- write_sequnlock(&jiffies_lock);
+ write_seqcount_end(&jiffies_seq);
+ raw_spin_unlock(&jiffies_lock);
update_wall_time();
}
@@ -105,12 +108,14 @@ static ktime_t tick_init_jiffy_update(vo
{
ktime_t period;
- write_seqlock(&jiffies_lock);
+ raw_spin_lock(&jiffies_lock);
+ write_seqcount_begin(&jiffies_seq);
/* Did we start the jiffies update yet ? */
if (last_jiffies_update == 0)
last_jiffies_update = tick_next_period;
period = last_jiffies_update;
- write_sequnlock(&jiffies_lock);
+ write_seqcount_end(&jiffies_seq);
+ raw_spin_unlock(&jiffies_lock);
return period;
}
@@ -676,10 +681,10 @@ static ktime_t tick_nohz_next_event(stru
/* Read jiffies and the time when jiffies were updated last */
do {
- seq = read_seqbegin(&jiffies_lock);
+ seq = read_seqcount_begin(&jiffies_seq);
basemono = last_jiffies_update;
basejiff = jiffies;
- } while (read_seqretry(&jiffies_lock, seq));
+ } while (read_seqcount_retry(&jiffies_seq, seq));
ts->last_jiffies = basejiff;
ts->timer_expires_base = basemono;
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -2397,8 +2397,10 @@ EXPORT_SYMBOL(hardpps);
*/
void xtime_update(unsigned long ticks)
{
- write_seqlock(&jiffies_lock);
+ raw_spin_lock(&jiffies_lock);
+ write_seqcount_begin(&jiffies_seq);
do_timer(ticks);
- write_sequnlock(&jiffies_lock);
+ write_seqcount_end(&jiffies_seq);
+ raw_spin_unlock(&jiffies_lock);
update_wall_time();
}
--- a/kernel/time/timekeeping.h
+++ b/kernel/time/timekeeping.h
@@ -25,7 +25,8 @@ static inline void sched_clock_resume(vo
extern void do_timer(unsigned long ticks);
extern void update_wall_time(void);
-extern seqlock_t jiffies_lock;
+extern raw_spinlock_t jiffies_lock;
+extern seqcount_t jiffies_seq;
#define CS_NAME_LEN 32
^ permalink raw reply
* [patch V2 06/15] rcuwait: Add @state argument to rcuwait_wait_event()
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: linux-usb, Peter Zijlstra, linux-pci, Sebastian Andrzej Siewior,
Joel Fernandes, Will Deacon, Ingo Molnar, Davidlohr Bueso,
Arnd Bergmann, Logan Gunthorpe, Paul E . McKenney, linuxppc-dev,
Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer, Kalle Valo,
Felipe Balbi, Greg Kroah-Hartman, Randy Dunlap, linux-wireless,
Oleg Nesterov, netdev, Linus Torvalds, David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
Extend rcuwait_wait_event() with a state variable so that it is not
restricted to UNINTERRUPTIBLE waits.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
---
include/linux/rcuwait.h | 12 ++++++++++--
kernel/locking/percpu-rwsem.c | 2 +-
2 files changed, 11 insertions(+), 3 deletions(-)
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -3,6 +3,7 @@
#define _LINUX_RCUWAIT_H_
#include <linux/rcupdate.h>
+#include <linux/sched/signal.h>
/*
* rcuwait provides a way of blocking and waking up a single
@@ -30,23 +31,30 @@ extern void rcuwait_wake_up(struct rcuwa
* The caller is responsible for locking around rcuwait_wait_event(),
* such that writes to @task are properly serialized.
*/
-#define rcuwait_wait_event(w, condition) \
+#define rcuwait_wait_event(w, condition, state) \
({ \
+ int __ret = 0; \
rcu_assign_pointer((w)->task, current); \
for (;;) { \
/* \
* Implicit barrier (A) pairs with (B) in \
* rcuwait_wake_up(). \
*/ \
- set_current_state(TASK_UNINTERRUPTIBLE); \
+ set_current_state(state); \
if (condition) \
break; \
\
+ if (signal_pending_state(state, current)) { \
+ __ret = -EINTR; \
+ break; \
+ } \
+ \
schedule(); \
} \
\
WRITE_ONCE((w)->task, NULL); \
__set_current_state(TASK_RUNNING); \
+ __ret; \
})
#endif /* _LINUX_RCUWAIT_H_ */
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -162,7 +162,7 @@ void percpu_down_write(struct percpu_rw_
*/
/* Wait for all now active readers to complete. */
- rcuwait_wait_event(&sem->writer, readers_active_check(sem));
+ rcuwait_wait_event(&sem->writer, readers_active_check(sem), TASK_UNINTERRUPTIBLE);
}
EXPORT_SYMBOL_GPL(percpu_down_write);
^ permalink raw reply
* [patch V2 00/15] Lock ordering documentation and annotation for lockdep
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
This is the second version of this work. The first one can be found here:
https://lore.kernel.org/r/20200313174701.148376-1-bigeasy@linutronix.de
Changes since V1:
- Split the PCI/switchtec patch (picked up the fix from Logan) and
reworked the change log.
- Addressed Linus feedback vs. completions.
Most of the places which had open coded completion variants have been
analysed and fixed up to use the regular interfaces.
The PS3 one got converted by Peter Zijlstra to rcu_wait().
Add explanation in the change log why swait actually fits the
completion semantics.
- Addressed Randys feedback on documentation
Thanks,
tglx
^ permalink raw reply
* [patch V2 07/15] powerpc/ps3: Convert half completion to rcuwait
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, netdev, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Paul E . McKenney, Logan Gunthorpe,
Arnd Bergmann, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
The PS3 notification interrupt and kthread use a hacked up completion to
communicate. Since we're wanting to change the completion implementation and
this is abuse anyway, replace it with a simple rcuwait since there is only ever
the one waiter.
AFAICT the kthread uses TASK_INTERRUPTIBLE to not increase loadavg, kthreads
cannot receive signals by default and this one doesn't look different. Use
TASK_IDLE instead.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linuxppc-dev@lists.ozlabs.org
---
V2: New patch to avoid the magic completion wait variant
---
arch/powerpc/platforms/ps3/device-init.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -13,6 +13,7 @@
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/reboot.h>
+#include <linux/rcuwait.h>
#include <asm/firmware.h>
#include <asm/lv1call.h>
@@ -670,7 +671,8 @@ struct ps3_notification_device {
spinlock_t lock;
u64 tag;
u64 lv1_status;
- struct completion done;
+ struct rcuwait wait;
+ bool done;
};
enum ps3_notify_type {
@@ -712,7 +714,8 @@ static irqreturn_t ps3_notification_inte
pr_debug("%s:%u: completed, status 0x%llx\n", __func__,
__LINE__, status);
dev->lv1_status = status;
- complete(&dev->done);
+ dev->done = true;
+ rcuwait_wake_up(&dev->wait);
}
spin_unlock(&dev->lock);
return IRQ_HANDLED;
@@ -725,12 +728,12 @@ static int ps3_notification_read_write(s
unsigned long flags;
int res;
- init_completion(&dev->done);
spin_lock_irqsave(&dev->lock, flags);
res = write ? lv1_storage_write(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
&dev->tag)
: lv1_storage_read(dev->sbd.dev_id, 0, 0, 1, 0, lpar,
&dev->tag);
+ dev->done = false;
spin_unlock_irqrestore(&dev->lock, flags);
if (res) {
pr_err("%s:%u: %s failed %d\n", __func__, __LINE__, op, res);
@@ -738,14 +741,10 @@ static int ps3_notification_read_write(s
}
pr_debug("%s:%u: notification %s issued\n", __func__, __LINE__, op);
- res = wait_event_interruptible(dev->done.wait,
- dev->done.done || kthread_should_stop());
+ rcuwait_wait_event(&dev->wait, dev->done || kthread_should_stop(), TASK_IDLE);
+
if (kthread_should_stop())
res = -EINTR;
- if (res) {
- pr_debug("%s:%u: interrupted %s\n", __func__, __LINE__, op);
- return res;
- }
if (dev->lv1_status) {
pr_err("%s:%u: %s not completed, status 0x%llx\n", __func__,
^ permalink raw reply
* [patch V2 01/15] PCI/switchtec: Fix init_completion race condition with poll_wait()
From: Thomas Gleixner @ 2020-03-18 20:43 UTC (permalink / raw)
To: LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Logan Gunthorpe,
Paul E . McKenney, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204302.693307984@linutronix.de>
From: Logan Gunthorpe <logang@deltatee.com>
The call to init_completion() in mrpc_queue_cmd() can theoretically
race with the call to poll_wait() in switchtec_dev_poll().
poll() write()
switchtec_dev_poll() switchtec_dev_write()
poll_wait(&s->comp.wait); mrpc_queue_cmd()
init_completion(&s->comp)
init_waitqueue_head(&s->comp.wait)
To my knowledge, no one has hit this bug.
Fix this by using reinit_completion() instead of init_completion() in
mrpc_queue_cmd().
Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lkml.kernel.org/r/20200313183608.2646-1-logang@deltatee.com
---
drivers/pci/switch/switchtec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index a823b4b8ef8a..81dc7ac01381 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -175,7 +175,7 @@ static int mrpc_queue_cmd(struct switchtec_user *stuser)
kref_get(&stuser->kref);
stuser->read_len = sizeof(stuser->data);
stuser_set_state(stuser, MRPC_QUEUED);
- init_completion(&stuser->comp);
+ reinit_completion(&stuser->comp);
list_add_tail(&stuser->list, &stdev->mrpc_queue);
mrpc_cmd_submit(stdev);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] tpm2: Export tpm2_get_cc_attrs_tbl for ibmvtpm driver as module
From: Stefan Berger @ 2020-03-18 19:53 UTC (permalink / raw)
To: Jarkko Sakkinen, Stefan Berger
Cc: sachinp, linux-kernel, linux-next, linux-integrity, linuxppc-dev
In-Reply-To: <20200318194247.GC48177@linux.intel.com>
On 3/18/20 3:42 PM, Jarkko Sakkinen wrote:
> On Tue, Mar 17, 2020 at 09:08:19AM -0400, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> This patch fixes the following problem when the ibmvtpm driver
>> is built as a module:
>>
>> ERROR: modpost: "tpm2_get_cc_attrs_tbl" [drivers/char/tpm/tpm_ibmvtpm.ko] undefined!
>> make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
>> make: *** [Makefile:1298: modules] Error 2
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Hi, wrong tag (we use "tpm:"), missing fixes tag and please cc stable.
> Thanks.
I did not add the fixes tag because I do not know the final commit hash,
or is it the final commit hash once it is in linux-next? I doubt it with
all the merging that can occur.
Stefan
>
> /Jarkko
^ permalink raw reply
* Re: [PATCH] tpm2: Export tpm2_get_cc_attrs_tbl for ibmvtpm driver as module
From: Jarkko Sakkinen @ 2020-03-18 19:42 UTC (permalink / raw)
To: Stefan Berger
Cc: sachinp, linux-kernel, linux-next, linux-integrity, linuxppc-dev,
Stefan Berger
In-Reply-To: <20200317130819.720318-1-stefanb@linux.vnet.ibm.com>
On Tue, Mar 17, 2020 at 09:08:19AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> This patch fixes the following problem when the ibmvtpm driver
> is built as a module:
>
> ERROR: modpost: "tpm2_get_cc_attrs_tbl" [drivers/char/tpm/tpm_ibmvtpm.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
> make: *** [Makefile:1298: modules] Error 2
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Hi, wrong tag (we use "tpm:"), missing fixes tag and please cc stable.
Thanks.
/Jarkko
^ permalink raw reply
* Re: [PATCH v2 3/4] mm: Implement reset_numa_mem
From: Christopher Lameter @ 2020-03-18 19:20 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Sachin Sant, Nathan Lynch, Michal Hocko, linux-mm, Kirill Tkhai,
Mel Gorman, Andrew Morton, Bharata B Rao, linuxppc-dev,
Joonsoo Kim, Vlastimil Babka
In-Reply-To: <20200318072810.9735-4-srikar@linux.vnet.ibm.com>
On Wed, 18 Mar 2020, Srikar Dronamraju wrote:
> For a memoryless or offline nodes, node_numa_mem refers to a N_MEMORY
> fallback node. Currently kernel has an API set_numa_mem that sets
> node_numa_mem for memoryless node. However this API cannot be used for
> offline nodes. Hence all offline nodes will have their node_numa_mem set
> to 0. However systems can themselves have node 0 as offline i.e
That is a significant change to the basic assumptions for memory less
nodes. Node 0 needed to have memory and processors. Not sure what else
may break.
^ permalink raw reply
* Re: [PATCH 3/3] mm/page_alloc: Keep memoryless cpuless node 0 offline
From: Christopher Lameter @ 2020-03-18 18:57 UTC (permalink / raw)
To: Michal Hocko
Cc: Srikar Dronamraju, Linus Torvalds, linux-kernel, linux-mm,
Mel Gorman, Kirill A. Shutemov, Andrew Morton, linuxppc-dev,
Vlastimil Babka
In-Reply-To: <20200316085425.GB11482@dhcp22.suse.cz>
On Mon, 16 Mar 2020, Michal Hocko wrote:
> > We can dynamically number the nodes right? So just make sure that the
> > firmware properly creates memory on node 0?
>
> Are you suggesting that the OS would renumber NUMA nodes coming
> from FW just to satisfy node 0 existence? If yes then I believe this is
> really a bad idea because it would make HW/LPAR configuration matching
> to the resulting memory layout really hard to follow.
NUMA nodes are created by the OS based on information provided by the
firmware. Either the FW would need to ensure that a viable node 0 exists
or the bootstrap arch code could setup things to the same effect.
^ permalink raw reply
* Re: [PATCH 02/15] powerpc/watchpoint: Add SPRN macros for second DAWR
From: Segher Boessenkool @ 2020-03-18 18:30 UTC (permalink / raw)
To: Christophe Leroy
Cc: Ravi Bangoria, mikey, apopple, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <0a45786d-f44b-8717-3aed-dfcfcb1856bb@c-s.fr>
On Tue, Mar 17, 2020 at 11:16:34AM +0100, Christophe Leroy wrote:
>
>
> Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> >Future Power architecture is introducing second DAWR. Add SPRN_ macros
> >for the same.
>
> I'm not sure this is called 'macros'. For me a macro is something more
> complex.
It is called "macros" in the C standard, and in common usage as well.
"Object-like macros", as opposed to "function-like macros": there are
no arguments.
> For me those are 'constants'.
That would be more like "static const" in C since 1990 ;-)
Segher
^ permalink raw reply
* Re: [PATCH 1/3] KVM: PPC: Fix kernel crash with PR KVM
From: Sean Christopherson @ 2020-03-18 18:16 UTC (permalink / raw)
To: Greg Kurz; +Cc: kvm-ppc, Paolo Bonzini, linuxppc-dev
In-Reply-To: <158455341029.178873.15248663726399374882.stgit@bahia.lan>
On Wed, Mar 18, 2020 at 06:43:30PM +0100, Greg Kurz wrote:
> It turns out that this is only relevant to PR KVM actually. And both
> 32 and 64 backends need vcpu->arch.book3s to be valid when calling
> kvmppc_mmu_destroy_pr(). So instead of calling kvmppc_mmu_destroy()
> from kvm_arch_vcpu_destroy(), call kvmppc_mmu_destroy_pr() at the
> beginning of kvmppc_core_vcpu_free_pr(). This is consistent with
> kvmppc_mmu_init() being the last call in kvmppc_core_vcpu_create_pr().
>
> For the same reason, if kvmppc_core_vcpu_create_pr() returns an
> error then this means that kvmppc_mmu_init() was either not called
> or failed, in which case kvmppc_mmu_destroy() should not be called.
> Drop the line in the error path of kvm_arch_vcpu_create().
>
> Fixes: ff030fdf5573 ("KVM: PPC: Move kvm_vcpu_init() invocation to common code")
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
Dang, I see where I went wrong. Sorry :-(
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
^ 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