Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sat, Mar 22, 2014 at 03:59:11PM -0700, James Bottomley wrote: On Sat, 2014-03-22 at 22:52 +, Russell King - ARM Linux wrote: 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. 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. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sun, Mar 23, 2014 at 02:04:46PM +1100, Benjamin Herrenschmidt wrote: 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 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. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sun, 2014-03-23 at 00:03 -0700, Christoph Hellwig wrote: On Sun, Mar 23, 2014 at 02:04:46PM +1100, Benjamin Herrenschmidt wrote: 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 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. Oh right... sorry for the noise. Ben. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sat, Mar 22, 2014 at 11:13:51AM -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. [...] 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 Acked-by: Heiko Carstens heiko.carst...@de.ibm.com FWIW, it would have been nice to keep the list of selected configs sorted. However no need to resend. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND][PATCH 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 li...@arm.linux.org.uk Cc: Tony Luck tony.l...@intel.com Cc: Fenghua Yu fenghua...@intel.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Ingo Molnar mi...@redhat.com Cc: H. Peter Anvin h...@zytor.com Cc: James E.J. Bottomley jbottom...@parallels.com Cc: Fenghua Yu fenghua...@intel.com Cc: Tony Luck tony.l...@intel.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Martin Schwidefsky schwidef...@de.ibm.com Cc: Heiko Carstens heiko.carst...@de.ibm.com Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Laura Abbott lau...@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..000 --- 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..000 ---
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
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 li...@arm.linux.org.uk Cc: Tony Luck tony.l...@intel.com Cc: Fenghua Yu fenghua...@intel.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Ingo Molnar mi...@redhat.com Cc: H. Peter Anvin h...@zytor.com Cc: James E.J. Bottomley jbottom...@parallels.com Cc: Fenghua Yu fenghua...@intel.com Cc: Tony Luck tony.l...@intel.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Martin Schwidefsky schwidef...@de.ibm.com Cc: Heiko Carstens heiko.carst...@de.ibm.com Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Laura Abbott lau...@codeaurora.org For the x86 part: Acked-by: Thomas Gleixner t...@linutronix.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
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 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
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 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sat, Mar 22, 2014 at 02:31:21PM -0700, James Bottomley wrote: 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. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sat, 2014-03-22 at 22:23 +, Russell King - ARM Linux wrote: On Sat, Mar 22, 2014 at 02:31:21PM -0700, James Bottomley wrote: 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) 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. OK, so lets see what the actual effort is. James -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sat, Mar 22, 2014 at 03:37:40PM -0700, James Bottomley wrote: On Sat, 2014-03-22 at 22:23 +, Russell King - ARM Linux wrote: On Sat, Mar 22, 2014 at 02:31:21PM -0700, James Bottomley wrote: 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 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. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
On Sat, 2014-03-22 at 22:52 +, Russell King - ARM Linux wrote: On Sat, Mar 22, 2014 at 03:37:40PM -0700, James Bottomley wrote: On Sat, 2014-03-22 at 22:23 +, Russell King - ARM Linux wrote: On Sat, Mar 22, 2014 at 02:31:21PM -0700, James Bottomley wrote: 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 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. 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 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
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 Heh, a self-selecting config option... I didn't know that trick ! Ben. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH 1/2] lib/scatterlist: Make ARCH_HAS_SG_CHAIN an actual Kconfig
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. Cc: Russell King li...@arm.linux.org.uk Cc: Tony Luck tony.l...@intel.com Cc: Fenghua Yu fenghua...@intel.com For powerpc Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Ingo Molnar mi...@redhat.com Cc: H. Peter Anvin h...@zytor.com Cc: James E.J. Bottomley jbottom...@parallels.com Cc: Fenghua Yu fenghua...@intel.com Cc: Tony Luck tony.l...@intel.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: Martin Schwidefsky schwidef...@de.ibm.com Cc: Heiko Carstens heiko.carst...@de.ibm.com Cc: Andrew Morton a...@linux-foundation.org Signed-off-by: Laura Abbott lau...@codeaurora.org -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
