* [PATCH] sd, mmc, virtio_blk, string_helpers: fix block size units
@ 2015-03-06 2:47 James Bottomley
2015-03-13 10:07 ` Ulf Hansson
0 siblings, 1 reply; 2+ messages in thread
From: James Bottomley @ 2015-03-06 2:47 UTC (permalink / raw)
To: linux-scsi; +Cc: Andrew Morton, linux-mmc, virtualization
From: James Bottomley <JBottomley@Parallels.com>
The current string_get_size() overflows when the device size goes over
2^64 bytes because the string helper routine computes the suffix from
the size in bytes. However, the entirety of Linux thinks in terms of
blocks, not bytes, so this will artificially induce an overflow on very
large devices. Fix this by making the function string_get_size() take
blocks and the block size instead of bytes. This should allow us to
keep working until the current SCSI standard overflows.
Also fix virtio_blk and mmc (both of which were also artificially
multiplying by the block size to pass a byte side to string_get_size()).
The mathematics of this is pretty simple: we're taking a product of
size in blocks (S) and block size (B) and trying to re-express this in
exponential form: S*B = R*N^E (where N, the exponent is either 1000 or
1024) and R < N. Mathematically, S = RS*N^ES and B=RB*N^EB, so if RS*RB
< N it's easy to see that S*B = RS*RB*N^(ES+EB). However, if RS*BS > N,
we can see that this can be re-expressed as RS*BS = R*N (where R =
RS*BS/N < N) so the whole exponent becomes R*N^(ES+EB+1)
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
---
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index cdfbd21..26d2440 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -344,7 +344,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
struct request_queue *q = vblk->disk->queue;
char cap_str_2[10], cap_str_10[10];
char *envp[] = { "RESIZE=1", NULL };
- u64 capacity, size;
+ u64 capacity;
/* Host must always specify the capacity. */
virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
@@ -356,9 +356,10 @@ static void virtblk_config_changed_work(struct work_struct *work)
capacity = (sector_t)-1;
}
- size = capacity * queue_logical_block_size(q);
- string_get_size(size, STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
- string_get_size(size, STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
+ string_get_size(capacity, queue_logical_block_size(q),
+ STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
+ string_get_size(capacity, queue_logical_block_size(q),
+ STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
dev_notice(&vdev->dev,
"new size: %llu %d-byte logical blocks (%s/%s)\n",
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index c69afb5..2fc4269 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2230,7 +2230,7 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
part_md->part_type = part_type;
list_add(&part_md->part, &md->part);
- string_get_size((u64)get_capacity(part_md->disk) << 9, STRING_UNITS_2,
+ string_get_size((u64)get_capacity(part_md->disk), 512, STRING_UNITS_2,
cap_str, sizeof(cap_str));
pr_info("%s: %s %s partition %u %s\n",
part_md->disk->disk_name, mmc_card_id(card),
@@ -2436,7 +2436,7 @@ static int mmc_blk_probe(struct device *dev)
if (IS_ERR(md))
return PTR_ERR(md);
- string_get_size((u64)get_capacity(md->disk) << 9, STRING_UNITS_2,
+ string_get_size((u64)get_capacity(md->disk), 512, STRING_UNITS_2,
cap_str, sizeof(cap_str));
pr_info("%s: %s %s %s %s\n",
md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6b78476..3c7fe4e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2235,11 +2235,11 @@ got_data:
{
char cap_str_2[10], cap_str_10[10];
- u64 sz = (u64)sdkp->capacity << ilog2(sector_size);
- string_get_size(sz, STRING_UNITS_2, cap_str_2,
- sizeof(cap_str_2));
- string_get_size(sz, STRING_UNITS_10, cap_str_10,
+ string_get_size(sdkp->capacity, sector_size,
+ STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
+ string_get_size(sdkp->capacity, sector_size,
+ STRING_UNITS_10, cap_str_10,
sizeof(cap_str_10));
if (sdkp->first_scan || old_capacity != sdkp->capacity) {
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 6575718..2633280 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -10,7 +10,7 @@ enum string_size_units {
STRING_UNITS_2, /* use binary powers of 2^10 */
};
-void string_get_size(u64 size, enum string_size_units units,
+void string_get_size(u64 size, u64 blk_size, enum string_size_units units,
char *buf, int len);
#define UNESCAPE_SPACE 0x01
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 8f8c441..6ac8d5a 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -4,6 +4,7 @@
* Copyright 31 August 2008 James Bottomley
* Copyright (C) 2013, Intel Corporation
*/
+#include <linux/bug.h>
#include <linux/kernel.h>
#include <linux/math64.h>
#include <linux/export.h>
@@ -14,7 +15,8 @@
/**
* string_get_size - get the size in the specified units
- * @size: The size to be converted
+ * @size: The size to be converted in blocks
+ * @blk_size: Size of the block (use 1 for size in bytes)
* @units: units to use (powers of 1000 or 1024)
* @buf: buffer to format to
* @len: length of buffer
@@ -24,14 +26,14 @@
* at least 9 bytes and will always be zero terminated.
*
*/
-void string_get_size(u64 size, const enum string_size_units units,
+void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
char *buf, int len)
{
static const char *const units_10[] = {
- "B", "kB", "MB", "GB", "TB", "PB", "EB"
+ "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
};
static const char *const units_2[] = {
- "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB"
+ "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
};
static const char *const *const units_str[] = {
[STRING_UNITS_10] = units_10,
@@ -42,31 +44,58 @@ void string_get_size(u64 size, const enum string_size_units units,
[STRING_UNITS_2] = 1024,
};
int i, j;
- u32 remainder = 0, sf_cap;
+ u32 remainder = 0, sf_cap, exp;
char tmp[8];
+ const char *unit;
tmp[0] = '\0';
i = 0;
- if (size >= divisor[units]) {
- while (size >= divisor[units]) {
- remainder = do_div(size, divisor[units]);
- i++;
- }
+ if (!size)
+ goto out;
- sf_cap = size;
- for (j = 0; sf_cap*10 < 1000; j++)
- sf_cap *= 10;
+ while (blk_size >= divisor[units]) {
+ remainder = do_div(blk_size, divisor[units]);
+ i++;
+ }
- if (j) {
- remainder *= 1000;
- remainder /= divisor[units];
- snprintf(tmp, sizeof(tmp), ".%03u", remainder);
- tmp[j+1] = '\0';
- }
+ exp = divisor[units];
+ do_div(exp, blk_size);
+ if (size >= exp) {
+ remainder = do_div(size, divisor[units]);
+ remainder *= blk_size;
+ i++;
+ } else {
+ remainder *= size;
+ }
+
+ size *= blk_size;
+ size += (remainder/divisor[units]);
+ remainder %= divisor[units];
+
+ while (size >= divisor[units]) {
+ remainder = do_div(size, divisor[units]);
+ i++;
}
+ sf_cap = size;
+ for (j = 0; sf_cap*10 < 1000; j++)
+ sf_cap *= 10;
+
+ if (j) {
+ remainder *= 1000;
+ remainder /= divisor[units];
+ snprintf(tmp, sizeof(tmp), ".%03u", remainder);
+ tmp[j+1] = '\0';
+ }
+
+ out:
+ if (i >= ARRAY_SIZE(units_2))
+ unit = "UNK";
+ else
+ unit = units_str[units][i];
+
snprintf(buf, len, "%u%s %s", (u32)size,
- tmp, units_str[units][i]);
+ tmp, unit);
}
EXPORT_SYMBOL(string_get_size);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] sd, mmc, virtio_blk, string_helpers: fix block size units
2015-03-06 2:47 [PATCH] sd, mmc, virtio_blk, string_helpers: fix block size units James Bottomley
@ 2015-03-13 10:07 ` Ulf Hansson
0 siblings, 0 replies; 2+ messages in thread
From: Ulf Hansson @ 2015-03-13 10:07 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-scsi, Andrew Morton, linux-mmc, virtualization
On 6 March 2015 at 03:47, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> From: James Bottomley <JBottomley@Parallels.com>
>
> The current string_get_size() overflows when the device size goes over
> 2^64 bytes because the string helper routine computes the suffix from
> the size in bytes. However, the entirety of Linux thinks in terms of
> blocks, not bytes, so this will artificially induce an overflow on very
> large devices. Fix this by making the function string_get_size() take
> blocks and the block size instead of bytes. This should allow us to
> keep working until the current SCSI standard overflows.
>
> Also fix virtio_blk and mmc (both of which were also artificially
> multiplying by the block size to pass a byte side to string_get_size()).
>
> The mathematics of this is pretty simple: we're taking a product of
> size in blocks (S) and block size (B) and trying to re-express this in
> exponential form: S*B = R*N^E (where N, the exponent is either 1000 or
> 1024) and R < N. Mathematically, S = RS*N^ES and B=RB*N^EB, so if RS*RB
> < N it's easy to see that S*B = RS*RB*N^(ES+EB). However, if RS*BS > N,
> we can see that this can be re-expressed as RS*BS = R*N (where R =
> RS*BS/N < N) so the whole exponent becomes R*N^(ES+EB+1)
>
> Signed-off-by: James Bottomley <JBottomley@Parallels.com>
For the mmc parts;
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
>
> ---
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index cdfbd21..26d2440 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -344,7 +344,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
> struct request_queue *q = vblk->disk->queue;
> char cap_str_2[10], cap_str_10[10];
> char *envp[] = { "RESIZE=1", NULL };
> - u64 capacity, size;
> + u64 capacity;
>
> /* Host must always specify the capacity. */
> virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
> @@ -356,9 +356,10 @@ static void virtblk_config_changed_work(struct work_struct *work)
> capacity = (sector_t)-1;
> }
>
> - size = capacity * queue_logical_block_size(q);
> - string_get_size(size, STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
> - string_get_size(size, STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
> + string_get_size(capacity, queue_logical_block_size(q),
> + STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
> + string_get_size(capacity, queue_logical_block_size(q),
> + STRING_UNITS_10, cap_str_10, sizeof(cap_str_10));
>
> dev_notice(&vdev->dev,
> "new size: %llu %d-byte logical blocks (%s/%s)\n",
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index c69afb5..2fc4269 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2230,7 +2230,7 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
> part_md->part_type = part_type;
> list_add(&part_md->part, &md->part);
>
> - string_get_size((u64)get_capacity(part_md->disk) << 9, STRING_UNITS_2,
> + string_get_size((u64)get_capacity(part_md->disk), 512, STRING_UNITS_2,
> cap_str, sizeof(cap_str));
> pr_info("%s: %s %s partition %u %s\n",
> part_md->disk->disk_name, mmc_card_id(card),
> @@ -2436,7 +2436,7 @@ static int mmc_blk_probe(struct device *dev)
> if (IS_ERR(md))
> return PTR_ERR(md);
>
> - string_get_size((u64)get_capacity(md->disk) << 9, STRING_UNITS_2,
> + string_get_size((u64)get_capacity(md->disk), 512, STRING_UNITS_2,
> cap_str, sizeof(cap_str));
> pr_info("%s: %s %s %s %s\n",
> md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 6b78476..3c7fe4e 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2235,11 +2235,11 @@ got_data:
>
> {
> char cap_str_2[10], cap_str_10[10];
> - u64 sz = (u64)sdkp->capacity << ilog2(sector_size);
>
> - string_get_size(sz, STRING_UNITS_2, cap_str_2,
> - sizeof(cap_str_2));
> - string_get_size(sz, STRING_UNITS_10, cap_str_10,
> + string_get_size(sdkp->capacity, sector_size,
> + STRING_UNITS_2, cap_str_2, sizeof(cap_str_2));
> + string_get_size(sdkp->capacity, sector_size,
> + STRING_UNITS_10, cap_str_10,
> sizeof(cap_str_10));
>
> if (sdkp->first_scan || old_capacity != sdkp->capacity) {
> diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
> index 6575718..2633280 100644
> --- a/include/linux/string_helpers.h
> +++ b/include/linux/string_helpers.h
> @@ -10,7 +10,7 @@ enum string_size_units {
> STRING_UNITS_2, /* use binary powers of 2^10 */
> };
>
> -void string_get_size(u64 size, enum string_size_units units,
> +void string_get_size(u64 size, u64 blk_size, enum string_size_units units,
> char *buf, int len);
>
> #define UNESCAPE_SPACE 0x01
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 8f8c441..6ac8d5a 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -4,6 +4,7 @@
> * Copyright 31 August 2008 James Bottomley
> * Copyright (C) 2013, Intel Corporation
> */
> +#include <linux/bug.h>
> #include <linux/kernel.h>
> #include <linux/math64.h>
> #include <linux/export.h>
> @@ -14,7 +15,8 @@
>
> /**
> * string_get_size - get the size in the specified units
> - * @size: The size to be converted
> + * @size: The size to be converted in blocks
> + * @blk_size: Size of the block (use 1 for size in bytes)
> * @units: units to use (powers of 1000 or 1024)
> * @buf: buffer to format to
> * @len: length of buffer
> @@ -24,14 +26,14 @@
> * at least 9 bytes and will always be zero terminated.
> *
> */
> -void string_get_size(u64 size, const enum string_size_units units,
> +void string_get_size(u64 size, u64 blk_size, const enum string_size_units units,
> char *buf, int len)
> {
> static const char *const units_10[] = {
> - "B", "kB", "MB", "GB", "TB", "PB", "EB"
> + "B", "kB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"
> };
> static const char *const units_2[] = {
> - "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB"
> + "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"
> };
> static const char *const *const units_str[] = {
> [STRING_UNITS_10] = units_10,
> @@ -42,31 +44,58 @@ void string_get_size(u64 size, const enum string_size_units units,
> [STRING_UNITS_2] = 1024,
> };
> int i, j;
> - u32 remainder = 0, sf_cap;
> + u32 remainder = 0, sf_cap, exp;
> char tmp[8];
> + const char *unit;
>
> tmp[0] = '\0';
> i = 0;
> - if (size >= divisor[units]) {
> - while (size >= divisor[units]) {
> - remainder = do_div(size, divisor[units]);
> - i++;
> - }
> + if (!size)
> + goto out;
>
> - sf_cap = size;
> - for (j = 0; sf_cap*10 < 1000; j++)
> - sf_cap *= 10;
> + while (blk_size >= divisor[units]) {
> + remainder = do_div(blk_size, divisor[units]);
> + i++;
> + }
>
> - if (j) {
> - remainder *= 1000;
> - remainder /= divisor[units];
> - snprintf(tmp, sizeof(tmp), ".%03u", remainder);
> - tmp[j+1] = '\0';
> - }
> + exp = divisor[units];
> + do_div(exp, blk_size);
> + if (size >= exp) {
> + remainder = do_div(size, divisor[units]);
> + remainder *= blk_size;
> + i++;
> + } else {
> + remainder *= size;
> + }
> +
> + size *= blk_size;
> + size += (remainder/divisor[units]);
> + remainder %= divisor[units];
> +
> + while (size >= divisor[units]) {
> + remainder = do_div(size, divisor[units]);
> + i++;
> }
>
> + sf_cap = size;
> + for (j = 0; sf_cap*10 < 1000; j++)
> + sf_cap *= 10;
> +
> + if (j) {
> + remainder *= 1000;
> + remainder /= divisor[units];
> + snprintf(tmp, sizeof(tmp), ".%03u", remainder);
> + tmp[j+1] = '\0';
> + }
> +
> + out:
> + if (i >= ARRAY_SIZE(units_2))
> + unit = "UNK";
> + else
> + unit = units_str[units][i];
> +
> snprintf(buf, len, "%u%s %s", (u32)size,
> - tmp, units_str[units][i]);
> + tmp, unit);
> }
> EXPORT_SYMBOL(string_get_size);
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-03-13 10:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-06 2:47 [PATCH] sd, mmc, virtio_blk, string_helpers: fix block size units James Bottomley
2015-03-13 10:07 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox