[PATCH v4] string_helpers: fix precision loss for some inputs

[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

v4: use explicit top bit checking in logarithmic reductions.  Only adjust
remainder precision where necessary

diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 5939f63..5c88204 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -43,50 +43,73 @@ 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 };
+   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 >> 32) {
+   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 >> 32) {
+   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++;
}
 
+   /* work out in j how many digits of precision we need from the
+* remainder */
sf_cap = size;
for (j = 0; sf_cap*10 < 1000; j++)
sf_cap *= 10;
 
-   if (j) {
+   if (units == STRING_UNITS_2) {
+   /* express the remainder as a decimal.  It's currently the
+* numerator of a fraction whose denominator is
+* divisor[units], which is 1 << 10 for STRING_UNITS_2 */
remainder *= 1000;
-   remainder /= divisor[units];
+   remainder >>= 10;
+   }
+
+   /* 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) {
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: [ANNOUNCE]: SCST 3.1 pre-release freeze

[Vladislav Bolkhovitin]

Hi,

Bike & Snow wrote on 11/06/2015 10:55 AM:
> Hello Vlad
> 
> Excellent news on all the updates.
> 
> Regarding this:
> - QLogic target driver has been significantly improved.
> 
> Does that mean I should stop building the QLogic target driver from here?
> git://git.qlogic.com/scst-qla2xxx.git 
> 
> Or are you saying the git.qlogic.com  has been 
> improved?

It is saying that qla2x00t was improved.

The ultimate goal is to have the mainstream (git) QLogic target driver to be 
the main
and the only QLogic target driver, but, unfortunately, this driver not yet 
reached
level of quality and maturity of qla2x00t. We with QLogic are working toward it.

> If I stop building the one from git.qlogic.com , does 
> the 3.2.0
> one support NPIV?

Yes, it has full NPIV support.

Vlad

--
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] st: trivial: remove form feed characters

[Maurizio Lombardi]

On 11/04/2015 10:04 PM, "Kai Mäkisara (Kolumbus)" wrote:
> What’s the point? Is there an “official” rule that form feeds are not allowed 
> (to
> put different things to different pages in printout)?


I wrote it just because on some editors - and with thunderbird in particular -
those characters make the code look weird.
It's not a real issue so if you want to keep them it's ok for me.

Regards,
Maurizio Lombardi
--
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

[Bug 107371] I/O error when accessing disk in standby

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=107371

Mikael Grahn  changed:

   What|Removed |Added

 Kernel Version|4.1.2   |4.1.12

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 01/27] hpsa: remove unused parameter hostno

[Tomas Henzl]

On 4.11.2015 22:50, Don Brace wrote:
> This parameter was once used before scan_start was defined
> but now it is no longer used.
>
> Signed-off-by: Don Brace 

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 v2 00/27] hpsa updates

[Don Brace]

Please repost, I'll sign off.
Thank-you

On 11/06/2015 08:30 AM, Tomas Henzl wrote:

Please add to this series the "hpsa: move lockup_detected attribute to host 
attr"
http://www.spinics.net/lists/linux-scsi/msg88129.html.

Maybe the maintainer could add it too, or should I repost the patch?

Thanks,
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


--
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 RESEND] hpsa: move lockup_detected attribute to host attr

[Tomas Henzl]

This patch fixes a 'general protection fault' issue by
moving the attribute to where it was likely meant.

Signed-off-by: Tomas Henzl 
Signed-off-by: Don Brace 
---
 drivers/scsi/hpsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 57166e68fc..6d4412359d 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -867,7 +867,6 @@ static struct device_attribute *hpsa_sdev_attrs[] = {
_attr_unique_id,
_attr_hp_ssd_smart_path_enabled,
_attr_path_info,
-   _attr_lockup_detected,
NULL,
 };
 
@@ -879,6 +878,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {
_attr_resettable,
_attr_hp_ssd_smart_path_status,
_attr_raid_offload_debug,
+   _attr_lockup_detected,
NULL,
 };
 
-- 
2.4.3

--
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 RESEND] hpsa: move lockup_detected attribute to host attr

[Don Brace]

On 11/06/2015 09:24 AM, Tomas Henzl wrote:

This patch fixes a 'general protection fault' issue by
moving the attribute to where it was likely meant.

Signed-off-by: Tomas Henzl 
Signed-off-by: Don Brace 
---
  drivers/scsi/hpsa.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 57166e68fc..6d4412359d 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -867,7 +867,6 @@ static struct device_attribute *hpsa_sdev_attrs[] = {
_attr_unique_id,
_attr_hp_ssd_smart_path_enabled,
_attr_path_info,
-   _attr_lockup_detected,
NULL,
  };
  
@@ -879,6 +878,7 @@ static struct device_attribute *hpsa_shost_attrs[] = {

_attr_resettable,
_attr_hp_ssd_smart_path_status,
_attr_raid_offload_debug,
+   _attr_lockup_detected,
NULL,
  };
  

Signed-off-by: Don Brace 
--
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 RESEND] scsi_sysfs: protect against double execution of __scsi_remove_device()

[Vitaly Kuznetsov]

On some host errors storvsc module tries to remove sdev by scheduling a job
which does the following:

   sdev = scsi_device_lookup(wrk->host, 0, 0, wrk->lun);
   if (sdev) {
   scsi_remove_device(sdev);
   scsi_device_put(sdev);
   }

While this code seems correct the following crash is observed:

 general protection fault:  [#1] SMP DEBUG_PAGEALLOC
 RIP: 0010:[]  [] bdi_destroy+0x39/0x220
 ...
 [] ? _raw_spin_unlock_irq+0x2c/0x40
 [] blk_cleanup_queue+0x17b/0x270
 [] __scsi_remove_device+0x54/0xd0 [scsi_mod]
 [] scsi_remove_device+0x2b/0x40 [scsi_mod]
 [] storvsc_remove_lun+0x3d/0x60 [hv_storvsc]
 [] process_one_work+0x1b1/0x530
 ...

The problem comes with the fact that many such jobs (for the same device)
are being scheduled simultaneously. While scsi_remove_device() uses
shost->scan_mutex and scsi_device_lookup() will fail for a device in
SDEV_DEL state there is no protection against someone who did
scsi_device_lookup() before we actually entered __scsi_remove_device(). So
the whole scenario looks like that: two callers do simultaneous (or
preemption happens) calls to scsi_device_lookup() ant these calls succeed
for both of them, after that they try doing scsi_remove_device().
shost->scan_mutex only serializes their calls to __scsi_remove_device()
and we end up doing the cleanup path twice.

Signed-off-by: Vitaly Kuznetsov 
---
 drivers/scsi/scsi_sysfs.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dff8faf..3b7e2bb 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1078,6 +1078,14 @@ void __scsi_remove_device(struct scsi_device *sdev)
 {
struct device *dev = >sdev_gendev;
 
+   /*
+* This cleanup path is not reentrant and while it is impossible
+* to get a new reference with scsi_device_get() someone can still
+* hold a previously acquired one.
+*/
+   if (sdev->sdev_state == SDEV_DEL)
+   return;
+
if (sdev->is_visible) {
if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
return;
-- 
2.4.3

--
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

[Bug 107371] I/O error when accessing disk in standby

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=107371

Mikael Grahn  changed:

   What|Removed |Added

   Hardware|All |x86-64

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 RESEND] scsi_sysfs: protect against double execution of __scsi_remove_device()

[KY Srinivasan]

> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Friday, November 6, 2015 7:49 AM
> To: James E.J. Bottomley 
> Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; KY Srinivasan
> ; Bart Van Assche 
> Subject: [PATCH RESEND] scsi_sysfs: protect against double execution of
> __scsi_remove_device()
> 
> On some host errors storvsc module tries to remove sdev by scheduling a job
> which does the following:
> 
>sdev = scsi_device_lookup(wrk->host, 0, 0, wrk->lun);
>if (sdev) {
>scsi_remove_device(sdev);
>scsi_device_put(sdev);
>}
> 
> While this code seems correct the following crash is observed:
> 
>  general protection fault:  [#1] SMP DEBUG_PAGEALLOC
>  RIP: 0010:[]  [] bdi_destroy+0x39/0x220
>  ...
>  [] ? _raw_spin_unlock_irq+0x2c/0x40
>  [] blk_cleanup_queue+0x17b/0x270
>  [] __scsi_remove_device+0x54/0xd0 [scsi_mod]
>  [] scsi_remove_device+0x2b/0x40 [scsi_mod]
>  [] storvsc_remove_lun+0x3d/0x60 [hv_storvsc]
>  [] process_one_work+0x1b1/0x530
>  ...
> 
> The problem comes with the fact that many such jobs (for the same device)
> are being scheduled simultaneously. While scsi_remove_device() uses
> shost->scan_mutex and scsi_device_lookup() will fail for a device in
> SDEV_DEL state there is no protection against someone who did
> scsi_device_lookup() before we actually entered __scsi_remove_device().
> So
> the whole scenario looks like that: two callers do simultaneous (or
> preemption happens) calls to scsi_device_lookup() ant these calls succeed
> for both of them, after that they try doing scsi_remove_device().
> shost->scan_mutex only serializes their calls to __scsi_remove_device()
> and we end up doing the cleanup path twice.
> 
> Signed-off-by: Vitaly Kuznetsov 

James,

I too have a bunch of patches in your queue (sent about a month ago).
Should I resend them as well.

Regards,

K. Y
--
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

[BUG] Boot failures with mpt2sas / Intel RMS25JB080 module

[Matthew Vernon]

Hi,

[These lists are in the MAINTAINERS file for mpt2sas; I hope this is
the correct place to report this problem. Xen-devel CCd as this is
failing on trying to boot Xen]

When booting Xen, mpt2sas finds my Intel RMS25JB080, but fails to load
it correctly.

This is a Debian jessie (stable) system, kernel version
3.16.0-4-amd64, Xen version 4.4.1.

Relevant snippet of log output of failed boot:

[   64.095008] mpt2sas0: reply_post_free pool: pci_pool_alloc failed
[   64.535255] mpt2sas0: failure at 
/build/linux-xkTWug/linux-3.16.7-ckt11/drivers/scsi/mpt2sas/mpt2sas_scsih.c:8234/_scsih_probe()!

...it then fails to find any of my attached drives, so cannot boot,
and I get dumped in the initramfs.

Having googled a bit, I tried pci=realloc=off on the kernel
command-line, as well as swiotlb=26422 ; neither of them changed the
failure mode

Booting bare-metal does work; relevant snippet of log output of
successful (i.e. bare-metal) boot:

[3.977037] mpt2sas0: diag reset: SUCCESS
[4.178627] mpt2sas0: Allocated physical memory: size(17390 kB)
[4.178630] mpt2sas0: Current Controller Queue Depth(7931), Max Controller 
Queue Depth(8192)
[4.178631] mpt2sas0: Scatter Gather Elements per IO(128)
[4.409752] mpt2sas0: LSISAS2308: FWVersion(20.00.02.00), 
ChipRevision(0x05), BiosVersion(07.39.00.00)
[4.409755] mpt2sas0: Intel(R) Integrated RAID Module RMS25JB080
[4.409756] mpt2sas0: Protocol=(Initiator), 
Capabilities=(Raid,TLR,EEDP,Snapshot Buffer,Diag Trace Buffer,Task Set Full,NCQ)
[4.409846] mpt2sas0: sending port enable !!
...
[5.941978] mpt2sas0: host_add: handle(0x0001), 
sas_addr(0x5001e67c17ff5000), phys(8)
[5.980601] device-mapper: uevent: version 1.0.3
[5.980773] device-mapper: ioctl: 4.27.0-ioctl (2013-10-30) initialised: 
dm-de...@redhat.com
[   11.823480] mpt2sas0: port enable: SUCCESS

(and then it finds the attached disks)

Fuller outputs:

i) bare-metal boot dmesg:
http://www-uxsup.csx.cam.ac.uk/~mcv21/debug/bare_metal_dmesg

ii) output from failed Xen boot (captured over a serial line with
earlyprintk=xen and loglvl=all guest_loglvl=all):
http://www-uxsup.csx.cam.ac.uk/~mcv21/debug/failedboot-output-2

iii) kernel config (stock Debian):
http://www-uxsup.csx.cam.ac.uk/~mcv21/debug/config-3.16.0-4-amd64

iv) lspci -vvv output:
http://www-uxsup.csx.cam.ac.uk/~mcv21/debug/lspcivvv

Regards,

Matthew
--
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

[Bug 107371] New: I/O error when accessing disk in standby

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=107371

Bug ID: 107371
   Summary: I/O error when accessing disk in standby
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: 4.1.2
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: nikm...@home.se
Regression: No

I got a LSI 2308 SAS controller (using the mpt2sas driver) and are noticing odd
I/O errors on my disk.

Example:

[398374.597662] sd 0:0:4:0: [sde] UNKNOWN(0x2003) Result: hostbyte=0x00
driverbyte=0x00
[398374.597665] sd 0:0:4:0: [sde] CDB: opcode=0x88 88 00 00 00 00 00 c0 52 c3
90 00 00 00 08 00 00
[398374.597667] blk_update_request: I/O error, dev sde, sector 3226649488

It happenes to all disks on the controller. I can't seem to notice any problems
due to this, but something does not seem right about getting this error. I also
checked the offending sectors and it's no problem reading them.

So i started to investigate this problem and soon found that the problem occurs
when the disk is in standby mode. Ie. when I try to access the disk when it is
in standby mode. Somehow the system tries to access the disk without spinning
it up first, and hence getting this error. I also checked the offending sectors
and there is no problem reading them.

I don't notice any problems due to this error, but I don't think this error
should occur.

It's very easy to replicate.

1. run "hdparm -y" on the disk to spin it down.
2. Access the disk in any way, like list a directory.
3. Getting this I/O error above, but this getting the dirlist as if nothing
wen't wrong.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
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