* Re: [PATCH 05/20] swiotlb: allow the architecture to provide a get_required_mask hook
From: Konrad Rzeszutek Wilk @ 2018-08-27 16:06 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Tony Luck, Fenghua Yu, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <20180730163824.10064-6-hch@lst.de>
On Mon, Jul 30, 2018 at 06:38:09PM +0200, Christoph Hellwig wrote:
> For now this allows consolidating the powerpc code. In the long run
> we should grow a generic implementation of dma_get_required_mask that
> returns the dma mask required to avoid bounce buffering.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Thank you!
> ---
> kernel/dma/swiotlb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 904541055792..1bb420244753 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -1084,5 +1084,9 @@ const struct dma_map_ops swiotlb_dma_ops = {
> .map_page = swiotlb_map_page,
> .unmap_page = swiotlb_unmap_page,
> .dma_supported = dma_direct_supported,
> +#ifdef swiotlb_get_required_mask
> + .get_required_mask = swiotlb_get_required_mask,
> +#endif
> +
> };
> EXPORT_SYMBOL(swiotlb_dma_ops);
> --
> 2.18.0
>
^ permalink raw reply
* [PATCH v2] drivers/video/fbdev: use ioremap_wc() instead of __ioremap(_PAGE_NO_CACHE)
From: Christophe Leroy @ 2018-08-27 16:05 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: dri-devel, linux-fbdev, linux-kernel, linuxppc-dev
_PAGE_NO_CACHE is a target specific flag. In addition, this flag
is misleading because one would think it requests a noncached page
whereas a noncached page is _PAGE_NO_CACHE | _PAGE_GUARDED
_PAGE_NO_CACHE alone means write combined noncached page, so lets
use ioremap_wc() instead.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: removed unnecessary include and enhanced commit log follow Segher's comment
drivers/video/fbdev/chipsfb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index f103665cad43..40182ed85648 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -27,7 +27,6 @@
#include <linux/init.h>
#include <linux/pci.h>
#include <linux/console.h>
-#include <asm/io.h>
#ifdef CONFIG_PMAC_BACKLIGHT
#include <asm/backlight.h>
@@ -401,7 +400,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent)
#endif /* CONFIG_PMAC_BACKLIGHT */
#ifdef CONFIG_PPC
- p->screen_base = __ioremap(addr, 0x200000, _PAGE_NO_CACHE);
+ p->screen_base = ioremap_wc(addr, 0x200000);
#else
p->screen_base = ioremap(addr, 0x200000);
#endif
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] treewide: remove current_text_addr
From: H. Peter Anvin @ 2018-08-27 13:33 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Nick Desaulniers, Linus Torvalds, deller, Andrew Morton,
Eric W . Biederman, Thomas Gleixner, mingo, Simon Horman,
Nathan Chancellor, Philippe Ombredanne, Kate Stewart, Greg KH,
rth, ink, mattst88, vgupta, linux, Catalin Marinas, Will Deacon,
msalter, jacquiot.aurelien, Yoshinori Sato, rkuo, tony.luck,
fenghua.yu, Geert Uytterhoeven, monstr, ralf, paul.burton, jhogan,
green.hu, deanbo422, lftan, jonas, stefan.kristiansson,
Stafford Horne, jejb, benh, paulus, mpe, palmer, aou, schwidefsky,
heiko.carstens, dalias, David S. Miller, gxt, x86, jdike, richard,
chris, jcmvbkbc, Tobias Klauser, noamc, mickael.guene,
nicolas.pitre, Kees Cook, Dave Martin, Marc Zyngier, alex.bennee,
Laura Abbott, Yury Norov, Mark Rutland, chenhc, macro,
Arnd Bergmann, dhowells, sukadev, Nicholas Piggin, aneesh.kumar,
felix, linuxram, christophe.leroy, cohuck, gor, nick.alcock,
shannon.nelson, nagarathnam.muthusamy, luto, bp, dave.hansen,
vkuznets, jkosina, linux-alpha, LKML, linux-snps-arc, Linux ARM,
linux-c6x-dev, uclinux-h8-devel, linux-hexagon, linux-ia64,
linux-m68k, linux-mips, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-s390, linux-sh, sparclinux,
linux-um
In-Reply-To: <20180827131103.GD24124@hirez.programming.kicks-ass.net>
On 08/27/18 06:11, Peter Zijlstra wrote:
> On Mon, Aug 27, 2018 at 05:26:53AM -0700, H. Peter Anvin wrote:
>
>> _THIS_IP_, however, is completely ill-defined, other than being an
>> address *somewhere* in the same global function (not even necessarily
>> the same function if the function is static!) As my experiment show, in
>> many (nearly) cases gcc will hoist the address all the way to the top of
>> the function, at least for the current generic implementation.
>
> It seems to have mostly worked so far... did anything change?
>
Most likely because the major architectures contain a arch-specific
assembly implementation. The generic implementation used in some places
is completely broken, as my experiments show.
>> For the case where _THIS_IP_ is passed to an out-of-line function in all
>> cases, it is extra pointless because all it does is increase the
>> footprint of every caller: _RET_IP_ is inherently passed to the function
>> anyway, and with tailcall protection it will uniquely identify a callsite.
>
> So I think we can convert many of the lockdep _THIS_IP_ calls to
> _RET_IP_ on the other side, with a wee bit of care.
>
> A little something like so perhaps...
I don't have time to look at this right now (I'm on sabbatical, and I'm
dealing with personal legal stuff right at the moment), but I think it
is the right direction.
-hpa
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: Peter Zijlstra @ 2018-08-27 13:11 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Nick Desaulniers, Linus Torvalds, deller, Andrew Morton,
Eric W . Biederman, Thomas Gleixner, mingo, Simon Horman,
Nathan Chancellor, Philippe Ombredanne, Kate Stewart, Greg KH,
rth, ink, mattst88, vgupta, linux, Catalin Marinas, Will Deacon,
msalter, jacquiot.aurelien, Yoshinori Sato, rkuo, tony.luck,
fenghua.yu, Geert Uytterhoeven, monstr, ralf, paul.burton, jhogan,
green.hu, deanbo422, lftan, jonas, stefan.kristiansson,
Stafford Horne, jejb, benh, paulus, mpe, palmer, aou, schwidefsky,
heiko.carstens, dalias, David S. Miller, gxt, x86, jdike, richard,
chris, jcmvbkbc, Tobias Klauser, noamc, mickael.guene,
nicolas.pitre, Kees Cook, Dave Martin, Marc Zyngier, alex.bennee,
Laura Abbott, Yury Norov, Mark Rutland, chenhc, macro,
Arnd Bergmann, dhowells, sukadev, Nicholas Piggin, aneesh.kumar,
felix, linuxram, christophe.leroy, cohuck, gor, nick.alcock,
shannon.nelson, nagarathnam.muthusamy, luto, bp, dave.hansen,
vkuznets, jkosina, linux-alpha, LKML, linux-snps-arc, Linux ARM,
linux-c6x-dev, uclinux-h8-devel, linux-hexagon, linux-ia64,
linux-m68k, linux-mips, nios2-dev, openrisc, linux-parisc,
linuxppc-dev, linux-riscv, linux-s390, linux-sh, sparclinux,
linux-um
In-Reply-To: <f9896d68-4a49-e666-cea5-a9c0522f1658@zytor.com>
On Mon, Aug 27, 2018 at 05:26:53AM -0700, H. Peter Anvin wrote:
> _THIS_IP_, however, is completely ill-defined, other than being an
> address *somewhere* in the same global function (not even necessarily
> the same function if the function is static!) As my experiment show, in
> many (nearly) cases gcc will hoist the address all the way to the top of
> the function, at least for the current generic implementation.
It seems to have mostly worked so far... did anything change?
> For the case where _THIS_IP_ is passed to an out-of-line function in all
> cases, it is extra pointless because all it does is increase the
> footprint of every caller: _RET_IP_ is inherently passed to the function
> anyway, and with tailcall protection it will uniquely identify a callsite.
So I think we can convert many of the lockdep _THIS_IP_ calls to
_RET_IP_ on the other side, with a wee bit of care.
A little something like so perhaps...
---
drivers/md/bcache/btree.c | 2 +-
fs/jbd2/transaction.c | 6 +++---
fs/super.c | 4 ++--
include/linux/fs.h | 4 ++--
include/linux/jbd2.h | 4 ++--
include/linux/lockdep.h | 21 ++++++++++-----------
include/linux/percpu-rwsem.h | 22 ++++++++++------------
include/linux/rcupdate.h | 8 ++++----
include/linux/ww_mutex.h | 2 +-
kernel/locking/lockdep.c | 14 ++++++++------
kernel/printk/printk.c | 14 +++++++-------
kernel/sched/core.c | 4 ++--
lib/locking-selftest.c | 32 ++++++++++++++++----------------
13 files changed, 68 insertions(+), 69 deletions(-)
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index c19f7716df88..21ede9b317de 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -940,7 +940,7 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
hlist_del_init_rcu(&b->hash);
hlist_add_head_rcu(&b->hash, mca_hash(c, k));
- lock_set_subclass(&b->lock.dep_map, level + 1, _THIS_IP_);
+ lock_set_subclass(&b->lock.dep_map, level + 1);
b->parent = (void *) ~0UL;
b->flags = 0;
b->written = 0;
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index c0b66a7a795b..40aa71321f8a 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -382,7 +382,7 @@ static int start_this_handle(journal_t *journal, handle_t *handle,
read_unlock(&journal->j_state_lock);
current->journal_info = handle;
- rwsem_acquire_read(&journal->j_trans_commit_map, 0, 0, _THIS_IP_);
+ rwsem_acquire_read(&journal->j_trans_commit_map, 0, 0);
jbd2_journal_free_transaction(new_transaction);
/*
* Ensure that no allocations done while the transaction is open are
@@ -677,7 +677,7 @@ int jbd2__journal_restart(handle_t *handle, int nblocks, gfp_t gfp_mask)
if (need_to_start)
jbd2_log_start_commit(journal, tid);
- rwsem_release(&journal->j_trans_commit_map, 1, _THIS_IP_);
+ rwsem_release(&journal->j_trans_commit_map, 1);
handle->h_buffer_credits = nblocks;
/*
* Restore the original nofs context because the journal restart
@@ -1771,7 +1771,7 @@ int jbd2_journal_stop(handle_t *handle)
wake_up(&journal->j_wait_transaction_locked);
}
- rwsem_release(&journal->j_trans_commit_map, 1, _THIS_IP_);
+ rwsem_release(&journal->j_trans_commit_map, 1);
if (wait_for_commit)
err = jbd2_log_wait_commit(journal, tid);
diff --git a/fs/super.c b/fs/super.c
index 50728d9c1a05..ec650a558f09 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1431,7 +1431,7 @@ static void lockdep_sb_freeze_release(struct super_block *sb)
int level;
for (level = SB_FREEZE_LEVELS - 1; level >= 0; level--)
- percpu_rwsem_release(sb->s_writers.rw_sem + level, 0, _THIS_IP_);
+ percpu_rwsem_release(sb->s_writers.rw_sem + level, 0);
}
/*
@@ -1442,7 +1442,7 @@ static void lockdep_sb_freeze_acquire(struct super_block *sb)
int level;
for (level = 0; level < SB_FREEZE_LEVELS; ++level)
- percpu_rwsem_acquire(sb->s_writers.rw_sem + level, 0, _THIS_IP_);
+ percpu_rwsem_acquire(sb->s_writers.rw_sem + level, 0);
}
static void sb_freeze_unlock(struct super_block *sb)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1ec33fd0423f..2ba14e5362e4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1505,9 +1505,9 @@ void __sb_end_write(struct super_block *sb, int level);
int __sb_start_write(struct super_block *sb, int level, bool wait);
#define __sb_writers_acquired(sb, lev) \
- percpu_rwsem_acquire(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
+ percpu_rwsem_acquire(&(sb)->s_writers.rw_sem[(lev)-1], 1)
#define __sb_writers_release(sb, lev) \
- percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
+ percpu_rwsem_release(&(sb)->s_writers.rw_sem[(lev)-1], 1)
/**
* sb_end_write - drop write access to a superblock
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index b708e5169d1d..7c31176ec8ae 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1155,8 +1155,8 @@ struct journal_s
#define jbd2_might_wait_for_commit(j) \
do { \
- rwsem_acquire(&j->j_trans_commit_map, 0, 0, _THIS_IP_); \
- rwsem_release(&j->j_trans_commit_map, 1, _THIS_IP_); \
+ rwsem_acquire(&j->j_trans_commit_map, 0, 0); \
+ rwsem_release(&j->j_trans_commit_map, 1); \
} while (0)
/* journal feature predicate functions */
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 6fc77d4dbdcd..ed3daf41ae7b 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -348,16 +348,15 @@ static inline int lock_is_held(const struct lockdep_map *lock)
#define lockdep_is_held_type(lock, r) lock_is_held_type(&(lock)->dep_map, (r))
extern void lock_set_class(struct lockdep_map *lock, const char *name,
- struct lock_class_key *key, unsigned int subclass,
- unsigned long ip);
+ struct lock_class_key *key, unsigned int subclass);
-static inline void lock_set_subclass(struct lockdep_map *lock,
- unsigned int subclass, unsigned long ip)
+static __always_inline void
+lock_set_subclass(struct lockdep_map *lock, unsigned int subclass)
{
- lock_set_class(lock, lock->name, lock->key, subclass, ip);
+ lock_set_class(lock, lock->name, lock->key, subclass);
}
-extern void lock_downgrade(struct lockdep_map *lock, unsigned long ip);
+extern void lock_downgrade(struct lockdep_map *lock);
struct pin_cookie { unsigned int val; };
@@ -401,11 +400,11 @@ static inline void lockdep_on(void)
{
}
-# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
-# define lock_release(l, n, i) do { } while (0)
-# define lock_downgrade(l, i) do { } while (0)
-# define lock_set_class(l, n, k, s, i) do { } while (0)
-# define lock_set_subclass(l, s, i) do { } while (0)
+# define lock_acquire(l, s, t, r, c, n) do { } while (0)
+# define lock_release(l, n) do { } while (0)
+# define lock_downgrade(l) do { } while (0)
+# define lock_set_class(l, n, k, s) do { } while (0)
+# define lock_set_subclass(l, s) do { } while (0)
# define lockdep_info() do { } while (0)
# define lockdep_init_map(lock, name, key, sub) \
do { (void)(name); (void)(key); } while (0)
diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h
index 79b99d653e03..4ebf14e99034 100644
--- a/include/linux/percpu-rwsem.h
+++ b/include/linux/percpu-rwsem.h
@@ -29,11 +29,11 @@ static struct percpu_rw_semaphore name = { \
extern int __percpu_down_read(struct percpu_rw_semaphore *, int);
extern void __percpu_up_read(struct percpu_rw_semaphore *);
-static inline void percpu_down_read_preempt_disable(struct percpu_rw_semaphore *sem)
+static __always_inline void percpu_down_read_preempt_disable(struct percpu_rw_semaphore *sem)
{
might_sleep();
- rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_);
+ rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0);
preempt_disable();
/*
@@ -60,7 +60,7 @@ static inline void percpu_down_read(struct percpu_rw_semaphore *sem)
preempt_enable();
}
-static inline int percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
+static __always_inline int percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
{
int ret = 1;
@@ -78,12 +78,12 @@ static inline int percpu_down_read_trylock(struct percpu_rw_semaphore *sem)
*/
if (ret)
- rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 1, _RET_IP_);
+ rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 1);
return ret;
}
-static inline void percpu_up_read_preempt_enable(struct percpu_rw_semaphore *sem)
+static __always_inline void percpu_up_read_preempt_enable(struct percpu_rw_semaphore *sem)
{
/*
* The barrier() prevents the compiler from
@@ -99,7 +99,7 @@ static inline void percpu_up_read_preempt_enable(struct percpu_rw_semaphore *sem
__percpu_up_read(sem); /* Unconditional memory barrier */
preempt_enable();
- rwsem_release(&sem->rw_sem.dep_map, 1, _RET_IP_);
+ rwsem_release(&sem->rw_sem.dep_map, 1);
}
static inline void percpu_up_read(struct percpu_rw_semaphore *sem)
@@ -127,20 +127,18 @@ extern void percpu_free_rwsem(struct percpu_rw_semaphore *);
#define percpu_rwsem_assert_held(sem) \
lockdep_assert_held(&(sem)->rw_sem)
-static inline void percpu_rwsem_release(struct percpu_rw_semaphore *sem,
- bool read, unsigned long ip)
+static __always_inline void percpu_rwsem_release(struct percpu_rw_semaphore *sem, bool read)
{
- lock_release(&sem->rw_sem.dep_map, 1, ip);
+ lock_release(&sem->rw_sem.dep_map, 1);
#ifdef CONFIG_RWSEM_SPIN_ON_OWNER
if (!read)
sem->rw_sem.owner = RWSEM_OWNER_UNKNOWN;
#endif
}
-static inline void percpu_rwsem_acquire(struct percpu_rw_semaphore *sem,
- bool read, unsigned long ip)
+static __always_inline void percpu_rwsem_acquire(struct percpu_rw_semaphore *sem, bool read)
{
- lock_acquire(&sem->rw_sem.dep_map, 0, 1, read, 1, NULL, ip);
+ lock_acquire(&sem->rw_sem.dep_map, 0, 1, read, 1, NULL);
#ifdef CONFIG_RWSEM_SPIN_ON_OWNER
if (!read)
sem->rw_sem.owner = current;
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 75e5b393cf44..6c1a35555e9d 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -239,14 +239,14 @@ static inline bool rcu_lockdep_current_cpu_online(void) { return true; }
#ifdef CONFIG_DEBUG_LOCK_ALLOC
-static inline void rcu_lock_acquire(struct lockdep_map *map)
+static __always_inline void rcu_lock_acquire(struct lockdep_map *map)
{
- lock_acquire(map, 0, 0, 2, 0, NULL, _THIS_IP_);
+ lock_acquire(map, 0, 0, 2, 0, NULL);
}
-static inline void rcu_lock_release(struct lockdep_map *map)
+static __always_inline void rcu_lock_release(struct lockdep_map *map)
{
- lock_release(map, 1, _THIS_IP_);
+ lock_release(map, 1);
}
extern struct lockdep_map rcu_lock_map;
diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
index 3af7c0e03be5..524aa28eef33 100644
--- a/include/linux/ww_mutex.h
+++ b/include/linux/ww_mutex.h
@@ -182,7 +182,7 @@ static inline void ww_acquire_done(struct ww_acquire_ctx *ctx)
static inline void ww_acquire_fini(struct ww_acquire_ctx *ctx)
{
#ifdef CONFIG_DEBUG_MUTEXES
- mutex_release(&ctx->dep_map, 0, _THIS_IP_);
+ mutex_release(&ctx->dep_map, 0);
DEBUG_LOCKS_WARN_ON(ctx->acquired);
if (!IS_ENABLED(CONFIG_PROVE_LOCKING))
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 5fa4d3138bf1..0b7c4f94a7a3 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3868,9 +3868,9 @@ static void check_flags(unsigned long flags)
}
void lock_set_class(struct lockdep_map *lock, const char *name,
- struct lock_class_key *key, unsigned int subclass,
- unsigned long ip)
+ struct lock_class_key *key, unsigned int subclass)
{
+ unsigned long ip = _RET_IP_;
unsigned long flags;
if (unlikely(current->lockdep_recursion))
@@ -3886,8 +3886,9 @@ void lock_set_class(struct lockdep_map *lock, const char *name,
}
EXPORT_SYMBOL_GPL(lock_set_class);
-void lock_downgrade(struct lockdep_map *lock, unsigned long ip)
+void lock_downgrade(struct lockdep_map *lock)
{
+ unsigned long ip = _RET_IP_;
unsigned long flags;
if (unlikely(current->lockdep_recursion))
@@ -3909,8 +3910,9 @@ EXPORT_SYMBOL_GPL(lock_downgrade);
*/
void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
int trylock, int read, int check,
- struct lockdep_map *nest_lock, unsigned long ip)
+ struct lockdep_map *nest_lock)
{
+ unsigned long ip = _RET_IP_;
unsigned long flags;
if (unlikely(current->lockdep_recursion))
@@ -3928,9 +3930,9 @@ void lock_acquire(struct lockdep_map *lock, unsigned int subclass,
}
EXPORT_SYMBOL_GPL(lock_acquire);
-void lock_release(struct lockdep_map *lock, int nested,
- unsigned long ip)
+void lock_release(struct lockdep_map *lock, int nested)
{
+ unsigned long ip = _RET_IP_;
unsigned long flags;
if (unlikely(current->lockdep_recursion))
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 90b6ab01db59..9c8654be08bb 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1583,7 +1583,7 @@ static void console_lock_spinning_enable(void)
raw_spin_unlock(&console_owner_lock);
/* The waiter may spin on us after setting console_owner */
- spin_acquire(&console_owner_dep_map, 0, 0, _THIS_IP_);
+ spin_acquire(&console_owner_dep_map, 0, 0);
}
/**
@@ -1611,20 +1611,20 @@ static int console_lock_spinning_disable_and_check(void)
raw_spin_unlock(&console_owner_lock);
if (!waiter) {
- spin_release(&console_owner_dep_map, 1, _THIS_IP_);
+ spin_release(&console_owner_dep_map, 1);
return 0;
}
/* The waiter is now free to continue */
WRITE_ONCE(console_waiter, false);
- spin_release(&console_owner_dep_map, 1, _THIS_IP_);
+ spin_release(&console_owner_dep_map, 1);
/*
* Hand off console_lock to waiter. The waiter will perform
* the up(). After this, the waiter is the console_lock owner.
*/
- mutex_release(&console_lock_dep_map, 1, _THIS_IP_);
+ mutex_release(&console_lock_dep_map, 1);
return 1;
}
@@ -1674,11 +1674,11 @@ static int console_trylock_spinning(void)
}
/* We spin waiting for the owner to release us */
- spin_acquire(&console_owner_dep_map, 0, 0, _THIS_IP_);
+ spin_acquire(&console_owner_dep_map, 0, 0);
/* Owner will clear console_waiter on hand off */
while (READ_ONCE(console_waiter))
cpu_relax();
- spin_release(&console_owner_dep_map, 1, _THIS_IP_);
+ spin_release(&console_owner_dep_map, 1);
printk_safe_exit_irqrestore(flags);
/*
@@ -1687,7 +1687,7 @@ static int console_trylock_spinning(void)
* this as a trylock. Otherwise lockdep will
* complain.
*/
- mutex_acquire(&console_lock_dep_map, 0, 1, _THIS_IP_);
+ mutex_acquire(&console_lock_dep_map, 0, 1);
return 1;
}
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 454adf9f8180..a3d146cc2cb9 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2557,7 +2557,7 @@ prepare_lock_switch(struct rq *rq, struct task_struct *next, struct rq_flags *rf
* do an early lockdep release here:
*/
rq_unpin_lock(rq, rf);
- spin_release(&rq->lock.dep_map, 1, _THIS_IP_);
+ spin_release(&rq->lock.dep_map, 1);
#ifdef CONFIG_DEBUG_SPINLOCK
/* this is a valid case when another task releases the spinlock */
rq->lock.owner = next;
@@ -2571,7 +2571,7 @@ static inline void finish_lock_switch(struct rq *rq)
* fix up the runqueue lock - which gets 'carried over' from
* prev into current:
*/
- spin_acquire(&rq->lock.dep_map, 0, 0, _THIS_IP_);
+ spin_acquire(&rq->lock.dep_map, 0, 0);
raw_spin_unlock_irq(&rq->lock);
}
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 1e1bbf171eca..d9599c7d0426 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -1475,7 +1475,7 @@ static void ww_test_edeadlk_normal(void)
mutex_lock(&o2.base);
o2.ctx = &t2;
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
WWAI(&t);
t2 = t;
@@ -1488,7 +1488,7 @@ static void ww_test_edeadlk_normal(void)
WARN_ON(ret != -EDEADLK);
o2.ctx = NULL;
- mutex_acquire(&o2.base.dep_map, 0, 1, _THIS_IP_);
+ mutex_acquire(&o2.base.dep_map, 0, 1);
mutex_unlock(&o2.base);
WWU(&o);
@@ -1500,7 +1500,7 @@ static void ww_test_edeadlk_normal_slow(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
WWAI(&t);
@@ -1514,7 +1514,7 @@ static void ww_test_edeadlk_normal_slow(void)
WARN_ON(ret != -EDEADLK);
o2.ctx = NULL;
- mutex_acquire(&o2.base.dep_map, 0, 1, _THIS_IP_);
+ mutex_acquire(&o2.base.dep_map, 0, 1);
mutex_unlock(&o2.base);
WWU(&o);
@@ -1527,7 +1527,7 @@ static void ww_test_edeadlk_no_unlock(void)
mutex_lock(&o2.base);
o2.ctx = &t2;
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
WWAI(&t);
t2 = t;
@@ -1540,7 +1540,7 @@ static void ww_test_edeadlk_no_unlock(void)
WARN_ON(ret != -EDEADLK);
o2.ctx = NULL;
- mutex_acquire(&o2.base.dep_map, 0, 1, _THIS_IP_);
+ mutex_acquire(&o2.base.dep_map, 0, 1);
mutex_unlock(&o2.base);
WWL(&o2, &t);
@@ -1551,7 +1551,7 @@ static void ww_test_edeadlk_no_unlock_slow(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
WWAI(&t);
@@ -1565,7 +1565,7 @@ static void ww_test_edeadlk_no_unlock_slow(void)
WARN_ON(ret != -EDEADLK);
o2.ctx = NULL;
- mutex_acquire(&o2.base.dep_map, 0, 1, _THIS_IP_);
+ mutex_acquire(&o2.base.dep_map, 0, 1);
mutex_unlock(&o2.base);
ww_mutex_lock_slow(&o2, &t);
@@ -1576,7 +1576,7 @@ static void ww_test_edeadlk_acquire_more(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
WWAI(&t);
@@ -1597,7 +1597,7 @@ static void ww_test_edeadlk_acquire_more_slow(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
WWAI(&t);
@@ -1618,11 +1618,11 @@ static void ww_test_edeadlk_acquire_more_edeadlk(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
mutex_lock(&o3.base);
- mutex_release(&o3.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o3.base.dep_map, 1);
o3.ctx = &t2;
WWAI(&t);
@@ -1644,11 +1644,11 @@ static void ww_test_edeadlk_acquire_more_edeadlk_slow(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
mutex_lock(&o3.base);
- mutex_release(&o3.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o3.base.dep_map, 1);
o3.ctx = &t2;
WWAI(&t);
@@ -1669,7 +1669,7 @@ static void ww_test_edeadlk_acquire_wrong(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
WWAI(&t);
@@ -1694,7 +1694,7 @@ static void ww_test_edeadlk_acquire_wrong_slow(void)
int ret;
mutex_lock(&o2.base);
- mutex_release(&o2.base.dep_map, 1, _THIS_IP_);
+ mutex_release(&o2.base.dep_map, 1);
o2.ctx = &t2;
WWAI(&t);
^ permalink raw reply related
* Re: [PATCH] treewide: remove current_text_addr
From: H. Peter Anvin @ 2018-08-27 12:26 UTC (permalink / raw)
To: Peter Zijlstra, Nick Desaulniers
Cc: Linus Torvalds, deller, Andrew Morton, Eric W . Biederman,
Thomas Gleixner, mingo, Simon Horman, Nathan Chancellor,
Philippe Ombredanne, Kate Stewart, Greg KH, rth, ink, mattst88,
vgupta, linux, Catalin Marinas, Will Deacon, msalter,
jacquiot.aurelien, Yoshinori Sato, rkuo, tony.luck, fenghua.yu,
Geert Uytterhoeven, monstr, ralf, paul.burton, jhogan, green.hu,
deanbo422, lftan, jonas, stefan.kristiansson, Stafford Horne,
jejb, benh, paulus, mpe, palmer, aou, schwidefsky, heiko.carstens,
dalias, David S. Miller, gxt, x86, jdike, richard, chris,
jcmvbkbc, Tobias Klauser, noamc, mickael.guene, nicolas.pitre,
Kees Cook, Dave Martin, Marc Zyngier, alex.bennee, Laura Abbott,
Yury Norov, Mark Rutland, chenhc, macro, Arnd Bergmann, dhowells,
sukadev, Nicholas Piggin, aneesh.kumar, felix, linuxram,
christophe.leroy, cohuck, gor, nick.alcock, shannon.nelson,
nagarathnam.muthusamy, luto, bp, dave.hansen, vkuznets, jkosina,
linux-alpha, LKML, linux-snps-arc, Linux ARM, linux-c6x-dev,
uclinux-h8-devel, linux-hexagon, linux-ia64, linux-m68k,
linux-mips, nios2-dev, openrisc, linux-parisc, linuxppc-dev,
linux-riscv, linux-s390, linux-sh, sparclinux, linux-um
In-Reply-To: <20180827073358.GV24124@hirez.programming.kicks-ass.net>
On 08/27/18 00:33, Peter Zijlstra wrote:
>
> What problem are we trying to solve? _THIS_IP_ and _RET_IP_ work fine.
> We're 'good' at dealing with text addresses, we use them for call stacks
> and all sorts. Why does this need changing?
>
_RET_IP_ works fine, with the following two caveats:
1. To get a unique IP for each call site, the function call needs to be
tailcall protected (easily done by wrapping the function in an
__always_inline function with the notailcall() function I described
earlier. Alternatively, a generic macro wrapper for the same thing:
#define notailcall(x) ({ typeof(x) _x = (x); asm volatile(""); _x; })
2. To uniformly get the return IP, it needs to be defined as:
#define _RET_IP_((unsigned long) \
__builtin_extract_return_addr(__builtin_return_address(0)))
[sorry for the line wrapping]
Using the type unsigned long instead of void * seems kind of pointless
though.
_THIS_IP_, however, is completely ill-defined, other than being an
address *somewhere* in the same global function (not even necessarily
the same function if the function is static!) As my experiment show, in
many (nearly) cases gcc will hoist the address all the way to the top of
the function, at least for the current generic implementation.
For the case where _THIS_IP_ is passed to an out-of-line function in all
cases, it is extra pointless because all it does is increase the
footprint of every caller: _RET_IP_ is inherently passed to the function
anyway, and with tailcall protection it will uniquely identify a callsite.
For the case where _THIS_IP_ is used inline, I believe the version I
described will at the very least avoid hoisting around volatile accesses
like READ_ONCE(). Surrounding the marked code with asm volatile("");
[which should be turned into a macro or inline, obviously] might be
necessary for it to make any kind of inherent sense.
The proposed "location identifier" does have a serious problem: with
inline functions you might very well have a bunch of duplicates pointing
into the inline function, so a single callsite isn't identifiable.
-hpa
^ permalink raw reply
* Re: [PATCH v8 5/5] powernv/pseries: consolidate code for mce early handling.
From: Mahesh Jagannath Salgaonkar @ 2018-08-27 10:32 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linuxppc-dev, Ananth Narayan, Laurent Dufour, Aneesh Kumar K.V,
Michal Suchanek, Michael Ellerman
In-Reply-To: <20180823190258.28570701@roar.ozlabs.ibm.com>
On 08/23/2018 02:32 PM, Nicholas Piggin wrote:
> On Thu, 23 Aug 2018 14:13:13 +0530
> Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>
>> On 08/20/2018 05:04 PM, Nicholas Piggin wrote:
>>> On Sun, 19 Aug 2018 22:38:39 +0530
>>> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>>>
>>>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>>
>>>> Now that other platforms also implements real mode mce handler,
>>>> lets consolidate the code by sharing existing powernv machine check
>>>> early code. Rename machine_check_powernv_early to
>>>> machine_check_common_early and reuse the code.
>>>>
>>>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>> ---
>>>> arch/powerpc/kernel/exceptions-64s.S | 155 ++++++----------------------------
>>>> 1 file changed, 28 insertions(+), 127 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>>>> index 12f056179112..2f85a7baf026 100644
>>>> --- a/arch/powerpc/kernel/exceptions-64s.S
>>>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>>>> @@ -243,14 +243,13 @@ EXC_REAL_BEGIN(machine_check, 0x200, 0x100)
>>>> SET_SCRATCH0(r13) /* save r13 */
>>>> EXCEPTION_PROLOG_0(PACA_EXMC)
>>>> BEGIN_FTR_SECTION
>>>> - b machine_check_powernv_early
>>>> + b machine_check_common_early
>>>> FTR_SECTION_ELSE
>>>> b machine_check_pSeries_0
>>>> ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
>>>> EXC_REAL_END(machine_check, 0x200, 0x100)
>>>> EXC_VIRT_NONE(0x4200, 0x100)
>>>> -TRAMP_REAL_BEGIN(machine_check_powernv_early)
>>>> -BEGIN_FTR_SECTION
>>>> +TRAMP_REAL_BEGIN(machine_check_common_early)
>>>> EXCEPTION_PROLOG_1(PACA_EXMC, NOTEST, 0x200)
>>>> /*
>>>> * Register contents:
>>>> @@ -306,7 +305,9 @@ BEGIN_FTR_SECTION
>>>> /* Save r9 through r13 from EXMC save area to stack frame. */
>>>> EXCEPTION_PROLOG_COMMON_2(PACA_EXMC)
>>>> mfmsr r11 /* get MSR value */
>>>> +BEGIN_FTR_SECTION
>>>> ori r11,r11,MSR_ME /* turn on ME bit */
>>>> +END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
>>>> ori r11,r11,MSR_RI /* turn on RI bit */
>>>> LOAD_HANDLER(r12, machine_check_handle_early)
>>>> 1: mtspr SPRN_SRR0,r12
>>>> @@ -325,7 +326,6 @@ BEGIN_FTR_SECTION
>>>> andc r11,r11,r10 /* Turn off MSR_ME */
>>>> b 1b
>>>> b . /* prevent speculative execution */
>>>> -END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
>>>>
>>>> TRAMP_REAL_BEGIN(machine_check_pSeries)
>>>> .globl machine_check_fwnmi
>>>> @@ -333,7 +333,7 @@ machine_check_fwnmi:
>>>> SET_SCRATCH0(r13) /* save r13 */
>>>> EXCEPTION_PROLOG_0(PACA_EXMC)
>>>> BEGIN_FTR_SECTION
>>>> - b machine_check_pSeries_early
>>>> + b machine_check_common_early
>>>> END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
>>>> machine_check_pSeries_0:
>>>> EXCEPTION_PROLOG_1(PACA_EXMC, KVMTEST_PR, 0x200)
>>>> @@ -346,103 +346,6 @@ machine_check_pSeries_0:
>>>>
>>>> TRAMP_KVM_SKIP(PACA_EXMC, 0x200)
>>>>
>>>> -TRAMP_REAL_BEGIN(machine_check_pSeries_early)
>>>> -BEGIN_FTR_SECTION
>>>> - EXCEPTION_PROLOG_1(PACA_EXMC, NOTEST, 0x200)
>>>> - mr r10,r1 /* Save r1 */
>>>> - lhz r11,PACA_IN_MCE(r13)
>>>> - cmpwi r11,0 /* Are we in nested machine check */
>>>> - bne 0f /* Yes, we are. */
>>>> - /* First machine check entry */
>>>> - ld r1,PACAMCEMERGSP(r13) /* Use MC emergency stack */
>>>> -0: subi r1,r1,INT_FRAME_SIZE /* alloc stack frame */
>>>> - addi r11,r11,1 /* increment paca->in_mce */
>>>> - sth r11,PACA_IN_MCE(r13)
>>>> - /* Limit nested MCE to level 4 to avoid stack overflow */
>>>> - cmpwi r11,MAX_MCE_DEPTH
>>>> - bgt 1f /* Check if we hit limit of 4 */
>>>> - mfspr r11,SPRN_SRR0 /* Save SRR0 */
>>>> - mfspr r12,SPRN_SRR1 /* Save SRR1 */
>>>> - EXCEPTION_PROLOG_COMMON_1()
>>>> - EXCEPTION_PROLOG_COMMON_2(PACA_EXMC)
>>>> - EXCEPTION_PROLOG_COMMON_3(0x200)
>>>> - addi r3,r1,STACK_FRAME_OVERHEAD
>>>> - BRANCH_LINK_TO_FAR(machine_check_early) /* Function call ABI */
>>>> - ld r12,_MSR(r1)
>>>> - andi. r11,r12,MSR_PR /* See if coming from user. */
>>>> - bne 2f /* continue in V mode if we are. */
>>>> -
>>>> - /*
>>>> - * At this point we are not sure about what context we come from.
>>>> - * We may be in the middle of swithing stack. r1 may not be valid.
>>>> - * Hence stay on emergency stack, call machine_check_exception and
>>>> - * return from the interrupt.
>>>> - * But before that, check if this is an un-recoverable exception.
>>>> - * If yes, then stay on emergency stack and panic.
>>>> - */
>>>> - andi. r11,r12,MSR_RI
>>>> - beq 1f
>>>> -
>>>> - /*
>>>> - * Check if we have successfully handled/recovered from error, if not
>>>> - * then stay on emergency stack and panic.
>>>> - */
>>>> - cmpdi r3,0 /* see if we handled MCE successfully */
>>>> - beq 1f /* if !handled then panic */
>>>> -
>>>> - /* Stay on emergency stack and return from interrupt. */
>>>> - LOAD_HANDLER(r10,mce_return)
>>>> - mtspr SPRN_SRR0,r10
>>>> - ld r10,PACAKMSR(r13)
>>>> - mtspr SPRN_SRR1,r10
>>>> - RFI_TO_KERNEL
>>>> - b .
>>>> -
>>>> -1: LOAD_HANDLER(r10,unrecover_mce)
>>>> - mtspr SPRN_SRR0,r10
>>>> - ld r10,PACAKMSR(r13)
>>>> - /*
>>>> - * We are going down. But there are chances that we might get hit by
>>>> - * another MCE during panic path and we may run into unstable state
>>>> - * with no way out. Hence, turn ME bit off while going down, so that
>>>> - * when another MCE is hit during panic path, hypervisor will
>>>> - * power cycle the lpar, instead of getting into MCE loop.
>>>> - */
>>>> - li r3,MSR_ME
>>>> - andc r10,r10,r3 /* Turn off MSR_ME */
>>>> - mtspr SPRN_SRR1,r10
>>>> - RFI_TO_KERNEL
>>>> - b .
>>>> -
>>>> - /* Move original SRR0 and SRR1 into the respective regs */
>>>> -2: ld r9,_MSR(r1)
>>>> - mtspr SPRN_SRR1,r9
>>>> - ld r3,_NIP(r1)
>>>> - mtspr SPRN_SRR0,r3
>>>> - ld r9,_CTR(r1)
>>>> - mtctr r9
>>>> - ld r9,_XER(r1)
>>>> - mtxer r9
>>>> - ld r9,_LINK(r1)
>>>> - mtlr r9
>>>> - REST_GPR(0, r1)
>>>> - REST_8GPRS(2, r1)
>>>> - REST_GPR(10, r1)
>>>> - ld r11,_CCR(r1)
>>>> - mtcr r11
>>>> - /* Decrement paca->in_mce. */
>>>> - lhz r12,PACA_IN_MCE(r13)
>>>> - subi r12,r12,1
>>>> - sth r12,PACA_IN_MCE(r13)
>>>> - REST_GPR(11, r1)
>>>> - REST_2GPRS(12, r1)
>>>> - /* restore original r1. */
>>>> - ld r1,GPR1(r1)
>>>> - SET_SCRATCH0(r13) /* save r13 */
>>>> - EXCEPTION_PROLOG_0(PACA_EXMC)
>>>> - b machine_check_pSeries_0
>>>> -END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
>>>> -
>>>> EXC_COMMON_BEGIN(machine_check_common)
>>>> /*
>>>> * Machine check is different because we use a different
>>>> @@ -541,6 +444,9 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>>>> bl machine_check_early
>>>> std r3,RESULT(r1) /* Save result */
>>>> ld r12,_MSR(r1)
>>>> +BEGIN_FTR_SECTION
>>>> + b 4f
>>>> +END_FTR_SECTION_IFCLR(CPU_FTR_HVMODE)
>>>>
>>>> #ifdef CONFIG_PPC_P7_NAP
>>>> /*
>>>> @@ -564,10 +470,11 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>>>> */
>>>> rldicl. r11,r12,4,63 /* See if MC hit while in HV mode. */
>>>> beq 5f
>>>> - andi. r11,r12,MSR_PR /* See if coming from user. */
>>>> +4: andi. r11,r12,MSR_PR /* See if coming from user. */
>>>> bne 9f /* continue in V mode if we are. */
>>>>
>>>> 5:
>>>> +BEGIN_FTR_SECTION
>>>> #ifdef CONFIG_KVM_BOOK3S_64_HANDLER
>>>> /*
>>>> * We are coming from kernel context. Check if we are coming from
>>>> @@ -578,6 +485,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>>>> cmpwi r11,0 /* Check if coming from guest */
>>>> bne 9f /* continue if we are. */
>>>> #endif
>>>> +END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
>>>
>>> Put these inside the ifdef?
>>>
>>>
>>>> /*
>>>> * At this point we are not sure about what context we come from.
>>>> * Queue up the MCE event and return from the interrupt.
>>>> @@ -611,6 +519,7 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>>>> cmpdi r3,0 /* see if we handled MCE successfully */
>>>>
>>>> beq 1b /* if !handled then panic */
>>>> +BEGIN_FTR_SECTION
>>>> /*
>>>> * Return from MC interrupt.
>>>> * Queue up the MCE event so that we can log it later, while
>>>> @@ -619,10 +528,24 @@ EXC_COMMON_BEGIN(machine_check_handle_early)
>>>> bl machine_check_queue_event
>>>> MACHINE_CHECK_HANDLER_WINDUP
>>>> RFI_TO_USER_OR_KERNEL
>>>> +FTR_SECTION_ELSE
>>>> + /*
>>>> + * pSeries: Return from MC interrupt. Before that stay on emergency
>>>> + * stack and call machine_check_exception to log the MCE event.
>>>> + */
>>>> + LOAD_HANDLER(r10,mce_return)
>>>> + mtspr SPRN_SRR0,r10
>>>> + ld r10,PACAKMSR(r13)
>>>> + mtspr SPRN_SRR1,r10
>>>> + RFI_TO_KERNEL
>>>> + b .
>>>> +ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
>>>
>>> Do you still need mce_return? Why can't you consolidate it as well? ...
>>> Hmm, okay so now I look back at patch 2, I don't think you should call
>>> machine_check_exception there. You're supposed to call
>>> machine_check_queue_event here and it will be handled by irq work.
>>
>> machine_check_queue_event does not handle RTAS mce event.
>
> Yes it would need a bit of work.
>
>> Also, we need
>> to call fwnmi_release_errinfo() as early as possible which is why I am
>> calling machine_check_exception() in mce_return path for pSeries.
>> Otherwise if we get another MCE before calling fwnmi_release_errinfo()
>> then lpar will get rebooted without any logs getting printed.
>
> I think you can call that in your early handler, but then defer
> the printing to the irq work.
>
> Although hmm, maybe that's less of a problem now we do nmi_enter
> in machine check exception so I think printk will use an NMI safe
> buffer.
>
> We have to be careful actually of soft irq state if we take a
> machine check in an un-reconciled state or in the middle of
> the irq replay code I'm not actually sure we do the right thing,
> but that would be a bug in existing code too. And we definitely
> have MSR[RI] vs DAR/DSISR bugs in existing code, sigh.
>
> I don't know... maybe just push what you have and we'll try to do
> some more fixes and cleanups on top of that.
Sure. I will respin the next version addressing your other minor
comments. Will work on more improvements as separate change.
Thanks,
-Mahesh.
^ permalink raw reply
* Re: Build regressions/improvements in v4.19-rc1
From: Geert Uytterhoeven @ 2018-08-27 9:55 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Greentime Hu, Vincent Chen, sparclinux, linuxppc-dev
In-Reply-To: <20180827094926.15512-1-geert@linux-m68k.org>
On Mon, Aug 27, 2018 at 11:50 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Below is the list of build error/warning regressions/improvements in
> v4.19-rc1[1] compared to v4.18[2].
>
> Summarized:
> - build errors: +5/-3
> - build warnings: +14732/-179
>
> Note that there may be false regressions, as some logs are incomplete.
> Still, they're build errors/warnings.
>
> Happy fixing! ;-)
>
> Thanks to the linux-next team for providing the build service.
>
> [1] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/5b394b2ddf0347bef56e50c69a58773c94343ff3/ (all 240 configs)
> [2] http://kisskb.ellerman.id.au/kisskb/branch/linus/head/94710cac0ef4ee177a63b5227664b38c95bbf703/ (239 out of 240 configs)
>
>
> *** ERRORS ***
>
> 5 error regressions:
> + /kisskb/src/arch/nds32/include/asm/elf.h: error: expected '}' before ';' token: => 126:29
nds32-allmodconfig, nds32-allyesconfig
> + error: arch/sparc/kernel/.tmp_head_32.o: relocation truncated to fit: R_SPARC_WDISP22 against `.init.text': => (.head.text+0x5100), (.head.text+0x5040)
> + error: arch/sparc/kernel/.tmp_head_32.o: relocation truncated to fit: R_SPARC_WDISP22 against symbol `leon_smp_cpu_startup' defined in .text section in arch/sparc/kernel/trampoline_32.o: => (.init.text+0xa4)
> + error: arch/sparc/kernel/process_32.o: relocation truncated to fit: R_SPARC_WDISP22 against `.text': => (.fixup+0x4), (.fixup+0xc)
> + error: arch/sparc/kernel/signal_32.o: relocation truncated to fit: R_SPARC_WDISP22 against `.text': => (.fixup+0x0), (.fixup+0x8), (.fixup+0x10), (.fixup+0x18), (.fixup+0x20)
sparc64/sparc-allmodconfig
> *** WARNINGS ***
>
> [Deleted 14564 lines about "warning: -ffunction-sections disabled; it makes profiling impossible [enabled by default]" on powerpc-all{mod,yes}config*]
Parisc finally managed to get rid of these, and now powerpc got them :-(
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 2/2] powerpc/64s/radix: Explicitly flush ERAT with local LPID invalidation
From: Nicholas Piggin @ 2018-08-27 9:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <9fd2cb24976fc3b4534d7ab6186032ac4e27c9d5.camel@kernel.crashing.org>
On Mon, 27 Aug 2018 18:16:05 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Mon, 2018-08-27 at 13:03 +1000, Nicholas Piggin wrote:
> > Local radix TLB flush operations that operate on congruence classes
> > have explicit ERAT flushes for POWER9. The process scoped LPID flush
> > did not have a flush, so add it.
>
> Paul, is that an actual bug ? I think the ERAT is flushed on LPID
> changes...
FWIW I'd like to add the following patch after these fixes. While I
have your attention...
[PATCH] powerpc/64s/radix: keep kernel ERAT over local process/guest
invalidates
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/ppc-opcode.h | 10 +++++++++-
arch/powerpc/mm/tlb-radix.c | 6 +++---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 665af14850e4..5fe617ab680a 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -568,7 +568,15 @@
#define PPC_SLBIA(IH) stringify_in_c(.long PPC_INST_SLBIA | \
((IH & 0x7) << 21))
-#define PPC_INVALIDATE_ERAT PPC_SLBIA(7)
+
+/*
+ * These may only be used by ARCH_300.
+ * GUEST/USER invalidates should only be used by radix mode, on HPT they also
+ * invalidate SLBs so the SLBIA instruction should be used directly.
+ */
+#define PPC_INVALIDATE_ERAT PPC_SLBIA(7)
+#define PPC_INVALIDATE_GUEST_ERAT PPC_SLBIA(6)
+#define PPC_INVALIDATE_USER_ERAT PPC_SLBIA(3)
#define VCMPEQUD_RC(vrt, vra, vrb) stringify_in_c(.long PPC_INST_VCMPEQUD | \
___PPC_RT(vrt) | ___PPC_RA(vra) | \
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 4e798f33c530..6887e4b2568b 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -262,7 +262,7 @@ static inline void _tlbiel_pid(unsigned long pid, unsigned long ric)
__tlbiel_pid(pid, set, RIC_FLUSH_TLB);
asm volatile("ptesync": : :"memory");
- asm volatile(PPC_INVALIDATE_ERAT "; isync" : : :"memory");
+ asm volatile(PPC_INVALIDATE_USER_ERAT "; isync" : : :"memory");
}
static inline void _tlbie_pid(unsigned long pid, unsigned long ric)
@@ -314,7 +314,7 @@ static inline void _tlbiel_lpid(unsigned long lpid, unsigned long ric)
__tlbiel_lpid(lpid, set, RIC_FLUSH_TLB);
asm volatile("ptesync": : :"memory");
- asm volatile(PPC_INVALIDATE_ERAT "; isync" : : :"memory");
+ asm volatile(PPC_INVALIDATE_GUEST_ERAT "; isync" : : :"memory");
}
static inline void _tlbie_lpid(unsigned long lpid, unsigned long ric)
@@ -366,7 +366,7 @@ static inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
__tlbiel_lpid_guest(lpid, set, RIC_FLUSH_TLB);
asm volatile("ptesync": : :"memory");
- asm volatile(PPC_INVALIDATE_ERAT : : :"memory");
+ asm volatile(PPC_INVALIDATE_GUEST_ERAT : : :"memory");
}
--
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc/64s/radix: Explicitly flush ERAT with local LPID invalidation
From: Nicholas Piggin @ 2018-08-27 9:11 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <9fd2cb24976fc3b4534d7ab6186032ac4e27c9d5.camel@kernel.crashing.org>
On Mon, 27 Aug 2018 18:16:05 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Mon, 2018-08-27 at 13:03 +1000, Nicholas Piggin wrote:
> > Local radix TLB flush operations that operate on congruence classes
> > have explicit ERAT flushes for POWER9. The process scoped LPID flush
> > did not have a flush, so add it. =20
>=20
> Paul, is that an actual bug ? I think the ERAT is flushed on LPID
> changes...
We also have a PPC_INVALIDATE_ERAT in tlbiel_lpid. I'd like to add some
comments for these things because I'm not entirely clear on them myself.
The P9 UM says, "Additionally, mtpidr and mtlpidr instructions perform
an implicit slbia with IH =3D x=E2=80=983=E2=80=99."
Although slbia IH=3D3 does not appear to invalidate LPID!=3D0 && PID=3D=3D0
entries (guest OS mappigs).
The UM also says that tlbiels will clear the involved ERATs, but this
might not match hardware?
Thanks,
Nick
>=20
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/mm/tlb-radix.c | 1 +
> > 1 file changed, 1 insertion(+)
> >=20
> > diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> > index fef3e1eb3a19..4e798f33c530 100644
> > --- a/arch/powerpc/mm/tlb-radix.c
> > +++ b/arch/powerpc/mm/tlb-radix.c
> > @@ -366,6 +366,7 @@ static inline void _tlbiel_lpid_guest(unsigned long=
lpid, unsigned long ric)
> > __tlbiel_lpid_guest(lpid, set, RIC_FLUSH_TLB);
> > =20
> > asm volatile("ptesync": : :"memory");
> > + asm volatile(PPC_INVALIDATE_ERAT : : :"memory");
> > }
> > =20
> > =20
>=20
^ permalink raw reply
* [PATCH] powerpc: wire up memtest
From: Christophe Leroy @ 2018-08-27 9:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
Add call to early_memtest() so that kernel compiled with
CONFIG_MEMTEST really perform memtest at startup when requested
via 'memtest' boot parameter.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/setup-common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 93fa0c99681e..904b728eb20d 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -33,6 +33,7 @@
#include <linux/serial_8250.h>
#include <linux/percpu.h>
#include <linux/memblock.h>
+#include <linux/bootmem.h>
#include <linux/of_platform.h>
#include <linux/hugetlb.h>
#include <asm/debugfs.h>
@@ -917,6 +918,8 @@ void __init setup_arch(char **cmdline_p)
/* Parse memory topology */
mem_topology_setup();
+ early_memtest(min_low_pfn << PAGE_SHIFT, max_low_pfn << PAGE_SHIFT);
+
/*
* Release secondary cpus out of their spinloops at 0x60 now that
* we can map physical -> logical CPU ids.
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 16/20] powerpc/dma: use dma_direct_{alloc,free}
From: Scott Wood @ 2018-08-27 8:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christoph Hellwig, Paul Mackerras,
Michael Ellerman, Tony Luck, Fenghua Yu
Cc: Konrad Rzeszutek Wilk, Robin Murphy, linuxppc-dev, iommu,
linux-ia64
In-Reply-To: <4d5c934ea7bc9802eb3653e6afe7583cb05e4484.camel@kernel.crashing.org>
On Thu, 2018-08-09 at 10:52 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2018-07-30 at 18:38 +0200, Christoph Hellwig wrote:
> > These do the same functionality as the existing helpers, but do it
> > simpler, and also allow the (optional) use of CMA.
> >
> > Note that the swiotlb code now calls into the dma_direct code directly,
> > given that it doesn't work with noncoherent caches at all, and isn't
> > called
> > when we have an iommu either, so the iommu special case in
> > dma_nommu_alloc_coherent isn't required for swiotlb.
>
> I am not convinced that this will produce the same results due to
> the way the zone picking works.
>
> As for the interaction with swiotlb, we'll need the FSL guys to have
> a look. Scott, do you remember what this is about ?
dma_direct_alloc() has similar (though not identical[1]) zone picking, so I
think it will work. Needs testing though, and I no longer have a book3e
machine with a PCIe card in it.
The odd thing about this platform (fsl book3e) is the 31-bit[2] limitation on
PCI. We currently use ZONE_DMA32 for this, rather than ZONE_DMA, at Ben's
request[3]. dma_direct_alloc() regards ZONE_DMA32 as being fixed at 32-bits,
but it doesn't really matter as long as limit_zone_pfn() still works, and the
allocation is made below 2 GiB. If we were to switch to ZONE_DMA, and have
both 31-bit and 32-bit zones, then dma_direct_alloc() would have a problem
knowing when to use the 31-bit zone since it's based on a non-power-of-2 limit
that isn't reflected in the dma mask.
-Scott
[1] The logic in dma_direct_alloc() seems wrong -- the zone should need to fit
in the mask, not the other way around. If ARCH_ZONE_DMA_BITS is 24, then
0x007fffff should be a failure rather than GFP_DMA, 0x7fffffff should be
GFP_DMA rather than GFP_DMA32, and 0x3ffffffff should be GFP_DMA32 rather than
an unrestricted allocation (in each case assuming that the end of RAM is
beyond the mask).
[2] The actual limit is closer to 4 GiB, but not quite due to special windows.
swiotlb still uses the real limit when deciding whether to bounce, so the dma
mask is still 32 bits.
[3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/099593.html
^ permalink raw reply
* [PATCH] powerpc/sysdev/ipic: check primary_ipic NULL pointer before using it
From: Christophe Leroy @ 2018-08-27 8:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
ipic_get_mcp_status() is used by targets implementing NMI
watchdog in target specific machine check handler in order
to known whether a machine check results from a watchdog
NMI reset.
In case of very early machine check, primary_ipic pointer
might not have been set yet, so ipic_get_mcp_status() needs
to check it for nullity before using it.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/sysdev/ipic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index 535cf1f6941c..6300123ce965 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -846,7 +846,7 @@ void ipic_disable_mcp(enum ipic_mcp_irq mcp_irq)
u32 ipic_get_mcp_status(void)
{
- return ipic_read(primary_ipic->regs, IPIC_SERSR);
+ return primary_ipic ? ipic_read(primary_ipic->regs, IPIC_SERSR) : 0;
}
void ipic_clear_mcp_status(u32 mask)
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 2/2] powerpc/64s/radix: Explicitly flush ERAT with local LPID invalidation
From: Benjamin Herrenschmidt @ 2018-08-27 8:16 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <20180827030302.17541-3-npiggin@gmail.com>
On Mon, 2018-08-27 at 13:03 +1000, Nicholas Piggin wrote:
> Local radix TLB flush operations that operate on congruence classes
> have explicit ERAT flushes for POWER9. The process scoped LPID flush
> did not have a flush, so add it.
Paul, is that an actual bug ? I think the ERAT is flushed on LPID
changes...
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/mm/tlb-radix.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index fef3e1eb3a19..4e798f33c530 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -366,6 +366,7 @@ static inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
> __tlbiel_lpid_guest(lpid, set, RIC_FLUSH_TLB);
>
> asm volatile("ptesync": : :"memory");
> + asm volatile(PPC_INVALIDATE_ERAT : : :"memory");
> }
>
>
^ permalink raw reply
* 11 minute NTP hw clock update racy?
From: Joakim Tjernlund @ 2018-08-27 8:01 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
V2Ugc2VlIGNvcnJ1cHQgSFcgY2xvY2sgdGltZSBldmVyeSBub3cgYW5kIHRoZW4ocmVhbGx5IGhh
cmQgdG8gcmVwcm9kdWNlKQ0KT3VyIFJUQyBpcyBhIERTMTM4OCBvbiBhbiBJMkMgYnVzLg0KDQpM
b29raW5nIGF0IG50cF9ub3RpZnlfY21vc190aW1lcigpIGFuZCBpdCdzIGRlbGF5ZWQgd29yayBx
dWV1ZSBpbXBsLiBJIHdvbmRlcg0KaWYgdGhlcmUgY291bGQgYmUgYSByYWNlIGhlcmUgdy5yLnQg
cmVib290ID8NCg0KQ291bGQgdGhlIDExIG1pbnV0ZSB1cGRhdGUga2ljayBpbiBqdXN0IGFzIHRo
ZSBzeXN0ZW0gaXMgYWJvdXQgdG8gcmVzZXQNCnRoZSBDUFU/DQoNCkkgYW0gb24gNC4xNC41MSwg
cHBjMzIgYW5kIHVzaW5nIHRoZSBwcGNfbWQucmVzdGFydCgpIGhvb2sgd2hpY2ggd2lsbA0KcmVz
ZXQgdGhlIENQVSBpbW1lZGlhdGVseS4NCg0KUXVlc3Rpb24sIGlzIHNhZmUgdG8gY2FsbCBudHBf
bm90aWZ5X2Ntb3NfdGltZXIoKSB3aGVuIHRoZSB3b3JrIHF1ZXVlIGlzIGFscmVhZHkNCmFybWVk
KGxpa2UgZG9fYWRqdGltZXgoKSBkb2VzKSA/IA0KDQogSm9ja2UNCg==
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: Nicholas Piggin @ 2018-08-27 7:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Anvin, Helge Deller, Nick Desaulniers, linux-alpha,
Linux Kernel Mailing List, arcml, linux-arm-kernel, linux-c6x-dev,
moderated list:H8/300 ARCHITECTURE, linux-hexagon,
linux-ia64@vger.kernel.org, Linux/m68k, linux-mips,
moderated list:NIOS2 ARCHITECTURE, openrisc, Parisc List, ppc-dev,
linux-riscv, linux-s390, Linux-sh list, sparclinux, linux-um
In-Reply-To: <CA+55aFzuSCKfmgT9efHuwtan+m3+bPh4BpwbZwn5gGX_H=Thuw@mail.gmail.com>
[ Trimmed the cc list because my SMTP didn't accept that many
addresses. ]
On Sun, 26 Aug 2018 13:25:14 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, Aug 26, 2018 at 12:32 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >
> > Here is a full-blown (user space) test program demonstrating the whole
> > technique and how to use it.
>
> So while I agree that some _THIS_IP_ users might be better off being
> converted to __builtin_return_address(0) at the caller, I also think
> that the whole "notailcall" thing shows why that can easily be more
> problematic than just our currnet _THIS_IP_ solution.
>
> Honestly, I'd suggest:
>
> - just do the current_text_addr() to _THIS_IP_ conversion
>
> - keep _THIS_IP_ and make it be the generic one, and screw the whole
> "some architectures might implement is better" issue. Nobody cares.
>
> - try to convince people to move away from the "we want the kernel
> instruction pointer for the call" model entirely, and consider this a
> "legacy" issue.
>
> The whole instruction pointer is a nasty thing. We should discourage
> it and not make complex infrastructure for it.
>
> Instead, maybe we could encourage something like
>
> struct kernel_loc { const char *file; const char *fn; int line; };
>
> #define __GEN_LOC__(n) \
> ({ static const struct kernel_loc n = { \
> __FILE__, __FUNCTION__, __LINE__ \
> }; &n; })
>
> #define _THIS_LOC_ __GEN_LOC__(__UNIQUE_ID(loc))
>
> which is a hell of a lot nicer to use, and actually allows gcc to
> optimize things (try it: if you pass a _THIS_LOC_ off to an inline
> function, and that inline function uses the name and line number, gcc
> will pick them up directly, without the extra structure dereference.
>
> Wouldn't it be much nicer to pass these kinds of "location pointer"
> around, rather than the nasty _THIS_IP_ thing?
Seems nice. Do you even need this unique ID thing? AFAIKS the name
would never really be useful.
It could perhaps go into a cold data section too, I assume the common
case is that you do not access it. Although gcc will end up putting
the file and function names into regular rodata.
Possibly we could add a printk specifier for it, pass it through to
existing BUG, etc macros that want exactly this, etc. Makes a lot of
sense.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: Peter Zijlstra @ 2018-08-27 7:33 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Linus Torvalds, hpa, deller, Andrew Morton, Eric W . Biederman,
Thomas Gleixner, mingo, Simon Horman, Nathan Chancellor,
Philippe Ombredanne, Kate Stewart, Greg KH, rth, ink, mattst88,
vgupta, linux, Catalin Marinas, Will Deacon, msalter,
jacquiot.aurelien, Yoshinori Sato, rkuo, tony.luck, fenghua.yu,
Geert Uytterhoeven, monstr, ralf, paul.burton, jhogan, green.hu,
deanbo422, lftan, jonas, stefan.kristiansson, Stafford Horne,
jejb, benh, paulus, mpe, palmer, aou, schwidefsky, heiko.carstens,
dalias, David S. Miller, gxt, x86, jdike, richard, chris,
jcmvbkbc, Tobias Klauser, noamc, mickael.guene, nicolas.pitre,
Kees Cook, Dave Martin, Marc Zyngier, alex.bennee, Laura Abbott,
Yury Norov, Mark Rutland, chenhc, macro, Arnd Bergmann, dhowells,
sukadev, Nicholas Piggin, aneesh.kumar, felix, linuxram,
christophe.leroy, cohuck, gor, nick.alcock, shannon.nelson,
nagarathnam.muthusamy, luto, bp, dave.hansen, vkuznets, jkosina,
linux-alpha, LKML, linux-snps-arc, Linux ARM, linux-c6x-dev,
uclinux-h8-devel, linux-hexagon, linux-ia64, linux-m68k,
linux-mips, nios2-dev, openrisc, linux-parisc, linuxppc-dev,
linux-riscv, linux-s390, linux-sh, sparclinux, linux-um
In-Reply-To: <CAKwvOd=wAaPBkFHAcWxgMW91a--9gbvu7xrt3j-q8c+-mT=7Lw@mail.gmail.com>
On Sun, Aug 26, 2018 at 07:52:59PM -0700, Nick Desaulniers wrote:
> On Sun, Aug 26, 2018 at 1:25 PM Linus Torvalds
> > Instead, maybe we could encourage something like
> >
> > struct kernel_loc { const char *file; const char *fn; int line; };
> >
> > #define __GEN_LOC__(n) \
> > ({ static const struct kernel_loc n = { \
> > __FILE__, __FUNCTION__, __LINE__ \
> > }; &n; })
> >
> > #define _THIS_LOC_ __GEN_LOC__(__UNIQUE_ID(loc))
> >
> > which is a hell of a lot nicer to use, and actually allows gcc to
> > optimize things (try it: if you pass a _THIS_LOC_ off to an inline
> > function, and that inline function uses the name and line number, gcc
> > will pick them up directly, without the extra structure dereference.
> >
> > Wouldn't it be much nicer to pass these kinds of "location pointer"
> > around, rather than the nasty _THIS_IP_ thing?
> >
> > Certainly lockdep looks like it could easily take that "const struct
> > kernel_loc *" instead of "unsigned long ip". Makes it easy to print
> > out the lockdep info.
> This is extremely reasonable. I can follow up with the lockdep folks
> to see if they really need _THIS_IP_ to solve their problem, or if
> there's a simpler solution that can solve their needs. Sometimes
> taking a step back and asking for clarity around the big picture
> allows simpler solutions to shake out.
What problem are we trying to solve? _THIS_IP_ and _RET_IP_ work fine.
We're 'good' at dealing with text addresses, we use them for call stacks
and all sorts. Why does this need changing?
^ permalink raw reply
* [PATCH 2/2] powerpc/64s/radix: Explicitly flush ERAT with local LPID invalidation
From: Nicholas Piggin @ 2018-08-27 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180827030302.17541-1-npiggin@gmail.com>
Local radix TLB flush operations that operate on congruence classes
have explicit ERAT flushes for POWER9. The process scoped LPID flush
did not have a flush, so add it.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/tlb-radix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index fef3e1eb3a19..4e798f33c530 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -366,6 +366,7 @@ static inline void _tlbiel_lpid_guest(unsigned long lpid, unsigned long ric)
__tlbiel_lpid_guest(lpid, set, RIC_FLUSH_TLB);
asm volatile("ptesync": : :"memory");
+ asm volatile(PPC_INVALIDATE_ERAT : : :"memory");
}
--
2.18.0
^ permalink raw reply related
* [PATCH 1/2] powerpc/64s/hash: do not use PPC_INVALIDATE_ERAT on CPUs before POWER9
From: Nicholas Piggin @ 2018-08-27 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180827030302.17541-1-npiggin@gmail.com>
PPC_INVALIDATE_ERAT is slbia IH=7 which is a new variant introduced
with POWER9, and the result is undefined on earlier CPUs.
Commits 7b9f71f974 ("powerpc/64s: POWER9 machine check handler") and
d4748276ae ("powerpc/64s: Improve local TLB flush for boot and MCE on
POWER9") caused POWER7/8 code to use this instruction. Remove it. An
ERAT flush can be made by invalidatig the SLB, but before POWER9 that
requires a flush and rebolt.
Fixes: 7b9f71f974 ("powerpc/64s: POWER9 machine check handler")
Fixes: d4748276ae ("powerpc/64s: Improve local TLB flush for boot and
MCE on POWER9")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/mce_power.c | 7 +++++++
arch/powerpc/mm/hash_native_64.c | 4 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 3497c8329c1d..3022d67f0c48 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -89,6 +89,13 @@ static void flush_and_reload_slb(void)
static void flush_erat(void)
{
+#ifdef CONFIG_PPC_BOOK3S_64
+ if (!early_cpu_has_feature(CPU_FTR_ARCH_300)) {
+ flush_and_reload_slb();
+ return;
+ }
+#endif
+ /* PPC_INVALIDATE_ERAT can only be used on ISA v3 and newer */
asm volatile(PPC_INVALIDATE_ERAT : : :"memory");
}
diff --git a/arch/powerpc/mm/hash_native_64.c b/arch/powerpc/mm/hash_native_64.c
index 729f02df8290..aaa28fd918fe 100644
--- a/arch/powerpc/mm/hash_native_64.c
+++ b/arch/powerpc/mm/hash_native_64.c
@@ -115,6 +115,8 @@ static void tlbiel_all_isa300(unsigned int num_sets, unsigned int is)
tlbiel_hash_set_isa300(0, is, 0, 2, 1);
asm volatile("ptesync": : :"memory");
+
+ asm volatile(PPC_INVALIDATE_ERAT "; isync" : : :"memory");
}
void hash__tlbiel_all(unsigned int action)
@@ -140,8 +142,6 @@ void hash__tlbiel_all(unsigned int action)
tlbiel_all_isa206(POWER7_TLB_SETS, is);
else
WARN(1, "%s called on pre-POWER7 CPU\n", __func__);
-
- asm volatile(PPC_INVALIDATE_ERAT "; isync" : : :"memory");
}
static inline unsigned long ___tlbie(unsigned long vpn, int psize,
--
2.18.0
^ permalink raw reply related
* [PATCH 0/2] ERAT invalidation fixes
From: Nicholas Piggin @ 2018-08-27 3:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
First one is a real bug-fix for POWER8 and earlier CPUs, second I'm
not exactly sure how POWER9's ERAT behaves in this situation but it
seems to match what we do in other places.
Thanks,
Nick
Nicholas Piggin (2):
powerpc/64s/hash: do not use PPC_INVALIDATE_ERAT on CPUs before POWER9
powerpc/64s/radix: Explicitly flush ERAT with local LPID invalidation
arch/powerpc/kernel/mce_power.c | 7 +++++++
arch/powerpc/mm/hash_native_64.c | 4 ++--
arch/powerpc/mm/tlb-radix.c | 1 +
3 files changed, 10 insertions(+), 2 deletions(-)
--
2.18.0
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: Nick Desaulniers @ 2018-08-27 2:52 UTC (permalink / raw)
To: Linus Torvalds
Cc: hpa, Peter Zijlstra, deller, Andrew Morton, Eric W . Biederman,
Thomas Gleixner, mingo, Simon Horman, Nathan Chancellor,
Philippe Ombredanne, Kate Stewart, Greg KH, rth, ink, mattst88,
vgupta, linux, Catalin Marinas, Will Deacon, msalter,
jacquiot.aurelien, Yoshinori Sato, rkuo, tony.luck, fenghua.yu,
Geert Uytterhoeven, monstr, ralf, paul.burton, jhogan, green.hu,
deanbo422, lftan, jonas, stefan.kristiansson, Stafford Horne,
jejb, benh, paulus, mpe, palmer, aou, schwidefsky, heiko.carstens,
dalias, David S. Miller, gxt, x86, jdike, richard, chris,
jcmvbkbc, Tobias Klauser, noamc, mickael.guene, nicolas.pitre,
Kees Cook, Dave Martin, Marc Zyngier, alex.bennee, Laura Abbott,
Yury Norov, Mark Rutland, chenhc, macro, Arnd Bergmann, dhowells,
sukadev, Nicholas Piggin, aneesh.kumar, felix, linuxram,
christophe.leroy, cohuck, gor, nick.alcock, shannon.nelson,
nagarathnam.muthusamy, luto, bp, dave.hansen, vkuznets, jkosina,
linux-alpha, LKML, linux-snps-arc, Linux ARM, linux-c6x-dev,
uclinux-h8-devel, linux-hexagon, linux-ia64, linux-m68k,
linux-mips, nios2-dev, openrisc, linux-parisc, linuxppc-dev,
linux-riscv, linux-s390, linux-sh, sparclinux, linux-um
In-Reply-To: <CA+55aFzuSCKfmgT9efHuwtan+m3+bPh4BpwbZwn5gGX_H=Thuw@mail.gmail.com>
On Sun, Aug 26, 2018 at 1:25 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Honestly, I'd suggest:
>
> - just do the current_text_addr() to _THIS_IP_ conversion
>
> - keep _THIS_IP_ and make it be the generic one, and screw the whole
> "some architectures might implement is better" issue. Nobody cares.
And mention it to the compiler vendors as this seems like a case where
code gen can be improved.
>
> - try to convince people to move away from the "we want the kernel
> instruction pointer for the call" model entirely, and consider this a
> "legacy" issue.
>
> The whole instruction pointer is a nasty thing. We should discourage
> it and not make complex infrastructure for it.
Yes, please. I think we should strive for simplicity here.
>
> Instead, maybe we could encourage something like
>
> struct kernel_loc { const char *file; const char *fn; int line; };
>
> #define __GEN_LOC__(n) \
> ({ static const struct kernel_loc n = { \
> __FILE__, __FUNCTION__, __LINE__ \
> }; &n; })
>
> #define _THIS_LOC_ __GEN_LOC__(__UNIQUE_ID(loc))
>
> which is a hell of a lot nicer to use, and actually allows gcc to
> optimize things (try it: if you pass a _THIS_LOC_ off to an inline
> function, and that inline function uses the name and line number, gcc
> will pick them up directly, without the extra structure dereference.
>
> Wouldn't it be much nicer to pass these kinds of "location pointer"
> around, rather than the nasty _THIS_IP_ thing?
>
> Certainly lockdep looks like it could easily take that "const struct
> kernel_loc *" instead of "unsigned long ip". Makes it easy to print
> out the lockdep info.
>
> Ok, I didn't try to convert anybody, so maybe people who currently use
> _THIS_IP_ or current_text_addr() have some fundamental reason why they
> want just that, but let's not male _THIS_IP_ more complex than it
> needs to be.
>
> Hmm?
>
> Linus
This is extremely reasonable. I can follow up with the lockdep folks
to see if they really need _THIS_IP_ to solve their problem, or if
there's a simpler solution that can solve their needs. Sometimes
taking a step back and asking for clarity around the big picture
allows simpler solutions to shake out.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* [PATCH] powerpc: remove pre-build checks for old GCC versions
From: Masahiro Yamada @ 2018-08-27 2:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Masahiro Yamada, linuxppc-dev, linux-kernel
Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6")
bumped the minimum GCC version to 4.6 for all architectures.
The GCC version checks in arch/powerpc/Makefile are obsolete now.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
arch/powerpc/Makefile | 38 +-------------------------------------
1 file changed, 1 insertion(+), 37 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 11a1acb..57e7358 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -400,40 +400,8 @@ archclean:
archprepare: checkbin
-# Use the file '.tmp_gas_check' for binutils tests, as gas won't output
-# to stdout and these checks are run even on install targets.
-TOUT := .tmp_gas_check
-
-# Check gcc and binutils versions:
-# - gcc-3.4 and binutils-2.14 are a fatal combination
-# - Require gcc 4.0 or above on 64-bit
-# - gcc-4.2.0 has issues compiling modules on 64-bit
+# Check binutils versions:
checkbin:
- @if test "$(cc-name)" != "clang" \
- && test "$(cc-version)" = "0304" ; then \
- if ! /bin/echo mftb 5 | $(AS) -v -mppc -many -o $(TOUT) >/dev/null 2>&1 ; then \
- echo -n '*** ${VERSION}.${PATCHLEVEL} kernels no longer build '; \
- echo 'correctly with gcc-3.4 and your version of binutils.'; \
- echo '*** Please upgrade your binutils or downgrade your gcc'; \
- false; \
- fi ; \
- fi
- @if test "$(cc-name)" != "clang" \
- && test "$(cc-version)" -lt "0400" \
- && test "x${CONFIG_PPC64}" = "xy" ; then \
- echo -n "Sorry, GCC v4.0 or above is required to build " ; \
- echo "the 64-bit powerpc kernel." ; \
- false ; \
- fi
- @if test "$(cc-name)" != "clang" \
- && test "$(cc-fullversion)" = "040200" \
- && test "x${CONFIG_MODULES}${CONFIG_PPC64}" = "xyy" ; then \
- echo -n '*** GCC-4.2.0 cannot compile the 64-bit powerpc ' ; \
- echo 'kernel with modules enabled.' ; \
- echo -n '*** Please use a different GCC version or ' ; \
- echo 'disable kernel modules' ; \
- false ; \
- fi
@if test "x${CONFIG_CPU_LITTLE_ENDIAN}" = "xy" \
&& $(LD) --version | head -1 | grep ' 2\.24$$' >/dev/null ; then \
echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
@@ -441,7 +409,3 @@ checkbin:
echo -n '*** Please use a different binutils version.' ; \
false ; \
fi
-
-
-CLEAN_FILES += $(TOUT)
-
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 6/8] kbuild: consolidate Devicetree dtb build rules
From: Rob Herring @ 2018-08-26 23:56 UTC (permalink / raw)
To: Masahiro Yamada
Cc: devicetree, linux-kernel@vger.kernel.org, Frank Rowand,
Michal Marek, Vineet Gupta, Russell King, Catalin Marinas,
Will Deacon, Yoshinori Sato, Michal Simek, Ralf Baechle,
Paul Burton, James Hogan, Ley Foon Tan, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Chris Zankel, Max Filippov,
Linux Kbuild mailing list, arcml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:H8/300 ARCHITECTURE, Linux-MIPS, nios2-dev,
linuxppc-dev, linux-xtensa
In-Reply-To: <CAK7LNATcYSvwh0BEEdyUuDa_Y9X-AqQAzA5MrbNhOSMmSzqhTg@mail.gmail.com>
On Sat, Aug 25, 2018 at 9:06 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Hi Rob,
>
>
> 2018-08-22 6:55 GMT+09:00 Rob Herring <robh@kernel.org>:
> > There is nothing arch specific about building dtb files other than their
> > location under /arch/*/boot/dts/. Keeping each arch aligned is a pain.
> > The dependencies and supported targets are all slightly different.
> > Also, a cross-compiler for each arch is needed, but really the host
> > compiler preprocessor is perfectly fine for building dtbs. Move the
> > build rules to a common location and remove the arch specific ones. This
> > is done in a single step to avoid warnings about overriding rules.
> >
> > The build dependencies had been a mixture of 'scripts' and/or 'prepare'.
> > These pull in several dependencies some of which need a target compiler
> > (specifically devicetable-offsets.h) and aren't needed to build dtbs.
> > All that is really needed is dtc, so adjust the dependencies to only be
> > dtc.
> >
> > This change enables support 'dtbs_install' on some arches which were
> > missing the target.
> >
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Cc: Michal Marek <michal.lkml@markovi.net>
> > Cc: Vineet Gupta <vgupta@synopsys.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> > Cc: Michal Simek <monstr@monstr.eu>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Paul Burton <paul.burton@mips.com>
> > Cc: James Hogan <jhogan@kernel.org>
> > Cc: Ley Foon Tan <lftan@altera.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Chris Zankel <chris@zankel.net>
> > Cc: Max Filippov <jcmvbkbc@gmail.com>
> > Cc: linux-kbuild@vger.kernel.org
> > Cc: linux-snps-arc@lists.infradead.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: uclinux-h8-devel@lists.sourceforge.jp
> > Cc: linux-mips@linux-mips.org
> > Cc: nios2-dev@lists.rocketboards.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-xtensa@linux-xtensa.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > Makefile | 30 ++++++++++++++++++++++++++++++
> > arch/arc/Makefile | 6 ------
> > arch/arm/Makefile | 20 +-------------------
> > arch/arm64/Makefile | 17 +----------------
> > arch/c6x/Makefile | 2 --
> > arch/h8300/Makefile | 11 +----------
> > arch/microblaze/Makefile | 4 +---
> > arch/mips/Makefile | 15 +--------------
> > arch/nds32/Makefile | 2 +-
> > arch/nios2/Makefile | 7 -------
> > arch/nios2/boot/Makefile | 4 ----
> > arch/powerpc/Makefile | 3 ---
> > arch/xtensa/Makefile | 12 +-----------
> > scripts/Makefile | 1 -
> > scripts/Makefile.lib | 2 +-
> > 15 files changed, 38 insertions(+), 98 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index c13f8b85ba60..6d89e673f192 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1212,6 +1212,30 @@ kselftest-merge:
> > $(srctree)/tools/testing/selftests/*/config
> > +$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
> >
> > +# ---------------------------------------------------------------------------
> > +# Devicetree files
> > +
> > +dtstree := $(wildcard arch/$(SRCARCH)/boot/dts)
BTW, there's an error here too. It doesn't work right with
KBUILD_OUTPUT set and should be:
ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/boot/dts/),)
dtstree := arch/$(SRCARCH)/boot/dts
endif
> > +
> > +ifdef CONFIG_OF_EARLY_FLATTREE
> > +
> > +%.dtb %.dtb.S %.dtb.o: | dtc
>
> I think the pipe operator is unnecessary
> because Kbuild will descend to $(dtstree) anyway.
The pipe means 'order-only', right? So it is just a weaker dependency
for things which are not input files as dtc is not. The 'dtc' here is
just the dtc rule below, not the actual executable. Or am I missing
something?
> > + $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@
> > +
> > +PHONY += dtbs
> > +dtbs: | dtc
>
> Ditto.
>
>
> > + $(Q)$(MAKE) $(build)=$(dtstree)
> > +
> > +dtbs_install: dtbs
> > + $(Q)$(MAKE) $(dtbinst)=$(dtstree)
> > +
> > +all: dtbs
> > +
> > +dtc:
> > + $(Q)$(MAKE) $(build)=scripts/dtc
> > +
> > +endif
> > +
>
>
> arch/*/boot/dts/ are not only directories that
> require dtc.
Ah yes, of course...
> > diff --git a/scripts/Makefile b/scripts/Makefile
> > index 61affa300d25..a716a6b10954 100644
> > --- a/scripts/Makefile
> > +++ b/scripts/Makefile
> > @@ -39,7 +39,6 @@ build_unifdef: $(obj)/unifdef
> > subdir-$(CONFIG_MODVERSIONS) += genksyms
> > subdir-y += mod
> > subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > -subdir-$(CONFIG_DTC) += dtc
> > subdir-$(CONFIG_GDB_SCRIPTS) += gdb
> >
> > # Let clean descend into subdirs
Looks like I need to leave this line to fix the above and cleaning.
Thanks for the review.
Rob
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: H. Peter Anvin @ 2018-08-26 23:20 UTC (permalink / raw)
To: Helge Deller, Nick Desaulniers, torvalds, akpm
Cc: ebiederm, tglx, mingo, horms, natechancellor, pombredanne,
kstewart, gregkh, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Vineet Gupta, Russell King, Catalin Marinas, Will Deacon,
Mark Salter, Aurelien Jacquiot, Yoshinori Sato, Richard Kuo,
Tony Luck, Fenghua Yu, Geert Uytterhoeven, Michal Simek,
Ralf Baechle, Paul Burton, James Hogan, Greentime Hu,
Vincent Chen, Ley Foon Tan, Jonas Bonn, Stefan Kristiansson,
Stafford Horne, James E.J. Bottomley, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller,
Guan Xuetao, x86, Jeff Dike, Richard Weinberger, Chris Zankel,
Max Filippov, Tobias Klauser, Noam Camus, Mickael GUENE,
Nicolas Pitre, Kees Cook, Dave Martin, Marc Zyngier,
Alex Bennée, Laura Abbott, Yury Norov, Mark Rutland,
Huacai Chen, Maciej W. Rozycki, Arnd Bergmann, David Howells,
Sukadev Bhattiprolu, Nicholas Piggin, Aneesh Kumar K.V,
Philippe Bergheaud, Ram Pai, Christophe Leroy, Cornelia Huck,
Vasily Gorbik, Nick Alcock, Shannon Nelson, Nagarathnam Muthusamy,
Andy Lutomirski, Borislav Petkov, Dave Hansen, Vitaly Kuznetsov,
Jiri Kosina, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-c6x-dev, uclinux-h8-devel, linux-hexagon,
linux-ia64, linux-m68k, linux-mips, nios2-dev, openrisc,
linux-parisc, linuxppc-dev, linux-riscv, linux-s390, linux-sh,
sparclinux, linux-um
In-Reply-To: <6ca8a1d3-ff95-e9f4-f003-0a5af85bcb6f@zytor.com>
On 08/26/18 12:30, H. Peter Anvin wrote:
> Here is a full-blown (user space) test program demonstrating the whole
> technique and how to use it.
>
> -hpa
Incidentally, it looks like _RET_IP_ really should be defined as:
/*
* Is there any reason whatsoever to have _RET_IP_ an unsigned int
* rather than a pointer throughout?
*/
#define _RET_IP_PTR_ \
__builtin_extract_return_addr(__builtin_return_addr(0))
#define _RET_IP_ ((unsigned long)_RET_IP_PTR_)
On some architectures __builtin_extract_return_addr() is apparently
necessary; its a nop on x86. Why that isn't part of
__builtin_return_addr() one can really wonder.
So, checking into all of this, the generic _THIS_IP_ DOES NOT WORK on
x86. I have tried a tons of variants, including adding various asm
volatile(...) instructions, and no matter what I do, it will always
return the address of the surrounding function rather than any kind of
local IP. The only way to get a localized address seems to be in
assembly, but even so, there is absolutely no guarantee that the value
of _THIS_IP_ has anything to do with where the code is otherwise
localized in the function.
>From examining the output of gcc, the fundamental problem seems to be
that *no matter what* one do with the label, unless gcc actually
produces a computed goto somewhere in the code, that it can't remove
with dead-code elimination or constant propagation, it will arbitrarily
hoist the labels all the way to the beginning of the function. Given
that, I suspect that other versions of gcc might have similar problems.
This is the closest thing to arch-neutral I have been able to find that
also works on x86, while not at the same time horribly polluting the
namespace:
#define __here(n) ___here_ ## n
#define __hereasm(n) ".L___here_" #n
#define _THIS_IP_CTR_(n) \
({ \
extern const char __here(n) asm(__hereasm(n)); \
asm volatile(__hereasm(n) ": /* _THIS_IP_ */"); \
(unsigned long)&__here(n); \
})
#define _THIS_IP_ _THIS_IP_CTR_(__COUNTER__)
The use of asm volatile() to define a label means that the position in
the instruction stream is at least reasonably well-defined.
-hpa
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: Linus Torvalds @ 2018-08-26 20:25 UTC (permalink / raw)
To: Peter Anvin, Peter Zijlstra
Cc: Helge Deller, Nick Desaulniers, Andrew Morton, Eric W. Biederman,
Thomas Gleixner, Ingo Molnar, Simon Horman, Nathan Chancellor,
Philippe Ombredanne, Kate Stewart, Greg Kroah-Hartman,
Richard Henderson, Ivan Kokshaysky, Matt Turner, Vineet Gupta,
Russell King - ARM Linux, Catalin Marinas, Will Deacon,
Mark Salter, jacquiot.aurelien, Yoshinori Sato, Richard Kuo,
Tony Luck, Fenghua Yu, Geert Uytterhoeven, Michal Simek,
Ralf Baechle, Paul Burton, James Hogan, Greentime Hu, deanbo422,
Ley Foon Tan, Jonas Bonn, Stefan Kristiansson, Stafford Horne,
James E.J. Bottomley, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Palmer Dabbelt, aou, Martin Schwidefsky,
Heiko Carstens, Rich Felker, David Miller, gxt,
the arch/x86 maintainers, Jeff Dike, Richard Weinberger,
Chris Zankel, Max Filippov, Tobias Klauser, noamc, mickael.guene,
Nicolas Pitre, Kees Cook, Dave Martin, Marc Zyngier, alex.bennee,
Laura Abbott, ynorov, Mark Rutland, chenhc@lemote.com,
Maciej W. Rozycki, Arnd Bergmann, David Howells, sukadev,
Nick Piggin, Aneesh Kumar K. V, felix, Ram Pai, Christophe Leroy,
Cornelia Huck, gor, nick.alcock, shannon.nelson,
nagarathnam.muthusamy, Andrew Lutomirski, Borislav Petkov,
Dave Hansen, Vitaly Kuznetsov, Jiri Kosina, linux-alpha,
Linux Kernel Mailing List, arcml, linux-arm-kernel, linux-c6x-dev,
moderated list:H8/300 ARCHITECTURE, linux-hexagon,
linux-ia64@vger.kernel.org, Linux/m68k, linux-mips,
moderated list:NIOS2 ARCHITECTURE, openrisc, Parisc List, ppc-dev,
linux-riscv, linux-s390, Linux-sh list, sparclinux, linux-um
In-Reply-To: <6ca8a1d3-ff95-e9f4-f003-0a5af85bcb6f@zytor.com>
On Sun, Aug 26, 2018 at 12:32 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> Here is a full-blown (user space) test program demonstrating the whole
> technique and how to use it.
So while I agree that some _THIS_IP_ users might be better off being
converted to __builtin_return_address(0) at the caller, I also think
that the whole "notailcall" thing shows why that can easily be more
problematic than just our currnet _THIS_IP_ solution.
Honestly, I'd suggest:
- just do the current_text_addr() to _THIS_IP_ conversion
- keep _THIS_IP_ and make it be the generic one, and screw the whole
"some architectures might implement is better" issue. Nobody cares.
- try to convince people to move away from the "we want the kernel
instruction pointer for the call" model entirely, and consider this a
"legacy" issue.
The whole instruction pointer is a nasty thing. We should discourage
it and not make complex infrastructure for it.
Instead, maybe we could encourage something like
struct kernel_loc { const char *file; const char *fn; int line; };
#define __GEN_LOC__(n) \
({ static const struct kernel_loc n = { \
__FILE__, __FUNCTION__, __LINE__ \
}; &n; })
#define _THIS_LOC_ __GEN_LOC__(__UNIQUE_ID(loc))
which is a hell of a lot nicer to use, and actually allows gcc to
optimize things (try it: if you pass a _THIS_LOC_ off to an inline
function, and that inline function uses the name and line number, gcc
will pick them up directly, without the extra structure dereference.
Wouldn't it be much nicer to pass these kinds of "location pointer"
around, rather than the nasty _THIS_IP_ thing?
Certainly lockdep looks like it could easily take that "const struct
kernel_loc *" instead of "unsigned long ip". Makes it easy to print
out the lockdep info.
Ok, I didn't try to convert anybody, so maybe people who currently use
_THIS_IP_ or current_text_addr() have some fundamental reason why they
want just that, but let's not male _THIS_IP_ more complex than it
needs to be.
Hmm?
Linus
^ permalink raw reply
* Re: [PATCH] treewide: remove current_text_addr
From: H. Peter Anvin @ 2018-08-26 19:30 UTC (permalink / raw)
To: Helge Deller, Nick Desaulniers, torvalds, akpm
Cc: ebiederm, tglx, mingo, horms, natechancellor, pombredanne,
kstewart, gregkh, Richard Henderson, Ivan Kokshaysky, Matt Turner,
Vineet Gupta, Russell King, Catalin Marinas, Will Deacon,
Mark Salter, Aurelien Jacquiot, Yoshinori Sato, Richard Kuo,
Tony Luck, Fenghua Yu, Geert Uytterhoeven, Michal Simek,
Ralf Baechle, Paul Burton, James Hogan, Greentime Hu,
Vincent Chen, Ley Foon Tan, Jonas Bonn, Stefan Kristiansson,
Stafford Horne, James E.J. Bottomley, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Palmer Dabbelt, Albert Ou,
Martin Schwidefsky, Heiko Carstens, Rich Felker, David S. Miller,
Guan Xuetao, x86, Jeff Dike, Richard Weinberger, Chris Zankel,
Max Filippov, Tobias Klauser, Noam Camus, Mickael GUENE,
Nicolas Pitre, Kees Cook, Dave Martin, Marc Zyngier,
Alex Bennée, Laura Abbott, Yury Norov, Mark Rutland,
Huacai Chen, Maciej W. Rozycki, Arnd Bergmann, David Howells,
Sukadev Bhattiprolu, Nicholas Piggin, Aneesh Kumar K.V,
Philippe Bergheaud, Ram Pai, Christophe Leroy, Cornelia Huck,
Vasily Gorbik, Nick Alcock, Shannon Nelson, Nagarathnam Muthusamy,
Andy Lutomirski, Borislav Petkov, Dave Hansen, Vitaly Kuznetsov,
Jiri Kosina, linux-alpha, linux-kernel, linux-snps-arc,
linux-arm-kernel, linux-c6x-dev, uclinux-h8-devel, linux-hexagon,
linux-ia64, linux-m68k, linux-mips, nios2-dev, openrisc,
linux-parisc, linuxppc-dev, linux-riscv, linux-s390, linux-sh,
sparclinux, linux-um
In-Reply-To: <7f49eeab-a5cc-867f-58fb-abd266f9c2c9@zytor.com>
[-- Attachment #1: Type: text/plain, Size: 108 bytes --]
Here is a full-blown (user space) test program demonstrating the whole
technique and how to use it.
-hpa
[-- Attachment #2: str.c --]
[-- Type: text/x-csrc, Size: 4208 bytes --]
#include <stddef.h>
#include <string.h>
#define _RET_IP_ ((unsigned long)__builtin_return_address(0))
#define noinline __attribute__((noinline))
#define used __attribute__((used))
/* __always_inline is defined in glibc already */
#define ifconst(x,y) __builtin_choose_expr(__builtin_constant_p(x),(x),(y))
static inline void notailcall(void)
{
asm volatile("");
}
/* Change this to a null string to make all functions global */
#define STATIC static
struct myputs_string {
unsigned short len;
char str[0];
};
STATIC int _myputs_struct(const struct myputs_string * const strs);
STATIC int _myputs_string(const char *str);
STATIC int __myputs(unsigned long ip, const char *str, size_t len);
#if 1
#include <stdio.h>
STATIC void dump_caller(unsigned long where)
{
const char *opname = NULL;
const char *wheretoname = NULL;
char ichar;
unsigned long whereto = 0;
#if defined(__i386__) || defined(__x86_64__)
char opname_buf[4];
unsigned char opcode;
where -= 5;
opcode = *(unsigned char *)where;
switch (opcode) {
case 0xe8:
opname = "call";
whereto = where + 5 + *(signed int *)(where + 1);
break;
case 0xe9:
opname = "jmp";
whereto = where + 5 + *(signed int *)(where + 1);
break;
default:
snprintf(opname_buf, sizeof opname_buf, "?%02x", opcode);
opname = opname_buf;
break;
}
#elif defined(__sparc__)
const char regtype[4] = "gilo";
unsigned int opcode, op1, op3, ibit;
signed int simm13, simm30;
char opname_buf[32];
char *p;
where -= 8;
opcode = *(signed int *)where;
op1 = opcode >> 30;
op3 = (opcode >> 19) & 0x3f;
ibit = (opcode >> 13) & 1;
simm13 = (opcode & 0x1fff) << 2;
simm30 = (opcode & 0x3fffffff) << 2;
opname = opname_buf;
if (op1 == 1) {
opname = "call";
whereto = where + simm30;
} else if (op1 == 2 && op3 == 0x38) {
if (ibit) {
snprintf(opname_buf, sizeof opname_buf,
"jmpl %%%c%u %c 0x%x",
regtype[(opcode >> 17) & 3],
(opcode >> 14) & 7,
simm13 < 0 ? '-' : '+',
abs(simm13));
} else {
snprintf(opname_buf, sizeof opname_buf,
"jmpl %%%c%u + %%%c%u",
regtype[(opcode >> 17) & 3],
(opcode >> 14) & 7,
regtype[(opcode >> 3) & 3],
opcode & 7);
}
} else {
snprintf(opname_buf, sizeof opname_buf,
"?0x08x", opcode);
}
#else
/* Unknown architecture */
#endif
if (whereto == (unsigned long)_myputs_struct) {
wheretoname = "_myputs_struct";
} else if (whereto == (unsigned long)_myputs_string) {
wheretoname = "_myputs_string";
} else {
wheretoname = "?";
}
ichar = '[';
if (opname) {
printf("%c%p: %s",
ichar, (void *)where, opname);
ichar = ' ';
}
if (whereto) {
printf("%c%p <%s>", ichar, (void *)whereto, wheretoname);
ichar = ' ';
}
if (ichar != '[')
putchar(']');
}
STATIC int __myputs(unsigned long where, const char *str, size_t len)
{
size_t slen = strlen(str);
size_t rv;
len--;
rv = printf("%p: \"%.*s\"%*s", (void *)where, (int)len, str,
16-(int)slen, "");
dump_caller(where);
if (slen != len)
printf(" <err: strlen = %zu, len = %zu>\n", slen, len);
else
printf(" <ok: len = %zu>\n", len);
return rv;
}
STATIC int noinline _myputs_struct(const struct myputs_string * const strs)
{
return __myputs(_RET_IP_, strs->str, strs->len);
}
STATIC int noinline _myputs_string(const char *str)
{
return __myputs(_RET_IP_, str, strlen(str)+1);
}
#endif
#define myputs(s) \
({ \
int _rv; \
if (__builtin_constant_p(s) && \
__builtin_constant_p(strlen(s)) && \
strlen(s)+1 == sizeof(s) && \
sizeof(s) <= (size_t)65535) { \
static const struct { \
struct myputs_string _mps_hdr; \
char _mps_str[sizeof(s)]; \
} _mps = { \
._mps_hdr.len = sizeof(s), \
._mps_str = ifconst(s,""), \
}; \
_rv = _myputs_struct(&_mps._mps_hdr); \
} else { \
_rv = _myputs_string(s); \
} \
notailcall(); \
_rv; \
})
STATIC int test1(void);
STATIC int test2(const char *strx);
STATIC int test1(void)
{
return myputs("Foobar");
}
STATIC int test2(const char *strx)
{
return myputs(strx);
}
int main(int argc, char *argv[])
{
(void)argc;
test1();
test2(argv[0]);
return 0;
}
^ 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