Re: [PATCH 1/4] drivers: target: Move prototype declaration of function to header file target_core_pr.h
On Wed, Dec 18, 2013 at 11:54:32PM +0530, Rashika Kheria wrote: > Move prototype declaration of function > spc_parse_naa_6h_vendor_specific() from target_core_xcopy.c to header > file target_core_pr.h because it is used by more than one file. > > This eliminates the following warning in target_core_spc.c: > drivers/target/target_core_spc.c:138:6: warning: no previous prototype for > ‘spc_parse_naa_6h_vendor_specific’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/target/target_core_pr.h|5 + > drivers/target/target_core_xcopy.c |4 > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/target/target_core_pr.h b/drivers/target/target_core_pr.h > index ed75cdd..2ee2936 100644 > --- a/drivers/target/target_core_pr.h > +++ b/drivers/target/target_core_pr.h > @@ -43,6 +43,11 @@ > #define PR_APTPL_MAX_IPORT_LEN 256 > #define PR_APTPL_MAX_TPORT_LEN 256 > > +/* > + * Function defined in target_core_spc.c > + */ > +void spc_parse_naa_6h_vendor_specific(struct se_device *, unsigned char *); > + > extern struct kmem_cache *t10_pr_reg_cache; > > extern void core_pr_dump_initiator_port(struct t10_pr_registration *, > diff --git a/drivers/target/target_core_xcopy.c > b/drivers/target/target_core_xcopy.c > index 6b88a99..669c536 100644 > --- a/drivers/target/target_core_xcopy.c > +++ b/drivers/target/target_core_xcopy.c > @@ -40,10 +40,6 @@ > > static struct workqueue_struct *xcopy_wq = NULL; > /* > - * From target_core_spc.c > - */ > -extern void spc_parse_naa_6h_vendor_specific(struct se_device *, unsigned > char *); > -/* > * From target_core_device.c > */ > extern struct mutex g_device_mutex; > -- > 1.7.9.5 > -- 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: [PATCH 2/4] drivers: target: Mark function as static in target_core_iblock.c
On Wed, Dec 18, 2013 at 11:56:44PM +0530, Rashika Kheria wrote: > Mark function iblock_get_write_cache() as static in target_core_iblock.c > because it is not used outside this file. > > This eliminates the following warning in target_core_iblock.c: > drivers/target/target_core_iblock.c:766:6: warning: no previous prototype for > ‘iblock_get_write_cache’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/target/target_core_iblock.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_iblock.c > b/drivers/target/target_core_iblock.c > index c87959f..15d9121 100644 > --- a/drivers/target/target_core_iblock.c > +++ b/drivers/target/target_core_iblock.c > @@ -763,7 +763,7 @@ iblock_parse_cdb(struct se_cmd *cmd) > return sbc_parse_cdb(cmd, &iblock_sbc_ops); > } > > -bool iblock_get_write_cache(struct se_device *dev) > +static bool iblock_get_write_cache(struct se_device *dev) > { > struct iblock_dev *ib_dev = IBLOCK_DEV(dev); > struct block_device *bd = ib_dev->ibd_bd; > -- > 1.7.9.5 > -- 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: [PATCH 3/4] drivers: target: Mark functions as static in tcm_loop.c
On Thu, Dec 19, 2013 at 12:02:54AM +0530, Rashika Kheria wrote: > Mark functions tcm_loop_make_naa_tpg(), tcm_loop_drop_naa_tpg(), > tcm_loop_make_scsi_hba() and tcm_loop_drop_scsi_hba() as static in > loopback/tcm_loop.c because they are not used outside this file. > > This eliminates the following warning in loopback/tcm_loop.c: > drivers/target/loopback/tcm_loop.c:1231:25: warning: no previous prototype > for ‘tcm_loop_make_naa_tpg’ [-Wmissing-prototypes] > drivers/target/loopback/tcm_loop.c:1276:6: warning: no previous prototype for > ‘tcm_loop_drop_naa_tpg’ [-Wmissing-prototypes] > drivers/target/loopback/tcm_loop.c:1308:16: warning: no previous prototype > for ‘tcm_loop_make_scsi_hba’ [-Wmissing-prototypes] > drivers/target/loopback/tcm_loop.c:1378:6: warning: no previous prototype for > ‘tcm_loop_drop_scsi_hba’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/target/loopback/tcm_loop.c |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/target/loopback/tcm_loop.c > b/drivers/target/loopback/tcm_loop.c > index 1b41e67..763ee45 100644 > --- a/drivers/target/loopback/tcm_loop.c > +++ b/drivers/target/loopback/tcm_loop.c > @@ -1228,7 +1228,7 @@ static struct configfs_attribute *tcm_loop_tpg_attrs[] > = { > > /* Start items for tcm_loop_naa_cit */ > > -struct se_portal_group *tcm_loop_make_naa_tpg( > +static struct se_portal_group *tcm_loop_make_naa_tpg( > struct se_wwn *wwn, > struct config_group *group, > const char *name) > @@ -1273,7 +1273,7 @@ struct se_portal_group *tcm_loop_make_naa_tpg( > return &tl_tpg->tl_se_tpg; > } > > -void tcm_loop_drop_naa_tpg( > +static void tcm_loop_drop_naa_tpg( > struct se_portal_group *se_tpg) > { > struct se_wwn *wwn = se_tpg->se_tpg_wwn; > @@ -1305,7 +1305,7 @@ void tcm_loop_drop_naa_tpg( > > /* Start items for tcm_loop_cit */ > > -struct se_wwn *tcm_loop_make_scsi_hba( > +static struct se_wwn *tcm_loop_make_scsi_hba( > struct target_fabric_configfs *tf, > struct config_group *group, > const char *name) > @@ -1375,7 +1375,7 @@ out: > return ERR_PTR(ret); > } > > -void tcm_loop_drop_scsi_hba( > +static void tcm_loop_drop_scsi_hba( > struct se_wwn *wwn) > { > struct tcm_loop_hba *tl_hba = container_of(wwn, > -- > 1.7.9.5 > -- 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: [PATCH 4/4] drivers: target: Mark functions and structures as static in tfc_conf.c
On Thu, Dec 19, 2013 at 12:05:59AM +0530, Rashika Kheria wrote: > Mark functions ft_tpg_alloc_fabric_acl(), ft_register_configfs() and > ft_deregister_configfs() as static in tcm_fc/tfc_conf.c because they are > not used outside this file. > > This eliminates the following warnings in tcm_fc/tfc_conf.c: > drivers/target/tcm_fc/tfc_conf.c:270:21: warning: no previous prototype for > ‘ft_tpg_alloc_fabric_acl’ [-Wmissing-prototypes] > drivers/target/tcm_fc/tfc_conf.c:555:5: warning: no previous prototype for > ‘ft_register_configfs’ [-Wmissing-prototypes] > drivers/target/tcm_fc/tfc_conf.c:602:6: warning: no previous prototype for > ‘ft_deregister_configfs’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/target/tcm_fc/tfc_conf.c |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/target/tcm_fc/tfc_conf.c > b/drivers/target/tcm_fc/tfc_conf.c > index c6932fb..e879da8 100644 > --- a/drivers/target/tcm_fc/tfc_conf.c > +++ b/drivers/target/tcm_fc/tfc_conf.c > @@ -267,7 +267,7 @@ struct ft_node_acl *ft_acl_get(struct ft_tpg *tpg, struct > fc_rport_priv *rdata) > return found; > } > > -struct se_node_acl *ft_tpg_alloc_fabric_acl(struct se_portal_group *se_tpg) > +static struct se_node_acl *ft_tpg_alloc_fabric_acl(struct se_portal_group > *se_tpg) > { > struct ft_node_acl *acl; > > @@ -552,7 +552,7 @@ static struct target_core_fabric_ops ft_fabric_ops = { > .fabric_drop_nodeacl = &ft_del_acl, > }; > > -int ft_register_configfs(void) > +static int ft_register_configfs(void) > { > struct target_fabric_configfs *fabric; > int ret; > @@ -599,7 +599,7 @@ int ft_register_configfs(void) > return 0; > } > > -void ft_deregister_configfs(void) > +static void ft_deregister_configfs(void) > { > if (!ft_configfs) > return; > -- > 1.7.9.5 > -- 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: [PATCH] VMW_PVSCSI: Fix the issue of DMA-API related warnings.
On Fri, Mar 21, 2014 at 2:08 PM, Arvind Kumar wrote: > The driver is missing calls to pci_dma_mapping_error() after > performing the DMA mapping, which caused DMA-API warning to > show up in dmesg's output. Though that happens only when > DMA_API_DEBUG option is enabled. This change fixes the issue > and makes pvscsi_map_buffers() function more robust. > > Signed-off-by: Arvind Kumar > Cc: Josh Boyer This patch has been sent and pinged for 3 months now. It's gotten no comments at all. Should we send it to Linus so it actually gets picked up? josh > --- > drivers/scsi/vmw_pvscsi.c | 45 > +++-- > drivers/scsi/vmw_pvscsi.h |2 +- > 2 files changed, 40 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c > index c88e146..9478a00 100644 > --- a/drivers/scsi/vmw_pvscsi.c > +++ b/drivers/scsi/vmw_pvscsi.c > @@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx, > * Map all data buffers for a command into PCI space and > * setup the scatter/gather list if needed. > */ > -static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, > - struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, > - struct PVSCSIRingReqDesc *e) > +static int pvscsi_map_buffers(struct pvscsi_adapter *adapter, > + struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, > + struct PVSCSIRingReqDesc *e) > { > unsigned count; > unsigned bufflen = scsi_bufflen(cmd); > @@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter > *adapter, > e->dataLen = bufflen; > e->dataAddr = 0; > if (bufflen == 0) > - return; > + return 0; > > sg = scsi_sglist(cmd); > count = scsi_sg_count(cmd); > if (count != 0) { > int segs = scsi_dma_map(cmd); > - if (segs > 1) { > + > + if (segs == -ENOMEM) { > + scmd_printk(KERN_ERR, cmd, > + "vmw_pvscsi: Failed to map cmd sglist for > DMA.\n"); > + return -1; > + } else if (segs > 1) { > pvscsi_create_sg(ctx, sg, segs); > > e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST; > ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl, > SGL_SIZE, > PCI_DMA_TODEVICE); > + if (pci_dma_mapping_error(adapter->dev, ctx->sglPA)) { > + scmd_printk(KERN_ERR, cmd, > + "vmw_pvscsi: Failed to map ctx > sglist for DMA.\n"); > + scsi_dma_unmap(cmd); > + ctx->sglPA = 0; > + return -1; > + } > e->dataAddr = ctx->sglPA; > } else > e->dataAddr = sg_dma_address(sg); > @@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter > *adapter, > */ > ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen, > cmd->sc_data_direction); > + if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) { > + scmd_printk(KERN_ERR, cmd, > + "vmw_pvscsi: Failed to map direct data > buffer for DMA.\n"); > + return -1; > + } > e->dataAddr = ctx->dataPA; > } > + > + return 0; > } > > static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter, > @@ -712,6 +731,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter > *adapter, > ctx->sensePA = pci_map_single(adapter->dev, cmd->sense_buffer, > SCSI_SENSE_BUFFERSIZE, > PCI_DMA_FROMDEVICE); > + if (pci_dma_mapping_error(adapter->dev, ctx->sensePA)) { > + scmd_printk(KERN_ERR, cmd, > + "vmw_pvscsi: Failed to map sense buffer > for DMA.\n"); > + ctx->sensePA = 0; > + return -1; > + } > e->senseAddr = ctx->sensePA; >
Re: WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
On Tue, Jul 29, 2014 at 02:26:11PM +0200, Christoph Hellwig wrote: > On Mon, Jul 28, 2014 at 11:49:22AM +0400, James Bottomley wrote: > > That needs to be > > > > From: James Bottomley > > > > As well. I do list handling on hansenpartnership.com to minimise > > exchange wreckage on mailinglists, but I should acknowledge Parallels as > > supporting the work I do. > > Thanks, I've update the author, added a Cc to ћtable and pushed it out to > the core-for-3.17 branch. This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there? josh -- 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: [PATCH v2 6/8] tools rpmb: add RPBM access tool
Hi Tomas, On Mon, Apr 04, 2016 at 02:11:22PM +0300, Tomas Winkler wrote: > Add simple RPMB host testing tool. It can be used > to program key, write and read data block, and retrieve > write counter. > > Signed-off-by: Tomas Winkler > --- > V2: resend > MAINTAINERS | 1 + > tools/Makefile| 16 +- > tools/rpmb/.gitignore | 2 + > tools/rpmb/Makefile | 32 ++ > tools/rpmb/rpmb.c | 807 > ++ > 5 files changed, 853 insertions(+), 5 deletions(-) > create mode 100644 tools/rpmb/.gitignore > create mode 100644 tools/rpmb/Makefile > create mode 100644 tools/rpmb/rpmb.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 07bd6f380460..b7966b95c265 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9425,6 +9425,7 @@ F: drivers/char/rpmb/* > F: include/uapi/linux/rpmb.h > F: include/linux/rpmb.h > F: Documentation/ABI/testing/sysfs-class-rpmb > +F: tools/rpmb/ > > RTL2830 MEDIA DRIVER > M: Antti Palosaari > diff --git a/tools/Makefile b/tools/Makefile > index 60c7e6c8ff17..5b37ccd95cab 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -19,6 +19,7 @@ help: > @echo ' lguest - a minimal 32-bit x86 hypervisor' > @echo ' net- misc networking tools' > @echo ' perf - Linux performance measurement and > analysis tool' > + @echo ' rpmb - Replay protected memory block access > tool' > @echo ' selftests - various kernel selftests' > @echo ' spi- spi tools' > @echo ' objtool- an ELF object analysis tool' > @@ -55,7 +56,8 @@ acpi: FORCE > cpupower: FORCE > $(call descend,power/$@) > > -cgroup firewire hv guest spi usb virtio vm net iio gpio objtool: FORCE > +cgroup firewire hv guest rpmb spi usb virtio vm net iio gpio objtool: FORCE > +cgroup firewire hv guest rpmb spi usb virtio vm net iio: FORCE This looks like an artifact from a bad merge resolution. The second line needs to be deleted. This probably explains the kbuild robot build errors, since CONFIG_STACK_VALIDATION requires objtool to be built. > $(call descend,$@) > > liblockdep: FORCE > @@ -85,7 +87,7 @@ freefall: FORCE > $(call descend,laptop/$@) > > all: acpi cgroup cpupower hv firewire lguest \ > - perf selftests turbostat usb \ > + perf rpmb selftests turbostat usb \ > virtio vm net x86_energy_perf_policy \ > tmon freefall objtool > > @@ -95,7 +97,8 @@ acpi_install: > cpupower_install: > $(call descend,power/$(@:_install=),install) > > -cgroup_install firewire_install hv_install lguest_install perf_install > usb_install virtio_install vm_install net_install objtool_install: > +cgroup_install firewire_install hv_install lguest_install perf_install > rpmb_install usb_install virtio_install vm_install net_install > objtool_install: > +cgroup_install firewire_install hv_install lguest_install perf_install > rpmb_install usb_install virtio_install vm_install net_install: Same here. -- Josh -- 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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Wed, Apr 13, 2016 at 09:55:09AM -0700, James Bottomley wrote: > On Wed, 2016-04-13 at 10:15 -0500, Josh Poimboeuf wrote: > > On Wed, Apr 13, 2016 at 07:36:07AM -0500, Josh Poimboeuf wrote: > > > On Wed, Apr 13, 2016 at 02:12:25PM +0200, Denys Vlasenko wrote: > > > > On 04/13/2016 05:36 AM, Josh Poimboeuf wrote: > > > > > On Thu, Feb 04, 2016 at 08:45:35PM +0100, Denys Vlasenko wrote: > > > > > > Sometimes gcc mysteriously doesn't inline > > > > > > very small functions we expect to be inlined. See > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66122 > > > > > > > > > > > > With this .config: > > > > > > http://busybox.net/~vda/kernel_config_OPTIMIZE_INLINING_and_O > > > > > > s, > > > > > > the following functions get deinlined many times. > > > > > > Examples of disassembly: > > > > > > > > > > > > (12 copies, 51 calls): > > > > > >66 8b 07mov(%rdi),%ax > > > > > >55 push %rbp > > > > > >48 89 e5mov%rsp,%rbp > > > > > >86 e0 xchg %ah,%al > > > > > >5d pop%rbp > > > > > >c3 retq > > > > ... > > > > > > This patch fixes this via s/inline/__always_inline/. > > > > > > Code size decrease after the patch is ~4.5k: > > > > > > > > > > > > text data bss dec hex filename > > > > > > 92202377 20826112 36417536 149446025 8e85d89 vmlinux > > > > > > 92197848 20826112 36417536 149441496 8e84bd8 > > > > > > vmlinux5_swap_after > > > > > > > > > > > > Signed-off-by: Denys Vlasenko > > > > > > Cc: Ingo Molnar > > > > > > Cc: Thomas Graf > > > > > > Cc: Peter Zijlstra > > > > > > Cc: David Rientjes > > > > > > Cc: Andrew Morton > > > > > > Cc: linux-ker...@vger.kernel.org > > > > > > --- > > > > > > include/uapi/linux/byteorder/big_endian.h| 24 > > > > > > > > > > > > include/uapi/linux/byteorder/little_endian.h | 24 > > > > > > > > > > > > include/uapi/linux/swab.h| 10 +- > > > > > > 3 files changed, 29 insertions(+), 29 deletions(-) > > > > > > > > > > Hi, > > > > > > > > > > This patch seems to trigger a gcc bug which can produce corrupt > > > > > code. I > > > > > discovered it when investigating an objtool warning reported by > > > > > kbuild > > > > > bot: > > > > > > > > > > http://www.spinics.net/lists/linux-scsi/msg95481.html > > > > > > > > > > From the disassembly of drivers/scsi/qla2xxx/qla_attr.o: > > > > > > > > > > 2f53 : > > > > > 2f53: 55 push %rbp > > > > > 2f54: 48 89 e5mov%rsp,%rbp > > > > > > > > > > 2f57 : > > > > > 2f57: 55 push %rbp > > > > > 2f58: b9 e8 00 00 00 mov$0xe8,%ecx > > > > > 2f5d: 48 89 e5mov%rsp,%rbp > > > > > ... > > > > > > > > > > Note that qla2x00_get_host_fabric_name() is inexplicably > > > > > truncated after > > > > > setting up the frame pointer. It falls through to the next > > > > > function, which is > > > > > very wrong. > > > > > > > > Wow, that's ... interesting. > > > > > > > > > > > > > I can recreate it with either gcc 5.3.1 or gcc 6.0 on > > > > > linus/master with > > > > > the .config from the above link. > > > > > > > > > > The call chain which appears to trigger the problem is: > > > > > > > > > > qla2x00_get_host_fabric_name() > > > > > wwn_to_u64() > > > > > get_unaligned_be64() > > > > > be64
Re: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Thu, Apr 14, 2016 at 05:29:06PM +0200, Denys Vlasenko wrote: > On 04/13/2016 07:10 PM, Josh Poimboeuf wrote: > >>>>>> From the disassembly of drivers/scsi/qla2xxx/qla_attr.o: > >>>>>> > >>>>>> 2f53 : > >>>>>> 2f53: 55 push %rbp > >>>>>> 2f54: 48 89 e5mov%rsp,%rbp > >>>>>> > >>>>>> 2f57 : > >>>>>> 2f57: 55 push %rbp > >>>>>> 2f58: b9 e8 00 00 00 mov$0xe8,%ecx > >>>>>> 2f5d: 48 89 e5mov%rsp,%rbp > >>>>>> ... > >>>>>> > >>>>>> Note that qla2x00_get_host_fabric_name() is inexplicably > >>>>>> truncated after > >>>>>> setting up the frame pointer. It falls through to the next > >>>>>> function, which is > >>>>>> very wrong. > >>>>> > >>>>> Wow, that's ... interesting. > >>>>> > >>>>> > >>>>>> I can recreate it with either gcc 5.3.1 or gcc 6.0 on > >>>>>> linus/master with > >>>>>> the .config from the above link. > >>>>>> > >>>>>> The call chain which appears to trigger the problem is: > >>>>>> > >>>>>> qla2x00_get_host_fabric_name() > >>>>>> wwn_to_u64() > >>>>>> get_unaligned_be64() > >>>>>> be64_to_cpup() > >>>>>> __be64_to_cpup() <- changed to __always_inline by this > >>>>>> patch > >>>>>> > >>>>>> It occurs with the combination of the following two recent > >>>>>> commits: > >>>>>> > >>>>>> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force > >>>>>> inlining of some byteswap operations") > >>>>>> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn > >>>>>> access") > >>>>>> > >>>>>> I can confirm that reverting either patch makes the problem go > >>>>>> away. > >>>>>> I'm planning on opening a gcc bug tomorrow. > >>>>> > >>>>> > >>>>> Note that if CONFIG_OPTIMIZE_INLINING is not set, _all_ "inline" > >>>>> keywords are in fact __always_inline, so the bug must be > >>>>> triggering > >>>>> even without the patch. > >>>> > >>>> Makes sense in theory, but the bug doesn't actually trigger when I > >>>> revert the patch and set CONFIG_OPTIMIZE_INLINING=n. > >>>> > >>>> Perhaps even more surprising, it doesn't trigger *with* the patch > >>>> and > >>>> CONFIG_OPTIMIZE_INLINING=n. > >>> > >>> [ Adding James to CC since this bug affects scsi. ] > >>> > >>> Here's the gcc bug: > >>> > >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 > >>> > >> > >> > >> Actually, adding me doesn't help, I've added linux-scsi. The summary > >> is that there's a but in gcc-5.3.1 which is miscompiling qla_attr.c ... > >> this means we're going to have to ask the compiler version of reported > >> crashes. > > > > The bug isn't specific to a compiler version. I've seen it with gcc > > 5.3.1 and gcc 6.0. I haven't tried any older versions. And the gcc bug > > hasn't been resolved (or even investigated) yet. > > > > The bug is triggered by a combination of the above two commits from the > > 4.6 merge window, so presumably we'd need to revert one of them to avoid > > crashes in 4.6. > > The bug is indeed in the compiler. 4.9 and all later versions are affected. > gcc bugzilla now has a reproducer. In abridged form: > > > static inline __attribute__((always_inline)) u64 __swab64p(const u64 *p) > { > return (__builtin_constant_p((u64)(*p)) ? ((u64)( (((u64)(*p) & > (u64)0x00ffULL) << 56) | (((u64)(*p) & > (u64)0xff00ULL) << 40) | (((u64)(*p) & > (u64)0x00ffULL) << 24) | (((u64)(*p) & > (u64)0x00
Re: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Fri, Apr 15, 2016 at 07:45:19AM +0200, Ingo Molnar wrote: > > * Denys Vlasenko wrote: > > > > In fact, the following patch seems to fix it: > > > > > > diff --git a/include/scsi/scsi_transport_fc.h > > > b/include/scsi/scsi_transport_fc.h > > > index bf66ea6..56b9e81 100644 > > > --- a/include/scsi/scsi_transport_fc.h > > > +++ b/include/scsi/scsi_transport_fc.h > > > @@ -796,7 +796,7 @@ fc_remote_port_chkready(struct fc_rport *rport) > > > return result; > > > } > > > > > > -static inline u64 wwn_to_u64(u8 *wwn) > > > +static __always_inline u64 wwn_to_u64(u8 *wwn) > > > { > > > return get_unaligned_be64(wwn); > > > } > > > > It is not a guarantee. > > Of course it's a workaround - but is there any deterministic way to turn off > this > GCC bug (by activating some GCC command line switch), or do we have to live > with > objtool warning about this GCC? I don't think we know yet if there's a reliable way to turn the bug off. Also, according to the gcc guys, this bug won't always result in a truncated function, and may sometimes just make some inline function call sites disappear: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646#c14 though I haven't been able to confirm that experimentally. But if it's true, that means that objtool won't be able to detect all cases of the bug and some function calls may just silently disappear! There's a lot of activity in the bug now, so hopefully they'll be able to tell us soon if there's a reliable way to avoid it and/or detect it. BTW, Denys posted a workaround patch for the qla2 code: https://lkml.kernel.org/r/1460716583-15673-1-git-send-email-dvlas...@redhat.com -- Josh -- 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: [PATCH] qla2xxx: rewrite code to avoid hitting gcc bug 70646
On Fri, Apr 15, 2016 at 12:05:26PM -0700, James Bottomley wrote: > On Fri, 2016-04-15 at 20:56 +0200, Denys Vlasenko wrote: > > and now *many* users of qla2x00 and new-ish gcc are going to > > very much notice it, as their kernels will start crashing reliably. > > > > The commits can be reverted, sure, but they per se do not contain > > anything unusual. They, together with not very typical construct > > in qla2x00_get_host_fabric_name, one > > which boils down to "swab64p(constant_array_of_8_bytes)", > > just happen to nudge gcc in a right way to finally trigger the bug. > > > > So I came with another idea how to forestall the imminent deluge of > > qla2x00 oops reports - this patch. > > There are actually a raft of checkers that run the upstream code which > aren't seeing any problem; likely because the code is harder to trigger > than you think. So, lets wait until the resolution of the other thread > before we panic, especially since we're only at -rc3. Regardless of the outcome of the gcc bug, it seems kind of silly to byteswap a constant value of 0x. uint8_t node_name[WWN_SIZE] = { 0xFF, 0xFF, 0xFF, 0xFF, \ 0xFF, 0xFF, 0xFF, 0xFF}; u64 fabric_name = wwn_to_u64(node_name); Similar to what Denys suggested, it can just be: u64 fabric_name = -1; or u64 fabric_name = 0x; Wouldn't that be an improvement to the code regardless? -- Josh -- 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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Fri, Apr 15, 2016 at 08:47:45AM -0500, Josh Poimboeuf wrote: > On Fri, Apr 15, 2016 at 07:45:19AM +0200, Ingo Molnar wrote: > > > > * Denys Vlasenko wrote: > > > > > > In fact, the following patch seems to fix it: > > > > > > > > diff --git a/include/scsi/scsi_transport_fc.h > > > > b/include/scsi/scsi_transport_fc.h > > > > index bf66ea6..56b9e81 100644 > > > > --- a/include/scsi/scsi_transport_fc.h > > > > +++ b/include/scsi/scsi_transport_fc.h > > > > @@ -796,7 +796,7 @@ fc_remote_port_chkready(struct fc_rport *rport) > > > > return result; > > > > } > > > > > > > > -static inline u64 wwn_to_u64(u8 *wwn) > > > > +static __always_inline u64 wwn_to_u64(u8 *wwn) > > > > { > > > > return get_unaligned_be64(wwn); > > > > } > > > > > > It is not a guarantee. > > > > Of course it's a workaround - but is there any deterministic way to turn > > off this > > GCC bug (by activating some GCC command line switch), or do we have to live > > with > > objtool warning about this GCC? > > I don't think we know yet if there's a reliable way to turn the bug off. > > Also, according to the gcc guys, this bug won't always result in a > truncated function, and may sometimes just make some inline function > call sites disappear: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646#c14 > > though I haven't been able to confirm that experimentally. But if it's > true, that means that objtool won't be able to detect all cases of the > bug and some function calls may just silently disappear! > > There's a lot of activity in the bug now, so hopefully they'll be able > to tell us soon if there's a reliable way to avoid it and/or detect it. > > BTW, Denys posted a workaround patch for the qla2 code: > > > https://lkml.kernel.org/r/1460716583-15673-1-git-send-email-dvlas...@redhat.com Martin Jambor wrote a succinct summary of the conditions needed for this bug: "This bug can occur when an inlineable function containing a call to __builtin_constant_p, which checks a parameter or a value it references and a (possibly indirect) caller of the function actually passes a constant, but stores it using a type of a different size." So to prevent it from happening elsewhere in the kernel, it sounds like we'd have to either remove all uses of __builtin_constant_p() or disable inlining completely. There's also no reliable way to detect the bug has occurred, though objtool will detect it in cases when the function gets truncated. -- Josh -- 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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Sat, Apr 16, 2016 at 09:42:37AM +0200, Arnd Bergmann wrote: > On Friday 15 April 2016 07:45:19 Ingo Molnar wrote: > > > > * Denys Vlasenko wrote: > > > > > > In fact, the following patch seems to fix it: > > > > > > > > diff --git a/include/scsi/scsi_transport_fc.h > > > > b/include/scsi/scsi_transport_fc.h > > > > index bf66ea6..56b9e81 100644 > > > > --- a/include/scsi/scsi_transport_fc.h > > > > +++ b/include/scsi/scsi_transport_fc.h > > > > @@ -796,7 +796,7 @@ fc_remote_port_chkready(struct fc_rport *rport) > > > > return result; > > > > } > > > > > > > > -static inline u64 wwn_to_u64(u8 *wwn) > > > > +static __always_inline u64 wwn_to_u64(u8 *wwn) > > > > { > > > > return get_unaligned_be64(wwn); > > > > } > > > > > > It is not a guarantee. > > > > Of course it's a workaround - but is there any deterministic way to turn > > off this > > GCC bug (by activating some GCC command line switch), or do we have to live > > with > > objtool warning about this GCC? > > > > Which, by the way, is pretty cool! > > I have done a patch for the asm-generic/unaligned handling recently that > reworks the implementation to avoid an ARM specific bug (gcc uses certain > CPU instructions that require aligned data when we tell it that unaligned > data is not). > > It changes the code enough that the gcc bug might not be triggered any more, > aside from generating far superior code in some cases. I tried this patch, but unfortunately it doesn't make the gcc bug go away. -- Josh -- 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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Sat, Apr 16, 2016 at 11:03:32AM +0200, Ingo Molnar wrote: > > * Josh Poimboeuf wrote: > > > > I don't think we know yet if there's a reliable way to turn the bug off. > > > > > > Also, according to the gcc guys, this bug won't always result in a > > > truncated function, and may sometimes just make some inline function > > > call sites disappear: > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646#c14 > > > > > > though I haven't been able to confirm that experimentally. But if it's > > > true, that means that objtool won't be able to detect all cases of the > > > bug and some function calls may just silently disappear! > > > > > > There's a lot of activity in the bug now, so hopefully they'll be able > > > to tell us soon if there's a reliable way to avoid it and/or detect it. > > > > > > BTW, Denys posted a workaround patch for the qla2 code: > > > > > > > > > https://lkml.kernel.org/r/1460716583-15673-1-git-send-email-dvlas...@redhat.com > > > > Martin Jambor wrote a succinct summary of the conditions needed for this > > bug: > > > > "This bug can occur when an inlineable function containing a call to > > __builtin_constant_p, which checks a parameter or a value it > > references and a (possibly indirect) caller of the function actually > > passes a constant, but stores it using a type of a different size." > > > > So to prevent it from happening elsewhere in the kernel, it sounds like > > we'd have to either remove all uses of __builtin_constant_p() or disable > > inlining completely. > > > > There's also no reliable way to detect the bug has occurred, though > > objtool will detect it in cases when the function gets truncated. > > So it appears to me that due to the hard to detect nature of the GCC bug the > fix > will probably be backported by them, so I think we should be fine with > relying on > objtool to detect weird code sequences in the kernel, and should work around > specific instances of the bug. I agree. So how should we work around the bug in this case? There have been several suggestions: - change wwn_to_u64() to __always_inline - change qla2x00_get_host_fabric_name() to skip the unnecessary call to wwn_to_u64() - revert one of the two commits: bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some byteswap operations") ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") -- Josh -- 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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)
On Mon, Apr 18, 2016 at 04:07:51PM +0200, Arnd Bergmann wrote: > On Monday 18 April 2016 08:39:32 Josh Poimboeuf wrote: > > > > I agree. So how should we work around the bug in this case? There have > > been several suggestions: > > > > - change wwn_to_u64() to __always_inline > > > > - change qla2x00_get_host_fabric_name() to skip the unnecessary call to > > wwn_to_u64() > > > > - revert one of the two commits: > > bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some > > byteswap operations") > > ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") > > What about the patch to change get_unaligned_be64() that I posted? > > I think we want to merge that anyway, I just don't know if that helps > with this particular problem as well. I replied to your other email about that -- it doesn't seem to help this issue. -- Josh -- 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
[PATCH] scsi: fc: force inlining of wwn conversion functions
objtool reports [1] the following warning: drivers/scsi/qla2xxx/qla_attr.o: warning: objtool: qla2x00_get_host_fabric_name() falls through to next function qla2x00_get_starget_port_name() This warning is due to a gcc bug [2] which causes corrupt code: 2f53 : 2f53: 55 push %rbp 2f54: 48 89 e5mov%rsp,%rbp 2f57 : 2f57: 55 push %rbp 2f58: b9 e8 00 00 00 mov$0xe8,%ecx 2f5d: 48 89 e5mov%rsp,%rbp ... Note that qla2x00_get_host_fabric_name() is inexplicably truncated after setting up the frame pointer. It falls through to the next function, which is very bad. It occurs with the combination of the following two recent commits: bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some byteswap operations") ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") The call chain which appears to trigger the problem is: qla2x00_get_host_fabric_name() wwn_to_u64() get_unaligned_be64() be64_to_cpup() __be64_to_cpup() The bug requires very specific conditions to trigger. According to Martin Jambor (from the gcc bugzilla): "This bug can occur when an inlineable function containing a call to __builtin_constant_p, which checks a parameter or a value it references and a (possibly indirect) caller of the function actually passes a constant, but stores it using a type of a different size." There's no reliable way to avoid (or even detect) the bug. Until it gets fixed in released versions of gcc, the least intrusive workaround for this particular issue is to force the wwn conversion functions to be inlined. [1] https://lists.01.org/pipermail/kbuild-all/2016-April/019579.html [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 Reported-by: kbuild test robot Signed-off-by: Josh Poimboeuf --- include/scsi/scsi_transport_fc.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h index bf66ea6..1919cd4 100644 --- a/include/scsi/scsi_transport_fc.h +++ b/include/scsi/scsi_transport_fc.h @@ -796,12 +796,12 @@ fc_remote_port_chkready(struct fc_rport *rport) return result; } -static inline u64 wwn_to_u64(u8 *wwn) +static __always_inline u64 wwn_to_u64(u8 *wwn) { return get_unaligned_be64(wwn); } -static inline void u64_to_wwn(u64 inm, u8 *wwn) +static __always_inline void u64_to_wwn(u64 inm, u8 *wwn) { put_unaligned_be64(inm, wwn); } -- 2.4.11 -- 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: [PATCH] scsi: fc: force inlining of wwn conversion functions
James, Can you merge this patch for 4.6? On Tue, Apr 19, 2016 at 08:56:00AM -0500, Josh Poimboeuf wrote: > objtool reports [1] the following warning: > > drivers/scsi/qla2xxx/qla_attr.o: warning: objtool: > qla2x00_get_host_fabric_name() falls through to next function > qla2x00_get_starget_port_name() > > This warning is due to a gcc bug [2] which causes corrupt code: > > 2f53 : > 2f53: 55 push %rbp > 2f54: 48 89 e5mov%rsp,%rbp > > 2f57 : > 2f57: 55 push %rbp > 2f58: b9 e8 00 00 00 mov$0xe8,%ecx > 2f5d: 48 89 e5mov%rsp,%rbp > ... > > Note that qla2x00_get_host_fabric_name() is inexplicably truncated after > setting up the frame pointer. It falls through to the next function, > which is very bad. > > It occurs with the combination of the following two recent commits: > > bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some > byteswap operations") > ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") > > The call chain which appears to trigger the problem is: > > qla2x00_get_host_fabric_name() > wwn_to_u64() > get_unaligned_be64() > be64_to_cpup() > __be64_to_cpup() > > The bug requires very specific conditions to trigger. According to Martin > Jambor (from the gcc bugzilla): > > "This bug can occur when an inlineable function containing a call to > __builtin_constant_p, which checks a parameter or a value it > references and a (possibly indirect) caller of the function actually > passes a constant, but stores it using a type of a different size." > > There's no reliable way to avoid (or even detect) the bug. Until it > gets fixed in released versions of gcc, the least intrusive workaround > for this particular issue is to force the wwn conversion functions to be > inlined. > > [1] https://lists.01.org/pipermail/kbuild-all/2016-April/019579.html > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 > > Reported-by: kbuild test robot > Signed-off-by: Josh Poimboeuf > --- > include/scsi/scsi_transport_fc.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/scsi/scsi_transport_fc.h > b/include/scsi/scsi_transport_fc.h > index bf66ea6..1919cd4 100644 > --- a/include/scsi/scsi_transport_fc.h > +++ b/include/scsi/scsi_transport_fc.h > @@ -796,12 +796,12 @@ fc_remote_port_chkready(struct fc_rport *rport) > return result; > } > > -static inline u64 wwn_to_u64(u8 *wwn) > +static __always_inline u64 wwn_to_u64(u8 *wwn) > { > return get_unaligned_be64(wwn); > } > > -static inline void u64_to_wwn(u64 inm, u8 *wwn) > +static __always_inline void u64_to_wwn(u64 inm, u8 *wwn) > { > put_unaligned_be64(inm, wwn); > } > -- > 2.4.11 > -- Josh -- 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: [PATCH, RFT] byteswap: try to avoid __builtin_constant_p gcc bug
On Thu, Apr 28, 2016 at 12:00:36AM +0200, Arnd Bergmann wrote: > This is another attempt to avoid a regression in wwn_to_u64() > after that started using get_unaligned_be64(), which in turn > ran into a bug on gcc-4.9 through 6.1. > > As part of the problem is how __builtin_constant_p gets evaluated > on an argument passed by reference into an inline function, this > avoids the use of __builtin_constant_p() for all architectures > that set CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not > set ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably > do not suffer from the problem in the qla2xxx driver, but they > might still run into it elsewhere. > > I have not been able to reproduce the original problem, so I don't > know if this patch solves it, but at least it leads to simpler > code doing the same thing, so at least there should be no downsides. > > Please test. > > Signed-off-by: Arnd Bergmann Nice patch. I can confirm it fixes the issue with gcc 5.3.1. Tested-by: Josh Poimboeuf Reviewed-by: Josh Poimboeuf > diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h > index 3f10e5317b46..de56fd54428d 100644 > --- a/include/uapi/linux/swab.h > +++ b/include/uapi/linux/swab.h > @@ -45,9 +45,7 @@ > > static inline __attribute_const__ __u16 __fswab16(__u16 val) > { > -#ifdef __HAVE_BUILTIN_BSWAP16__ > - return __builtin_bswap16(val); > -#elif defined (__arch_swab16) > +#if defined (__arch_swab16) > return __arch_swab16(val); > #else > return ___constant_swab16(val); > @@ -56,9 +54,7 @@ static inline __attribute_const__ __u16 __fswab16(__u16 val) > > static inline __attribute_const__ __u32 __fswab32(__u32 val) > { > -#ifdef __HAVE_BUILTIN_BSWAP32__ > - return __builtin_bswap32(val); > -#elif defined(__arch_swab32) > +#if defined(__arch_swab32) > return __arch_swab32(val); > #else > return ___constant_swab32(val); > @@ -67,9 +63,7 @@ static inline __attribute_const__ __u32 __fswab32(__u32 val) > > static inline __attribute_const__ __u64 __fswab64(__u64 val) > { > -#ifdef __HAVE_BUILTIN_BSWAP64__ > - return __builtin_bswap64(val); > -#elif defined (__arch_swab64) > +#if defined (__arch_swab64) > return __arch_swab64(val); > #elif defined(__SWAB_64_THRU_32__) > __u32 h = val >> 32; > @@ -102,28 +96,40 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 > val) > * __swab16 - return a byteswapped 16-bit value > * @x: value to byteswap > */ > +#ifdef __HAVE_BUILTIN_BSWAP16__ > +#define __swab16(x) __builtin_bswap16((__u16)(x)) > +#else > #define __swab16(x) \ > (__builtin_constant_p((__u16)(x)) ? \ > ___constant_swab16(x) : \ > __fswab16(x)) > +#endif > > /** > * __swab32 - return a byteswapped 32-bit value > * @x: value to byteswap > */ > +#ifdef __HAVE_BUILTIN_BSWAP32__ > +#define __swab32(x) __builtin_bswap32((__u32)(x)) > +#else > #define __swab32(x) \ > (__builtin_constant_p((__u32)(x)) ? \ > ___constant_swab32(x) : \ > __fswab32(x)) > +#endif > > /** > * __swab64 - return a byteswapped 64-bit value > * @x: value to byteswap > */ > +#ifdef __HAVE_BUILTIN_BSWAP64__ > +#define __swab64(x) __builtin_bswap64((__u64)(x)) > +#else > #define __swab64(x) \ > (__builtin_constant_p((__u64)(x)) ? \ > ___constant_swab64(x) : \ > __fswab64(x)) > +#endif > > /** > * __swahw32 - return a word-swapped 32-bit value > -- Josh -- 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: [PATCH 1/3] scsi: ufs: Allow vendor specific initialization
On Tue, Aug 13, 2013 at 04:30:18PM +0530, Sujit Reddy Thumma wrote: [..] > diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c > index a823cf4..829f7a4 100644 > --- a/drivers/scsi/ufs/ufshcd-pci.c > +++ b/drivers/scsi/ufs/ufshcd-pci.c > @@ -191,7 +191,13 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *id) > return err; > } > > - err = ufshcd_init(&pdev->dev, &hba, mmio_base, pdev->irq); > + err = ufshcd_alloc_host(&pdev->dev, &hba); > + if (err) { > + dev_err(&pdev->dev, "Allocation failed\n"); > + goto out_iounmap You seem to be missing a semicolon here. Also, which tree were these patches generated against? They fail to apply at least on the last few 3.11-rc's. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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: scsi: Implement per-cpu logging buffer
On Thu, Feb 12, 2015 at 02:29:35PM +0100, Hannes Reinecke wrote: > On 02/12/2015 01:36 PM, Geert Uytterhoeven wrote: > > On Wed, Feb 11, 2015 at 8:16 PM, Linux Kernel Mailing List > > wrote: > >> Gitweb: > >> http://git.kernel.org/linus/;a=commit;h=ded85c193a391a84076d5c6a7a5668fe164a490e > >> Commit: ded85c193a391a84076d5c6a7a5668fe164a490e > >> Parent: b0a93d96b2814c725161f91a4e35d0c29ec0f95b > >> Refname:refs/heads/master > >> Author: Hannes Reinecke > >> AuthorDate: Thu Jan 8 07:43:42 2015 +0100 > >> Committer: Christoph Hellwig > >> CommitDate: Fri Jan 9 15:44:28 2015 +0100 > >> > >> scsi: Implement per-cpu logging buffer > >> > >> Implement a per-cpu buffer for formatting messages to avoid line breaks > >> up under high load. This patch implements scmd_printk() and > >> sdev_prefix_printk() using the per-cpu buffer and makes sdev_printk() a > >> wrapper for sdev_prefix_printk(). > >> > >> Tested-by: Robert Elliott > >> Reviewed-by: Robert Elliott > >> Signed-off-by: Hannes Reinecke > >> Signed-off-by: Christoph Hellwig > > > >> --- /dev/null > >> +++ b/drivers/scsi/scsi_logging.c > > > >> +#define SCSI_LOG_SPOOLSIZE 4096 > >> +#define SCSI_LOG_BUFSIZE 128 > >> + > >> +#if (SCSI_LOG_SPOOLSIZE / SCSI_LOG_BUFSIZE) > BITS_PER_LONG > >> +#warning SCSI logging bitmask too large > >> +#endif > >> + > >> +struct scsi_log_buf { > >> + char buffer[SCSI_LOG_SPOOLSIZE]; > >> + unsigned long map; > >> +}; > >> + > >> +static DEFINE_PER_CPU(struct scsi_log_buf, scsi_format_log); > > > > Do we really need a static 4 KiB per-CPU buffer? > > > > bloat-o-meter: > > > > add/remove: 183/94 grow/shrink: 314/211 up/down: 33467/-21291 (12176) > > function old new delta > > scsi_format_log-4100 +4100 > > handle_mm_fault 17942750+956 > > scsi_log_print_sense_hdr - 774+774 > > proc_keys_show - 770+770 > > > Define 'need'. > We don't absolutely 'need' it. (Configure it out and it's gone). > > But when we want to avoid several logging messages coming in from > various CPUs overwriting each other and _not_ introduce additional > latency by locking a single buffer, then yes. > > We can possibly reduce it to, say, 1KiB or even lower by imposing > stricter rules on the logging functions. > But I don't see a way around the per-CPU buffer. It seems very odd to introduce a mechanism like this specifically for SCSI, rather than introducing a generic per-CPU buffered-print mechanism in printk, controlled by a config option. That option could then automatically go away when !SMP, or !PRINTK, or if users don't actually care about message ordering. Also, this doesn't seem to be configurable at all. - Josh Triplett -- 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: scsi: Implement per-cpu logging buffer
On Fri, Feb 13, 2015 at 09:48:36AM +0100, Hannes Reinecke wrote: > On 02/12/2015 06:18 PM, Josh Triplett wrote: > > On Thu, Feb 12, 2015 at 02:29:35PM +0100, Hannes Reinecke wrote: > >> On 02/12/2015 01:36 PM, Geert Uytterhoeven wrote: > >>> On Wed, Feb 11, 2015 at 8:16 PM, Linux Kernel Mailing List > >>> wrote: > >>>> Gitweb: > >>>> http://git.kernel.org/linus/;a=commit;h=ded85c193a391a84076d5c6a7a5668fe164a490e > >>>> Commit: ded85c193a391a84076d5c6a7a5668fe164a490e > >>>> Parent: b0a93d96b2814c725161f91a4e35d0c29ec0f95b > >>>> Refname:refs/heads/master > >>>> Author: Hannes Reinecke > >>>> AuthorDate: Thu Jan 8 07:43:42 2015 +0100 > >>>> Committer: Christoph Hellwig > >>>> CommitDate: Fri Jan 9 15:44:28 2015 +0100 > >>>> > >>>> scsi: Implement per-cpu logging buffer > >>>> > >>>> Implement a per-cpu buffer for formatting messages to avoid line > >>>> breaks > >>>> up under high load. This patch implements scmd_printk() and > >>>> sdev_prefix_printk() using the per-cpu buffer and makes > >>>> sdev_printk() a > >>>> wrapper for sdev_prefix_printk(). > >>>> > >>>> Tested-by: Robert Elliott > >>>> Reviewed-by: Robert Elliott > >>>> Signed-off-by: Hannes Reinecke > >>>> Signed-off-by: Christoph Hellwig > >>> > >>>> --- /dev/null > >>>> +++ b/drivers/scsi/scsi_logging.c > >>> > >>>> +#define SCSI_LOG_SPOOLSIZE 4096 > >>>> +#define SCSI_LOG_BUFSIZE 128 > >>>> + > >>>> +#if (SCSI_LOG_SPOOLSIZE / SCSI_LOG_BUFSIZE) > BITS_PER_LONG > >>>> +#warning SCSI logging bitmask too large > >>>> +#endif > >>>> + > >>>> +struct scsi_log_buf { > >>>> + char buffer[SCSI_LOG_SPOOLSIZE]; > >>>> + unsigned long map; > >>>> +}; > >>>> + > >>>> +static DEFINE_PER_CPU(struct scsi_log_buf, scsi_format_log); > >>> > >>> Do we really need a static 4 KiB per-CPU buffer? > >>> > >>> bloat-o-meter: > >>> > >>> add/remove: 183/94 grow/shrink: 314/211 up/down: 33467/-21291 (12176) > >>> function old new delta > >>> scsi_format_log-4100 +4100 > >>> handle_mm_fault 17942750+956 > >>> scsi_log_print_sense_hdr - 774+774 > >>> proc_keys_show - 770+770 > >>> > >> Define 'need'. > >> We don't absolutely 'need' it. (Configure it out and it's gone). > >> > >> But when we want to avoid several logging messages coming in from > >> various CPUs overwriting each other and _not_ introduce additional > >> latency by locking a single buffer, then yes. > >> > >> We can possibly reduce it to, say, 1KiB or even lower by imposing > >> stricter rules on the logging functions. > >> But I don't see a way around the per-CPU buffer. > > > > It seems very odd to introduce a mechanism like this specifically for > > SCSI, rather than introducing a generic per-CPU buffered-print mechanism > > in printk, controlled by a config option. That option could then > > automatically go away when !SMP, or !PRINTK, or if users don't actually > > care about message ordering. > > > But then we ran afoul with the printk purists. > > Thing is, if we were to use per-CPU buffers for printk() out of > necessity we have to queue these buffers for writing out. > So there is a time window during which the message already is in the > per-CPU buffer but still not printed out as printk() is currently > writing out one of the other per-CPU buffers. > > If there is a consensus that such a delayed printk() is useful and a > valid use case then yes, sure I can give it a go. > > Personally I think printk() currently has an unfortunate double > purpose: on the one hand it should print out emergency messages > immediate so that they'll be visible if the system crashes. On the > other hand it is used as a general logging facility, where frankly > most of the subsystems simple do not care at all if and when the > message are printed. > S
Re: scsi: Implement per-cpu logging buffer
On Sat, Feb 14, 2015 at 03:29:37PM +0100, Hannes Reinecke wrote: > On 02/13/2015 04:45 PM, Josh Triplett wrote: > > On Fri, Feb 13, 2015 at 09:48:36AM +0100, Hannes Reinecke wrote: > >> On 02/12/2015 06:18 PM, Josh Triplett wrote: > >>> On Thu, Feb 12, 2015 at 02:29:35PM +0100, Hannes Reinecke wrote: > >>>> On 02/12/2015 01:36 PM, Geert Uytterhoeven wrote: > >>>>> On Wed, Feb 11, 2015 at 8:16 PM, Linux Kernel Mailing List > >>>>> wrote: > >>>>>> Gitweb: > >>>>>> http://git.kernel.org/linus/;a=commit;h=ded85c193a391a84076d5c6a7a5668fe164a490e > >>>>>> Commit: ded85c193a391a84076d5c6a7a5668fe164a490e > >>>>>> Parent: b0a93d96b2814c725161f91a4e35d0c29ec0f95b > >>>>>> Refname:refs/heads/master > >>>>>> Author: Hannes Reinecke > >>>>>> AuthorDate: Thu Jan 8 07:43:42 2015 +0100 > >>>>>> Committer: Christoph Hellwig > >>>>>> CommitDate: Fri Jan 9 15:44:28 2015 +0100 > >>>>>> > >>>>>> scsi: Implement per-cpu logging buffer > >>>>>> > >>>>>> Implement a per-cpu buffer for formatting messages to avoid line > >>>>>> breaks > >>>>>> up under high load. This patch implements scmd_printk() and > >>>>>> sdev_prefix_printk() using the per-cpu buffer and makes > >>>>>> sdev_printk() a > >>>>>> wrapper for sdev_prefix_printk(). > >>>>>> > >>>>>> Tested-by: Robert Elliott > >>>>>> Reviewed-by: Robert Elliott > >>>>>> Signed-off-by: Hannes Reinecke > >>>>>> Signed-off-by: Christoph Hellwig > >>>>> > >>>>>> --- /dev/null > >>>>>> +++ b/drivers/scsi/scsi_logging.c > >>>>> > >>>>>> +#define SCSI_LOG_SPOOLSIZE 4096 > >>>>>> +#define SCSI_LOG_BUFSIZE 128 > >>>>>> + > >>>>>> +#if (SCSI_LOG_SPOOLSIZE / SCSI_LOG_BUFSIZE) > BITS_PER_LONG > >>>>>> +#warning SCSI logging bitmask too large > >>>>>> +#endif > >>>>>> + > >>>>>> +struct scsi_log_buf { > >>>>>> + char buffer[SCSI_LOG_SPOOLSIZE]; > >>>>>> + unsigned long map; > >>>>>> +}; > >>>>>> + > >>>>>> +static DEFINE_PER_CPU(struct scsi_log_buf, scsi_format_log); > >>>>> > >>>>> Do we really need a static 4 KiB per-CPU buffer? > >>>>> > >>>>> bloat-o-meter: > >>>>> > >>>>> add/remove: 183/94 grow/shrink: 314/211 up/down: 33467/-21291 (12176) > >>>>> function old new delta > >>>>> scsi_format_log-4100 +4100 > >>>>> handle_mm_fault 17942750+956 > >>>>> scsi_log_print_sense_hdr - 774+774 > >>>>> proc_keys_show - 770+770 > >>>>> > >>>> Define 'need'. > >>>> We don't absolutely 'need' it. (Configure it out and it's gone). > >>>> > >>>> But when we want to avoid several logging messages coming in from > >>>> various CPUs overwriting each other and _not_ introduce additional > >>>> latency by locking a single buffer, then yes. > >>>> > >>>> We can possibly reduce it to, say, 1KiB or even lower by imposing > >>>> stricter rules on the logging functions. > >>>> But I don't see a way around the per-CPU buffer. > >>> > >>> It seems very odd to introduce a mechanism like this specifically for > >>> SCSI, rather than introducing a generic per-CPU buffered-print mechanism > >>> in printk, controlled by a config option. That option could then > >>> automatically go away when !SMP, or !PRINTK, or if users don't actually > >>> care about message ordering. > >>> > >> But then we ran afoul with the printk purists. > >> > >> Thing is, if we were to use per-CPU buffers for printk() out of > >> necessity we have to queue th
[PATCH Resend] VMW_PVSCSI: Fix the issue of DMA-API related warnings.
The driver is missing calls to pci_dma_mapping_error() after performing the DMA mapping, which caused DMA-API warning to show up in dmesg's output. Though that happens only when DMA_API_DEBUG option is enabled. This change fixes the issue and makes pvscsi_map_buffers() function more robust. Signed-off-by: Arvind Kumar Cc: Josh Boyer Reviewed-by: Thomas Hellstrom Signed-off-by: Josh Boyer --- - Resend of patch that was never committed for some reason drivers/scsi/vmw_pvscsi.c | 45 +++-- drivers/scsi/vmw_pvscsi.h | 2 +- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c index 0f133c1817de..19734494f9ec 100644 --- a/drivers/scsi/vmw_pvscsi.c +++ b/drivers/scsi/vmw_pvscsi.c @@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx, * Map all data buffers for a command into PCI space and * setup the scatter/gather list if needed. */ -static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, - struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, - struct PVSCSIRingReqDesc *e) +static int pvscsi_map_buffers(struct pvscsi_adapter *adapter, + struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, + struct PVSCSIRingReqDesc *e) { unsigned count; unsigned bufflen = scsi_bufflen(cmd); @@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, e->dataLen = bufflen; e->dataAddr = 0; if (bufflen == 0) - return; + return 0; sg = scsi_sglist(cmd); count = scsi_sg_count(cmd); if (count != 0) { int segs = scsi_dma_map(cmd); - if (segs > 1) { + + if (segs == -ENOMEM) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map cmd sglist for DMA.\n"); + return -1; + } else if (segs > 1) { pvscsi_create_sg(ctx, sg, segs); e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST; ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl, SGL_SIZE, PCI_DMA_TODEVICE); + if (pci_dma_mapping_error(adapter->dev, ctx->sglPA)) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map ctx sglist for DMA.\n"); + scsi_dma_unmap(cmd); + ctx->sglPA = 0; + return -1; + } e->dataAddr = ctx->sglPA; } else e->dataAddr = sg_dma_address(sg); @@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, */ ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen, cmd->sc_data_direction); + if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map direct data buffer for DMA.\n"); + return -1; + } e->dataAddr = ctx->dataPA; } + + return 0; } static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter, @@ -690,6 +709,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, ctx->sensePA = pci_map_single(adapter->dev, cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE, PCI_DMA_FROMDEVICE); + if (pci_dma_mapping_error(adapter->dev, ctx->sensePA)) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map sense buffer for DMA.\n"); + ctx->sensePA = 0; + return -1; + } e->senseAddr = ctx->sensePA; e->senseLen = SCSI_SENSE_BUFFERSIZE; } else { @@ -711,7 +736,15 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, else e->flags = 0; - pvscsi_map_buffers(adapter, ctx, cmd, e); + if (pvscsi_map_buffers(adapter, ctx, cmd, e) != 0) { + if (cmd->sense_buffer) { + pci_unmap_single(adapter->dev, ctx->sensePA, +SCSI_SENSE_BUFFERSIZE, +PCI_DMA_FROMDEVICE); + ctx->sensePA = 0; + }
Re: [PATCH Resend] VMW_PVSCSI: Fix the issue of DMA-API related warnings.
On Wed, Dec 2, 2015 at 3:42 AM, Johannes Thumshirn wrote: > Hi Josh, > > On Tue, 2015-12-01 at 11:34 -0500, Josh Boyer wrote: >> The driver is missing calls to pci_dma_mapping_error() after >> performing the DMA mapping, which caused DMA-API warning to >> show up in dmesg's output. Though that happens only when >> DMA_API_DEBUG option is enabled. This change fixes the issue >> and makes pvscsi_map_buffers() function more robust. >> >> Signed-off-by: Arvind Kumar >> Cc: Josh Boyer >> Reviewed-by: Thomas Hellstrom >> Signed-off-by: Josh Boyer >> --- >> >> - Resend of patch that was never committed for some reason >> >> drivers/scsi/vmw_pvscsi.c | 45 +++-- >> drivers/scsi/vmw_pvscsi.h | 2 +- >> 2 files changed, 40 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c >> index 0f133c1817de..19734494f9ec 100644 >> --- a/drivers/scsi/vmw_pvscsi.c >> +++ b/drivers/scsi/vmw_pvscsi.c >> @@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx, >> * Map all data buffers for a command into PCI space and >> * setup the scatter/gather list if needed. >> */ >> -static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, >> -struct pvscsi_ctx *ctx, struct scsi_cmnd >> *cmd, >> -struct PVSCSIRingReqDesc *e) >> +static int pvscsi_map_buffers(struct pvscsi_adapter *adapter, >> + struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, >> + struct PVSCSIRingReqDesc *e) >> { >> unsigned count; >> unsigned bufflen = scsi_bufflen(cmd); >> @@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter >> *adapter, >> e->dataLen = bufflen; >> e->dataAddr = 0; >> if (bufflen == 0) >> - return; >> + return 0; >> >> sg = scsi_sglist(cmd); >> count = scsi_sg_count(cmd); >> if (count != 0) { >> int segs = scsi_dma_map(cmd); >> - if (segs > 1) { >> + >> + if (segs == -ENOMEM) { >> + scmd_printk(KERN_ERR, cmd, >> + "vmw_pvscsi: Failed to map cmd sglist >> for DMA.\n"); >> + return -1; > > Please return -ENOMEM instead of -1 > >> + } else if (segs > 1) { >> pvscsi_create_sg(ctx, sg, segs); >> >> e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST; >> ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl, >> SGL_SIZE, >> PCI_DMA_TODEVICE); >> + if (pci_dma_mapping_error(adapter->dev, ctx->sglPA)) >> { >> + scmd_printk(KERN_ERR, cmd, >> + "vmw_pvscsi: Failed to map ctx >> sglist for DMA.\n"); >> + scsi_dma_unmap(cmd); >> + ctx->sglPA = 0; >> + return -1; > > Same here. > >> + } >> e->dataAddr = ctx->sglPA; >> } else >> e->dataAddr = sg_dma_address(sg); >> @@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter >> *adapter, >>*/ >> ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen, >>cmd->sc_data_direction); >> + if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) { >> + scmd_printk(KERN_ERR, cmd, >> + "vmw_pvscsi: Failed to map direct data >> buffer for DMA.\n"); >> + return -1; > > And here. > >> + } >> e->dataAddr = ctx->dataPA; >> } >> + >> + return 0; >> } >> >> static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter, >> @@ -690,6 +709,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter >> *adapter, >> ctx->sensePA = pci_map_single(adapter->dev, cmd- >> >sense_buffer, >> SCSI_SENSE_BUFFERSIZE, >> PCI_DMA_FROMDEV
[PATCH v2] VMW_PVSCSI: Fix the issue of DMA-API related warnings.
The driver is missing calls to pci_dma_mapping_error() after performing the DMA mapping, which caused DMA-API warning to show up in dmesg's output. Though that happens only when DMA_API_DEBUG option is enabled. This change fixes the issue and makes pvscsi_map_buffers() function more robust. Signed-off-by: Arvind Kumar Cc: Josh Boyer Reviewed-by: Thomas Hellstrom Signed-off-by: Josh Boyer --- v2: Use -ENOMEM instead of -1 for the error return code as suggested by Johannes Thumshirn drivers/scsi/vmw_pvscsi.c | 45 +++-- drivers/scsi/vmw_pvscsi.h | 2 +- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c index 0f133c1817de..6164634aff18 100644 --- a/drivers/scsi/vmw_pvscsi.c +++ b/drivers/scsi/vmw_pvscsi.c @@ -349,9 +349,9 @@ static void pvscsi_create_sg(struct pvscsi_ctx *ctx, * Map all data buffers for a command into PCI space and * setup the scatter/gather list if needed. */ -static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, - struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, - struct PVSCSIRingReqDesc *e) +static int pvscsi_map_buffers(struct pvscsi_adapter *adapter, + struct pvscsi_ctx *ctx, struct scsi_cmnd *cmd, + struct PVSCSIRingReqDesc *e) { unsigned count; unsigned bufflen = scsi_bufflen(cmd); @@ -360,18 +360,30 @@ static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, e->dataLen = bufflen; e->dataAddr = 0; if (bufflen == 0) - return; + return 0; sg = scsi_sglist(cmd); count = scsi_sg_count(cmd); if (count != 0) { int segs = scsi_dma_map(cmd); - if (segs > 1) { + + if (segs == -ENOMEM) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map cmd sglist for DMA.\n"); + return -ENOMEM; + } else if (segs > 1) { pvscsi_create_sg(ctx, sg, segs); e->flags |= PVSCSI_FLAG_CMD_WITH_SG_LIST; ctx->sglPA = pci_map_single(adapter->dev, ctx->sgl, SGL_SIZE, PCI_DMA_TODEVICE); + if (pci_dma_mapping_error(adapter->dev, ctx->sglPA)) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map ctx sglist for DMA.\n"); + scsi_dma_unmap(cmd); + ctx->sglPA = 0; + return -ENOMEM; + } e->dataAddr = ctx->sglPA; } else e->dataAddr = sg_dma_address(sg); @@ -382,8 +394,15 @@ static void pvscsi_map_buffers(struct pvscsi_adapter *adapter, */ ctx->dataPA = pci_map_single(adapter->dev, sg, bufflen, cmd->sc_data_direction); + if (pci_dma_mapping_error(adapter->dev, ctx->dataPA)) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map direct data buffer for DMA.\n"); + return -ENOMEM; + } e->dataAddr = ctx->dataPA; } + + return 0; } static void pvscsi_unmap_buffers(const struct pvscsi_adapter *adapter, @@ -690,6 +709,12 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, ctx->sensePA = pci_map_single(adapter->dev, cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE, PCI_DMA_FROMDEVICE); + if (pci_dma_mapping_error(adapter->dev, ctx->sensePA)) { + scmd_printk(KERN_ERR, cmd, + "vmw_pvscsi: Failed to map sense buffer for DMA.\n"); + ctx->sensePA = 0; + return -ENOMEM; + } e->senseAddr = ctx->sensePA; e->senseLen = SCSI_SENSE_BUFFERSIZE; } else { @@ -711,7 +736,15 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, else e->flags = 0; - pvscsi_map_buffers(adapter, ctx, cmd, e); + if (pvscsi_map_buffers(adapter, ctx, cmd, e) != 0) { + if (cmd->sense_buffer) { + pci_unmap_single(adapter->dev, ctx->sensePA, +SCSI_SENSE_BUFFERSIZE, +PCI_DMA_FROMDEVICE); +
Re: [PATCH] Remove pointless casts from void pointers,
On Fri, 26 Oct 2007 05:40:22 -0400 (EDT) Jeff Garzik <[EMAIL PROTECTED]> wrote: > mostly in and around irq handlers. > > Signed-off-by: Jeff Garzik <[EMAIL PROTECTED]> > --- > drivers/serial/uartlite.c |2 +- Acked-by: Josh Boyer <[EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: BusLogic: Fix an oops when intializing multimaster adapter
On Wed, Sep 25, 2013 at 10:45 AM, Khalid Aziz wrote: > This fixes an oops caused by buslogic driver when initializing a BusLogic > MultiMaster adapter. Initialization code used scope of a variable > incorrectly which created a NULL pointer. Oops message is below: > > BUG: unable to handle kernel NULL pointer dereference at 000c > IP: [] blogic_init_mm_probeinfo.isra.17+0x20a/0x583 > *pde = > Oops: 002 [#1] PREEMPT SMP > Modules linked in: > CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.11.1.puz1 #1 > Hardware name:/Canterwood, BIOS 6.00 PG 05/16/2003 > task: f705 ti: f7054000 task.ti: f7054000 > EIP: 0060:[] EFLAGS: 00010246 CPU:1 > EIP is at blogic_init_mm_probeinfo.isra.17+0x20a/0x583 > EAX: 0013 EBX: ECX: EDX: f8001000 > ESI: f71cb800 EDI: f7388000 EBP: 7800 ESP: f7055c84 > DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 > CR0: 8005003b CR2: 000c CR3: 0154f000 CR4: 07d0 > Stack: > 001c c11a59f6 f7055c98 8130 > 0003 0013 f8001000 0001 03d0 > c14e3f84 f78803c8 f738c000 00e9 > Call Trace: > [] ? pci_get_subsys+0x33/0x38 > [] ? blogic_init_probeinfo_list+0x4b/0x19e > [] ? __alloc_pages_nodemask+0xe3/0x623 > [] ? __alloc_pages_nodemask+0xe3/0x623 > [] ? sysfs_link_sibling+0x61/0x8d > [] ? kmem_cache_alloc+0x8b/0xb5 > [] ? blogic_init+0xa1/0x10e8 > [] ? sysfs_add_one+0x10/0x9d > [] ? sysfs_addrm_finish+0x12/0x85 > [] ? sysfs_do_create_link_sd+0x9d/0x1b4 > [] ? blk_register_queue+0x69/0xb3 > [] ? sysfs_create_link+0x1a/0x2c > [] ? add_disk+0x1a1/0x3c7 > [] ? klist_next+0x60/0xc3 > [] ? scsi_dh_detach+0x68/0x68 > [] ? bus_for_each_dev+0x51/0x61 > [] ? do_one_initcall+0x22/0x12c > [] ? __proc_create+0x8c/0xba > [] ? blogic_setup+0x5f6/0x5f6 > [] ? repair_env_string+0xf/0x4d > [] ? do_early_param+0x71/0x71 > [] ? parse_args+0x21f/0x33d > [] ? kernel_init_freeable+0xdf/0x17d > [] ? do_early_param+0x71/0x71 > [] ? kernel_init+0x8/0xc0 > [] ? ret_from_kernel_thread+0x6/0x28 > [] ? ret_from_kernel_thread+0x1b/0x28 > [] ? rest_init+0x6c/0x6c > Code: 89 44 24 10 0f b6 44 24 3d 89 44 24 0c c7 44 24 08 00 00 00 00 c7 44 24 > 04 38 62 46 c1 c7 04 24 02 00 00 00 e8 78 13 d2 ff 31 db <89> 6b 0c b0 20 89 > ea ee > c7 44 24 08 04 00 00 00 8d 44 24 4c 89 > EIP: [] blogic_init_mm_probeinfo.isra.17+0x20a/0x583 SS:ESP > 0068:f7055c84 > CR2: 000c > ---[ end trace 17f45f5196d40487 ]--- > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 > > Signed-off-by: Khalid Aziz > Cc: # 3.11.x > Cc: Khalid Aziz > Reported-by: Pierre Uszynski > Tested-by: Pierre Uszynski We had a user report an issue starting VMWare guests using BusLogic with the 3.11 kernel in Fedora. They tested a kernel build based on 3.11.5 plus this patch and it fixes their issue. Details here: https://bugzilla.redhat.com/show_bug.cgi?id=1015558 You can add a Tested-by: Bojan Smojver if you'd like. josh > --- > drivers/scsi/BusLogic.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c > index feab3a5..757eb07 100644 > --- a/drivers/scsi/BusLogic.c > +++ b/drivers/scsi/BusLogic.c > @@ -696,7 +696,7 @@ static int __init blogic_init_mm_probeinfo(struct > blogic_adapter *adapter) > while ((pci_device = pci_get_device(PCI_VENDOR_ID_BUSLOGIC, > PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER, > pci_device)) != NULL) { > - struct blogic_adapter *adapter = adapter; > + struct blogic_adapter *host_adapter = adapter; > struct blogic_adapter_info adapter_info; > enum blogic_isa_ioport mod_ioaddr_req; > unsigned char bus; > @@ -744,9 +744,9 @@ static int __init blogic_init_mm_probeinfo(struct > blogic_adapter *adapter) >known and enabled, note that the particular Standard ISA > I/O >Address should not be probed. > */ > - adapter->io_addr = io_addr; > - blogic_intreset(adapter); > - if (blogic_cmd(adapter, BLOGIC_INQ_PCI_INFO, NULL, 0, > + host_adapter->io_addr = io_addr; > + blogic_intreset(host_adapter); > + if (blogic_cmd(host_adapter, BLOGIC_INQ_PCI_INFO, NULL, 0, > &adapter_info, sizeof(adapter_info)) == > sizeof(adapter_info)) { > if (adapter_
Re: [3.12-rc] sg_open: leaving the kernel with locks still held!
On Wed, Oct 23, 2013 at 12:44 AM, James Bottomley wrote: > On Tue, 2013-10-22 at 20:41 -0400, Douglas Gilbert wrote: >> On 13-10-22 04:56 PM, Simon Kirby wrote: >> > Hello! >> > >> > While trying to figure out why the request queue to sda (ext4) was >> > clogging up on one of our btrfs backup boxes, I noticed a megarc process >> > in D state, so enabled locking debugging, and got this (on 3.12-rc6): >> > >> > [ 205.372823] >> > [ 205.372901] [ BUG: lock held when returning to user space! ] >> > [ 205.372979] 3.12.0-rc6-hw-debug-pagealloc+ #67 Not tainted >> > [ 205.373055] >> > [ 205.373132] megarc.bin/5283 is leaving the kernel with locks still held! >> > [ 205.373212] 1 lock held by megarc.bin/5283: >> > [ 205.373285] #0: (&sdp->o_sem){.+.+..}, at: [] >> > sg_open+0x3a0/0x4d0 >> > >> > Vaughan, it seems you touched this area last in 15b06f9a02406e, and git >> > tag --contains says this went in for 3.12-rc. We didn't see this on 3.11, >> > though I haven't tried with lockdep. >> > >> > This is caused by some of our internal RAID monitoring scripts that run >> > "megarc.bin -dispCfg -a0" (even though that controller isn't present on >> > this server -- a PowerEdge 2950 w/Perc 5). >> > >> > strace output of the program execution that causes the above message is >> > here: http://0x.ca/sim/ref/3.12-rc6/megarc_strace.txt >> >> This has been reported. That patch will be reverted or, >> if there is enough time, a fix will (or at least should) >> go in before the release of lk 3.12 . > > I think you've got about a week to prove you can fix it (before 3.12 > goes final). I'll send my current set of fixes to Linus without doing > anything about sg. In the event that a suitable fix isn't found, are you going to revert the commit(s) that caused the issue? josh -- 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: [PATCH 1/3] drivers: message: Mark function mpt_SoftResetHandler() as static in mptbase.c
On Fri, Dec 13, 2013 at 11:33:11AM +0530, Rashika Kheria wrote: > This patch marks the function mpt_SoftResetHandler() as static in mptbase.c > because it is not used outside this file. > > Thus, it also eliminates the following warning in fusion/mptbase.c: > drivers/message/fusion/mptbase.c:7011:1: warning: no previous prototype for > ‘mpt_SoftResetHandler’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/message/fusion/mptbase.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/message/fusion/mptbase.c > b/drivers/message/fusion/mptbase.c > index 767ff4d..c783a42 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -7007,7 +7007,7 @@ EXPORT_SYMBOL(mpt_halt_firmware); > * IOC doesn't reply to any outstanding request. This will transfer IOC > * to READY state. > **/ > -int > +static int > mpt_SoftResetHandler(MPT_ADAPTER *ioc, int sleepFlag) > { > int rc; > -- > 1.7.9.5 > -- 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: [PATCH 2/3] drivers: message: Mark function mptscsih_quiesce_raid() as static in mptspi.c
On Fri, Dec 13, 2013 at 11:36:54AM +0530, Rashika Kheria wrote: > This patch marks the function mptscsih_quiesce_raid() as static in > fusion/mptspi.c because it is not used outside this function. > > Thus, it also eliminates the following warning in fusion/mptspi.c: > drivers/message/fusion/mptspi.c:624:1: warning: no previous prototype for > ‘mptscsih_quiesce_raid’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/message/fusion/mptspi.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/message/fusion/mptspi.c b/drivers/message/fusion/mptspi.c > index 5653e50..ed8de0a 100644 > --- a/drivers/message/fusion/mptspi.c > +++ b/drivers/message/fusion/mptspi.c > @@ -620,7 +620,7 @@ static void mptspi_read_parameters(struct scsi_target > *starget) > spi_width(starget) = (nego & MPI_SCSIDEVPAGE0_NP_WIDE) ? 1 : 0; > } > > -int > +static int > mptscsih_quiesce_raid(MPT_SCSI_HOST *hd, int quiesce, u8 channel, u8 id) > { > MPT_ADAPTER *ioc = hd->ioc; > -- > 1.7.9.5 > -- 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: [PATCH 3/3] drivers: message: Mark functions as static in mptsas.c
On Fri, Dec 13, 2013 at 11:40:35AM +0530, Rashika Kheria wrote: > This patch marks the function mptsas_refreshing_device_handles(), > mptsas_expander_add() and mptsas_shutdown() as static in fusion/mptsas.c > because they are not used outside this file. > > Thus, it also eliminates the following warnings in fusion/mptsas.c: > drivers/message/fusion/mptsas.c:1579:1: warning: no previous prototype for > ‘mptsas_refreshing_device_handles’ [-Wmissing-prototypes] > drivers/message/fusion/mptsas.c:3654:1: warning: no previous prototype for > ‘mptsas_expander_add’ [-Wmissing-prototypes] > drivers/message/fusion/mptsas.c:5327:1: warning: no previous prototype for > ‘mptsas_shutdown’ [-Wmissing-prototypes] > > Signed-off-by: Rashika Kheria Reviewed-by: Josh Triplett > drivers/message/fusion/mptsas.c |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c > index dd239bd..906f448 100644 > --- a/drivers/message/fusion/mptsas.c > +++ b/drivers/message/fusion/mptsas.c > @@ -1575,7 +1575,7 @@ mptsas_del_end_device(MPT_ADAPTER *ioc, struct > mptsas_phyinfo *phy_info) > mptsas_port_delete(ioc, phy_info->port_details); > } > > -struct mptsas_phyinfo * > +static struct mptsas_phyinfo * > mptsas_refreshing_device_handles(MPT_ADAPTER *ioc, > struct mptsas_devinfo *sas_device) > { > @@ -3650,7 +3650,7 @@ mptsas_send_expander_event(struct fw_event_work > *fw_event) > * @handle: > * > */ > -struct mptsas_portinfo * > +static struct mptsas_portinfo * > mptsas_expander_add(MPT_ADAPTER *ioc, u16 handle) > { > struct mptsas_portinfo buffer, *port_info; > @@ -5323,7 +5323,7 @@ mptsas_probe(struct pci_dev *pdev, const struct > pci_device_id *id) > return error; > } > > -void > +static void > mptsas_shutdown(struct pci_dev *pdev) > { > MPT_ADAPTER *ioc = pci_get_drvdata(pdev); > -- > 1.7.9.5 > -- 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
DMA-API mapping errors in vmw_pvscsi
Hi All, We've had a few reports[1][2] now on the vmw_pvscsi driver throwing DMA-API mapping errors when the DMA_API_DEBUG option is enabled. I've included one from a recent 3.14-rc6 kernel below. Looking at vmw_pvscsi.c, I can see pvscsi_map_buffers is missing the calls to pci_dma_mapping_error, which is what causes the warnings to be thrown. However, I'm not familiar with this driver and I can't see what the proper error path should be in this case. pvscsi_map_buffers is a void function and doesn't currently have the ability to return an error to the caller. Even if it did, I'm not sure what the proper response to an error should be. Thoughts? josh [2.962772] [ cut here ] [2.963764] WARNING: CPU: 1 PID: 6 at lib/dma-debug.c:1140 check_unmap+0x4ee/0x9e0() [2.965382] vmw_pvscsi :03:00.0: DMA-API: device driver failed to check map error[device address=0x78520f80] [size=96 bytes] [mapped as single] [2.968214] Modules linked in: [2.968897] vmwgfx(+) ttm drm ata_generic vmw_pvscsi(+) i2c_core pata_acpi [2.970302] CPU: 1 PID: 6 Comm: kworker/u4:0 Not tainted 3.14.0-0.rc6.git4.1.fc21.x86_64 #1 [2.972028] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/30/2013 [2.974230] Workqueue: events_unbound async_run_entry_fn [2.975375] 3e328dbc 88007fc03bf0 817d19a8 [2.977032] 88007fc03c38 88007fc03c28 8109671d 88007bf63540 [2.978694] 88007bc52e50 82d74a90 0082 81a2cce8 [2.980352] Call Trace: [2.980882][] dump_stack+0x4d/0x66 [2.982147] [] warn_slowpath_common+0x7d/0xa0 [2.983398] [] warn_slowpath_fmt+0x5c/0x80 [2.984606] [] check_unmap+0x4ee/0x9e0 [2.985731] [] debug_dma_unmap_page+0x70/0x90 [2.986981] [] pvscsi_unmap_buffers.isra.12+0x123/0x200 [vmw_pvscsi] [2.988635] [] pvscsi_process_completion_ring+0x106/0x2f0 [vmw_pvscsi] [2.990325] [] pvscsi_isr+0x34/0xa0 [vmw_pvscsi] [2.991631] [] handle_irq_event_percpu+0x3e/0x370 [2.992952] [] handle_irq_event+0x3d/0x60 [2.994129] [] handle_edge_irq+0x77/0x130 [2.995320] [] handle_irq+0xbf/0x150 [2.996413] [] ? irq_enter+0x42/0x90 [2.997514] [] do_IRQ+0x4f/0xf0 [2.998521] [] common_interrupt+0x72/0x72 [2.999703][] ? _raw_spin_unlock_irqrestore+0x3b/0x70 [3.001587] [] pvscsi_queue+0x130/0x840 [vmw_pvscsi] [3.002970] [] ? ftrace_raw_event_scsi_dispatch_cmd_error+0x220/0x220 [3.004681] [] scsi_dispatch_cmd+0xb7/0x4e0 [3.006095] [] scsi_request_fn+0x33c/0x540 [3.007299] [] __blk_run_queue+0x33/0x40 [3.008464] [] blk_execute_rq_nowait+0xa9/0x140 [3.009751] [] blk_execute_rq+0x133/0x1e0 [3.010954] [] ? bio_phys_segments+0x19/0x20 [3.012195] [] ? blk_rq_bio_prep+0x72/0xf0 [3.013393] [] scsi_execute+0xd7/0x160 [3.014523] [] scsi_execute_req_flags+0x8c/0x100 [3.015833] [] scsi_probe_and_add_lun+0x235/0xc50 [3.036404] [] ? __pm_runtime_resume+0x5c/0x90 [3.037679] [] __scsi_scan_target+0x110/0x6d0 [3.038936] [] ? _raw_spin_unlock_irqrestore+0x36/0x70 [3.040345] [] ? trace_hardirqs_on_caller+0x105/0x1d0 [3.041741] [] ? trace_hardirqs_on+0xd/0x10 [3.042957] [] scsi_scan_channel.part.6+0x66/0x90 [3.044288] [] scsi_scan_host_selected+0xf9/0x1c0 [3.045773] [] do_scsi_scan_host+0x91/0xa0 [3.046977] [] do_scan_async+0x1c/0x160 [3.048125] [] async_run_entry_fn+0x39/0x120 [3.049358] [] process_one_work+0x220/0x6f0 [3.050572] [] ? process_one_work+0x1b4/0x6f0 [3.051822] [] worker_thread+0x11b/0x3a0 [3.052987] [] ? process_one_work+0x6f0/0x6f0 [3.054239] [] kthread+0xff/0x120 [3.055285] [] ? insert_kthread_work+0x80/0x80 [3.056558] [] ret_from_fork+0x7c/0xb0 [3.057686] [] ? insert_kthread_work+0x80/0x80 [3.058954] ---[ end trace df8f36ebf71b314f ]--- [3.059922] Mapped at: [3.060431] [] debug_dma_map_page+0x91/0x140 [3.061699] [] pvscsi_queue+0x281/0x840 [vmw_pvscsi] [3.063105] [] scsi_dispatch_cmd+0xb7/0x4e0 [3.064351] [] scsi_request_fn+0x33c/0x540 [3.065607] [] __blk_run_queue+0x33/0x40 [1] https://bugzilla.redhat.com/show_bug.cgi?id=926917 [2] https://bugzilla.redhat.com/show_bug.cgi?id=1077118 -- 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: [PATCH 19/55] scsi: Mark function as static in isci/phy.c
On Mon, Mar 31, 2014 at 08:54:49AM +, Dorau, Lukasz wrote: > On Saturday, March 29, 2014 7:04 PM Rashika Kheria > wrote: > > > > Mark function as static in isci/phy.c because it is not used outside > > this file. > > > > This eliminates the following warning in isci/phy.c: > > drivers/scsi/isci/phy.c:672:6: warning: no previous prototype for > > ‘scu_link_layer_set_txcomsas_timeout’ [-Wmissing-prototypes] > > > > Signed-off-by: Rashika Kheria > > Reviewed-by: Josh Triplett > > Acked-by: Lukasz Dorau Since you're a maintainer of the driver in question, can you accept the three patches you acked, or would you suggest that they go through another tree? - Josh Triplett -- 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
3.15 drivers/net/ethernet/broadcom/cnic.c:576 suspicious rcu_dereference_check() usage!
Hi All, We've had a report [1] of the bnx2i/cnic driver(s) throwing suspicious RCU usage with 3.15 merge window kernels on an i686 machine. This corresponds to Linux v3.14-12812-g321d03c86732. I've included the dump below. Has anyone seen this issue before? josh [1] https://bugzilla.redhat.com/show_bug.cgi?id=1087813 [ 90.432418] === [ 90.511920] [ INFO: suspicious RCU usage. ] [ 90.511922] 3.15.0-0.rc0.git13.1.fc21.i686 #1 Not tainted [ 90.511922] --- [ 90.511923] drivers/net/ethernet/broadcom/cnic.c:576 suspicious rcu_dereference_check() usage! [ 90.511923] [ 90.511923] other info that might help us debug this: [ 90.511923] [ 90.511924] [ 90.511924] rcu_scheduler_active = 1, debug_locks = 0 [ 90.511925] 3 locks held by anaconda/1320: [ 90.511932] #0: (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x14/0x20 [ 90.511937] #1: (&bnx2i_dev_lock){+.+...}, at: [] bnx2i_ulp_init+0x2f/0x140 [bnx2i] [ 90.511940] #2: (cnic_lock){+.+...}, at: [] cnic_register_device+0x38/0x2d0 [cnic] [ 90.511941] [ 90.511941] stack backtrace: [ 90.511942] CPU: 3 PID: 1320 Comm: anaconda Not tainted 3.15.0-0.rc0.git13.1.fc21.i686 #1 [ 90.511943] Hardware name: HP ProLiant DL360 G7, BIOS P68 01/28/2011 [ 90.511946] 2cd0aecd dc0d3cf4 c0ae271d 0001 dc0d3d1c c04ac226 [ 90.511948] c0cb548e c0cdb161 0001 dc16 0001 ec75a2c0 ec75a32c [ 90.511950] dc0d3d90 f80c8442 0003 f80d45df 0001 df49393c ec75a330 [ 90.511951] Call Trace: [ 90.511956] [] dump_stack+0x48/0x60 [ 90.511959] [] lockdep_rcu_suspicious+0xd6/0x100 [ 90.511961] [] cnic_register_device+0x152/0x2d0 [cnic] [ 90.511967] [] ? bnx2i_ulp_init+0x2f/0x140 [bnx2i] [ 90.511969] [] ? trace_hardirqs_on+0xb/0x10 [ 90.511972] [] ? bnx2i_ulp_init+0x2f/0x140 [bnx2i] [ 90.511974] [] ? bnx2i_ulp_init+0x2f/0x140 [bnx2i] [ 90.511977] [] bnx2i_ulp_init+0x49/0x140 [bnx2i] [ 90.511979] [] cnic_register_driver+0xe1/0x180 [cnic] [ 90.511982] [] ? 0xf7d92fff [ 90.511984] [] bnx2i_mod_init+0x8b/0x1000 [bnx2i] [ 90.511986] [] ? 0xf7d92fff [ 90.511988] [] do_one_initcall+0xca/0x1a0 [ 90.511990] [] ? 0xf7d92fff [ 90.511992] [] ? set_memory_ro+0x37/0x40 [ 90.511995] [] load_module+0x1fe6/0x2480 [ 90.511999] [] ? copy_module_from_fd.isra.45+0x109/0x1a0 [ 90.512001] [] SyS_finit_module+0x8d/0xd0 [ 90.512003] [] ? up_write+0x1b/0x30 [ 90.512005] [] ? vm_mmap_pgoff+0x9b/0xc0 [ 90.512010] [] sysenter_do_call+0x12/0x38 [ 90.512012] [] ? mtrr_check.part.2+0x32/0x57 [ 90.512304] bnx2i [04:00.01]: ISCSI_INIT passed [ 155.484180] INFO: rcu_sched detected stalls on CPUs/tasks: {} (detected by 7, t=65099 jiffies, g=4371, c=4370, q=172) [ 155.634698] INFO: Stall ended before state dump start [-- MARK -- Tue Apr 15 09:30:01 2014] -- 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: [PATCH 1/1] bnx2x: Add FW 7.13.11.0.
On Sat, Feb 9, 2019 at 11:06 PM Rahul Verma wrote: > > From: Rahul Verma > > This patch adds new FW for bnx2x, which adds the following: > - TX VLAN filtering support. > - Enable TPA only for packets without VLAN. > > It also addresses few critical issues, > - Fairness algorithm misbehaviour when minimum bandwidth configured >for all PFs. > - Error recovery issue on TAPE devices. > - FW not discarding FIP frames that are not designated to PF. > - Kernel driver initialization failure after preboot driver. > - VxLAN stops working after sending inner IP fragmented traffic. > - Issues in the following FW flows: > SD VLAN update, TX packet drop, packet padding flow, vlan add/remove. > > Signed-off-by: Sudarsana Reddy Kalluru > Signed-off-by: Ariel Elior > Signed-off-by: Rahul Verma Applied and pushed out. josh
Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution
On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote: > On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina wrote: > > On Fri, 5 Jan 2018, Dan Williams wrote: > > > > [ ... snip ... ] > >> Andi Kleen (1): > >> x86, barrier: stop speculation for failed access_ok > >> > >> Dan Williams (13): > >> x86: implement nospec_barrier() > >> [media] uvcvideo: prevent bounds-check bypass via speculative > >> execution > >> carl9170: prevent bounds-check bypass via speculative execution > >> p54: prevent bounds-check bypass via speculative execution > >> qla2xxx: prevent bounds-check bypass via speculative execution > >> cw1200: prevent bounds-check bypass via speculative execution > >> Thermal/int340x: prevent bounds-check bypass via speculative > >> execution > >> ipv6: prevent bounds-check bypass via speculative execution > >> ipv4: prevent bounds-check bypass via speculative execution > >> vfs, fdtable: prevent bounds-check bypass via speculative execution > >> net: mpls: prevent bounds-check bypass via speculative execution > >> udf: prevent bounds-check bypass via speculative execution > >> userns: prevent bounds-check bypass via speculative execution > >> > >> Mark Rutland (4): > >> asm-generic/barrier: add generic nospec helpers > >> Documentation: document nospec helpers > >> arm64: implement nospec_ptr() > >> arm: implement nospec_ptr() > > > > So considering the recent publication of [1], how come we all of a sudden > > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and > > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT? > > > > Is this going to be handled in eBPF in some other way? > > > > Without that in place, and considering Jann Horn's paper, it would seem > > like PTI doesn't really lock it down fully, right? > > Here is the latest (v3) bpf fix: > > https://patchwork.ozlabs.org/patch/856645/ > > I currently have v2 on my 'nospec' branch and will move that to v3 for > the next update, unless it goes upstream before then. That patch seems specific to CONFIG_BPF_SYSCALL. Is the bpf() syscall the only attack vector? Or are there other ways to run bpf programs that we should be worried about? -- Josh
Re: [PATCH 1/1] qed: Add firmware 8.33.11.0
On Feb 28, Rahul Verma wrote: > This patch add a new qed firmware with fixes and added > support for new features. > -Support VLAN remove action in steering flow. > -Optimized the FW flow and several bug fixes. > -Allow VXLAN steering. > -Supports T10DIF and SRQ. > -Support for port redirection for RDMA bonding > > Signed-off-by: Rahul Verma > --- > WHENCE | 1 + > qed/qed_init_values_zipped-8.33.11.0.bin | Bin 0 -> 852456 bytes > 2 files changed, 1 insertion(+) > create mode 100755 qed/qed_init_values_zipped-8.33.11.0.bin I had to fixup a small conflict in WHENCE, but no big deal. Applied and pushed out. josh
Re: [PATCH linux-firmware] bnx2x: Add FW 7.13.15.0.
On Tue, Oct 22, 2019 at 10:02 AM Sudarsana Reddy Kalluru wrote: > > This patch adds new FW for bnx2x, which addresses the following issues: > - TCP packet with padding can open TPA aggregation in GRO mode. > - Tx Silent Drops could cause HW error when statistics is not enabled for > client. > - Transmission of tunneled packets over tx-only clients (with cos>0 in this > case) followed by load/unload with DCB update (for instance), resulted in a > Tx path halt. > - FORWARD_SETUP ramrod yielded a FW assert (x_eth_fp_hsi_ver_invalid). > > The FW also adds support for direct update of RSS indirection table entry. > > Signed-off-by: Sudarsana Reddy Kalluru > Signed-off-by: Ameen Rahman > --- > WHENCE | 3 +++ > bnx2x/bnx2x-e1-7.13.15.0.fw | Bin 0 -> 170168 bytes > bnx2x/bnx2x-e1h-7.13.15.0.fw | Bin 0 -> 178608 bytes > bnx2x/bnx2x-e2-7.13.15.0.fw | Bin 0 -> 323360 bytes > 4 files changed, 3 insertions(+) > create mode 100644 bnx2x/bnx2x-e1-7.13.15.0.fw > create mode 100644 bnx2x/bnx2x-e1h-7.13.15.0.fw > create mode 100644 bnx2x/bnx2x-e2-7.13.15.0.fw Applied and pushed out. josh
[PATCH] scsi: ufs: use PTR_ERR_OR_ZERO in ufs_hisi_get_resource()
Use PTR_ERR_OR_ZERO instead of IF_ERR() return PTR_ERR(). Signed-off-by: Joshua Abraham --- drivers/scsi/ufs/ufs-hisi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c index 46df707e6f2c..e79499469cb3 100644 --- a/drivers/scsi/ufs/ufs-hisi.c +++ b/drivers/scsi/ufs/ufs-hisi.c @@ -505,10 +505,8 @@ static int ufs_hisi_get_resource(struct ufs_hisi_host *host) /* get resource of ufs sys ctrl */ mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1); host->ufs_sys_ctrl = devm_ioremap_resource(dev, mem_res); - if (IS_ERR(host->ufs_sys_ctrl)) - return PTR_ERR(host->ufs_sys_ctrl); - return 0; + return PTR_ERR_OR_ZERO(host->ufs_sys_ctrl); } static void ufs_hisi_set_pm_lvl(struct ufs_hba *hba) -- 2.17.1
[PATCH] scsi: ufs: use PTR_ERR_OR_ZERO in ufs_hisi_get_resource()
This patch uses PTR_ERR_OR_ZERO instead of IF_ERR() return PTR_ERR(). Signed-off-by: Joshua Abraham --- drivers/scsi/ufs/ufs-hisi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c index 46df707e6f2c..e79499469cb3 100644 --- a/drivers/scsi/ufs/ufs-hisi.c +++ b/drivers/scsi/ufs/ufs-hisi.c @@ -505,10 +505,8 @@ static int ufs_hisi_get_resource(struct ufs_hisi_host *host) /* get resource of ufs sys ctrl */ mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1); host->ufs_sys_ctrl = devm_ioremap_resource(dev, mem_res); - if (IS_ERR(host->ufs_sys_ctrl)) - return PTR_ERR(host->ufs_sys_ctrl); - return 0; + return PTR_ERR_OR_ZERO(host->ufs_sys_ctrl); } static void ufs_hisi_set_pm_lvl(struct ufs_hba *hba) -- 2.17.1
Re: [PATCH 5/9] powerpc: PCI_MSI needs PCI
On Fri, Oct 19, 2018 at 02:09:48PM +0200, Christoph Hellwig wrote: > Various powerpc boards select the PCI_MSI config option without selecting > PCI, resulting in potentially not compilable configurations if the by > default enabled PCI option is disabled. Explicitly select PCI to ensure > we always have valid configs. [...] > --- a/arch/powerpc/platforms/44x/Kconfig > +++ b/arch/powerpc/platforms/44x/Kconfig > @@ -24,6 +24,7 @@ config BLUESTONE > default n > select PPC44x_SIMPLE > select APM821xx > + select PCI > select PCI_MSI > select PPC4xx_MSI > select PPC4xx_PCI_EXPRESS > @@ -78,6 +79,7 @@ config KATMAI > select 440SPe > select PCI > select PPC4xx_PCI_EXPRESS > + select PCI > select PCI_MSI This case already had PCI selected a couple of lines above. > select PPC4xx_MSI > help > @@ -219,6 +221,7 @@ config AKEBONO > select SWIOTLB > select 476FPE > select PPC4xx_PCI_EXPRESS > + select PCI > select PCI_MSI > select PPC4xx_HSTA_MSI > select I2C > -- > 2.19.1 >
[PATCH] Scsi: Fixed white space and tab errors.
Signed-off-by: Josh Taylor --- drivers/scsi/scsi.c | 66 +-- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 2936b44..00aded9 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -126,7 +126,7 @@ static const char *const scsi_device_types[] = { * @type: type number to look up */ -const char * scsi_device_type(unsigned type) +const char *scsi_device_type(unsigned type) { if (type == 0x1e) return "Well-known LUN "; @@ -609,7 +609,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) printk("FAILED\n"); break; case TIMEOUT_ERROR: - /* + /* * If called via scsi_times_out. */ printk("TIMEOUT\n"); @@ -642,7 +642,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) void scsi_cmd_get_serial(struct Scsi_Host *host, struct scsi_cmnd *cmd) { cmd->serial_number = host->cmd_serial_number++; - if (cmd->serial_number == 0) + if (cmd->serial_number == 0) cmd->serial_number = host->cmd_serial_number++; } EXPORT_SYMBOL(scsi_cmd_get_serial); @@ -675,7 +675,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) /* Check to see if the scsi lld made this device blocked. */ if (unlikely(scsi_device_blocked(cmd->device))) { - /* + /* * in blocked state, the command is just put back on * the device queue. The suspend state has already * blocked the queue so future requests should not @@ -694,7 +694,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) goto out; } - /* + /* * If SCSI-2 or lower, store the LUN value in cmnd. */ if (cmd->device->scsi_level <= SCSI_2 && @@ -805,17 +805,17 @@ void scsi_finish_command(struct scsi_cmnd *cmd) scsi_device_unbusy(sdev); -/* - * Clear the flags which say that the device/host is no longer - * capable of accepting new commands. These are set in scsi_queue.c - * for both the queue full condition on a device, and for a - * host full condition on the host. + /* +* Clear the flags which say that the device/host is no longer +* capable of accepting new commands. These are set in scsi_queue.c +* for both the queue full condition on a device, and for a +* host full condition on the host * * XXX(hch): What about locking? - */ -shost->host_blocked = 0; +*/ + shost->host_blocked = 0; starget->target_blocked = 0; -sdev->device_blocked = 0; + sdev->device_blocked = 0; /* * If we have valid sense information, then some kind of recovery @@ -829,7 +829,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd) "(result %x)\n", cmd->result)); good_bytes = scsi_bufflen(cmd); -if (cmd->request->cmd_type != REQ_TYPE_BLOCK_PC) { + if (cmd->request->cmd_type != REQ_TYPE_BLOCK_PC) { int old_good_bytes = good_bytes; drv = scsi_cmd_to_driver(cmd); if (drv->done) @@ -894,22 +894,22 @@ void scsi_adjust_queue_depth(struct scsi_device *sdev, int tagged, int tags) sdev->queue_depth = tags; switch (tagged) { - case MSG_ORDERED_TAG: - sdev->ordered_tags = 1; - sdev->simple_tags = 1; - break; - case MSG_SIMPLE_TAG: - sdev->ordered_tags = 0; - sdev->simple_tags = 1; - break; - default: - sdev_printk(KERN_WARNING, sdev, - "scsi_adjust_queue_depth, bad queue type, " - "disabled\n"); - case 0: - sdev->ordered_tags = sdev->simple_tags = 0; - sdev->queue_depth = tags; - break; + case MSG_ORDERED_TAG: + sdev->ordered_tags = 1; + sdev->simple_tags = 1; + break; + case MSG_SIMPLE_TAG: + sdev->ordered_tags = 0; + sdev->simple_tags = 1; + break; + default: + sdev_printk(KERN_WARNING, sdev, + "scsi_adjust_queue_depth, bad queue type, " +
Re: [PATCH v2 1/1] qed: Add firmware 8.37.2.0
On Mon, May 21, 2018 at 10:25 AM Rahul Verma wrote: > This patch add a new qed firmware with fixes and added > support for new features. > -Fix aRFS for tunneled traffic without inner IP. > -Fix chip configuration which may fail under heavy traffic conditions. > -Fix iSCSI recovery flow. > -Support receiving any-VNI in VXLAN and GENEVE RX classification. > -Support additional RoCE statistics for error cases. > -Support RoCE T10DIF. > v1->v2: > Added signoff for Ariel Elior. > Signed-off-by: Rahul Verma > Signed-off-by: Ariel Elior > --- > WHENCE | 1 + > qed/qed_init_values_zipped-8.37.2.0.bin | Bin 0 -> 867472 bytes > 2 files changed, 1 insertion(+) > create mode 100755 qed/qed_init_values_zipped-8.37.2.0.bin Applied and pushed out. josh