ATTENTION
Good day My Dear, I am seeking the immediate assistance,of a reliable and trustworthy Indian businessman who understands India local language (Hindi) and a little English; to handle a profitable business for our mutual benefit. I await your reply for more details. Thanks and best regards, Dr. Maria Rose Anthony. Email:mrsmariaro...@gmail.com -- 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] megaraid_sas : Add locking to megasas_aen_polling
> -Original Message- > From: Ben Guthro [mailto:ben.gut...@gmail.com] On Behalf Of Ben Guthro > Sent: Monday, November 02, 2015 5:49 PM > To: megaraidlinux@avagotech.com; linux-scsi@vger.kernel.org > Cc: Glenn Watkins; Ben Guthro; Yang, Bo; sta...@vger.kernel.org > Subject: [PATCH v2] megaraid_sas : Add locking to megasas_aen_polling > > From: Glenn Watkins > > Under conditions of offlining drives, and rescanning the scsi host, we can get > into situations that the megasas_aen_polling kthread can crash(GPF) in the > megasas_aen_polling work queue: > > [ 1206.568641] general protection fault: [#1] SMP [ 1206.569479] Modules > linked in: xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack > iptable_filter ip_tables x_tables coretemp crct10dif_pclmul crc32_pclmul > aesni_intel ablk_helper cryptd psmouse lrw vmwgfx gf128mul serio_raw > glue_helper aes_x86_64 ppdev ttm microcode vmw_balloon drm_kms_helper > drm parport_pc parport fb_sys_fops sysimgblt sysfillrect syscopyarea vmw_vmci > binfmt_misc floppy mptspi mptscsih vmw_pvscsi megaraid_sas pata_acpi > mptbase vmxnet3 [ 1206.576488] CPU: 0 PID: 1157 Comm: kworker/0:2 Not > tainted 4.3.0-rc7-svt1 #1 [ 1206.577520] Hardware name: VMware, Inc. > VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 > 04/14/2014 [ 1206.579101] Workqueue: events megasas_aen_polling > [megaraid_sas] [ 1206.580007] task: 8818bb7b8000 ti: 8818ca28 > task.ti: 8818ca28 [ 1206.581104] RIP: 0010:[] > [] bdi_unregister+0x3d/0x1e0 [ 1206.582339] RSP: > 0018:8818ca283cb8 EFLAGS: 00010246 [ 1206.583131] RAX: > dead0200 RBX: 8818bb603f08 RCX: 8818c6487800 [ > 1206.584184] RDX: 8818bb603f08 RSI: 7fff RDI: 81f9aa68 > [ 1206.585243] RBP: 8818ca283d18 R08: R09: > [ 1206.586294] R10: 000fffe0 R11: > dead0200 R12: 8818bb6042f0 [ 1206.587346] R13: > 8818bb604530 R14: 00ae R15: 0080 [ > 1206.588388] FS: () GS:88193fc0() > knlGS: [ 1206.589598] CS: 0010 DS: ES: CR0: > 8005003b [ 1206.590457] CR2: 01a89000 CR3: > 0018c07f2000 CR4: 000406f0 [ 1206.591545] Stack: > [ 1206.591870] 8818bb6042f0 8818bb603d78 00ae > 0080 [ 1206.593098] 8818ca283ce8 8108f683 > 8818ca283d18 813332b0 [ 1206.594308] 8818ca283d18 > 8818bb603d78 8818bb6042f0 8818bb604530 [ 1206.595532] Call > Trace: > [ 1206.595922] [] ? cancel_delayed_work_sync+0x13/0x20 > [ 1206.596903] [] ? blk_sync_queue+0x80/0x90 [ > 1206.597753] [] blk_cleanup_queue+0x114/0x150 [ > 1206.598645] [] __scsi_remove_device+0x54/0xd0 [ > 1206.599556] [] scsi_remove_device+0x2f/0x50 [ > 1206.600441] [] megasas_aen_polling+0x34d/0x670 > [megaraid_sas] [ 1206.601561] [] > process_one_work+0x14c/0x400 [ 1206.602449] [] > worker_thread+0x117/0x480 [ 1206.603295] [] ? > create_worker+0x1c0/0x1c0 [ 1206.604160] [] > kthread+0xc9/0xe0 [ 1206.604898] [] ? > flush_kthread_worker+0x90/0x90 [ 1206.605831] [] > ret_from_fork+0x3f/0x70 [ 1206.606659] [] ? > flush_kthread_worker+0x90/0x90 [ 1206.607585] Code: c7 c7 68 aa f9 81 48 83 > ec 48 e8 bf 76 59 00 48 8b 43 08 48 8b 13 49 bb 00 02 00 00 00 00 ad de 48 c7 c7 > 68 aa f9 81 48 89 42 08 <48> 89 10 4c 89 5b 08 e8 27 76 59 00 e8 32 92 f4 ff 48 > 8d 7b 50 [ 1206.611938] RIP [] bdi_unregister+0x3d/0x1e0 [ > 1206.612856] RSP > > This can be readily reproduced by a pair of shell scripts - one of which loops on > onlining / offlining drives via MegaCli (or storcli, if you prefer) > > #!/bin/bash > > while [ 1 ]; do > /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:0] a0 &>2 > /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:11] a0 &>2 > > /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:0] a0 &>2 > /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:11] a0 &>2 > done > > Meanwhile, the second script is looping on rescanning the scsi hosts: > > #!/bin/bash > while [ 1 ]; do > for (( l=0; l<4; l++ )); do > echo - - - > /sys/class/scsi_host/host$l/scan > done > done > > This was originally introduced in the following commit: > > commit 7e8a75f4dfbff173977b2f58799c3eceb7b09afd > Author: Yang, Bo > Date: Tue Oct 6 14:50:17 2009 -0600 > > [SCSI] megaraid_sas: Add the support for updating the OS after > adding/removing the devices from FW > > The fix for this is to add some locking around the AEN polling. > Since this affects all kernels since 2.6.33, I have also CC'ed the stable list. > > -- > Changes in v2: > - Fix contents of sign-off area > > Signed-off-by: Glenn Watkins > Signed-off-by: Ben Guthro > Cc: Yang, Bo > Cc: sta...@vger.kernel.org > > --- > drivers/scsi/megaraid/megaraid_sas_base.c |2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/driver
Re: [PATCH v4 11/11] scsi: ufs-exynos: add UFS host support for Exynos SoCs
Hi Kishon, Thanks for your time. On 10/28/2015 06:23 PM, Kishon Vijay Abraham I wrote: Hi, On Sunday 25 October 2015 05:34 PM, Alim Akhtar wrote: Hi Kishon Thanks again for you review. On Fri, Oct 23, 2015 at 8:48 PM, Kishon Vijay Abraham I wrote: Hi, On Thursday 15 October 2015 08:38 AM, Alim Akhtar wrote: +CCing kishon Vijay, On 10/14/2015 06:25 PM, Alim Akhtar wrote: From: Seungwon Jeon This patch introduces Exynos UFS host controller driver, which mainly handles vendor-specific operations including link startup, power mode change and hibernation/unhibernation. Signed-off-by: Seungwon Jeon Signed-off-by: Alim Akhtar --- drivers/scsi/ufs/Kconfig | 12 + drivers/scsi/ufs/Makefile|1 + drivers/scsi/ufs/ufs-exynos-hw.c | 131 drivers/scsi/ufs/ufs-exynos-hw.h | 43 ++ drivers/scsi/ufs/ufs-exynos.c| 1317 ++ drivers/scsi/ufs/ufs-exynos.h| 247 +++ drivers/scsi/ufs/ufshci.h| 26 +- drivers/scsi/ufs/unipro.h| 47 ++ 8 files changed, 1823 insertions(+), 1 deletion(-) create mode 100644 drivers/scsi/ufs/ufs-exynos-hw.c create mode 100644 drivers/scsi/ufs/ufs-exynos-hw.h create mode 100644 drivers/scsi/ufs/ufs-exynos.c create mode 100644 drivers/scsi/ufs/ufs-exynos.h . . . . diff --git a/drivers/scsi/ufs/ufs-exynos-hw.c b/drivers/scsi/ufs/ufs-exynos-hw.c new file mode 100644 index ..be6c61541a8f --- /dev/null +++ b/drivers/scsi/ufs/ufs-exynos-hw.c @@ -0,0 +1,131 @@ . . . . + +#define PWR_MODE_STR_LEN64 +static int exynos_ufs_post_pwr_mode(struct ufs_hba *hba, +struct ufs_pa_layer_attr *pwr_max, +struct ufs_pa_layer_attr *pwr_req) +{ +struct exynos_ufs *ufs = to_exynos_ufs(hba); +struct exynos_ufs_phy_info *phy_info = phy_get_drvdata(ufs->phy); This is abusing the interface. phy_get_drvdata is meant to be used only by the PHY driver. +struct exynos_ufs_phy_specific_ops *phy_ops = +phy_info->phy_specific_ops; I'm really not happy about having platform specific ops for PHY. We have to see if existing PHY ops can be used for this or in worst case add new PHY ops. Well you said you like the controller driver to use only PHY ops[1], I am sorry If I misunderstood that point, can you please help me to understand that? I meant PHY generic ops and not PHY ops. Ok, got it, will use only generic phy here in controller driver. - Will remove the platform specific PHY ops from phy driver introduce in this series (patch 02/11) [1]-> https://lkml.org/lkml/2015/9/18/29 +struct uic_pwr_mode *pwr = &ufs->pwr_act; +char pwr_str[PWR_MODE_STR_LEN] = ""; +int ret = 0; + +if (ufs->drv_data->post_pwr_change) +ufs->drv_data->post_pwr_change(ufs, pwr); + +if (IS_UFS_PWR_MODE_HS(pwr->mode)) { +switch (pwr->hs_series) { +case PA_HS_MODE_A: +case PA_HS_MODE_B: +phy_ops->calibrate_phy(ufs->phy, CFG_POST_PWR_HS, +PWR_MODE_HS(pwr->gear, pwr->hs_series)); +break; +} + +ret = phy_ops->wait_for_lock_acq(ufs->phy); +snprintf(pwr_str, sizeof(pwr_str), "Fast%s series_%s G_%d L_%d", +pwr->mode == FASTAUTO_MODE ? "_Auto" : "", +pwr->hs_series == PA_HS_MODE_A ? "A" : "B", +pwr->gear, pwr->lane); +} else if (IS_UFS_PWR_MODE_PWM(pwr->mode)) { +snprintf(pwr_str, sizeof(pwr_str), "Slow%s G_%d L_%d", +pwr->mode == SLOWAUTO_MODE ? "_Auto" : "", +pwr->gear, pwr->lane); +} + +dev_info(hba->dev, "Power mode change %d : %s\n", ret, pwr_str); +return ret; +} + +static void exynos_ufs_specify_nexus_t_xfer_req(struct ufs_hba *hba, +int tag, struct scsi_cmnd *cmd) +{ +struct exynos_ufs *ufs = to_exynos_ufs(hba); +u32 type; + +type = hci_readl(ufs, HCI_UTRL_NEXUS_TYPE); + +if (cmd) +hci_writel(ufs, type | (1 << tag), HCI_UTRL_NEXUS_TYPE); +else +hci_writel(ufs, type & ~(1 << tag), HCI_UTRL_NEXUS_TYPE); +} + +static void exynos_ufs_specify_nexus_t_tm_req(struct ufs_hba *hba, +int tag, u8 func) +{ +struct exynos_ufs *ufs = to_exynos_ufs(hba); +u32 type; + +type = hci_readl(ufs, HCI_UTMRL_NEXUS_TYPE); + +switch (func) { +case UFS_ABORT_TASK: +case UFS_QUERY_TASK: +hci_writel(ufs, type | (1 << tag), HCI_UTMRL_NEXUS_TYPE); +break; +case UFS_ABORT_TASK_SET: +case UFS_CLEAR_TASK_SET: +case UFS_LOGICAL_RESET: +case UFS_QUERY_TASK_SET: +hci_writel(ufs, type & ~(1 << tag), HCI_UTMRL_NEXUS_TYPE); +break; +} +} + +static void exynos_ufs_phy_init(struct exynos_ufs *ufs) +{ +struct ufs_hba *hba = ufs->hba; +struct exynos_ufs_phy_info *phy_info = phy_get_drvdata(ufs->phy); +struct exynos_ufs_phy_specific_ops *phy_ops = +phy_info->phy_specific_ops; + +if
Re: [PATCH v2 27/32] scsi: hisi_sas: add smp protocol support
On 02/11/2015 20:29, Arnd Bergmann wrote: On Monday 02 November 2015 17:03:58 John Garry wrote: Can do. Actually sg_req seems only ever has one element: expander.c, smp_execute_task() sg_init_one(&task->smp_task.smp_req, req, req_size); I tried replacing with dma_map_single, but I feel the code is not as clean as I need to manually set sg_dma_len() and sg_dma_address(): req_len = sg_dma_len(sg_req) = sg_req->length; sg_dma_address(sg_req) = dma_map_single(dev, sg_virt(sg_req), req_len, DMA_TO_DEVICE); if (dma_mapping_error(dev, sg_dma_address(sg_req))) return -ENOMEM; sg_dma_address(sg_req) is used in another function for unmap. opinion? What I meant was not using a struct scatterlist at all: replace the 'sg_req' variable with a normal pointer, and then do hdr->cmd_table_addr = cpu_to_le64(dma_map_single(dev, req, len, DMA_TO_DEVICE)); Any reason this won't work? Arnd . There seems to be some misunderstanding. Are you suggesting I change sas_smp_task? ./include/scsi/libsas.h struct sas_smp_task { struct scatterlist smp_req; struct scatterlist smp_resp; }; thanks, John -- 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 27/32] scsi: hisi_sas: add smp protocol support
On Tuesday 03 November 2015 11:42:30 John Garry wrote: > > > There seems to be some misunderstanding. Are you suggesting I change > sas_smp_task? > ./include/scsi/libsas.h > struct sas_smp_task { > struct scatterlist smp_req; > struct scatterlist smp_resp; > }; > Ah, no. I had not realized this comes from another file. Arnd -- 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 22/25] hpsa: enhance device messages
On 2.11.2015 17:54, Don Brace wrote: > On 10/30/2015 09:32 AM, Tomas Henzl wrote: >> RAID_UNKNOWN is used in few other places - raid_level_show for example, > raid_level_show handles physical devices by using "N/A", otherwise it > displays > the RAID level for logical devices. > > Other functions avoid the use of raid_type when the drive is not a RAID > device. > > Or, am I missing your point? Seems that I missed the fact that raid_level_show output is N/A, I thought it would be "RAID-Unknown". Last minor point is that you could replace + dev->raid_level > RAID_UNKNOWN ? "?" : + raid_label[dev->raid_level]); with + dev->raid_level > RAID_UNKNOWN ? raid_label[RAID_UNKNOWN] : + raid_label[dev->raid_level]); but that's not important. Reviewed-by: Tomas Henzl Tomas -- 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] pm80xx: remove the SCSI host before detaching from SAS transport
On Mon, Nov 02, 2015 at 09:29:13AM +0100, Jack Wang wrote: > Thanks for digging it down. Looks good to me. > > Seems other libsas based driver all affected, maybe we should also fix them? > Christoph (cc-ed), could you share your opinion about this fix, as the > commit cff549e4860f is from you? scsi_remove_host should be done first in ->remove and the commit was written under that assumption. So it looks like the other drivers will need fixes as well. -- 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 2/3] ibmvscsi: display default value for max_id, max_lun and max_channel.
As devices with values greater than that are silently ignored, this gives some hints to the sys admin to know why he doesn't see his devices... Signed-off-by: Laurent Vivier Reviewed-by: Brian King Acked-by: Tyrel Datwyler --- drivers/scsi/ibmvscsi/ibmvscsi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 3e76490..f9d7ec4 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -106,9 +106,9 @@ MODULE_LICENSE("GPL"); MODULE_VERSION(IBMVSCSI_VERSION); module_param_named(max_id, max_id, int, S_IRUGO); -MODULE_PARM_DESC(max_id, "Largest ID value for each channel"); +MODULE_PARM_DESC(max_id, "Largest ID value for each channel [Default=64]"); module_param_named(max_channel, max_channel, int, S_IRUGO); -MODULE_PARM_DESC(max_channel, "Largest channel value"); +MODULE_PARM_DESC(max_channel, "Largest channel value [Default=3]"); module_param_named(init_timeout, init_timeout, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(init_timeout, "Initialization timeout in seconds"); module_param_named(max_requests, max_requests, int, S_IRUGO); @@ -2294,6 +2294,10 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id) host->max_channel = max_channel; host->max_cmd_len = 16; + dev_info(dev, +"Maximum ID: %d Maximum LUN: %d Maximum Channel: %d\n", +host->max_id, host->max_lun, host->max_channel); + if (scsi_add_host(hostdata->host, hostdata->dev)) goto add_host_failed; -- 2.1.0 -- 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 3/3] ibmvscsi: Allow to configure maximum LUN
QEMU allows until 32 LUNs. Signed-off-by: Laurent Vivier Reviewed-by: Brian King Acked-by: Tyrel Datwyler --- drivers/scsi/ibmvscsi/ibmvscsi.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index f9d7ec4..e5478b0 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -84,6 +84,7 @@ */ static int max_id = 64; static int max_channel = 3; +static int max_lun = 8; static int init_timeout = 300; static int login_timeout = 60; static int info_timeout = 30; @@ -117,6 +118,8 @@ module_param_named(fast_fail, fast_fail, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(fast_fail, "Enable fast fail. [Default=1]"); module_param_named(client_reserve, client_reserve, int, S_IRUGO ); MODULE_PARM_DESC(client_reserve, "Attempt client managed reserve/release"); +module_param(max_lun, int, S_IRUGO); +MODULE_PARM_DESC(max_lun, "Maximum LUN value [Default=8]"); static void ibmvscsi_handle_crq(struct viosrp_crq *crq, struct ibmvscsi_host_data *hostdata); @@ -2289,7 +2292,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id) goto init_pool_failed; } - host->max_lun = 8; + host->max_lun = max_lun; host->max_id = max_id; host->max_channel = max_channel; host->max_cmd_len = 16; -- 2.1.0 -- 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 1/3] ibmvscsi: make parameters max_id and max_channel read-only
The value of the parameter is never re-read by the driver, so a new value is ignored. Let know the user he can't modify it by removing writable attribute. Signed-off-by: Laurent Vivier Reviewed-by: Brian King Acked-by: Tyrel Datwyler --- drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index 6a41c36..3e76490 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c @@ -105,9 +105,9 @@ MODULE_AUTHOR("Dave Boutcher"); MODULE_LICENSE("GPL"); MODULE_VERSION(IBMVSCSI_VERSION); -module_param_named(max_id, max_id, int, S_IRUGO | S_IWUSR); +module_param_named(max_id, max_id, int, S_IRUGO); MODULE_PARM_DESC(max_id, "Largest ID value for each channel"); -module_param_named(max_channel, max_channel, int, S_IRUGO | S_IWUSR); +module_param_named(max_channel, max_channel, int, S_IRUGO); MODULE_PARM_DESC(max_channel, "Largest channel value"); module_param_named(init_timeout, init_timeout, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(init_timeout, "Initialization timeout in seconds"); -- 2.1.0 -- 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] ibmvscsi: display default value for max_id, max_lun and max_channel.
Hi Laurent, [auto build test WARNING on scsi/for-next] [also WARNING on: v4.3 next-20151103] url: https://github.com/0day-ci/linux/commits/Laurent-Vivier/ibmvscsi-make-parameters-max_id-and-max_channel-read-only/20151103-223706 base: https://github.com/0day-ci/linux Laurent-Vivier/ibmvscsi-make-parameters-max_id-and-max_channel-read-only/20151103-223706 config: powerpc-defconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=powerpc All warnings (new ones prefixed by >>): In file included from include/linux/dma-mapping.h:5:0, from drivers/scsi/ibmvscsi/ibmvscsi.c:65: drivers/scsi/ibmvscsi/ibmvscsi.c: In function 'ibmvscsi_probe': >> drivers/scsi/ibmvscsi/ibmvscsi.c:2298:4: warning: format '%d' expects >> argument of type 'int', but argument 4 has type 'u64 {aka long long unsigned >> int}' [-Wformat=] "Maximum ID: %d Maximum LUN: %d Maximum Channel: %d\n", ^ include/linux/device.h:1166:51: note: in definition of macro 'dev_info' #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg) ^ vim +2298 drivers/scsi/ibmvscsi/ibmvscsi.c 2282 rc = ibmvscsi_init_crq_queue(&hostdata->queue, hostdata, max_events); 2283 if (rc != 0 && rc != H_RESOURCE) { 2284 dev_err(&vdev->dev, "couldn't initialize crq. rc=%d\n", rc); 2285 goto kill_kthread; 2286 } 2287 if (initialize_event_pool(&hostdata->pool, max_events, hostdata) != 0) { 2288 dev_err(&vdev->dev, "couldn't initialize event pool\n"); 2289 goto init_pool_failed; 2290 } 2291 2292 host->max_lun = 8; 2293 host->max_id = max_id; 2294 host->max_channel = max_channel; 2295 host->max_cmd_len = 16; 2296 2297 dev_info(dev, > 2298 "Maximum ID: %d Maximum LUN: %d Maximum Channel: %d\n", 2299 host->max_id, host->max_lun, host->max_channel); 2300 2301 if (scsi_add_host(hostdata->host, hostdata->dev)) 2302 goto add_host_failed; 2303 2304 /* we don't have a proper target_port_id so let's use the fake one */ 2305 memcpy(ids.port_id, hostdata->madapter_info.partition_name, 2306 sizeof(ids.port_id)); --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Awards
Prize Notification Contact: unitednationdiplomaticen...@outlook.it -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/5] ipr: Driver version 2.6.3.
Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index b16bcd1..a34c7a5 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -39,8 +39,8 @@ /* * Literals */ -#define IPR_DRIVER_VERSION "2.6.2" -#define IPR_DRIVER_DATE "(June 11, 2015)" +#define IPR_DRIVER_VERSION "2.6.3" +#define IPR_DRIVER_DATE "(October 17, 2015)" /* * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding -- 2.1.0 -- 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 v2 1/5] ipr: Add delay to ensure coherent dumps.
Add a holding pattern prior to collecting dump data, to wait for the IOA indication that the Mailbox register is stable and won't change without an explicit reset. This ensures we'll be collecting meaningful dump data, even when dumping right after an adapter reset. In the event of a timeout, we still force the dump, since a partial dump still might be useful. Changes since v1: - Fix checkpatch.pl warnings. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 51 +++ drivers/scsi/ipr.h | 3 +++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index b62836d..238efab 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -8277,6 +8277,42 @@ static int ipr_reset_get_unit_check_job(struct ipr_cmnd *ipr_cmd) return IPR_RC_JOB_RETURN; } +static int ipr_dump_mailbox_wait(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + + ENTER; + + if (ioa_cfg->sdt_state != GET_DUMP) + return IPR_RC_JOB_RETURN; + + if (!ioa_cfg->sis64 || !ipr_cmd->u.time_left || + (readl(ioa_cfg->regs.sense_interrupt_reg) & +IPR_PCII_MAILBOX_STABLE)) { + + if (!ipr_cmd->u.time_left) + dev_err(&ioa_cfg->pdev->dev, + "Timed out waiting for Mailbox register.\n"); + + ioa_cfg->sdt_state = READ_DUMP; + ioa_cfg->dump_timeout = 0; + if (ioa_cfg->sis64) + ipr_reset_start_timer(ipr_cmd, IPR_SIS64_DUMP_TIMEOUT); + else + ipr_reset_start_timer(ipr_cmd, IPR_SIS32_DUMP_TIMEOUT); + ipr_cmd->job_step = ipr_reset_wait_for_dump; + schedule_work(&ioa_cfg->work_q); + + } else { + ipr_cmd->u.time_left -= IPR_CHECK_FOR_RESET_TIMEOUT; + ipr_reset_start_timer(ipr_cmd, + IPR_CHECK_FOR_RESET_TIMEOUT); + } + + LEAVE; + return IPR_RC_JOB_RETURN; +} + /** * ipr_reset_restore_cfg_space - Restore PCI config space. * @ipr_cmd: ipr command struct @@ -8326,20 +8362,11 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd) if (ioa_cfg->in_ioa_bringdown) { ipr_cmd->job_step = ipr_ioa_bringdown_done; + } else if (ioa_cfg->sdt_state == GET_DUMP) { + ipr_cmd->job_step = ipr_dump_mailbox_wait; + ipr_cmd->u.time_left = IPR_WAIT_FOR_MAILBOX; } else { ipr_cmd->job_step = ipr_reset_enable_ioa; - - if (GET_DUMP == ioa_cfg->sdt_state) { - ioa_cfg->sdt_state = READ_DUMP; - ioa_cfg->dump_timeout = 0; - if (ioa_cfg->sis64) - ipr_reset_start_timer(ipr_cmd, IPR_SIS64_DUMP_TIMEOUT); - else - ipr_reset_start_timer(ipr_cmd, IPR_SIS32_DUMP_TIMEOUT); - ipr_cmd->job_step = ipr_reset_wait_for_dump; - schedule_work(&ioa_cfg->work_q); - return IPR_RC_JOB_RETURN; - } } LEAVE; diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index e4fb17a..69257c4 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -279,6 +279,9 @@ #define IPR_IPL_INIT_STAGE_TIME_MASK 0x #define IPR_PCII_IPL_STAGE_CHANGE (0x8000 >> 0) +#define IPR_PCII_MAILBOX_STABLE(0x8000 >> 4) +#define IPR_WAIT_FOR_MAILBOX (2 * HZ) + #define IPR_PCII_IOA_TRANS_TO_OPER (0x8000 >> 0) #define IPR_PCII_IOARCB_XFER_FAILED(0x8000 >> 3) #define IPR_PCII_IOA_UNIT_CHECKED (0x8000 >> 4) -- 2.1.0 -- 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 v2 4/5] ipr: Issue Configure Cache Parameters command.
Some new adapters require a special Configure Cache Parameters command to enable the adapter write cache, so send this during the adapter initialization if the adapter requires it. Changes since v1: - Fix checkpatch.pl warnings. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 59 +- drivers/scsi/ipr.h | 4 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 5efc7ef..79fd9ff 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -7675,6 +7675,63 @@ static int ipr_ioafp_query_ioa_cfg(struct ipr_cmnd *ipr_cmd) return IPR_RC_JOB_RETURN; } +static int ipr_ioa_service_action_failed(struct ipr_cmnd *ipr_cmd) +{ + u32 ioasc = be32_to_cpu(ipr_cmd->s.ioasa.hdr.ioasc); + + if (ioasc == IPR_IOASC_IR_INVALID_REQ_TYPE_OR_PKT) + return IPR_RC_JOB_CONTINUE; + + return ipr_reset_cmd_failed(ipr_cmd); +} + +static void ipr_build_ioa_service_action(struct ipr_cmnd *ipr_cmd, +__be32 res_handle, u8 sa_code) +{ + struct ipr_ioarcb *ioarcb = &ipr_cmd->ioarcb; + + ioarcb->res_handle = res_handle; + ioarcb->cmd_pkt.cdb[0] = IPR_IOA_SERVICE_ACTION; + ioarcb->cmd_pkt.cdb[1] = sa_code; + ioarcb->cmd_pkt.request_type = IPR_RQTYPE_IOACMD; +} + +/** + * ipr_ioafp_set_caching_parameters - Issue Set Cache parameters service + * action + * + * Return value: + * none + **/ +static int ipr_ioafp_set_caching_parameters(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioarcb *ioarcb = &ipr_cmd->ioarcb; + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + struct ipr_inquiry_pageC4 *pageC4 = &ioa_cfg->vpd_cbs->pageC4_data; + + ENTER; + + ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + + if (pageC4->cache_cap[0] & IPR_CAP_SYNC_CACHE) { + ipr_build_ioa_service_action(ipr_cmd, +cpu_to_be32(IPR_IOA_RES_HANDLE), +IPR_IOA_SA_CHANGE_CACHE_PARAMS); + + ioarcb->cmd_pkt.cdb[2] = 0x40; + + ipr_cmd->job_step_failed = ipr_ioa_service_action_failed; + ipr_do_req(ipr_cmd, ipr_reset_ioa_job, ipr_timeout, + IPR_SET_SUP_DEVICE_TIMEOUT); + + LEAVE; + return IPR_RC_JOB_RETURN; + } + + LEAVE; + return IPR_RC_JOB_CONTINUE; +} + /** * ipr_ioafp_inquiry - Send an Inquiry to the adapter. * @ipr_cmd: ipr command struct @@ -7742,7 +7799,7 @@ static int ipr_ioafp_pageC4_inquiry(struct ipr_cmnd *ipr_cmd) struct ipr_inquiry_pageC4 *pageC4 = &ioa_cfg->vpd_cbs->pageC4_data; ENTER; - ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + ipr_cmd->job_step = ipr_ioafp_set_caching_parameters; memset(pageC4, 0, sizeof(*pageC4)); if (ipr_inquiry_page_supported(page0, 0xC4)) { diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index 7be1271..b16bcd1 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -216,6 +216,10 @@ #define IPR_SET_ALL_SUPPORTED_DEVICES 0x80 #define IPR_IOA_SHUTDOWN 0xF7 #defineIPR_WR_BUF_DOWNLOAD_AND_SAVE0x05 +#define IPR_IOA_SERVICE_ACTION 0xD2 + +/* IOA Service Actions */ +#define IPR_IOA_SA_CHANGE_CACHE_PARAMS 0x14 /* * Timeouts -- 2.1.0 -- 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 v2 2/5] ipr: Don't set NO_ULEN_CHK bit when resource is a vset.
According to the IPR specification, Inhibit Underlength Checking bit must be disabled when issuing commands to vsets. Enabling it in this case might cause SCSI commands to fail with an Illegal Request, so make sure we keep this bit cleared when resource is a vset. Changes since v1: - Put gsci exclusive stuff in a separate block. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 238efab..6849b7f 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -6363,15 +6363,19 @@ static int ipr_queuecommand(struct Scsi_Host *shost, ipr_cmd->scsi_cmd = scsi_cmd; ipr_cmd->done = ipr_scsi_eh_done; - if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { + if (ipr_is_gscsi(res)) { if (scsi_cmd->underflow == 0) ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_ULEN_CHK; - ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_LINK_DESC; - if (ipr_is_gscsi(res) && res->reset_occurred) { + if (res->reset_occurred) { res->reset_occurred = 0; ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_DELAY_AFTER_RST; } + } + + if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { + ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_LINK_DESC; + ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_ALIGNED_BFR; if (scsi_cmd->flags & SCMD_TAGGED) ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_SIMPLE_TASK; -- 2.1.0 -- 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 v2 3/5] ipr: Inquiry IOA page 0xC4 during initialization.
Add an IOA Inquiry command for Page 0xC4 during IOA initialization to collect cache capabilities, particularly to check if Sync IOA Write Cache is supported. Inquiry will happen right after Cap Inquiry on page 0xD0; and will execute only if the "Supported Pages" field in Inquiry Page 0x0 shows support for Page 0xC4. Otherwise, assume Sync IOA Write Cache is not supported. Changes since v1: - Fix checkpatch.pl warnings. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 35 ++- drivers/scsi/ipr.h | 11 +++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 6849b7f..5efc7ef 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -7726,6 +7726,39 @@ static int ipr_inquiry_page_supported(struct ipr_inquiry_page0 *page0, u8 page) } /** + * ipr_ioafp_pageC4_inquiry - Send a Page 0xC4 Inquiry to the adapter. + * @ipr_cmd: ipr command struct + * + * This function sends a Page 0xC4 inquiry to the adapter + * to retrieve software VPD information. + * + * Return value: + * IPR_RC_JOB_CONTINUE / IPR_RC_JOB_RETURN + **/ +static int ipr_ioafp_pageC4_inquiry(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + struct ipr_inquiry_page0 *page0 = &ioa_cfg->vpd_cbs->page0_data; + struct ipr_inquiry_pageC4 *pageC4 = &ioa_cfg->vpd_cbs->pageC4_data; + + ENTER; + ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + memset(pageC4, 0, sizeof(*pageC4)); + + if (ipr_inquiry_page_supported(page0, 0xC4)) { + ipr_ioafp_inquiry(ipr_cmd, 1, 0xC4, + (ioa_cfg->vpd_cbs_dma + + offsetof(struct ipr_misc_cbs, + pageC4_data)), + sizeof(struct ipr_inquiry_pageC4)); + return IPR_RC_JOB_RETURN; + } + + LEAVE; + return IPR_RC_JOB_CONTINUE; +} + +/** * ipr_ioafp_cap_inquiry - Send a Page 0xD0 Inquiry to the adapter. * @ipr_cmd: ipr command struct * @@ -7742,7 +7775,7 @@ static int ipr_ioafp_cap_inquiry(struct ipr_cmnd *ipr_cmd) struct ipr_inquiry_cap *cap = &ioa_cfg->vpd_cbs->cap; ENTER; - ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + ipr_cmd->job_step = ipr_ioafp_pageC4_inquiry; memset(cap, 0, sizeof(*cap)); if (ipr_inquiry_page_supported(page0, 0xD0)) { diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index 69257c4..7be1271 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -849,6 +849,16 @@ struct ipr_inquiry_page0 { u8 page[IPR_INQUIRY_PAGE0_ENTRIES]; }__attribute__((packed)); +struct ipr_inquiry_pageC4 { + u8 peri_qual_dev_type; + u8 page_code; + u8 reserved1; + u8 len; + u8 cache_cap[4]; +#define IPR_CAP_SYNC_CACHE 0x08 + u8 reserved2[20]; +} __packed; + struct ipr_hostrcb_device_data_entry { struct ipr_vpd vpd; struct ipr_res_addr dev_res_addr; @@ -1322,6 +1332,7 @@ struct ipr_misc_cbs { struct ipr_inquiry_page0 page0_data; struct ipr_inquiry_page3 page3_data; struct ipr_inquiry_cap cap; + struct ipr_inquiry_pageC4 pageC4_data; struct ipr_mode_pages mode_pages; struct ipr_supported_device supp_dev; }; -- 2.1.0 -- 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] string_helpers: fix precision loss for some inputs
From: James Bottomley It was noticed that we lose precision in the final calculation for some inputs. The most egregious example is size=3000 blk_size=1900 in units of 10 should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the current algorithm doesn't correctly account for all the remainders in the logarithms. Fix this by doing a correct calculation in the remainders based on napier's algorithm. Additionally, now we have the correct result, we have to account for arithmetic rounding because we're printing 3 digits of precision. This means that if the fourth digit is five or greater, we have to round up, so add a section to ensure correct rounding. Finally account for all possible inputs correctly, including zero for block size. Reported-by: Vitaly Kuznetsov Cc: sta...@vger.kernel.org # delay backport by two months for testing Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 Signed-off-by: James Bottomley diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 5939f63..2ac77bf 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -43,37 +43,67 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, [STRING_UNITS_10] = 1000, [STRING_UNITS_2] = 1024, }; - int i, j; - u32 remainder = 0, sf_cap, exp; + static const unsigned int rounding[] = { 500, 50, 5, 0}; + int i = 0, j; + u32 remainder = 0, sf_cap, r1 = 0, r2 = 0; char tmp[8]; const char *unit; tmp[0] = '\0'; - i = 0; - if (!size) + + if (blk_size == 0) + size = 0; + if (size == 0) goto out; + /* This is napier's algorithm. Reduce the original block size to +* +* co * divisor[units]^i +* +* where co = blk_size + r1/divisor[units]; +* +* and the same for size. We simply add to the exponent i, because +* the final calculation we're looking for is +* +* (co1 * co2) * divisor[units]^(i1 +i2) +*/ + + while (blk_size >= divisor[units]) { - remainder = do_div(blk_size, divisor[units]); + r1 = do_div(blk_size, divisor[units]); i++; } - exp = divisor[units] / (u32)blk_size; - /* -* size must be strictly greater than exp here to ensure that remainder -* is greater than divisor[units] coming out of the if below. -*/ - if (size > exp) { - remainder = do_div(size, divisor[units]); - remainder *= blk_size; + while (size >= divisor[units]) { + r2 = do_div(size, divisor[units]); i++; - } else { - remainder *= size; } - size *= blk_size; - size += remainder / divisor[units]; - remainder %= divisor[units]; + /* +* We've already added the exponents in i, now multiply the +* coefficients: +* +* co1*co2 = (blk_size + r1/divisor[units])*(size + r2/divisor[units]) +* +* therefore +* +* co1*co2 = blk_size*size +* + (r1*size + r2*size)/divisor[units] +* + r1*r2/divisor[units]/divisor[units] +* +* reduce the exponent by 2 (it can now go negative) to perform this +* calculation without divisions (64 bit divisions are very painful on +* 32 bit architectures), then redo the logarithm adjustment to bring +* us back to the correct coefficient and exponent. This calculation +* can still not overflow because the largest term must be less than +* divisor[units]^4, which is 40 bits +* +*/ + + i -= 2; + size = size * blk_size * divisor[units] * divisor[units] + + (r1 * size + r2 * blk_size) * divisor[units] + + r1*r2; while (size >= divisor[units]) { remainder = do_div(size, divisor[units]); @@ -84,9 +114,20 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, for (j = 0; sf_cap*10 < 1000; j++) sf_cap *= 10; + /* express the remainder as a decimal (it's currently the numerator of +* a fraction whose denominator is divisor[units]) */ + remainder *= 1000; + remainder /= divisor[units]; + + /* add a 5 to the digit below what will be printed to ensure +* an arithmetical round up and carry it through to size */ + remainder += rounding[j]; + if (remainder >= 1000) { + remainder -= 1000; + size += 1; + } + if (j) { - remainder *= 1000; - remainder /= divisor[units]; snprintf(tmp, sizeof(tmp), ".%03u", remainder); tmp[j+1] = '\0'; } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a mess
Re: [PATCH v2 2/5] ipr: Don't set NO_ULEN_CHK bit when resource is a vset.
Gabriel: On applying this patch, I noticed that this statement seems to be unnecessary: else ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_UNTAGGED_TASK; As the value is 0x00: #define IPR_FLAGS_LO_UNTAGGED_TASK 0x00 You can resolve this in a future update. Reviewed-by: Manoj Kumar --- Manoj Kumar On 11/3/2015 12:26 PM, Gabriel Krisman Bertazi wrote: According to the IPR specification, Inhibit Underlength Checking bit must be disabled when issuing commands to vsets. Enabling it in this case might cause SCSI commands to fail with an Illegal Request, so make sure we keep this bit cleared when resource is a vset. Changes since v1: - Put gsci exclusive stuff in a separate block. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 238efab..6849b7f 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -6363,15 +6363,19 @@ static int ipr_queuecommand(struct Scsi_Host *shost, ipr_cmd->scsi_cmd = scsi_cmd; ipr_cmd->done = ipr_scsi_eh_done; - if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { + if (ipr_is_gscsi(res)) { if (scsi_cmd->underflow == 0) ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_ULEN_CHK; - ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_LINK_DESC; - if (ipr_is_gscsi(res) && res->reset_occurred) { + if (res->reset_occurred) { res->reset_occurred = 0; ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_DELAY_AFTER_RST; } + } + + if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { + ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_LINK_DESC; + ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_ALIGNED_BFR; if (scsi_cmd->flags & SCMD_TAGGED) ioarcb->cmd_pkt.flags_lo |= IPR_FLAGS_LO_SIMPLE_TASK; -- 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] string_helpers: fix precision loss for some inputs
From: James Bottomley It was noticed that we lose precision in the final calculation for some inputs. The most egregious example is size=3000 blk_size=1900 in units of 10 should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the current algorithm doesn't correctly account for all the remainders in the logarithms. Fix this by doing a correct calculation in the remainders based on napier's algorithm. Additionally, now we have the correct result, we have to account for arithmetic rounding because we're printing 3 digits of precision. This means that if the fourth digit is five or greater, we have to round up, so add a section to ensure correct rounding. Finally account for all possible inputs correctly, including zero for block size. Reported-by: Vitaly Kuznetsov Cc: sta...@vger.kernel.org # delay backport by two months for testing Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 Signed-off-by: James Bottomley -- v2: updated with a recommendation from Rasmus Villemoes to truncate the initial precision at just under 32 bits diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 5939f63..363faca 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, [STRING_UNITS_10] = 1000, [STRING_UNITS_2] = 1024, }; - int i, j; - u32 remainder = 0, sf_cap, exp; + static const unsigned int rounding[] = { 500, 50, 5, 0}; + int i = 0, j; + u32 remainder = 0, sf_cap; char tmp[8]; const char *unit; tmp[0] = '\0'; - i = 0; - if (!size) + + if (blk_size == 0) + size = 0; + if (size == 0) goto out; - while (blk_size >= divisor[units]) { - remainder = do_div(blk_size, divisor[units]); + /* This is napier's algorithm. Reduce the original block size to +* +* coefficient * divisor[units]^i +* +* we do the reduction so both coefficients are just under 32 bits so +* that multiplying them together won't overflow 64 bits and we keep +* as much precision as possible in the numbers +*/ + + while (blk_size >= UINT_MAX) i++; - } - exp = divisor[units] / (u32)blk_size; - /* -* size must be strictly greater than exp here to ensure that remainder -* is greater than divisor[units] coming out of the if below. -*/ - if (size > exp) { - remainder = do_div(size, divisor[units]); - remainder *= blk_size; + while (size >= UINT_MAX) i++; - } else { - remainder *= size; - } + + /* now perform the actual multiplication keeping i as the sum of the +* two logarithms */ size *= blk_size; - size += remainder / divisor[units]; - remainder %= divisor[units]; + /* and logarithmically reduce it until it's just under the divisor */ while (size >= divisor[units]) { remainder = do_div(size, divisor[units]); i++; @@ -84,9 +86,20 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, for (j = 0; sf_cap*10 < 1000; j++) sf_cap *= 10; + /* express the remainder as a decimal (it's currently the numerator of +* a fraction whose denominator is divisor[units]) */ + remainder *= 1000; + remainder /= divisor[units]; + + /* add a 5 to the digit below what will be printed to ensure +* an arithmetical round up and carry it through to size */ + remainder += rounding[j]; + if (remainder >= 1000) { + remainder -= 1000; + size += 1; + } + if (j) { - remainder *= 1000; - remainder /= divisor[units]; snprintf(tmp, sizeof(tmp), ".%03u", remainder); tmp[j+1] = '\0'; } -- 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] string_helpers: fix precision loss for some inputs
On Tue, Nov 03 2015, James Bottomley wrote: > From: James Bottomley > > It was noticed that we lose precision in the final calculation for some > inputs. The most egregious example is size=3000 blk_size=1900 in units of 10 > should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the > current algorithm doesn't correctly account for all the remainders in the > logarithms. Fix this by doing a correct calculation in the remainders based > on napier's algorithm. Additionally, now we have the correct result, we have > to account for arithmetic rounding because we're printing 3 digits of > precision. This means that if the fourth digit is five or greater, we have to > round up, so add a section to ensure correct rounding. Finally account for > all possible inputs correctly, including zero for block size. > > Reported-by: Vitaly Kuznetsov > Cc: sta...@vger.kernel.org# delay backport by two months for testing > Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 > Signed-off-by: James Bottomley > > -- > > v2: updated with a recommendation from Rasmus Villemoes to truncate the > initial precision at just under 32 bits > > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > index 5939f63..363faca 100644 > --- a/lib/string_helpers.c > +++ b/lib/string_helpers.c > @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const enum > string_size_units units, > [STRING_UNITS_10] = 1000, > [STRING_UNITS_2] = 1024, > }; > - int i, j; > - u32 remainder = 0, sf_cap, exp; > + static const unsigned int rounding[] = { 500, 50, 5, 0}; j necessarily ends up being 0, 1 or 2. Any reason to include the last entry? > + > + while (blk_size >= UINT_MAX) > i++; > - } > > - exp = divisor[units] / (u32)blk_size; > - /* > - * size must be strictly greater than exp here to ensure that remainder > - * is greater than divisor[units] coming out of the if below. > - */ > - if (size > exp) { > - remainder = do_div(size, divisor[units]); > - remainder *= blk_size; > + while (size >= UINT_MAX) > i++; Please spell it U32_MAX. Also, it's not clear why you left out the do_divs ;-) Rasmus -- 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] string_helpers: fix precision loss for some inputs
On Tue, 2015-11-03 at 13:37 -0800, Bart Van Assche wrote: > On 11/03/2015 01:21 PM, James Bottomley wrote: > > + while (blk_size >= UINT_MAX) > > i++; > > (reduced CC-list) Let's keep at least the lists in cc. > Hello James, > > Is the above loop an infinite loop if blk_size >= UINT_MAX ? Yes, a little too much truncation over the original. I'll post a v3. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] string_helpers: fix precision loss for some inputs
On Tue, 2015-11-03 at 23:13 +0100, Rasmus Villemoes wrote: > On Tue, Nov 03 2015, James Bottomley > wrote: > > > From: James Bottomley > > > > It was noticed that we lose precision in the final calculation for some > > inputs. The most egregious example is size=3000 blk_size=1900 in units of > > 10 > > should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the > > current algorithm doesn't correctly account for all the remainders in the > > logarithms. Fix this by doing a correct calculation in the remainders based > > on napier's algorithm. Additionally, now we have the correct result, we > > have > > to account for arithmetic rounding because we're printing 3 digits of > > precision. This means that if the fourth digit is five or greater, we have > > to > > round up, so add a section to ensure correct rounding. Finally account for > > all possible inputs correctly, including zero for block size. > > > > Reported-by: Vitaly Kuznetsov > > Cc: sta...@vger.kernel.org # delay backport by two months for testing > > Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 > > Signed-off-by: James Bottomley > > > > -- > > > > v2: updated with a recommendation from Rasmus Villemoes to truncate the > > initial precision at just under 32 bits > > > > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > > index 5939f63..363faca 100644 > > --- a/lib/string_helpers.c > > +++ b/lib/string_helpers.c > > @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const enum > > string_size_units units, > > [STRING_UNITS_10] = 1000, > > [STRING_UNITS_2] = 1024, > > }; > > - int i, j; > > - u32 remainder = 0, sf_cap, exp; > > + static const unsigned int rounding[] = { 500, 50, 5, 0}; > > j necessarily ends up being 0, 1 or 2. Any reason to include the last entry? No reason beyond a vague worry someone might try to increase the printed precision by one digit. > > + > > + while (blk_size >= UINT_MAX) > > i++; > > - } > > > > - exp = divisor[units] / (u32)blk_size; > > - /* > > -* size must be strictly greater than exp here to ensure that remainder > > -* is greater than divisor[units] coming out of the if below. > > -*/ > > - if (size > exp) { > > - remainder = do_div(size, divisor[units]); > > - remainder *= blk_size; > > + while (size >= UINT_MAX) > > i++; > > Please spell it U32_MAX Why? there's no reason not to use the arithmetic UINT_MAX here. Either works, of course but UINT_MAX is standard. > . Also, it's not clear why you left out the > do_divs ;-) Over reduction. James > Rasmus > -- > 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 > -- 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 v3] string_helpers: fix precision loss for some inputs
From: James Bottomley It was noticed that we lose precision in the final calculation for some inputs. The most egregious example is size=3000 blk_size=1900 in units of 10 should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the current algorithm doesn't correctly account for all the remainders in the logarithms. Fix this by doing a correct calculation in the remainders based on napier's algorithm. Additionally, now we have the correct result, we have to account for arithmetic rounding because we're printing 3 digits of precision. This means that if the fourth digit is five or greater, we have to round up, so add a section to ensure correct rounding. Finally account for all possible inputs correctly, including zero for block size. Reported-by: Vitaly Kuznetsov Cc: sta...@vger.kernel.org # delay backport by two months for testing Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 Signed-off-by: James Bottomley --- v2: updated with a recommendation from Rasmus Villemoes to truncate the initial precision at just under 32 bits v3: put back the missing do_divs diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 5939f63..c2ffb73 100644 --- a/lib/string_helpers.c +++ b/lib/string_helpers.c @@ -43,38 +43,45 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, [STRING_UNITS_10] = 1000, [STRING_UNITS_2] = 1024, }; - int i, j; - u32 remainder = 0, sf_cap, exp; + static const unsigned int rounding[] = { 500, 50, 5, 0}; + int i = 0, j; + u32 remainder = 0, sf_cap; char tmp[8]; const char *unit; tmp[0] = '\0'; - i = 0; - if (!size) + + if (blk_size == 0) + size = 0; + if (size == 0) goto out; - while (blk_size >= divisor[units]) { - remainder = do_div(blk_size, divisor[units]); + /* This is napier's algorithm. Reduce the original block size to +* +* coefficient * divisor[units]^i +* +* we do the reduction so both coefficients are just under 32 bits so +* that multiplying them together won't overflow 64 bits and we keep +* as much precision as possible in the numbers. +* +* Note: it's safe to throw away the remainders here because all the +* precision is in the coefficients. +*/ + while (blk_size >= UINT_MAX) { + do_div(blk_size, divisor[units]); i++; } - exp = divisor[units] / (u32)blk_size; - /* -* size must be strictly greater than exp here to ensure that remainder -* is greater than divisor[units] coming out of the if below. -*/ - if (size > exp) { - remainder = do_div(size, divisor[units]); - remainder *= blk_size; + while (size >= UINT_MAX) { + do_div(size, divisor[units]); i++; - } else { - remainder *= size; } + /* now perform the actual multiplication keeping i as the sum of the +* two logarithms */ size *= blk_size; - size += remainder / divisor[units]; - remainder %= divisor[units]; + /* and logarithmically reduce it until it's just under the divisor */ while (size >= divisor[units]) { remainder = do_div(size, divisor[units]); i++; @@ -84,9 +91,20 @@ void string_get_size(u64 size, u64 blk_size, const enum string_size_units units, for (j = 0; sf_cap*10 < 1000; j++) sf_cap *= 10; + /* express the remainder as a decimal (it's currently the numerator of +* a fraction whose denominator is divisor[units]) */ + remainder *= 1000; + remainder /= divisor[units]; + + /* add a 5 to the digit below what will be printed to ensure +* an arithmetical round up and carry it through to size */ + remainder += rounding[j]; + if (remainder >= 1000) { + remainder -= 1000; + size += 1; + } + if (j) { - remainder *= 1000; - remainder /= divisor[units]; snprintf(tmp, sizeof(tmp), ".%03u", remainder); tmp[j+1] = '\0'; } -- 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] string_helpers: fix precision loss for some inputs
On Tue, Nov 03 2015, James Bottomley wrote: > On Tue, 2015-11-03 at 23:13 +0100, Rasmus Villemoes wrote: >> On Tue, Nov 03 2015, James Bottomley >> wrote: >> >> > From: James Bottomley >> > >> > It was noticed that we lose precision in the final calculation for some >> > inputs. The most egregious example is size=3000 blk_size=1900 in units of >> > 10 >> > should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the >> > current algorithm doesn't correctly account for all the remainders in the >> > logarithms. Fix this by doing a correct calculation in the remainders >> > based >> > on napier's algorithm. Additionally, now we have the correct result, we >> > have >> > to account for arithmetic rounding because we're printing 3 digits of >> > precision. This means that if the fourth digit is five or greater, we >> > have to >> > round up, so add a section to ensure correct rounding. Finally account for >> > all possible inputs correctly, including zero for block size. >> > >> > Reported-by: Vitaly Kuznetsov >> > Cc: sta...@vger.kernel.org # delay backport by two months for testing >> > Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 >> > Signed-off-by: James Bottomley >> > >> > -- >> > >> > v2: updated with a recommendation from Rasmus Villemoes to truncate the >> > initial precision at just under 32 bits >> > >> > diff --git a/lib/string_helpers.c b/lib/string_helpers.c >> > index 5939f63..363faca 100644 >> > --- a/lib/string_helpers.c >> > +++ b/lib/string_helpers.c >> > @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const >> > enum string_size_units units, >> >[STRING_UNITS_10] = 1000, >> >[STRING_UNITS_2] = 1024, >> >}; >> > - int i, j; >> > - u32 remainder = 0, sf_cap, exp; >> > + static const unsigned int rounding[] = { 500, 50, 5, 0}; >> >> j necessarily ends up being 0, 1 or 2. Any reason to include the last entry? > > No reason beyond a vague worry someone might try to increase the printed > precision by one digit. But that would seem to require prepending 5000 to that array and changing various constants below to 1 (aside from checking all callers to see if they pass a sufficient buffer size) - the 0 doesn't serve any purpose in that scenario either. >> > + >> > + while (blk_size >= UINT_MAX) >> >i++; >> > - } >> > >> > - exp = divisor[units] / (u32)blk_size; >> > - /* >> > - * size must be strictly greater than exp here to ensure that remainder >> > - * is greater than divisor[units] coming out of the if below. >> > - */ >> > - if (size > exp) { >> > - remainder = do_div(size, divisor[units]); >> > - remainder *= blk_size; >> > + while (size >= UINT_MAX) >> >i++; >> >> Please spell it U32_MAX > > Why? there's no reason not to use the arithmetic UINT_MAX here. Either > works, of course but UINT_MAX is standard. We're dealing with explicitly sized integers, and the comment even says that we're reducing till we fit in 32 bits, so that we can do a 32x32->64 multiplication. U32_MAX is the natural name for the appropriate constant. Also, you could do > U32_MAX instead of >= U32_MAX, but that's unlikely to make any difference (well, except it might generate slightly better code, since it would allow gcc to just test the upper half for being 0, which might be cheaper on some architectures than comparing to a literal). Rasmus -- 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] string_helpers: fix precision loss for some inputs
On Wed, 2015-11-04 at 00:26 +0100, Rasmus Villemoes wrote: > On Tue, Nov 03 2015, James Bottomley > wrote: > > > On Tue, 2015-11-03 at 23:13 +0100, Rasmus Villemoes wrote: > >> On Tue, Nov 03 2015, James Bottomley > >> wrote: > >> > >> > From: James Bottomley > >> > > >> > It was noticed that we lose precision in the final calculation for some > >> > inputs. The most egregious example is size=3000 blk_size=1900 in units > >> > of 10 > >> > should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because > >> > the > >> > current algorithm doesn't correctly account for all the remainders in the > >> > logarithms. Fix this by doing a correct calculation in the remainders > >> > based > >> > on napier's algorithm. Additionally, now we have the correct result, we > >> > have > >> > to account for arithmetic rounding because we're printing 3 digits of > >> > precision. This means that if the fourth digit is five or greater, we > >> > have to > >> > round up, so add a section to ensure correct rounding. Finally account > >> > for > >> > all possible inputs correctly, including zero for block size. > >> > > >> > Reported-by: Vitaly Kuznetsov > >> > Cc: sta...@vger.kernel.org # delay backport by two months for > >> > testing > >> > Fixes: b9f28d863594c429e1df35a0474d2663ca28b307 > >> > Signed-off-by: James Bottomley > >> > > >> > -- > >> > > >> > v2: updated with a recommendation from Rasmus Villemoes to truncate the > >> > initial precision at just under 32 bits > >> > > >> > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > >> > index 5939f63..363faca 100644 > >> > --- a/lib/string_helpers.c > >> > +++ b/lib/string_helpers.c > >> > @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const > >> > enum string_size_units units, > >> > [STRING_UNITS_10] = 1000, > >> > [STRING_UNITS_2] = 1024, > >> > }; > >> > -int i, j; > >> > -u32 remainder = 0, sf_cap, exp; > >> > +static const unsigned int rounding[] = { 500, 50, 5, 0}; > >> > >> j necessarily ends up being 0, 1 or 2. Any reason to include the last > >> entry? > > > > No reason beyond a vague worry someone might try to increase the printed > > precision by one digit. > > But that would seem to require prepending 5000 to that array and > changing various constants below to 1 (aside from checking all > callers to see if they pass a sufficient buffer size) - the 0 doesn't > serve any purpose in that scenario either. > > >> > + > >> > +while (blk_size >= UINT_MAX) > >> > i++; > >> > -} > >> > > >> > -exp = divisor[units] / (u32)blk_size; > >> > -/* > >> > - * size must be strictly greater than exp here to ensure that > >> > remainder > >> > - * is greater than divisor[units] coming out of the if below. > >> > - */ > >> > -if (size > exp) { > >> > -remainder = do_div(size, divisor[units]); > >> > -remainder *= blk_size; > >> > +while (size >= UINT_MAX) > >> > i++; > >> > >> Please spell it U32_MAX > > > > Why? there's no reason not to use the arithmetic UINT_MAX here. Either > > works, of course but UINT_MAX is standard. > > We're dealing with explicitly sized integers An integer is explicitly sized: it's 32 bits. That's why UINT_MAX is a universal constant. > and the comment even says > that we're reducing till we fit in 32 bits, so that we can do a > 32x32->64 multiplication. U32_MAX is the natural name for the > appropriate constant. > > Also, you could do > U32_MAX instead of >= U32_MAX, but that's unlikely > to make any difference (well, except it might generate slightly better > code, since it would allow gcc to just test the upper half for being 0, > which might be cheaper on some architectures than comparing to a > literal). Heh if we're going to be that concerned about the code generation, then we should just tell gcc exactly how to do it instead of hoping it can work it out for itself, so while (blk_size >> 32) { ... James > Rasmus > -- > 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 > -- 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 3/5] ipr: Inquiry IOA page 0xC4 during initialization.
Acked-by: Brian King -- Brian King Power Linux I/O IBM Linux Technology Center -- 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 2/5] ipr: Don't set NO_ULEN_CHK bit when resource is a vset.
Acked-by: Brian King -- Brian King Power Linux I/O IBM Linux Technology Center -- 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 1/5] ipr: Add delay to ensure coherent dumps.
Acked-by: Brian King -- Brian King Power Linux I/O IBM Linux Technology Center -- 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 5/5] ipr: Driver version 2.6.3.
Acked-by: Brian King -- Brian King Power Linux I/O IBM Linux Technology Center -- 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 4/5] ipr: Issue Configure Cache Parameters command.
Acked-by: Brian King -- Brian King Power Linux I/O IBM Linux Technology Center -- 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