ATTENTION

2015-11-03 Thread
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

2015-11-03 Thread Sumit Saxena
> -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

2015-11-03 Thread Alim Akhtar

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

2015-11-03 Thread John Garry

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

2015-11-03 Thread Arnd Bergmann
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

2015-11-03 Thread Tomas Henzl
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

2015-11-03 Thread Christoph Hellwig
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.

2015-11-03 Thread Laurent Vivier
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

2015-11-03 Thread Laurent Vivier
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

2015-11-03 Thread Laurent Vivier
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.

2015-11-03 Thread kbuild test robot
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

2015-11-03 Thread RENEE CAMERON
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.

2015-11-03 Thread Gabriel Krisman Bertazi
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.

2015-11-03 Thread Gabriel Krisman Bertazi
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.

2015-11-03 Thread Gabriel Krisman Bertazi
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.

2015-11-03 Thread Gabriel Krisman Bertazi
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.

2015-11-03 Thread Gabriel Krisman Bertazi
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

2015-11-03 Thread James Bottomley
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.

2015-11-03 Thread Manoj Kumar

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

2015-11-03 Thread James Bottomley
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

2015-11-03 Thread Rasmus Villemoes
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

2015-11-03 Thread James Bottomley
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

2015-11-03 Thread James Bottomley
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

2015-11-03 Thread James Bottomley
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

2015-11-03 Thread Rasmus Villemoes
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

2015-11-03 Thread James Bottomley
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.

2015-11-03 Thread Brian King
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.

2015-11-03 Thread Brian King
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.

2015-11-03 Thread Brian King
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.

2015-11-03 Thread Brian King
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.

2015-11-03 Thread Brian King
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