* Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
From: Thomas Gleixner @ 2014-03-22 22:03 UTC (permalink / raw)
To: James Bottomley
Cc: Fenghua Yu, linux-ia64, Tony Luck, Russell King, linux-scsi,
linux-s390, x86, Heiko Carstens, linux-kernel, linux390,
sparclinux, Ingo Molnar, Paul Mackerras, H. Peter Anvin,
Martin Schwidefsky, Laura Abbott, Andrew Morton, linuxppc-dev,
David S. Miller, linux-arm-kernel
In-Reply-To: <1395523881.2143.58.camel@dabdike.int.hansenpartnership.com>
On Sat, 22 Mar 2014, James Bottomley wrote:
> On Sat, 2014-03-22 at 11:13 -0700, Laura Abbott wrote:
> > Rather than have architectures #define ARCH_HAS_SG_CHAIN in an architecture
> > specific scatterlist.h, make it a proper Kconfig option and use that
> > instead. At same time, remove the header files are are now mostly
> > useless and just include asm-generic/scatterlist.h.
>
> Well, the transformation looks fine. Perhaps part of the reason for the
> lack of response is that there's no compelling reason in the change log
> above for doing this. The usual reason for eliminating ARCH_HAS is that
> it's hiding something that would be better expressed a different way
> (that's actually intuitive to grep) or that it's expressing something
> that should be configurable. Neither of these reasons apply in this
> case, because SG_CHAIN definitely is a property of the architecture not
> the config space and it's not really hiding anything.
Getting rid of pointless copied code is definitely a good enough
reason and the patch removes quite some of that.
Thanks,
tglx
^ permalink raw reply
* Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
From: James Bottomley @ 2014-03-22 21:31 UTC (permalink / raw)
To: Laura Abbott
Cc: Fenghua Yu, Tony Luck, Russell King, Andrew Morton, linux-scsi,
linux-s390, x86, Heiko Carstens, linux-kernel, linux390,
sparclinux, Ingo Molnar, Paul Mackerras, H. Peter Anvin,
Martin Schwidefsky, linux-ia64, Thomas Gleixner, linuxppc-dev,
David S. Miller, linux-arm-kernel
In-Reply-To: <1395512032-20575-2-git-send-email-lauraa@codeaurora.org>
On Sat, 2014-03-22 at 11:13 -0700, Laura Abbott wrote:
> Rather than have architectures #define ARCH_HAS_SG_CHAIN in an architecture
> specific scatterlist.h, make it a proper Kconfig option and use that
> instead. At same time, remove the header files are are now mostly
> useless and just include asm-generic/scatterlist.h.
Well, the transformation looks fine. Perhaps part of the reason for the
lack of response is that there's no compelling reason in the change log
above for doing this. The usual reason for eliminating ARCH_HAS is that
it's hiding something that would be better expressed a different way
(that's actually intuitive to grep) or that it's expressing something
that should be configurable. Neither of these reasons apply in this
case, because SG_CHAIN definitely is a property of the architecture not
the config space and it's not really hiding anything.
Perhaps now might be the time to ask which are the remaining
architectures that cannot do SG chaining and then we can fix them and
pull the whole thing out.
James
^ permalink raw reply
* Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
From: Thomas Gleixner @ 2014-03-22 20:44 UTC (permalink / raw)
To: Laura Abbott
Cc: Fenghua Yu, Tony Luck, Russell King, linux-scsi, linux-s390, x86,
Heiko Carstens, linux-kernel, James E.J. Bottomley, linux390,
sparclinux, Ingo Molnar, Paul Mackerras, H. Peter Anvin,
Martin Schwidefsky, linux-ia64, Andrew Morton, linuxppc-dev,
David S. Miller, linux-arm-kernel
In-Reply-To: <1395512032-20575-2-git-send-email-lauraa@codeaurora.org>
On Sat, 22 Mar 2014, Laura Abbott wrote:
> Rather than have architectures #define ARCH_HAS_SG_CHAIN in an architecture
> specific scatterlist.h, make it a proper Kconfig option and use that
> instead. At same time, remove the header files are are now mostly
> useless and just include asm-generic/scatterlist.h.
>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
For the x86 part:
Acked-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
From: Laura Abbott @ 2014-03-22 18:13 UTC (permalink / raw)
To: Russell King, David S. Miller, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, James E.J. Bottomley, Fenghua Yu, Tony Luck,
Benjamin Herrenschmidt, Paul Mackerras, Martin Schwidefsky,
Heiko Carstens, Andrew Morton
Cc: linux-s390, Laura Abbott, linux-scsi, linux-kernel, linux390,
sparclinux, linux-ia64, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1395512032-20575-1-git-send-email-lauraa@codeaurora.org>
Rather than have architectures #define ARCH_HAS_SG_CHAIN in an architecture
specific scatterlist.h, make it a proper Kconfig option and use that
instead. At same time, remove the header files are are now mostly
useless and just include asm-generic/scatterlist.h.
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/Kbuild | 1 +
arch/arm/include/asm/scatterlist.h | 12 ------------
arch/arm64/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/ia64/include/asm/Kbuild | 1 +
arch/ia64/include/asm/scatterlist.h | 7 -------
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/powerpc/include/asm/scatterlist.h | 17 -----------------
arch/s390/Kconfig | 1 +
arch/s390/include/asm/Kbuild | 1 +
arch/s390/include/asm/scatterlist.h | 3 ---
arch/sparc/Kconfig | 1 +
arch/sparc/include/asm/Kbuild | 1 +
arch/sparc/include/asm/scatterlist.h | 8 --------
arch/x86/Kconfig | 1 +
arch/x86/include/asm/Kbuild | 1 +
arch/x86/include/asm/scatterlist.h | 8 --------
include/linux/scatterlist.h | 2 +-
include/scsi/scsi.h | 2 +-
lib/Kconfig | 7 +++++++
lib/scatterlist.c | 4 ++--
23 files changed, 24 insertions(+), 59 deletions(-)
delete mode 100644 arch/arm/include/asm/scatterlist.h
delete mode 100644 arch/ia64/include/asm/scatterlist.h
delete mode 100644 arch/powerpc/include/asm/scatterlist.h
delete mode 100644 arch/s390/include/asm/scatterlist.h
delete mode 100644 arch/sparc/include/asm/scatterlist.h
delete mode 100644 arch/x86/include/asm/scatterlist.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1594945..8122294 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -82,6 +82,7 @@ config ARM
<http://www.arm.linux.org.uk/>.
config ARM_HAS_SG_CHAIN
+ select ARCH_HAS_SG_CHAIN
bool
config NEED_SG_DMA_LENGTH
diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 3278afe..2357ed6 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -18,6 +18,7 @@ generic-y += param.h
generic-y += parport.h
generic-y += poll.h
generic-y += resource.h
+generic-y += scatterlist.h
generic-y += sections.h
generic-y += segment.h
generic-y += sembuf.h
diff --git a/arch/arm/include/asm/scatterlist.h b/arch/arm/include/asm/scatterlist.h
deleted file mode 100644
index cefdb8f..0000000
--- a/arch/arm/include/asm/scatterlist.h
+++ /dev/null
@@ -1,12 +0,0 @@
-#ifndef _ASMARM_SCATTERLIST_H
-#define _ASMARM_SCATTERLIST_H
-
-#ifdef CONFIG_ARM_HAS_SG_CHAIN
-#define ARCH_HAS_SG_CHAIN
-#endif
-
-#include <asm/memory.h>
-#include <asm/types.h>
-#include <asm-generic/scatterlist.h>
-
-#endif /* _ASMARM_SCATTERLIST_H */
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 27bbcfc..f2f95f4 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2,6 +2,7 @@ config ARM64
def_bool y
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_HAS_SG_CHAIN
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_WANT_OPTIONAL_GPIOLIB
select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 0c8e553..13e2e8b 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -44,6 +44,7 @@ config IA64
select HAVE_MOD_ARCH_SPECIFIC
select MODULES_USE_ELF_RELA
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_HAS_SG_CHAIN
default y
help
The Itanium Processor Family is Intel's 64-bit successor to
diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index 283a831..3906865 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -2,6 +2,7 @@
generic-y += clkdev.h
generic-y += exec.h
generic-y += kvm_para.h
+generic-y += scatterlist.h
generic-y += trace_clock.h
generic-y += preempt.h
generic-y += vtime.h
diff --git a/arch/ia64/include/asm/scatterlist.h b/arch/ia64/include/asm/scatterlist.h
deleted file mode 100644
index 08fd93b..0000000
--- a/arch/ia64/include/asm/scatterlist.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef _ASM_IA64_SCATTERLIST_H
-#define _ASM_IA64_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* _ASM_IA64_SCATTERLIST_H */
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 957bf34..659aee2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -141,6 +141,7 @@ config PPC
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_IRQ_EXIT_ON_IRQ_STACK
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
+ select ARCH_HAS_SG_CHAIN
config GENERIC_CSUM
def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index 6c0a955..ca596ec 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -1,6 +1,7 @@
generic-y += clkdev.h
generic-y += rwsem.h
+generic-y += scatterlist.h
generic-y += trace_clock.h
generic-y += preempt.h
generic-y += vtime.h
diff --git a/arch/powerpc/include/asm/scatterlist.h b/arch/powerpc/include/asm/scatterlist.h
deleted file mode 100644
index de1f620..0000000
--- a/arch/powerpc/include/asm/scatterlist.h
+++ /dev/null
@@ -1,17 +0,0 @@
-#ifndef _ASM_POWERPC_SCATTERLIST_H
-#define _ASM_POWERPC_SCATTERLIST_H
-/*
- * Copyright (C) 2001 PPC64 Team, IBM Corp
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
-
-#include <asm/dma.h>
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* _ASM_POWERPC_SCATTERLIST_H */
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 65a0775..d6c2059 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -142,6 +142,7 @@ config S390
select SYSCTL_EXCEPTION_TRACE
select VIRT_CPU_ACCOUNTING
select VIRT_TO_BUS
+ select ARCH_HAS_SG_CHAIN
config SCHED_OMIT_FRAME_POINTER
def_bool y
diff --git a/arch/s390/include/asm/Kbuild b/arch/s390/include/asm/Kbuild
index 8386a4a..14be6d0 100644
--- a/arch/s390/include/asm/Kbuild
+++ b/arch/s390/include/asm/Kbuild
@@ -4,3 +4,4 @@ generic-y += clkdev.h
generic-y += trace_clock.h
generic-y += preempt.h
generic-y += hash.h
+generic-y += scatterlist.h
diff --git a/arch/s390/include/asm/scatterlist.h b/arch/s390/include/asm/scatterlist.h
deleted file mode 100644
index 6d45ef6..0000000
--- a/arch/s390/include/asm/scatterlist.h
+++ /dev/null
@@ -1,3 +0,0 @@
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 7d8b7e9..7a179fe 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -42,6 +42,7 @@ config SPARC
select MODULES_USE_ELF_RELA
select ODD_RT_SIGACTION
select OLD_SIGSUSPEND
+ select ARCH_HAS_SG_CHAIN
config SPARC32
def_bool !64BIT
diff --git a/arch/sparc/include/asm/Kbuild b/arch/sparc/include/asm/Kbuild
index 4b60a0c..437f0c6 100644
--- a/arch/sparc/include/asm/Kbuild
+++ b/arch/sparc/include/asm/Kbuild
@@ -18,3 +18,4 @@ generic-y += types.h
generic-y += word-at-a-time.h
generic-y += preempt.h
generic-y += hash.h
+generic-y += scatterlist.h
diff --git a/arch/sparc/include/asm/scatterlist.h b/arch/sparc/include/asm/scatterlist.h
deleted file mode 100644
index 92bb638..0000000
--- a/arch/sparc/include/asm/scatterlist.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef _SPARC_SCATTERLIST_H
-#define _SPARC_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* !(_SPARC_SCATTERLIST_H) */
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..76997dc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
select HAVE_CC_STACKPROTECTOR
+ select ARCH_HAS_SG_CHAIN
config INSTRUCTION_DECODER
def_bool y
diff --git a/arch/x86/include/asm/Kbuild b/arch/x86/include/asm/Kbuild
index 7f66985..9b3d749 100644
--- a/arch/x86/include/asm/Kbuild
+++ b/arch/x86/include/asm/Kbuild
@@ -5,3 +5,4 @@ genhdr-y += unistd_64.h
genhdr-y += unistd_x32.h
generic-y += clkdev.h
+generic-y += scatterlist.h
diff --git a/arch/x86/include/asm/scatterlist.h b/arch/x86/include/asm/scatterlist.h
deleted file mode 100644
index 4240878..0000000
--- a/arch/x86/include/asm/scatterlist.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef _ASM_X86_SCATTERLIST_H
-#define _ASM_X86_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
-
-#endif /* _ASM_X86_SCATTERLIST_H */
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index a964f72..4b152c8 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -136,7 +136,7 @@ static inline void sg_set_buf(struct scatterlist *sg, const void *buf,
static inline void sg_chain(struct scatterlist *prv, unsigned int prv_nents,
struct scatterlist *sgl)
{
-#ifndef ARCH_HAS_SG_CHAIN
+#ifndef CONFIG_ARCH_HAS_SG_CHAIN
BUG();
#endif
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 0a4edfe..d34cf2d 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -31,7 +31,7 @@ enum scsi_timeouts {
* Like SCSI_MAX_SG_SEGMENTS, but for archs that have sg chaining. This limit
* is totally arbitrary, a setting of 2048 will get you at least 8mb ios.
*/
-#ifdef ARCH_HAS_SG_CHAIN
+#ifdef CONFIG_ARCH_HAS_SG_CHAIN
#define SCSI_MAX_SG_CHAIN_SEGMENTS 2048
#else
#define SCSI_MAX_SG_CHAIN_SEGMENTS SCSI_MAX_SG_SEGMENTS
diff --git a/lib/Kconfig b/lib/Kconfig
index 991c98b..32c68d3 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -451,4 +451,11 @@ config UCS2_STRING
source "lib/fonts/Kconfig"
+#
+# sg chaining option
+#
+
+config ARCH_HAS_SG_CHAIN
+ def_bool n
+
endmenu
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 3a8e8e8..4251cbd 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -73,7 +73,7 @@ EXPORT_SYMBOL(sg_nents);
**/
struct scatterlist *sg_last(struct scatterlist *sgl, unsigned int nents)
{
-#ifndef ARCH_HAS_SG_CHAIN
+#ifndef CONFIG_ARCH_HAS_SG_CHAIN
struct scatterlist *ret = &sgl[nents - 1];
#else
struct scatterlist *sg, *ret = NULL;
@@ -251,7 +251,7 @@ int __sg_alloc_table(struct sg_table *table, unsigned int nents,
if (nents == 0)
return -EINVAL;
-#ifndef ARCH_HAS_SG_CHAIN
+#ifndef CONFIG_ARCH_HAS_SG_CHAIN
if (WARN_ON_ONCE(nents > max_ents))
return -EINVAL;
#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply related
* [RESEND][PATCH 0/2] Cleanup asm/scatterlist.h
From: Laura Abbott @ 2014-03-22 18:13 UTC (permalink / raw)
To: Russell King, David S. Miller, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, x86, James E.J. Bottomley, Fenghua Yu, Tony Luck,
Benjamin Herrenschmidt, Paul Mackerras, Martin Schwidefsky,
Heiko Carstens, Andrew Morton, Mikael Starvik, Jesper Nilsson,
Hirokazu Takata, Koichi Yasutake, David Howells, Michal Simek,
Chen Liqin, Lennox Wu
Cc: linux-s390, linux-m32r, Laura Abbott, linux-cris-kernel,
linux-scsi, microblaze-uclinux, linux-alpha, linux-kernel,
linux-am33-list, linux390, sparclinux, linux-ia64, linuxppc-dev,
linux-arm-kernel
I haven't gotten many responses so I'm going to try sending again
(this time with a cover letter which I probably should have done in
the first place...)
ARCH_HAS_SG_CHAIN is currently defined as needed in asm/scatterlist.h.
It was suggested[1] that this should probably be a proper Kconfig.
At the same time, we can clean up most of the asm/scatterlist.h files
to reduce redundancy. This makes parisc the only architecture that still
has an asm/scatterlist.h file due to the #define sg_virt_addr (which
could probably be cleaned up further still)
Andrew, once we get a few more Acked-bys can we take this through your tree
as suggested by Will?
Thanks,
Laura
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/240435.html
Laura Abbott (2):
lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
Cleanup useless architecture versions of scatterlist.h
arch/alpha/include/asm/Kbuild | 1 +
arch/alpha/include/asm/scatterlist.h | 6 ------
arch/arm/Kconfig | 1 +
arch/arm/include/asm/Kbuild | 1 +
arch/arm/include/asm/scatterlist.h | 12 ------------
arch/arm64/Kconfig | 1 +
arch/cris/include/asm/Kbuild | 1 +
arch/cris/include/asm/scatterlist.h | 6 ------
arch/frv/include/asm/Kbuild | 1 +
arch/frv/include/asm/scatterlist.h | 6 ------
arch/ia64/Kconfig | 1 +
arch/ia64/include/asm/Kbuild | 1 +
arch/ia64/include/asm/scatterlist.h | 7 -------
arch/m32r/include/asm/Kbuild | 1 +
arch/m32r/include/asm/scatterlist.h | 6 ------
arch/microblaze/include/asm/Kbuild | 1 +
arch/microblaze/include/asm/scatterlist.h | 1 -
arch/mn10300/include/asm/Kbuild | 1 +
arch/mn10300/include/asm/scatterlist.h | 16 ----------------
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/Kbuild | 1 +
arch/powerpc/include/asm/scatterlist.h | 17 -----------------
arch/s390/Kconfig | 1 +
arch/s390/include/asm/Kbuild | 1 +
arch/s390/include/asm/scatterlist.h | 3 ---
arch/score/include/asm/Kbuild | 2 +-
arch/score/include/asm/scatterlist.h | 6 ------
arch/sparc/Kconfig | 1 +
arch/sparc/include/asm/Kbuild | 1 +
arch/sparc/include/asm/scatterlist.h | 8 --------
arch/x86/Kconfig | 1 +
arch/x86/include/asm/Kbuild | 1 +
arch/x86/include/asm/scatterlist.h | 8 --------
include/linux/scatterlist.h | 2 +-
include/scsi/scsi.h | 2 +-
lib/Kconfig | 7 +++++++
lib/scatterlist.c | 4 ++--
37 files changed, 31 insertions(+), 107 deletions(-)
delete mode 100644 arch/alpha/include/asm/scatterlist.h
delete mode 100644 arch/arm/include/asm/scatterlist.h
delete mode 100644 arch/cris/include/asm/scatterlist.h
delete mode 100644 arch/frv/include/asm/scatterlist.h
delete mode 100644 arch/ia64/include/asm/scatterlist.h
delete mode 100644 arch/m32r/include/asm/scatterlist.h
delete mode 100644 arch/microblaze/include/asm/scatterlist.h
delete mode 100644 arch/mn10300/include/asm/scatterlist.h
delete mode 100644 arch/powerpc/include/asm/scatterlist.h
delete mode 100644 arch/s390/include/asm/scatterlist.h
delete mode 100644 arch/score/include/asm/scatterlist.h
delete mode 100644 arch/sparc/include/asm/scatterlist.h
delete mode 100644 arch/x86/include/asm/scatterlist.h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [v3] mtd: m25p80: Modify the name of mtd_info
From: Hou Zhiqiang @ 2014-03-22 8:55 UTC (permalink / raw)
To: linux-mtd, linuxppc-dev; +Cc: scottwood, mingkai.hu, Hou Zhiqiang, dwmw2
To specify spi flash layouts by "mtdparts=..." in cmdline, we must
give mtd_info a fixed name,because the cmdlinepart's parser will
match the name of mtd_info given in cmdline.
Now, if use DT, the mtd_info's name will be spi->dev->name. It
consists of spi_master->bus_num, and the spi_master->bus_num maybe
dynamically fetched. So, in this case, replace the component bus_num
with thei physical address of spi master.
Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
---
v3:
Fix a bug, matching unsigned long long with "%llx".
v2:
1. Fix some code style issue.
2. Cast physical address to unsigned long long.
drivers/mtd/devices/m25p80.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 7eda71d..695890e 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -30,6 +30,7 @@
#include <linux/mtd/cfi.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/spi/spi.h>
@@ -934,9 +935,11 @@ static int m25p_probe(struct spi_device *spi)
struct flash_platform_data *data;
struct m25p *flash;
struct flash_info *info;
- unsigned i;
+ unsigned i, ret;
struct mtd_part_parser_data ppdata;
struct device_node *np = spi->dev.of_node;
+ struct resource res;
+ struct device_node *mnp = spi->master->dev.of_node;
/* Platform data helps sort out which chip type we have, as
* well as how this board partitions it. If we don't have
@@ -1009,8 +1012,18 @@ static int m25p_probe(struct spi_device *spi)
if (data && data->name)
flash->mtd.name = data->name;
- else
- flash->mtd.name = dev_name(&spi->dev);
+ else {
+ ret = of_address_to_resource(mnp, 0, &res);
+ if (ret) {
+ dev_err(&spi->dev, "failed to get spi master resource\n");
+ return ret;
+ }
+ flash->mtd.name = kasprintf(GFP_KERNEL, "spi%llx.%d",
+ (unsigned long long)res.start,
+ spi->chip_select);
+ if (!flash->mtd.name)
+ return -ENOMEM;
+ }
flash->mtd.type = MTD_NORFLASH;
flash->mtd.writesize = 1;
--
1.8.5
^ permalink raw reply related
* [v2] mtd: m25p80: Modify the name of mtd_info
From: Hou Zhiqiang @ 2014-03-22 7:58 UTC (permalink / raw)
To: linux-mtd, linuxppc-dev; +Cc: scottwood, mingkai.hu, Hou Zhiqiang, dwmw2
To specify spi flash layouts by "mtdparts=..." in cmdline, we must
give mtd_info a fixed name,because the cmdlinepart's parser will
match the name of mtd_info given in cmdline.
Now, if use DT, the mtd_info's name will be spi->dev->name. It
consists of spi_master->bus_num, and the spi_master->bus_num maybe
dynamically fetched. So, in this case, replace the component bus_num
with thei physical address of spi master.
Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
---
v2:
1. Fix some code style issue.
2. Cast physical address to unsigned long long.
drivers/mtd/devices/m25p80.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 7eda71d..cbb04aa 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -30,6 +30,7 @@
#include <linux/mtd/cfi.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/spi/spi.h>
@@ -934,9 +935,11 @@ static int m25p_probe(struct spi_device *spi)
struct flash_platform_data *data;
struct m25p *flash;
struct flash_info *info;
- unsigned i;
+ unsigned i, ret;
struct mtd_part_parser_data ppdata;
struct device_node *np = spi->dev.of_node;
+ struct resource res;
+ struct device_node *mnp = spi->master->dev.of_node;
/* Platform data helps sort out which chip type we have, as
* well as how this board partitions it. If we don't have
@@ -1009,8 +1012,18 @@ static int m25p_probe(struct spi_device *spi)
if (data && data->name)
flash->mtd.name = data->name;
- else
- flash->mtd.name = dev_name(&spi->dev);
+ else {
+ ret = of_address_to_resource(mnp, 0, &res);
+ if (ret) {
+ dev_err(&spi->dev, "failed to get spi master resource\n");
+ return ret;
+ }
+ flash->mtd.name = kasprintf(GFP_KERNEL, "spi%x.%d",
+ (unsigned long long)res.start,
+ spi->chip_select);
+ if (!flash->mtd.name)
+ return -ENOMEM;
+ }
flash->mtd.type = MTD_NORFLASH;
flash->mtd.writesize = 1;
--
1.8.5
^ permalink raw reply related
* Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver->get() method
From: Gautham R Shenoy @ 2014-03-22 7:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, ego, Linux PM list, Viresh Kumar
In-Reply-To: <1395442590.3460.85.camel@pasglop>
Hi Ben,
On Sat, Mar 22, 2014 at 09:56:30AM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2014-03-21 at 16:34 +0530, Gautham R Shenoy wrote:
>
> > > >
> > > > +/*
> > > > + * Computes the current frequency on this cpu
> > > > + * and stores the result in *ret_freq.
> > > > + */
> > > > +static void powernv_read_cpu_freq(void *ret_freq)
> > > > +{
> > > > + unsigned long pmspr_val;
> > > > + s8 local_pstate_id;
> > > > + int *cur_freq, freq, pstate_id;
> > > > +
> > > > + cur_freq = (int *)ret_freq;
> > >
> > > You don't need cur_freq variable at all..
> >
> > I don't like it either. But the compiler complains without this hack
> > :-(
>
> Casting integers into void * is a recipe for disaster... what is that
> supposed to be about ?
Like I mentioned elsewhere on this thread, we're calling
powernv_read_cpu_freq via an smp_call_function(). We use this to
obtain the frequency on the cpu where powernv_read_cpu_freq
executes and return it to the caller of smp_call_function.
> We lose all type checking and get exposed
> to endian issues etc... the day somebody uses a different type on both
> sides.
>
Yes, I understand the problem now. I'll think of a safer way to pass
the return value.
> Also is "freq" a frequency ? In this case an int isn't big enough.
freq is the frequency stored in the cpufreq_table. The value is in
kHz. So, int should be big enough.
> Cheers,
> Ben.
>
>
--
Thanks and Regards
gautham.
^ permalink raw reply
* [PATCH] arch/powerpc: Use RCU_INIT_POINTER(x, NULL) in platforms/cell/spu_syscalls.c
From: Monam Agarwal @ 2014-03-22 6:50 UTC (permalink / raw)
To: arnd, benh, paulus, linuxppc-dev, cbe-oss-dev, linux-kernel
Here rcu_assign_pointer() is ensuring that the
initialization of a structure is carried out before storing a pointer
to that structure.
So, rcu_assign_pointer(p, NULL) can always safely be converted to
RCU_INIT_POINTER(p, NULL).
Signed-off-by: Monam Agarwal <monamagarwal123@gmail.com>
---
arch/powerpc/platforms/cell/spu_syscalls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/cell/spu_syscalls.c b/arch/powerpc/platforms/cell/spu_syscalls.c
index 3844f13..38e0a1a 100644
--- a/arch/powerpc/platforms/cell/spu_syscalls.c
+++ b/arch/powerpc/platforms/cell/spu_syscalls.c
@@ -170,7 +170,7 @@ EXPORT_SYMBOL_GPL(register_spu_syscalls);
void unregister_spu_syscalls(struct spufs_calls *calls)
{
BUG_ON(spufs_calls->owner != calls->owner);
- rcu_assign_pointer(spufs_calls, NULL);
+ RCU_INIT_POINTER(spufs_calls, NULL);
synchronize_rcu();
}
EXPORT_SYMBOL_GPL(unregister_spu_syscalls);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3 1/5] powernv: cpufreq driver for powernv platform
From: Viresh Kumar @ 2014-03-22 3:43 UTC (permalink / raw)
To: ego@linux.vnet.ibm.com
Cc: Linux PM list, linuxppc-dev@ozlabs.org, Anton Blanchard,
Srivatsa S. Bhat
In-Reply-To: <20140321145452.GA8755@in.ibm.com>
On 21 March 2014 20:24, Gautham R Shenoy <ego@linux.vnet.ibm.com> wrote:
> Ok, I guess the right thing to do at this point is call
>
> cpufreq_table_validate_and_show(policy, powernv_freqs);
>
> Will fix the code to take care of this.
Yes.
^ permalink raw reply
* Re: [PATCH v3 1/5] powernv: cpufreq driver for powernv platform
From: Viresh Kumar @ 2014-03-22 3:43 UTC (permalink / raw)
To: Vaidyanathan Srinivasan
Cc: Gautham R Shenoy, Linux PM list, linuxppc-dev@ozlabs.org,
Anton Blanchard, Srivatsa S. Bhat
In-Reply-To: <20140321144818.GA30371@dirshya.in.ibm.com>
On 21 March 2014 20:18, Vaidyanathan Srinivasan
<svaidy@linux.vnet.ibm.com> wrote:
> Yeah, I had the driver written using driver_data to store pstates.
> Gautham found the bug that we are missing one PState when we match the
> ID with CPUFREQ_BOOST_FREQ!
I see..
> We did not know that you have taken care of those issues. Ideally
> I did expect that driver_data should not be touched by the framework.
> Thanks for fixing that and allowing the back-end driver to use
> driver_data.
No, I haven't fixed anything yet. And this piece of code still exists.
I will see if I can get this fixed, by that time you can continue the
way your code is there in this version.
^ permalink raw reply
* Re: Tasks stuck in futex code (in 3.14-rc6)
From: Davidlohr Bueso @ 2014-03-22 3:36 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Peter Zijlstra, Linus Torvalds, LKML, Paul Mackerras,
Thomas Gleixner, Paul McKenney, ppc-dev, Ingo Molnar
In-Reply-To: <20140322022754.GA21677@linux.vnet.ibm.com>
On Sat, 2014-03-22 at 07:57 +0530, Srikar Dronamraju wrote:
> > > So reverting and applying v3 3/4 and 4/4 patches works for me.
> >
> > Ok, I verified that the above endds up resulting in the same tree as
> > the minimal patch I sent out, modulo (a) some comments and (b) an
> > #ifdef CONFIG_SMP in futex_get_mm() that doesn't really matter.
> >
> > So I committed the minimal patch with your tested-by.
> >
>
> Just to be sure, I have verified with latest mainline with HEAD having
> commit 08edb33c4 (Merge branch 'drm-fixes' of
> git://people.freedesktop.org/~airlied/linux) which also has the commit
> 11d4616bd0 futex:( revert back to the explicit waiter counting code).
Thanks for double checking.
^ permalink raw reply
* Re: Tasks stuck in futex code (in 3.14-rc6)
From: Srikar Dronamraju @ 2014-03-22 2:27 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Zijlstra, LKML, Davidlohr Bueso, Paul Mackerras,
Thomas Gleixner, Paul McKenney, ppc-dev, Ingo Molnar
In-Reply-To: <CA+55aFyX0sj_MjTas3RYWWOa1W-xc3t0Af_0vdmsmgui8Apqpw@mail.gmail.com>
> > So reverting and applying v3 3/4 and 4/4 patches works for me.
>
> Ok, I verified that the above endds up resulting in the same tree as
> the minimal patch I sent out, modulo (a) some comments and (b) an
> #ifdef CONFIG_SMP in futex_get_mm() that doesn't really matter.
>
> So I committed the minimal patch with your tested-by.
>
Just to be sure, I have verified with latest mainline with HEAD having
commit 08edb33c4 (Merge branch 'drm-fixes' of
git://people.freedesktop.org/~airlied/linux) which also has the commit
11d4616bd0 futex:( revert back to the explicit waiter counting code).
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* [RFC][PATCH] perf: Add 'merge-recursive' callchain option
From: Sukadev Bhattiprolu @ 2014-03-22 2:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Anton Blanchard, linux-kernel, linuxppc-dev,
Paul Mackerras, Jiri Olsa
>From 9ad9432dab2bf4d1c8e6ff9201e88d5ae9f3994a Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Wed, 19 Mar 2014 20:24:22 -0500
Subject: [PATCH 1/1] perf: Add 'merge-recursive' callchain option
Powerpc saves the link register (LR) with each sample to help resolve
callchains for programs involving tail calls. Sometimes the value in the
LR is same as the NIP register. Other times it is not. This results in
callchains samples like these:
3547953334790 0x1ec0 [0x78]: PERF_RECORD_SAMPLE(IP, 2): 4667/4667: 0x80a7be3c58 period: 1773985 addr: 0
... chain: nr:9
..... 0: fffffffffffffe00
..... 1: 00000080a7be3c58 __random
..... 2: 00000080a7be3c40 __random
..... 3: 00000080a7be4270 rand
..... 4: 0000000010000784 do_my_sprintf
..... 5: 00000000100009d8 main
..... 6: 00000080a7bc482c
..... 7: 00000080a7bc4a34
..... 8: 0000000000000000
... thread: sprintft:4667
...... dso: /usr/lib64/libc-2.18.so
67470723107802 0x2f10 [0x78]: PERF_RECORD_SAMPLE(IP, 0x2): 5706/5706: 0x80a7be3c20 period: 872629 addr: 0
... chain: nr:9
..... 0: fffffffffffffe00
..... 1: 00000080a7be3c20 __random
..... 2: 00000080a7be4270 rand
..... 3: 00000080a7be4270 rand
..... 4: 0000000010000784
..... 5: 00000000100009d8
..... 6: 00000080a7bc482c
..... 7: 00000080a7bc4a34
..... 8: 0000000000000000
... thread: sprintft:5706
...... dso: /usr/lib64/libc-2.18.so
67470738362072 0x4cf8 [0x78]: PERF_RECORD_SAMPLE(IP, 0x2): 5706/5706: 0x80a7be3c14 period: 869774 addr: 0
... chain: nr:9
..... 0: fffffffffffffe00
..... 1: 00000080a7be3c14 __random
..... 2: 00000080a7be4270 rand
..... 3: 00000080a7be4270 rand
..... 4: 0000000010000784 do_my_sprintf
..... 5: 00000000100009d8 main
..... 6: 00000080a7bc482c
..... 7: 00000080a7bc4a34
..... 8: 0000000000000000
... thread: sprintft:5706
...... dso: /usr/lib64/libc-2.18.so
In the perf tool, these samples end up on different branches of the RB-Tree
resulting in duplicated arcs in the final call-graph.
15.02% sprintft libc-2.18.so [.] __random
|
--- __random
|
|--58.45%-- rand
| rand
| do_my_sprintf
| main
| generic_start_main.isra.0
| __libc_start_main
| 0x0
|
--41.55%-- __random
rand
do_my_sprintf
main
generic_start_main.isra.0
__libc_start_main
0x0
This is an RFC for adding a 'merge-recursive' call-graph option that would
discard the duplicate entries resulting in a more compact call-graph. The
duplicates can be either identical instruction pointer values or IP values
within the same function.
perf report --call-graph=fractal,0.5,callee,function,merge-recursive
15.02% sprintft libc-2.18.so [.] __random
|
---__random
rand
do_my_sprintf
main
generic_start_main.isra.0
__libc_start_main
(nil)
This option could also be used to collapse call-graphs of recursive functions.
AFAICT, the existing CCKEY_FUNCTION does not address this case because
the callchain lengths can vary across samples.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
tools/perf/builtin-report.c | 14 ++++++++++++--
tools/perf/util/callchain.h | 1 +
tools/perf/util/machine.c | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d882b6f..0b68749 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -647,6 +647,16 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
callchain_param.key = CCKEY_ADDRESS;
else
return -1;
+
+ tok2 = strtok(NULL, ",");
+ if (!tok2)
+ goto setup;
+
+ if (!strncmp(tok2, "merge-recursive", strlen("merge-recursive")))
+ callchain_param.merge_recursive = 1;
+ else
+ return -1;
+
setup:
if (callchain_register_param(&callchain_param) < 0) {
pr_err("Can't register callchain params\n");
@@ -762,8 +772,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
"regex filter to identify parent, see: '--sort parent'"),
OPT_BOOLEAN('x', "exclude-other", &symbol_conf.exclude_other,
"Only display entries with parent-match"),
- OPT_CALLBACK_DEFAULT('g', "call-graph", &report, "output_type,min_percent[,print_limit],call_order",
- "Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address). "
+ OPT_CALLBACK_DEFAULT('g', "call-graph", &report, "output_type,min_percent[,print_limit],call_order,merge-recursive",
+ "Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address), merge or don't merge recursive calls"
"Default: fractal,0.5,callee,function", &parse_callchain_opt, callchain_default_opt),
OPT_INTEGER(0, "max-stack", &report.max_stack,
"Set the maximum stack depth when parsing the callchain, "
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 8ad97e9..d8d0822 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -53,6 +53,7 @@ struct callchain_param {
sort_chain_func_t sort;
enum chain_order order;
enum chain_key key;
+ u32 merge_recursive;
};
struct callchain_list {
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index eb26544..e74ad95 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1272,6 +1272,32 @@ struct branch_info *sample__resolve_bstack(struct perf_sample *sample,
return bi;
}
+static int duplicate_symbol(struct symbol *sym, char **prev_name)
+{
+ char *prev = *prev_name;
+
+ if (sym) {
+ if (prev) {
+ if (!strcmp(sym->name, prev))
+ return 1; /* duplicate */
+ free(prev);
+ }
+
+ *prev_name = prev = strdup(sym->name);
+ if (!prev) {
+ pr_debug("%s: Unable to alloc memory\n", __func__);
+ return -ENOMEM;
+ }
+ } else if (prev) {
+ /* new symbol is NULL, so forget prev name */
+ free(prev);
+ *prev_name = NULL;
+ }
+
+ /* Not duplicate */
+ return 0;
+}
+
static int machine__resolve_callchain_sample(struct machine *machine,
struct thread *thread,
struct ip_callchain *chain,
@@ -1283,6 +1309,8 @@ static int machine__resolve_callchain_sample(struct machine *machine,
int chain_nr = min(max_stack, (int)chain->nr);
int i;
int err;
+ u64 prev_ip = 0;
+ char *prev_name = NULL;
callchain_cursor_reset(&callchain_cursor);
@@ -1324,6 +1352,10 @@ static int machine__resolve_callchain_sample(struct machine *machine,
continue;
}
+ if (callchain_param.merge_recursive && prev_ip == ip)
+ continue;
+ prev_ip = ip;
+
al.filtered = false;
thread__find_addr_location(thread, machine, cpumode,
MAP__FUNCTION, ip, &al);
@@ -1340,6 +1372,13 @@ static int machine__resolve_callchain_sample(struct machine *machine,
}
}
+ if (callchain_param.merge_recursive) {
+ if ((err = duplicate_symbol(al.sym, &prev_name)) > 0)
+ continue; /* duplicate */
+ else if (err < 0)
+ return err;
+ }
+
err = callchain_cursor_append(&callchain_cursor,
ip, al.map, al.sym);
if (err)
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver->get() method
From: Benjamin Herrenschmidt @ 2014-03-21 22:56 UTC (permalink / raw)
To: ego; +Cc: Viresh Kumar, Linux PM list, linuxppc-dev
In-Reply-To: <20140321110445.GB2493@in.ibm.com>
On Fri, 2014-03-21 at 16:34 +0530, Gautham R Shenoy wrote:
> > >
> > > +/*
> > > + * Computes the current frequency on this cpu
> > > + * and stores the result in *ret_freq.
> > > + */
> > > +static void powernv_read_cpu_freq(void *ret_freq)
> > > +{
> > > + unsigned long pmspr_val;
> > > + s8 local_pstate_id;
> > > + int *cur_freq, freq, pstate_id;
> > > +
> > > + cur_freq = (int *)ret_freq;
> >
> > You don't need cur_freq variable at all..
>
> I don't like it either. But the compiler complains without this hack
> :-(
Casting integers into void * is a recipe for disaster... what is that
supposed to be about ? We lose all type checking and get exposed
to endian issues etc... the day somebody uses a different type on both
sides.
Also is "freq" a frequency ? In this case an int isn't big enough.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 9/9] powerpc/pm: support deep sleep feature on T1040
From: Scott Wood @ 2014-03-21 21:16 UTC (permalink / raw)
To: David Laight
Cc: linuxppc-dev@lists.ozlabs.org, 'Kevin Hao', Chenhui Zhao,
Jason.Jin@freescale.com, linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6E40C4@AcuExch.aculab.com>
On Fri, 2014-03-21 at 09:21 +0000, David Laight wrote:
> From: Scott Wood [mailto:scottwood@freescale.com]
> > On Thu, 2014-03-20 at 11:59 +0000, David Laight wrote:
> > > I tried to work out what the 'twi, isync' instructions were for (in in_le32()).
> > > The best I could come up with was to ensure a synchronous bus-fault.
> > > But bus faults are probably only expected during device probing - not
> > > normal operation, and the instructions will have a significant cost.
> > >
> > > Additionally in_le32() and out_le32() both start with a 'sync' instruction.
> > > In many cases that isn't needed either - an explicit iosync() can be
> > > used after groups of instructions.
> >
> > The idea is that it's better to be maximally safe by default, and let
> > performance critical sections be optimized using raw accessors and
> > explicit synchronization if needed, than to have hard-to-debug bugs due
> > to missing/wrong sync. A lot of I/O is slow enough that the performance
> > impact doesn't really matter, but the brain-time cost of getting the
> > sync right is still there.
>
> Hmmm....
>
> That might be an excuse for the 'sync', but not the twi and isync.
That might be true if I/O is always cache inhibited and guarded, in
which case I think we can rely on that to ensure that the load has
completed before we do things like wrtee or rfi. In any case, I'd want
to hear Ben's explanation.
> I was setting up a dma request (for the ppc 83xx PCIe bridge) and
> was doing back to back little-endian writes into memory.
> I had difficulty finding and including header files containing
> the definitions for byteswapped accesses I needed.
> arch/powerpc/include/asm/swab.h contains some - but I couldn't
> work out how to get it included (apart from giving the full path).
>
> In any case you need to understand when synchronisation is
> required - otherwise you will get it wrong.
> Especially since non-byteswapped accesses are done by direct
> access.
Yes, it's bad that rawness combines the lack of byteswapping with the
lack of synchronization. Ideally the raw accessors would also come in
big and little endian form, plus a native endian form if it's really
needed.
-Scott
^ permalink raw reply
* Re: [PATCH 4/4] ARCH: AUDIT: audit_syscall_entry() should not require the arch
From: Richard Guy Briggs @ 2014-03-21 19:18 UTC (permalink / raw)
To: Eric Paris
Cc: linux-mips, linux-ia64, linux-parisc, user-mode-linux-devel,
linux-s390, linux-sh, microblaze-uclinux, linux-xtensa, x86,
linux-audit, linux-alpha, sparclinux, linuxppc-dev, linux,
linux-arm-kernel
In-Reply-To: <1395266643-3139-4-git-send-email-eparis@redhat.com>
On 14/03/19, Eric Paris wrote:
> We have a function where the arch can be queried, syscall_get_arch().
> So rather than have every single piece of arch specific code use and/or
> duplicate syscall_get_arch(), just have the audit code use the
> syscall_get_arch() code.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-ia64@vger.kernel.org
> Cc: microblaze-uclinux@itee.uq.edu.au
> Cc: linux-mips@linux-mips.org
> Cc: linux@lists.openrisc.net
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> Cc: user-mode-linux-devel@lists.sourceforge.net
> Cc: linux-xtensa@linux-xtensa.org
> Cc: x86@kernel.org
Acked-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> arch/alpha/kernel/ptrace.c | 2 +-
> arch/arm/kernel/ptrace.c | 4 ++--
> arch/ia64/kernel/ptrace.c | 2 +-
> arch/microblaze/kernel/ptrace.c | 3 +--
> arch/mips/kernel/ptrace.c | 4 +---
> arch/openrisc/kernel/ptrace.c | 3 +--
> arch/parisc/kernel/ptrace.c | 9 +++------
> arch/powerpc/kernel/ptrace.c | 7 ++-----
> arch/s390/kernel/ptrace.c | 4 +---
> arch/sh/kernel/ptrace_32.c | 14 +-------------
> arch/sh/kernel/ptrace_64.c | 17 +----------------
> arch/sparc/kernel/ptrace_64.c | 9 ++-------
> arch/um/kernel/ptrace.c | 3 +--
> arch/x86/kernel/ptrace.c | 8 ++------
> arch/x86/um/asm/ptrace.h | 4 ----
> arch/xtensa/kernel/ptrace.c | 2 +-
> include/linux/audit.h | 7 ++++---
> 17 files changed, 25 insertions(+), 77 deletions(-)
>
> diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
> index 86d8351..d9ee817 100644
> --- a/arch/alpha/kernel/ptrace.c
> +++ b/arch/alpha/kernel/ptrace.c
> @@ -321,7 +321,7 @@ asmlinkage unsigned long syscall_trace_enter(void)
> if (test_thread_flag(TIF_SYSCALL_TRACE) &&
> tracehook_report_syscall_entry(current_pt_regs()))
> ret = -1UL;
> - audit_syscall_entry(AUDIT_ARCH_ALPHA, regs->r0, regs->r16, regs->r17, regs->r18, regs->r19);
> + audit_syscall_entry(regs->r0, regs->r16, regs->r17, regs->r18, regs->r19);
> return ret ?: current_pt_regs()->r0;
> }
>
> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index 0dd3b79..c9d2b34 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -943,8 +943,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
> if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> trace_sys_enter(regs, scno);
>
> - audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0, regs->ARM_r1,
> - regs->ARM_r2, regs->ARM_r3);
> + audit_syscall_entry(scno, regs->ARM_r0, regs->ARM_r1, regs->ARM_r2,
> + regs->ARM_r3);
>
> return scno;
> }
> diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
> index b7a5fff..6f54d51 100644
> --- a/arch/ia64/kernel/ptrace.c
> +++ b/arch/ia64/kernel/ptrace.c
> @@ -1219,7 +1219,7 @@ syscall_trace_enter (long arg0, long arg1, long arg2, long arg3,
> ia64_sync_krbs();
>
>
> - audit_syscall_entry(AUDIT_ARCH_IA64, regs.r15, arg0, arg1, arg2, arg3);
> + audit_syscall_entry(regs.r15, arg0, arg1, arg2, arg3);
>
> return 0;
> }
> diff --git a/arch/microblaze/kernel/ptrace.c b/arch/microblaze/kernel/ptrace.c
> index 39cf508..bb10637 100644
> --- a/arch/microblaze/kernel/ptrace.c
> +++ b/arch/microblaze/kernel/ptrace.c
> @@ -147,8 +147,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
> */
> ret = -1L;
>
> - audit_syscall_entry(EM_MICROBLAZE, regs->r12, regs->r5, regs->r6,
> - regs->r7, regs->r8);
> + audit_syscall_entry(regs->r12, regs->r5, regs->r6, regs->r7, regs->r8);
>
> return ret ?: regs->r12;
> }
> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> index 65ba622..c06bb82 100644
> --- a/arch/mips/kernel/ptrace.c
> +++ b/arch/mips/kernel/ptrace.c
> @@ -671,9 +671,7 @@ asmlinkage void syscall_trace_enter(struct pt_regs *regs)
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> trace_sys_enter(regs, regs->regs[2]);
>
> - audit_syscall_entry(syscall_get_arch(),
> - regs->regs[2],
> - regs->regs[4], regs->regs[5],
> + audit_syscall_entry(regs->regs[2], regs->regs[4], regs->regs[5],
> regs->regs[6], regs->regs[7]);
> }
>
> diff --git a/arch/openrisc/kernel/ptrace.c b/arch/openrisc/kernel/ptrace.c
> index 71a2a0c..4f59fa4 100644
> --- a/arch/openrisc/kernel/ptrace.c
> +++ b/arch/openrisc/kernel/ptrace.c
> @@ -187,8 +187,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
> */
> ret = -1L;
>
> - audit_syscall_entry(AUDIT_ARCH_OPENRISC, regs->gpr[11],
> - regs->gpr[3], regs->gpr[4],
> + audit_syscall_entry(regs->gpr[11], regs->gpr[3], regs->gpr[4],
> regs->gpr[5], regs->gpr[6]);
>
> return ret ? : regs->gpr[11];
> diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
> index e842ee2..7481457 100644
> --- a/arch/parisc/kernel/ptrace.c
> +++ b/arch/parisc/kernel/ptrace.c
> @@ -276,14 +276,11 @@ long do_syscall_trace_enter(struct pt_regs *regs)
>
> #ifdef CONFIG_64BIT
> if (!is_compat_task())
> - audit_syscall_entry(AUDIT_ARCH_PARISC64,
> - regs->gr[20],
> - regs->gr[26], regs->gr[25],
> - regs->gr[24], regs->gr[23]);
> + audit_syscall_entry(regs->gr[20], regs->gr[26], regs->gr[25],
> + regs->gr[24], regs->gr[23]);
> else
> #endif
> - audit_syscall_entry(AUDIT_ARCH_PARISC,
> - regs->gr[20] & 0xffffffff,
> + audit_syscall_entry(regs->gr[20] & 0xffffffff,
> regs->gr[26] & 0xffffffff,
> regs->gr[25] & 0xffffffff,
> regs->gr[24] & 0xffffffff,
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 2e3d2bf..524a943 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -1788,14 +1788,11 @@ long do_syscall_trace_enter(struct pt_regs *regs)
>
> #ifdef CONFIG_PPC64
> if (!is_32bit_task())
> - audit_syscall_entry(AUDIT_ARCH_PPC64,
> - regs->gpr[0],
> - regs->gpr[3], regs->gpr[4],
> + audit_syscall_entry(regs->gpr[0], regs->gpr[3], regs->gpr[4],
> regs->gpr[5], regs->gpr[6]);
> else
> #endif
> - audit_syscall_entry(AUDIT_ARCH_PPC,
> - regs->gpr[0],
> + audit_syscall_entry(regs->gpr[0],
> regs->gpr[3] & 0xffffffff,
> regs->gpr[4] & 0xffffffff,
> regs->gpr[5] & 0xffffffff,
> diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
> index e65c91c..2e2e7bb5 100644
> --- a/arch/s390/kernel/ptrace.c
> +++ b/arch/s390/kernel/ptrace.c
> @@ -812,9 +812,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> trace_sys_enter(regs, regs->gprs[2]);
>
> - audit_syscall_entry(is_compat_task() ?
> - AUDIT_ARCH_S390 : AUDIT_ARCH_S390X,
> - regs->gprs[2], regs->orig_gpr2,
> + audit_syscall_entry(regs->gprs[2], regs->orig_gpr2,
> regs->gprs[3], regs->gprs[4],
> regs->gprs[5]);
> out:
> diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c
> index 668c816..c1a6b89 100644
> --- a/arch/sh/kernel/ptrace_32.c
> +++ b/arch/sh/kernel/ptrace_32.c
> @@ -484,17 +484,6 @@ long arch_ptrace(struct task_struct *child, long request,
> return ret;
> }
>
> -static inline int audit_arch(void)
> -{
> - int arch = EM_SH;
> -
> -#ifdef CONFIG_CPU_LITTLE_ENDIAN
> - arch |= __AUDIT_ARCH_LE;
> -#endif
> -
> - return arch;
> -}
> -
> asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
> {
> long ret = 0;
> @@ -513,8 +502,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> trace_sys_enter(regs, regs->regs[0]);
>
> - audit_syscall_entry(audit_arch(), regs->regs[3],
> - regs->regs[4], regs->regs[5],
> + audit_syscall_entry(regs->regs[3], regs->regs[4], regs->regs[5],
> regs->regs[6], regs->regs[7]);
>
> return ret ?: regs->regs[0];
> diff --git a/arch/sh/kernel/ptrace_64.c b/arch/sh/kernel/ptrace_64.c
> index af90339..5cea973 100644
> --- a/arch/sh/kernel/ptrace_64.c
> +++ b/arch/sh/kernel/ptrace_64.c
> @@ -504,20 +504,6 @@ asmlinkage int sh64_ptrace(long request, long pid,
> return sys_ptrace(request, pid, addr, data);
> }
>
> -static inline int audit_arch(void)
> -{
> - int arch = EM_SH;
> -
> -#ifdef CONFIG_64BIT
> - arch |= __AUDIT_ARCH_64BIT;
> -#endif
> -#ifdef CONFIG_CPU_LITTLE_ENDIAN
> - arch |= __AUDIT_ARCH_LE;
> -#endif
> -
> - return arch;
> -}
> -
> asmlinkage long long do_syscall_trace_enter(struct pt_regs *regs)
> {
> long long ret = 0;
> @@ -536,8 +522,7 @@ asmlinkage long long do_syscall_trace_enter(struct pt_regs *regs)
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> trace_sys_enter(regs, regs->regs[9]);
>
> - audit_syscall_entry(audit_arch(), regs->regs[1],
> - regs->regs[2], regs->regs[3],
> + audit_syscall_entry(regs->regs[1], regs->regs[2], regs->regs[3],
> regs->regs[4], regs->regs[5]);
>
> return ret ?: regs->regs[9];
> diff --git a/arch/sparc/kernel/ptrace_64.c b/arch/sparc/kernel/ptrace_64.c
> index c13c9f2..9ddc492 100644
> --- a/arch/sparc/kernel/ptrace_64.c
> +++ b/arch/sparc/kernel/ptrace_64.c
> @@ -1076,13 +1076,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
> trace_sys_enter(regs, regs->u_regs[UREG_G1]);
>
> - audit_syscall_entry((test_thread_flag(TIF_32BIT) ?
> - AUDIT_ARCH_SPARC :
> - AUDIT_ARCH_SPARC64),
> - regs->u_regs[UREG_G1],
> - regs->u_regs[UREG_I0],
> - regs->u_regs[UREG_I1],
> - regs->u_regs[UREG_I2],
> + audit_syscall_entry(regs->u_regs[UREG_G1], regs->u_regs[UREG_I0],
> + regs->u_regs[UREG_I1], regs->u_regs[UREG_I2],
> regs->u_regs[UREG_I3]);
>
> return ret;
> diff --git a/arch/um/kernel/ptrace.c b/arch/um/kernel/ptrace.c
> index 694d551..62435ef 100644
> --- a/arch/um/kernel/ptrace.c
> +++ b/arch/um/kernel/ptrace.c
> @@ -165,8 +165,7 @@ static void send_sigtrap(struct task_struct *tsk, struct uml_pt_regs *regs,
> */
> void syscall_trace_enter(struct pt_regs *regs)
> {
> - audit_syscall_entry(HOST_AUDIT_ARCH,
> - UPT_SYSCALL_NR(®s->regs),
> + audit_syscall_entry(UPT_SYSCALL_NR(®s->regs),
> UPT_SYSCALL_ARG1(®s->regs),
> UPT_SYSCALL_ARG2(®s->regs),
> UPT_SYSCALL_ARG3(®s->regs),
> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index 7461f50..46dfba6 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -1488,15 +1488,11 @@ long syscall_trace_enter(struct pt_regs *regs)
> trace_sys_enter(regs, regs->orig_ax);
>
> if (IS_IA32)
> - audit_syscall_entry(AUDIT_ARCH_I386,
> - regs->orig_ax,
> - regs->bx, regs->cx,
> + audit_syscall_entry(regs->orig_ax, regs->bx, regs->cx,
> regs->dx, regs->si);
> #ifdef CONFIG_X86_64
> else
> - audit_syscall_entry(AUDIT_ARCH_X86_64,
> - regs->orig_ax,
> - regs->di, regs->si,
> + audit_syscall_entry(regs->orig_ax, regs->di, regs->si,
> regs->dx, regs->r10);
> #endif
>
> diff --git a/arch/x86/um/asm/ptrace.h b/arch/x86/um/asm/ptrace.h
> index 54f8102..e59eef2 100644
> --- a/arch/x86/um/asm/ptrace.h
> +++ b/arch/x86/um/asm/ptrace.h
> @@ -47,8 +47,6 @@ struct user_desc;
>
> #ifdef CONFIG_X86_32
>
> -#define HOST_AUDIT_ARCH AUDIT_ARCH_I386
> -
> extern int ptrace_get_thread_area(struct task_struct *child, int idx,
> struct user_desc __user *user_desc);
>
> @@ -57,8 +55,6 @@ extern int ptrace_set_thread_area(struct task_struct *child, int idx,
>
> #else
>
> -#define HOST_AUDIT_ARCH AUDIT_ARCH_X86_64
> -
> #define PT_REGS_R8(r) UPT_R8(&(r)->regs)
> #define PT_REGS_R9(r) UPT_R9(&(r)->regs)
> #define PT_REGS_R10(r) UPT_R10(&(r)->regs)
> diff --git a/arch/xtensa/kernel/ptrace.c b/arch/xtensa/kernel/ptrace.c
> index 562fac6..4d54b48 100644
> --- a/arch/xtensa/kernel/ptrace.c
> +++ b/arch/xtensa/kernel/ptrace.c
> @@ -342,7 +342,7 @@ void do_syscall_trace_enter(struct pt_regs *regs)
> do_syscall_trace();
>
> #if 0
> - audit_syscall_entry(current, AUDIT_ARCH_XTENSA..);
> + audit_syscall_entry(...);
> #endif
> }
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 4b2983e..62c9d98 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -26,6 +26,7 @@
> #include <linux/sched.h>
> #include <linux/ptrace.h>
> #include <uapi/linux/audit.h>
> +#include <asm/syscall.h>
>
> struct audit_sig_info {
> uid_t uid;
> @@ -135,12 +136,12 @@ static inline void audit_free(struct task_struct *task)
> if (unlikely(task->audit_context))
> __audit_free(task);
> }
> -static inline void audit_syscall_entry(int arch, int major, unsigned long a0,
> +static inline void audit_syscall_entry(int major, unsigned long a0,
> unsigned long a1, unsigned long a2,
> unsigned long a3)
> {
> if (unlikely(current->audit_context))
> - __audit_syscall_entry(arch, major, a0, a1, a2, a3);
> + __audit_syscall_entry(syscall_get_arch(), major, a0, a1, a2, a3);
> }
> static inline void audit_syscall_exit(void *pt_regs)
> {
> @@ -316,7 +317,7 @@ static inline int audit_alloc(struct task_struct *task)
> }
> static inline void audit_free(struct task_struct *task)
> { }
> -static inline void audit_syscall_entry(int arch, int major, unsigned long a0,
> +static inline void audit_syscall_entry(int major, unsigned long a0,
> unsigned long a1, unsigned long a2,
> unsigned long a3)
> { }
> --
> 1.8.5.3
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH 3/4] ARCH: AUDIT: implement syscall_get_arch for all arches
From: Richard Guy Briggs @ 2014-03-21 19:13 UTC (permalink / raw)
To: Eric Paris
Cc: linux-mips, linux-ia64, linux-parisc, microblaze-uclinux, linux,
linux-audit, sparclinux, linuxppc-dev
In-Reply-To: <1395266643-3139-3-git-send-email-eparis@redhat.com>
On 14/03/19, Eric Paris wrote:
> For all arches which support audit implement syscall_get_arch()
> They are all pretty easy and straight forward, stolen from how the call
> to audit_syscall_entry() determines the arch.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
> Cc: linux-ia64@vger.kernel.org
> Cc: microblaze-uclinux@itee.uq.edu.au
> Cc: linux-mips@linux-mips.org
> Cc: linux@lists.openrisc.net
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
Acked-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> arch/ia64/include/asm/syscall.h | 6 ++++++
> arch/microblaze/include/asm/syscall.h | 5 +++++
> arch/mips/include/asm/syscall.h | 2 +-
> arch/openrisc/include/asm/syscall.h | 5 +++++
> arch/parisc/include/asm/syscall.h | 11 +++++++++++
> arch/powerpc/include/asm/syscall.h | 12 ++++++++++++
> arch/sparc/include/asm/syscall.h | 8 ++++++++
> include/uapi/linux/audit.h | 1 +
> 8 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/arch/ia64/include/asm/syscall.h b/arch/ia64/include/asm/syscall.h
> index a7ff1c6..1d0b875 100644
> --- a/arch/ia64/include/asm/syscall.h
> +++ b/arch/ia64/include/asm/syscall.h
> @@ -13,6 +13,7 @@
> #ifndef _ASM_SYSCALL_H
> #define _ASM_SYSCALL_H 1
>
> +#include <uapi/linux/audit.h>
> #include <linux/sched.h>
> #include <linux/err.h>
>
> @@ -79,4 +80,9 @@ static inline void syscall_set_arguments(struct task_struct *task,
>
> ia64_syscall_get_set_arguments(task, regs, i, n, args, 1);
> }
> +
> +static inline int syscall_get_arch(void)
> +{
> + return AUDIT_ARCH_IA64;
> +}
> #endif /* _ASM_SYSCALL_H */
> diff --git a/arch/microblaze/include/asm/syscall.h b/arch/microblaze/include/asm/syscall.h
> index 9bc4317..53cfaf3 100644
> --- a/arch/microblaze/include/asm/syscall.h
> +++ b/arch/microblaze/include/asm/syscall.h
> @@ -1,6 +1,7 @@
> #ifndef __ASM_MICROBLAZE_SYSCALL_H
> #define __ASM_MICROBLAZE_SYSCALL_H
>
> +#include <uapi/linux/audit.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> #include <asm/ptrace.h>
> @@ -99,4 +100,8 @@ static inline void syscall_set_arguments(struct task_struct *task,
> asmlinkage long do_syscall_trace_enter(struct pt_regs *regs);
> asmlinkage void do_syscall_trace_leave(struct pt_regs *regs);
>
> +static inline int syscall_get_arch(void)
> +{
> + return AUDIT_ARCH_MICROBLAZE;
> +}
> #endif /* __ASM_MICROBLAZE_SYSCALL_H */
> diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
> index fc556d8..992b6ab 100644
> --- a/arch/mips/include/asm/syscall.h
> +++ b/arch/mips/include/asm/syscall.h
> @@ -103,7 +103,7 @@ extern const unsigned long sysn32_call_table[];
>
> static inline int syscall_get_arch(void)
> {
> - int arch = EM_MIPS;
> + int arch = AUDIT_ARCH_MIPS;
> #ifdef CONFIG_64BIT
> arch |= __AUDIT_ARCH_64BIT;
> #endif
> diff --git a/arch/openrisc/include/asm/syscall.h b/arch/openrisc/include/asm/syscall.h
> index b752bb6..2db9f1c 100644
> --- a/arch/openrisc/include/asm/syscall.h
> +++ b/arch/openrisc/include/asm/syscall.h
> @@ -19,6 +19,7 @@
> #ifndef __ASM_OPENRISC_SYSCALL_H__
> #define __ASM_OPENRISC_SYSCALL_H__
>
> +#include <uapi/linux/audit.h>
> #include <linux/err.h>
> #include <linux/sched.h>
>
> @@ -71,4 +72,8 @@ syscall_set_arguments(struct task_struct *task, struct pt_regs *regs,
> memcpy(®s->gpr[3 + i], args, n * sizeof(args[0]));
> }
>
> +static inline int syscall_get_arch(void)
> +{
> + return AUDIT_ARCH_OPENRISC;
> +}
> #endif
> diff --git a/arch/parisc/include/asm/syscall.h b/arch/parisc/include/asm/syscall.h
> index 8bdfd2c..a5eba95 100644
> --- a/arch/parisc/include/asm/syscall.h
> +++ b/arch/parisc/include/asm/syscall.h
> @@ -3,6 +3,8 @@
> #ifndef _ASM_PARISC_SYSCALL_H_
> #define _ASM_PARISC_SYSCALL_H_
>
> +#include <uapi/linux/audit.h>
> +#include <linux/compat.h>
> #include <linux/err.h>
> #include <asm/ptrace.h>
>
> @@ -37,4 +39,13 @@ static inline void syscall_get_arguments(struct task_struct *tsk,
> }
> }
>
> +static inline int syscall_get_arch(void)
> +{
> + int arch = AUDIT_ARCH_PARISC;
> +#ifdef CONFIG_64BIT
> + if (!is_compat_task())
> + arch = AUDIT_ARCH_PARISC64;
> +#endif
> + return arch;
> +}
> #endif /*_ASM_PARISC_SYSCALL_H_*/
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index b54b2ad..4271544 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -13,6 +13,8 @@
> #ifndef _ASM_SYSCALL_H
> #define _ASM_SYSCALL_H 1
>
> +#include <uapi/linux/audit.h>
> +#include <linux/compat.h>
> #include <linux/sched.h>
>
> /* ftrace syscalls requires exporting the sys_call_table */
> @@ -86,4 +88,14 @@ static inline void syscall_set_arguments(struct task_struct *task,
> memcpy(®s->gpr[3 + i], args, n * sizeof(args[0]));
> }
>
> +static inline int syscall_get_arch(void)
> +{
> + int arch = AUDIT_ARCH_PPC;
> +
> +#ifdef CONFIG_PPC64
> + if (!is_32bit_task())
> + arch = AUDIT_ARCH_PPC64;
> +#endif
> + return arch;
> +}
> #endif /* _ASM_SYSCALL_H */
> diff --git a/arch/sparc/include/asm/syscall.h b/arch/sparc/include/asm/syscall.h
> index 025a02a..fed3d51 100644
> --- a/arch/sparc/include/asm/syscall.h
> +++ b/arch/sparc/include/asm/syscall.h
> @@ -1,9 +1,11 @@
> #ifndef __ASM_SPARC_SYSCALL_H
> #define __ASM_SPARC_SYSCALL_H
>
> +#include <uapi/linux/audit.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> #include <asm/ptrace.h>
> +#include <asm/thread_info.h>
>
> /*
> * The syscall table always contains 32 bit pointers since we know that the
> @@ -124,4 +126,10 @@ static inline void syscall_set_arguments(struct task_struct *task,
> regs->u_regs[UREG_I0 + i + j] = args[j];
> }
>
> +static inline int syscall_get_arch(void)
> +{
> + return test_thread_flag(TIF_32BIT) ? AUDIT_ARCH_SPARC
> + : AUDIT_ARCH_SPARC64;
> +}
> +
> #endif /* __ASM_SPARC_SYSCALL_H */
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 9af01d7..8496cfa 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -343,6 +343,7 @@ enum {
> #define AUDIT_ARCH_IA64 (EM_IA_64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_M32R (EM_M32R)
> #define AUDIT_ARCH_M68K (EM_68K)
> +#define AUDIT_ARCH_MICROBLAZE (EM_MICROBLAZE)
> #define AUDIT_ARCH_MIPS (EM_MIPS)
> #define AUDIT_ARCH_MIPSEL (EM_MIPS|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_MIPS64 (EM_MIPS|__AUDIT_ARCH_64BIT)
> --
> 1.8.5.3
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply
* Re: [PATCH 00/18] powerpc/boot: 64bit little endian wrapper
From: Cedric Le Goater @ 2014-03-21 17:40 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev
In-Reply-To: <1395422919.26958.6.camel@smoke>
On 03/21/2014 06:28 PM, Geoff Levand wrote:
> Hi Cédric,
>
> On Thu, 2014-03-20 at 16:09 +0100, Cédric Le Goater wrote:
>> The following patchset adds support for 64bit little endian boot
>> wrapper. It is based on original code from Andrew Tauferner.
>
> I tested this on PS3 (64 bit BE) and found no regression.
>
> Tested by: Geoff Levand <geoff@infradead.org>
Thanks !
C.
^ permalink raw reply
* Re: [PATCH] mtd: m25p80: Modify the name of mtd_info
From: Scott Wood @ 2014-03-21 17:34 UTC (permalink / raw)
To: Hou Zhiqiang; +Cc: linuxppc-dev, mingkai.hu, linux-mtd, dwmw2
In-Reply-To: <1395400578-5637-1-git-send-email-B48286@freescale.com>
On Fri, 2014-03-21 at 19:16 +0800, Hou Zhiqiang wrote:
> @@ -1009,8 +1012,17 @@ static int m25p_probe(struct spi_device *spi)
>
> if (data && data->name)
> flash->mtd.name = data->name;
> - else
> - flash->mtd.name = dev_name(&spi->dev);
> + else{
Whitespace
> + ret = of_address_to_resource(mnp, 0, &res);
> + if (ret) {
> + dev_err(&spi->dev, "failed to get spi master resource\n");
> + return ret;
> + }
> + flash->mtd.name = kasprintf(GFP_KERNEL, "spi%x.%d",
> + (unsigned)res.start, spi->chip_select);
Don't use "unsigned" by itself. Don't cast physical addresses to
"unsigned int" -- use "unsigned long long".
-Scott
^ permalink raw reply
* Re: [PATCH 00/18] powerpc/boot: 64bit little endian wrapper
From: Geoff Levand @ 2014-03-21 17:28 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <1395328213-19206-1-git-send-email-clg@fr.ibm.com>
Hi Cédric,
On Thu, 2014-03-20 at 16:09 +0100, Cédric Le Goater wrote:
> The following patchset adds support for 64bit little endian boot
> wrapper. It is based on original code from Andrew Tauferner.
I tested this on PS3 (64 bit BE) and found no regression.
Tested by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply
* RE: [PATCH v3 5/5] powernv:cpufreq: Implement the driver->get() method
From: David Laight @ 2014-03-21 15:01 UTC (permalink / raw)
To: 'Viresh Kumar'
Cc: ego@linux.vnet.ibm.com, Linux PM list, linuxppc-dev@ozlabs.org
In-Reply-To: <CAKohpo=E1UEcLhbZHtmBJcD3S3trm8ypT7++quF_t6XHDXAvHA@mail.gmail.com>
From: Viresh Kumar [mailto:viresh.kumar@linaro.org]
> On 21 March 2014 17:31, David Laight <David.Laight@aculab.com> wrote:
> >> *(int *)ret_freq =3D freq;
> >
> > Because it is very likely to be wrong.
> > In general casts of pointers to integer types are dangerous.
>=20
> Where are we converting pointers to integers? We are doing a
> cast from 'void * ' to 'int *' and then using indirection operator
> to set its value.
You mis-parsed what I wrote, try:
In general casts of 'pointer to integer' types are dangerous.
Somewhere, much higher up the call stack, the address of an integer
variable is being taken and then passed as the 'void *' parameter.
The 'problem' is that it is quite easily to pass the address of
a 'long' instead. On 32bit and LE systems this won't always
be a problem. On 64bit BE it all goes wrong.
David
^ permalink raw reply
* Re: [PATCH v3 1/5] powernv: cpufreq driver for powernv platform
From: Gautham R Shenoy @ 2014-03-21 14:54 UTC (permalink / raw)
To: Viresh Kumar
Cc: ego@linux.vnet.ibm.com, Linux PM list, linuxppc-dev,
Anton Blanchard, Srivatsa S. Bhat
In-Reply-To: <CAKohpo=JqHOPF_pdcAsvRgc1vENmS=Sg9pS8ZXPGhfMFnA8ZUA@mail.gmail.com>
On Fri, Mar 21, 2014 at 04:24:27PM +0530, Viresh Kumar wrote:
> >> > +static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
> >> > +{
> >> > + int base, i;
> >> > +
> >> > + base = cpu_first_thread_sibling(policy->cpu);
> >> > +
> >> > + for (i = 0; i < threads_per_core; i++)
> >> > + cpumask_set_cpu(base + i, policy->cpus);
> >> > + policy->cpuinfo.transition_latency = 25000;
> >> > +
> >> > + policy->cur = powernv_freqs[0].frequency;
> >> > + cpufreq_frequency_table_get_attr(powernv_freqs, policy->cpu);
> >>
> >> This doesn't exist anymore.
Ok, I guess the right thing to do at this point is call
cpufreq_table_validate_and_show(policy, powernv_freqs);
Will fix the code to take care of this.
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [PATCH v3 1/5] powernv: cpufreq driver for powernv platform
From: Vaidyanathan Srinivasan @ 2014-03-21 14:48 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Linux PM list, Viresh Kumar, linuxppc-dev, Anton Blanchard,
Srivatsa S. Bhat
In-Reply-To: <20140321104317.GA2493@in.ibm.com>
* Gautham R Shenoy <ego@linux.vnet.ibm.com> [2014-03-21 16:13:17]:
> Hi Viresh,
>
> On Fri, Mar 21, 2014 at 02:11:32PM +0530, Viresh Kumar wrote:
> > On Thu, Mar 20, 2014 at 5:40 PM, Gautham R. Shenoy
> > <ego@linux.vnet.ibm.com> wrote:
> > > From: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
> >
> > Hi Vaidy,
> >
Hi Viresh,
Thanks for the detailed review.
> > > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> > > index 4b029c0..4ba1632 100644
> > > --- a/drivers/cpufreq/Kconfig
> > > +++ b/drivers/cpufreq/Kconfig
> > > @@ -48,6 +48,7 @@ config CPU_FREQ_STAT_DETAILS
> > > choice
> > > prompt "Default CPUFreq governor"
> > > default CPU_FREQ_DEFAULT_GOV_USERSPACE if ARM_SA1100_CPUFREQ || ARM_SA1110_CPUFREQ
> > > + default CPU_FREQ_DEFAULT_GOV_ONDEMAND if POWERNV_CPUFREQ
> >
> > Probably we should remove SA1100's entry as well from here. This is
> > not the right way of doing it. Imagine 100 platforms having entries here.
> > If you want it, then select it from your platforms Kconfig.
>
> Sure. Will move these bits and the other governor related bits to the
> Powernv Kconfig.
>
> > > diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> > > new file mode 100644
> > > index 0000000..ab1551f
> > > --- /dev/null
> > > +
> > > +#define pr_fmt(fmt) "powernv-cpufreq: " fmt
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/cpufreq.h>
> > > +#include <linux/of.h>
> > > +#include <asm/cputhreads.h>
> >
> > That's it? Sure?
> >
> > Even if things compile for you, you must explicitly include all the
> > files on which
> > you depend.
>
> Ok.
>
> >
> > > +
> > > + WARN_ON(len_ids != len_freqs);
> > > + nr_pstates = min(len_ids, len_freqs) / sizeof(u32);
> > > + WARN_ON(!nr_pstates);
> >
> > Why do you want to continue here?
>
> Good point. We might be better off exiting at this point.
Yes, we could return here. We do not generally come till this point
if the platform has a problem discovering PStates from device tree.
But there is no useful debug info after this point if nr_pstates is 0.
> >
> > > + pr_debug("NR PStates %d\n", nr_pstates);
> > > + for (i = 0; i < nr_pstates; i++) {
> > > + u32 id = be32_to_cpu(pstate_ids[i]);
> > > + u32 freq = be32_to_cpu(pstate_freqs[i]);
> > > +
> > > + pr_debug("PState id %d freq %d MHz\n", id, freq);
> > > + powernv_freqs[i].driver_data = i;
> >
> > I don't think you are using this field at all and this is the field you can
> > use for driver_data and so you can get rid of powernv_pstate_ids[ ].
>
> Using driver_data to record powernv_pstate_ids won't work since
> powernv_pstate_ids can be negative. So a pstate_id -3 can be confused
> with CPUFREQ_BOOST_FREQ thereby not displaying the frequency
> corresponding to pstate id -3. So for now I think we will be retaining
> powernv_pstate_ids.
Yeah, I had the driver written using driver_data to store pstates.
Gautham found the bug that we are missing one PState when we match the
ID with CPUFREQ_BOOST_FREQ!
We did not know that you have taken care of those issues. Ideally
I did expect that driver_data should not be touched by the framework.
Thanks for fixing that and allowing the back-end driver to use
driver_data.
> >
> > > + powernv_freqs[i].frequency = freq * 1000; /* kHz */
> > > + powernv_pstate_ids[i] = id;
> > > + }
> > > + /* End of list marker entry */
> > > + powernv_freqs[i].driver_data = 0;
> >
> > Not required.
>
> Ok.
> >
> > > + powernv_freqs[i].frequency = CPUFREQ_TABLE_END;
> > > +
> > > + /* Print frequency table */
> > > + for (i = 0; powernv_freqs[i].frequency != CPUFREQ_TABLE_END; i++)
> > > + pr_debug("%d: %d\n", i, powernv_freqs[i].frequency);
> >
> > You have already printed this table earlier..
>
> Fair enough.
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static struct freq_attr *powernv_cpu_freq_attr[] = {
> > > + &cpufreq_freq_attr_scaling_available_freqs,
> > > + NULL,
> > > +};
> >
> > Can use this instead: cpufreq_generic_attr?
>
> In this patch yes. But later patch introduces an additional attribute
> for displaying the nominal frequency. Will handle that part in a clean
> way in the next version.
>
> >
> > > +/* Helper routines */
> > > +
> > > +/* Access helpers to power mgt SPR */
> > > +
> > > +static inline unsigned long get_pmspr(unsigned long sprn)
> >
> > Looks big enough not be inlined?
>
> It is called from one function. It has been defined separately for
> readability.
Let the compiler decide. The code is straight forward :)
I wanted to keep this SPR operations in a separate function to
facilitate debug and also introduce any timing/sequence change here if
required. Keeping this separate is helpful, if inlining is
successful, we get a bonus :)
> >
> > > +{
> > > + switch (sprn) {
> > > + case SPRN_PMCR:
> > > + return mfspr(SPRN_PMCR);
> > > +
> > > + case SPRN_PMICR:
> > > + return mfspr(SPRN_PMICR);
> > > +
> > > + case SPRN_PMSR:
> > > + return mfspr(SPRN_PMSR);
> > > + }
> > > + BUG();
> > > +}
> > > +
> > > +static inline void set_pmspr(unsigned long sprn, unsigned long val)
> > > +{
> >
> > Same here..
>
> Same reason as above.
>
> >
> > > + switch (sprn) {
> > > + case SPRN_PMCR:
> > > + mtspr(SPRN_PMCR, val);
> > > + return;
> > > +
> > > + case SPRN_PMICR:
> > > + mtspr(SPRN_PMICR, val);
> > > + return;
> > > +
> > > + case SPRN_PMSR:
> > > + mtspr(SPRN_PMSR, val);
> > > + return;
> > > + }
> > > + BUG();
> > > +}
> > > +
> > > +static void set_pstate(void *pstate)
> > > +{
> > > + unsigned long val;
> > > + unsigned long pstate_ul = *(unsigned long *) pstate;
> >
> > Why not sending value only to this routine instead of pointer?
>
> Well this function is called via an smp_call_function. so, cannot send
> a value :(
>
> >
> > > +
> > > + val = get_pmspr(SPRN_PMCR);
> > > + val = val & 0x0000ffffffffffffULL;
> >
> > Maybe a blank line here?
>
> Ok.
>
> >
> > > + /* Set both global(bits 56..63) and local(bits 48..55) PStates */
> > > + val = val | (pstate_ul << 56) | (pstate_ul << 48);
> >
> > here as well?
>
> Ok.
> >
>
> > > + pr_debug("Setting cpu %d pmcr to %016lX\n", smp_processor_id(), val);
> > > + set_pmspr(SPRN_PMCR, val);
> > > +}
> > > +
> > > +static int powernv_set_freq(cpumask_var_t cpus, unsigned int new_index)
> > > +{
> > > + unsigned long val = (unsigned long) powernv_pstate_ids[new_index];
> >
> > I think automatic type conversion will happen here.
>
> Ok. Will fix this.
Most of the conversions are verbose mainly to help readability of the
required sign-extensions. There is scope to make these concise.
> >
> > > +
> > > + /*
> > > + * Use smp_call_function to send IPI and execute the
> > > + * mtspr on target cpu. We could do that without IPI
> > > + * if current CPU is within policy->cpus (core)
> > > + */
> >
> > Hmm, interesting I also feel there are cases where this routine can
> > get called from other CPUs. Can you list those use cases where it can
> > happen? Governors will mostly call this from one of the CPUs present
> > in policy->cpus.
>
> Consider the case when the governor is userspace and we are executing
>
> # echo freqvalue >
> /sys/devices/system/cpu/cpu<i>/cpufreq/scaling_setspeed
>
> from a cpu <j> which is not in policy->cpus of cpu i.
>
>
> > > +static int powernv_cpufreq_cpu_init(struct cpufreq_policy *policy)
> > > +{
> > > + int base, i;
> > > +
> > > +#ifdef CONFIG_SMP
> >
> > What will break if you don't have this ifdef here? Without that as well
> > below code should work?
> >
> > > + base = cpu_first_thread_sibling(policy->cpu);
> > > +
> > > + for (i = 0; i < threads_per_core; i++)
> > > + cpumask_set_cpu(base + i, policy->cpus);
> > > +#endif
> > > + policy->cpuinfo.transition_latency = 25000;
> > > +
> > > + policy->cur = powernv_freqs[0].frequency;
> > > + cpufreq_frequency_table_get_attr(powernv_freqs, policy->cpu);
> >
> > This doesn't exist anymore.
>
> Didn't get this comment!
>
> >
> > > + return cpufreq_frequency_table_cpuinfo(policy, powernv_freqs);
> >
> > Have you written this driver long time back? CPUFreq core has been
> > cleaned up heavily since last few kernel releases and I think there are
> > better helper routines available now.
>
> Yup it was written quite a while ago. And yeah, CPUFreq has changed
> quite a bit since the last time I saw it :-)
Yes, the driver is more than a year old and based on even older
implementation that I had written. I kept up until I got a compiler
warning or functional issue. Definitely could not catch-up with your
cleanups :)
> >
> > > +}
> > > +
> > > +static int powernv_cpufreq_cpu_exit(struct cpufreq_policy *policy)
> > > +{
> > > + cpufreq_frequency_table_put_attr(policy->cpu);
> > > + return 0;
> > > +}
> >
> > You don't need this..
>
> Why not ?
>
> >
> > > +static int powernv_cpufreq_verify(struct cpufreq_policy *policy)
> > > +{
> > > + return cpufreq_frequency_table_verify(policy, powernv_freqs);
> > > +}
> >
> > use generic verify function pointer instead..
> >
> > > +static int powernv_cpufreq_target(struct cpufreq_policy *policy,
> > > + unsigned int target_freq,
> > > + unsigned int relation)
> >
> > use target_index() instead..
> >
> > > +{
> > > + int rc;
> > > + struct cpufreq_freqs freqs;
> > > + unsigned int new_index;
> > > +
> > > + cpufreq_frequency_table_target(policy, powernv_freqs, target_freq,
> > > + relation, &new_index);
> > > +
> > > + freqs.old = policy->cur;
> > > + freqs.new = powernv_freqs[new_index].frequency;
> > > + freqs.cpu = policy->cpu;
> > > +
> > > + mutex_lock(&freq_switch_mutex);
> >
> > Why do you need this lock for?
>
> I guess it was to serialize accesses to PMCR. But Srivatsa's patch
> converts this to a per-core lock which probably is no longer required
> after your cpufreq_freq_transition_begin/end() patch series.
Right. Frequency transitions are per-core, the h/w SPRs are per-core.
We need to prevent threads from colliding on h/w SPR access. We do
make the lock per-core later in the series as mentioned above.
Gautham has addressed most comments.
Thanks,
Vaidy
^ permalink raw reply
* Re: [PATCH v3 5/5] powernv:cpufreq: Implement the driver->get() method
From: Gautham R Shenoy @ 2014-03-21 14:25 UTC (permalink / raw)
To: Viresh Kumar
Cc: ego@linux.vnet.ibm.com, Linux PM list, linuxppc-dev@ozlabs.org
In-Reply-To: <CAKohpomHHToqOND=ZtMeqBd11X3VA=PeZ2K6Tyj31y2V4G9-uA@mail.gmail.com>
On Fri, Mar 21, 2014 at 06:42:50PM +0530, Viresh Kumar wrote:
> On 21 March 2014 18:34, Gautham R Shenoy <ego@linux.vnet.ibm.com> wrote:
> > Consider the case when pmspr = 0x00feffffffffffff;
> >
> > We are interested in extracting the value 'fe'. And ensure that when
> > we store this value into an int, we get the sign extension right.
> >
> > So the following doesn't work:
> >
> > pstate_id = (pmspr_val >> 48) & 0xFFFFFFFF;
>
> What about pstate_id = (pmspr_val >> 48) & 0xFF; ??
Nope.
Suppose the pmspr_val is contained in the register
%rax, and pstate_id corresponds to the address -20(%rbp) then:
pstate_id = (pmspr_val >> 48) & 0xFF;
would corrspond to
shrq $48, %rax // Left shift by 48 bits
andl $255, %eax // Mask the lower 32 bits of %rax with 0x000000FF
movl %eax, -20(%rbp) // Store the lower 32 bits of %rax
into pstate_id
On the other hand,
pstate_id = (int)((s8)((pmspr_val >> 48) & 0xFF));
would correspond to:
shrq $48, %rax // Left shift by 48 bits.
movsbl %al, %eax // Move the lower 8 bits of %rax to %eax
with sign-extension.
movl %eax, -20(%rbp) // store the result in pstate_id;
So with this, we are getting the sign extension due to the use of movsbl.
And if local_pstate_id corresponds to the address -1(%rbp) then:
local_pstate_id = (pmspr_val >> 48) & 0xFF;
pstate_id = local_pstate_id;
would correspond to:
shrq $48, %rax // Left shift by 48 bits
movb %al, -1(%rbp) //copy the lower 8 bits to local_pstate_id
movsbl -1(%rbp), %eax //move the contents of
local_pstate_id to %eax with sign extension.
movl %eax, -20(%rbp) // Store the result in pstate_id
Hope this helps :-)
--
Thanks and Regards
gautham.
^ 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