Skip to contentSkip to conversation

Cleanup asm/scatterlist.h

PATCH RESEND In review Laura Abbott 8 participants 17 messages Mar 22, 2014 – Mar 28, 2014

Subsystems (10)
Arm (32-bit) x86 SCSI PowerPC +6 more subsystems
lore
Share & tools
lore
#0 lore raw

cover Cleanup asm/scatterlist.h

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
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
#1 lore raw

PATCH RESEND 1/2 lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

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 <[email protected]>
Cc Tony Luck <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Tony Luck <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Martin Schwidefsky <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Andrew Morton <[email protected]>
---
 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
arch/arm/Kconfig +1 -0
index 1594945..8122294 100644
@@ -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
arch/arm/include/asm/Kbuild +1 -0
index 3278afe..2357ed6 100644
@@ -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
arch/arm/include/asm/scatterlist.h deleted +0 -12
deleted file mode 100644
index cefdb8f..0000000
@@ -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 */
arch/arm64/Kconfig +1 -0
index 27bbcfc..f2f95f4 100644
@@ -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
arch/ia64/Kconfig +1 -0
index 0c8e553..13e2e8b 100644
@@ -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
arch/ia64/include/asm/Kbuild +1 -0
index 283a831..3906865 100644
@@ -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
arch/ia64/include/asm/scatterlist.h deleted +0 -7
deleted file mode 100644
index 08fd93b..0000000
@@ -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 */
arch/powerpc/Kconfig +1 -0
index 957bf34..659aee2 100644
@@ -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
arch/powerpc/include/asm/Kbuild +1 -0
index 6c0a955..ca596ec 100644
@@ -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
arch/powerpc/include/asm/scatterlist.h deleted +0 -17
deleted file mode 100644
index de1f620..0000000
@@ -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 */
arch/s390/Kconfig +1 -0
index 65a0775..d6c2059 100644
@@ -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
arch/s390/include/asm/Kbuild +1 -0
index 8386a4a..14be6d0 100644
@@ -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
arch/s390/include/asm/scatterlist.h deleted +0 -3
deleted file mode 100644
index 6d45ef6..0000000
@@ -1,3 +0,0 @@
-#include <asm-generic/scatterlist.h>
-
-#define ARCH_HAS_SG_CHAIN
arch/sparc/Kconfig +1 -0
index 7d8b7e9..7a179fe 100644
@@ -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
arch/sparc/include/asm/Kbuild +1 -0
index 4b60a0c..437f0c6 100644
@@ -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
arch/sparc/include/asm/scatterlist.h deleted +0 -8
deleted file mode 100644
index 92bb638..0000000
@@ -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) */
arch/x86/Kconfig +1 -0
index 0af5250..76997dc 100644
@@ -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
arch/x86/include/asm/Kbuild +1 -0
index 7f66985..9b3d749 100644
@@ -5,3 +5,4 @@ genhdr-y += unistd_64.h
 genhdr-y += unistd_x32.h
 
 generic-y += clkdev.h
+generic-y += scatterlist.h
arch/x86/include/asm/scatterlist.h deleted +0 -8
deleted file mode 100644
index 4240878..0000000
@@ -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 */
include/linux/scatterlist.h +1 -1
index a964f72..4b152c8 100644
@@ -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
 
include/scsi/scsi.h +1 -1
index 0a4edfe..d34cf2d 100644
@@ -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
lib/Kconfig +7 -0
index 991c98b..32c68d3 100644
@@ -451,4 +451,11 @@ config UCS2_STRING
 
 source "lib/fonts/Kconfig"
 
+#
+# sg chaining option
+#
+
+config ARCH_HAS_SG_CHAIN
+	def_bool n
+
 endmenu
lib/scatterlist.c +2 -2
index 3a8e8e8..4251cbd 100644
@@ -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
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
Acked-by on v1 1/2
#2 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 5 lines · Laura Abbott

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.

Quoted 16 lines
Cc Russell King <[email protected]>
Cc Tony Luck <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Tony Luck <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Martin Schwidefsky <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Andrew Morton <[email protected]>

For the x86 part:

maintainer
#3 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

In reply to Laura Abbott
Quoted 4 lines · Laura Abbott

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

#4 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 6 lines · James Bottomley
Quoted 4 lines · Laura Abbott

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.

Quoted 8 lines

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

#5 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

In reply to James Bottomley
Quoted 3 lines · James Bottomley

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.

Not quite. You're making the assumption that we can be sure that all the scatterlist users on an architecture have been converted - that's simply not true on ARM. We have some which have, and some which still have not been audited.

The cases that get us here would be old platform DMA code which walks scatterlists handed to it from drivers - stuff like arch/arm/mach-rpc/dma.c (which probably can cope), and drivers/scsi/arm/* (which definitely can't because of their SCSI pointers save/restore handling message.) I know that's one case where SG_CHAIN definitely isn't supported on ARM.

So, we had decided not to enable it, but this means that new stuff isn't benefitting from this. I've recently asked arm-soc to enable it for the modern multi-platform builds, because modern stuff really be written with correct SG chaining in mind.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
maintainer
#6 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 8 lines · Russell King - ARM Linux
Quoted 3 lines · James Bottomley

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.

Not quite. You're making the assumption that we can be sure that all the scatterlist users on an architecture have been converted - that's simply not true on ARM.

No I'm not, I said "now might be the time to ask which are the remaining architectures that cannot do SG chaining" I think it's time to list them so we know what work remains. I know we've got a bunch in parisc (all of our iommu code in driver/parisc - about 5 different ones - are unconverted). However, the conversion is pretty simple; it's mostly replacing sglist++ with sglist=sg_next(sglist)

Quoted 10 lines

We have some which have, and some which still
have not been audited.

The cases that get us here would be old platform DMA code which walks scatterlists handed to it from drivers - stuff like arch/arm/mach-rpc/dma.c (which probably can cope), and drivers/scsi/arm/* (which definitely can't because of their SCSI pointers save/restore handling message.) I know that's one case where SG_CHAIN definitely isn't supported on ARM.

Quoted 4 lines

So, we had decided not to enable it, but this means that new stuff isn't benefitting from this. I've recently asked arm-soc to enable it for the modern multi-platform builds, because modern stuff really be written with correct SG chaining in mind.

OK, so lets see what the actual effort is.

James

#7 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 6 lines · James Bottomley
Quoted 5 lines · Russell King - ARM Linux
Quoted 3 lines · James Bottomley

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.

Quoted 6 lines
Quoted 3 lines

Not quite. You're making the assumption that we can be sure that all the scatterlist users on an architecture have been converted - that's simply not true on ARM.

No I'm not, I said "now might be the time to ask which are the remaining architectures that cannot do SG chaining"

And I'm disagreeing with that statement which implies that it's something that is an architecture wide property for any particular architecture.

Right now in mainline, if ARM has IOMMU support enabled, then SG_CHAIN support will also be enabled. I've a patch out of tree which I've been using for years which enables SG_CHAIN for a particular SoC (Dove). Otherwise, it doesn't have support for SG_CHAIN.

PARISC on the other hand (as you list) has no support to enable SG_CHAIN under any circumstances.

Where we're disagreeing is whether this is something that is always-on or always-off for any particular architecture.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
maintainer
#8 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 22 lines · Russell King - ARM Linux
Quoted 6 lines · James Bottomley
Quoted 5 lines · Russell King - ARM Linux
Quoted 3 lines · James Bottomley

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.

Quoted 6 lines
Quoted 3 lines

Not quite. You're making the assumption that we can be sure that all the scatterlist users on an architecture have been converted - that's simply not true on ARM.

No I'm not, I said "now might be the time to ask which are the remaining architectures that cannot do SG chaining"

And I'm disagreeing with that statement which implies that it's something that is an architecture wide property for any particular architecture.

Right now in mainline, if ARM has IOMMU support enabled, then SG_CHAIN support will also be enabled. I've a patch out of tree which I've been using for years which enables SG_CHAIN for a particular SoC (Dove). Otherwise, it doesn't have support for SG_CHAIN.

Quoted 5 lines

PARISC on the other hand (as you list) has no support to enable SG_CHAIN under any circumstances.

Where we're disagreeing is whether this is something that is always-on or always-off for any particular architecture.

Actually, I don't disagree with that. PA used to share sb_iommu with ia64 (it's the same chipset for the HP versions), but we can't now because ia64 is chained and we're not and there's no way to say chained for this platform but not for these other more legacy ones. If you have a proposal for this, I'd be interested, so I don't have to do an all or nothing conversion, but the config option isn't it because our platform configuration is runtime determined (we usually select every driver and let the actual one be chosen at runtime from the config table).

James

#9 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 15 lines · James Bottomley
Quoted 8 lines · Russell King - ARM Linux

And I'm disagreeing with that statement which implies that it's something that is an architecture wide property for any particular architecture.

Right now in mainline, if ARM has IOMMU support enabled, then SG_CHAIN support will also be enabled. I've a patch out of tree which I've been using for years which enables SG_CHAIN for a particular SoC (Dove). Otherwise, it doesn't have support for SG_CHAIN.

Quoted 5 lines

PARISC on the other hand (as you list) has no support to enable SG_CHAIN under any circumstances.

Where we're disagreeing is whether this is something that is always-on or always-off for any particular architecture.

Quoted 8 lines

Actually, I don't disagree with that. PA used to share sb_iommu with ia64 (it's the same chipset for the HP versions), but we can't now because ia64 is chained and we're not and there's no way to say chained for this platform but not for these other more legacy ones. If you have a proposal for this, I'd be interested, so I don't have to do an all or nothing conversion, but the config option isn't it because our platform configuration is runtime determined (we usually select every driver and let the actual one be chosen at runtime from the config table).

At runtime, it's obviously a lot harder to resolve. However, that's not what we're trying to do with this patch.

The main issue is that Laura is trying to add SG chain support to ARM64. At the moment, ARM64 uses the asm-generic version of scatterlist.h directly.

Her first spin of the patch involved creating arch/arm64/include/asm/scatterlist.h, which was nothing more than an include of the asm-generic header file of the same name, and the definition of ARCH_HAS_SG_CHAIN. That seemed to be extremely wasteful and sub-optimal way to handle this.

Moving this symbol into the Kconfig means that ARM64 no longer has to have this additional file (and probably a bunch of other architectures fall into that same camp) and the ARM specific CONFIG_ARM_HAS_SG_CHAIN can be eliminated in favour of the now generic cross-arch config symbol.

We have plenty of other Kconfig symbols which control similar features throughout the kernel source, there is no reason that this one should not be the same.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
#10 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

In reply to Laura Abbott

arch/arm/Kconfig config ARM · line 87

Quoted 12 lines
arch/arm/Kconfig +1 -0
index 1594945..8122294 100644
@@ -82,6 +82,7 @@ config ARM
 	  <http://www.arm.linux.org.uk/>.
 
 config ARM_HAS_SG_CHAIN
+	select ARCH_HAS_SG_CHAIN
 	bool
 

Heh, a self-selecting config option... I didn't know that trick !

Ben.

#11 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 8 lines · Benjamin Herrenschmidt
Quoted 8 lines
arch/arm/Kconfig +0 -0
index 1594945..8122294 100644
@@ -82,6 +82,7 @@ config ARM
 	  <http://www.arm.linux.org.uk/>.
 
Quoted 6 lines

arch/arm/Kconfig config ARM · line 87

Quoted 4 lines
arch/arm/Kconfig +1 -0
 config ARM_HAS_SG_CHAIN
+	select ARCH_HAS_SG_CHAIN
 	bool
 

Heh, a self-selecting config option... I didn't know that trick !

ARM vs ARCH. However the arm variant of the variable should probably be consolidated into the ARCH one as a follow up.

#12 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

Quoted 16 lines · Christoph Hellwig
Quoted 8 lines · Benjamin Herrenschmidt
Quoted 8 lines
arch/arm/Kconfig +0 -0
index 1594945..8122294 100644
@@ -82,6 +82,7 @@ config ARM
 	  <http://www.arm.linux.org.uk/>.
 
Quoted 6 lines

arch/arm/Kconfig config ARM · line 87

Quoted 4 lines
arch/arm/Kconfig +1 -0
 config ARM_HAS_SG_CHAIN
+	select ARCH_HAS_SG_CHAIN
 	bool
 

Heh, a self-selecting config option... I didn't know that trick !

Quoted 2 lines

ARM vs ARCH. However the arm variant of the variable should probably be consolidated into the ARCH one as a follow up.

Oh right... sorry for the noise.

Ben.

Acked-by on v1 1/2
#13 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

In reply to Laura Abbott
Quoted 8 lines · Laura Abbott

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 <[email protected]>
Cc Tony Luck <[email protected]>
Cc Fenghua Yu <[email protected]>

For powerpc

Quoted 12 lines
Cc Paul Mackerras <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc Fenghua Yu <[email protected]>
Cc Tony Luck <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Martin Schwidefsky <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Andrew Morton <[email protected]>
Acked-by on v1 1/2
#14 lore raw

Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig

In reply to Laura Abbott
Quoted 5 lines · Laura Abbott

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.

[...]

arch/s390/Kconfig config S390 · line 146

Quoted 10 lines
arch/s390/Kconfig +1 -0
index 65a0775..d6c2059 100644
@@ -142,6 +142,7 @@ config S390
 	select SYSCTL_EXCEPTION_TRACE
 	select VIRT_CPU_ACCOUNTING
 	select VIRT_TO_BUS
+	select ARCH_HAS_SG_CHAIN
 

FWIW, it would have been nice to keep the list of selected configs sorted. However no need to resend.

#15 lore raw

PATCH RESEND 2/2 Cleanup useless architecture versions of scatterlist.h

In reply to Laura Abbott

There's no need to have an architecture version of scatterlist.h if the only thing the file does is include asm-generic/scatterlist.h. Switch to the asm-generic versions directly.

Cc Mikael Starvik <[email protected]>
Cc Hirokazu Takata <[email protected]>,
Cc Michal Simek <[email protected]>
Cc David Howells <[email protected]>
Cc Koichi Yasutake <[email protected]>
Cc Chen Liqin <[email protected]>,
Cc Lennox Wu <[email protected]>
---
 arch/alpha/include/asm/Kbuild             |  1 +
 arch/alpha/include/asm/scatterlist.h      |  6 ------
 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/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/score/include/asm/Kbuild             |  2 +-
 arch/score/include/asm/scatterlist.h      |  6 ------
 14 files changed, 7 insertions(+), 48 deletions(-)
 delete mode 100644 arch/alpha/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/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/score/include/asm/scatterlist.h
arch/alpha/include/asm/Kbuild +1 -0
index a73a8e2..2eeb6f2 100644
@@ -5,3 +5,4 @@ generic-y += exec.h
 generic-y += trace_clock.h
 generic-y += preempt.h
 generic-y += hash.h
+generic-y += scatterlist.h
arch/alpha/include/asm/scatterlist.h deleted +0 -6
deleted file mode 100644
index 017d747..0000000
@@ -1,6 +0,0 @@
-#ifndef _ALPHA_SCATTERLIST_H
-#define _ALPHA_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#endif /* !(_ALPHA_SCATTERLIST_H) */
arch/cris/include/asm/Kbuild +1 -0
index f3fd876..d9e86c7 100644
@@ -14,3 +14,4 @@ generic-y += trace_clock.h
 generic-y += vga.h
 generic-y += xor.h
 generic-y += preempt.h
+generic-y += scatterlist.h
arch/cris/include/asm/scatterlist.h deleted +0 -6
deleted file mode 100644
index f11f8f4..0000000
@@ -1,6 +0,0 @@
-#ifndef __ASM_CRIS_SCATTERLIST_H
-#define __ASM_CRIS_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#endif /* !(__ASM_CRIS_SCATTERLIST_H) */
arch/frv/include/asm/Kbuild +1 -0
index bc42f14..5ccdc46 100644
@@ -4,3 +4,4 @@ generic-y += exec.h
 generic-y += trace_clock.h
 generic-y += preempt.h
 generic-y += hash.h
+generic-y += scatterlist.h
arch/frv/include/asm/scatterlist.h deleted +0 -6
deleted file mode 100644
index 0e5eb30..0000000
@@ -1,6 +0,0 @@
-#ifndef _ASM_SCATTERLIST_H
-#define _ASM_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#endif /* !_ASM_SCATTERLIST_H */
arch/m32r/include/asm/Kbuild +1 -0
index 932435a..8672b27 100644
@@ -2,6 +2,7 @@
 generic-y += clkdev.h
 generic-y += exec.h
 generic-y += module.h
+generic-y += scatterlist.h
 generic-y += trace_clock.h
 generic-y += preempt.h
 generic-y += hash.h
arch/m32r/include/asm/scatterlist.h deleted +0 -6
deleted file mode 100644
index 7370b8b..0000000
@@ -1,6 +0,0 @@
-#ifndef _ASM_M32R_SCATTERLIST_H
-#define _ASM_M32R_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#endif /* _ASM_M32R_SCATTERLIST_H */
arch/microblaze/include/asm/Kbuild +1 -0
index 2b98bc7..9270f69 100644
@@ -4,5 +4,6 @@ generic-y += clkdev.h
 generic-y += exec.h
 generic-y += hash.h
 generic-y += trace_clock.h
+generic-y += scatterlist.h
 generic-y += syscalls.h
 generic-y += preempt.h
arch/microblaze/include/asm/scatterlist.h deleted +0 -1
deleted file mode 100644
index 35d786f..0000000
@@ -1 +0,0 @@
-#include <asm-generic/scatterlist.h>
arch/mn10300/include/asm/Kbuild +1 -0
index 992e989..3045ffe 100644
@@ -3,5 +3,6 @@ generic-y += barrier.h
 generic-y += clkdev.h
 generic-y += exec.h
 generic-y += hash.h
+generic-y += scatterlist.h
 generic-y += trace_clock.h
 generic-y += preempt.h
arch/mn10300/include/asm/scatterlist.h deleted +0 -16
deleted file mode 100644
index 7baa400..0000000
@@ -1,16 +0,0 @@
-/* MN10300 Scatterlist definitions
- *
- * Copyright (C) 2007 Matsushita Electric Industrial Co., Ltd.
- * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
- * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
- */
-#ifndef _ASM_SCATTERLIST_H
-#define _ASM_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#endif /* _ASM_SCATTERLIST_H */
arch/score/include/asm/Kbuild +1 -1
index 146b9d5..62d35f1 100644
@@ -7,4 +7,4 @@ generic-y += hash.h
 generic-y += trace_clock.h
 generic-y += xor.h
 generic-y += preempt.h
-
+generic-y += scatterlist.h
arch/score/include/asm/scatterlist.h deleted +0 -6
deleted file mode 100644
index 9f533b8..0000000
@@ -1,6 +0,0 @@
-#ifndef _ASM_SCORE_SCATTERLIST_H
-#define _ASM_SCORE_SCATTERLIST_H
-
-#include <asm-generic/scatterlist.h>
-
-#endif /* _ASM_SCORE_SCATTERLIST_H */
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
maintainer
#16 lore raw
In reply to Laura Abbott
Quoted 11 lines · On Sat, 22 Mar 2014 11:13:50 -0700 Laura Abbott <[email protected]> wrote:

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)

Quoted 2 lines

Andrew, once we get a few more Acked-bys can we take this through your tree as suggested by Will?

Grabbed. It's super late for 3.15-rc1 and I'm thinking of merging this in 3.16-rc1. Or I can maybe squeeze it into 3.15-rc2 is there's an immediatish need for it?
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev