The capacity printk'd in bytes is divided by 1000000,
whereas 1048576 would be more consistent with the rest
of the OS and disk-related utilities ('df' etc.).
This change replaces the (sz - (sz/625 - 974))/1950
calculation with a simple right shift by 11 bits
(/2048).
Signed-off-by: Simon Arlott <[email protected]>
---
drivers/scsi/sd.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e5e7d78..e6fd6fd 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1441,10 +1441,8 @@ got_data:
sector_t mb = sz;
blk_queue_hardsect_size(queue, hard_sector);
- /* avoid 64-bit division on 32-bit platforms */
- sector_div(sz, 625);
- mb -= sz - 974;
- sector_div(mb, 1950);
+ /* Convert to megabytes (/2048) */
+ mb = sz >> 11;
sd_printk(KERN_NOTICE, sdkp,
"%llu %d-byte hardware sectors (%llu MB)\n",
--
1.5.6.5
--
Simon Arlott
linux-scsi is the correct list for this, cc's added
On Sat, 2008-08-30 at 15:08 +0100, Simon Arlott wrote:
> The capacity printk'd in bytes is divided by 1000000,
> whereas 1048576 would be more consistent with the rest
> of the OS and disk-related utilities ('df' etc.).
>
> This change replaces the (sz - (sz/625 - 974))/1950
> calculation with a simple right shift by 11 bits
> (/2048).
>
> Signed-off-by: Simon Arlott <[email protected]>
> ---
> drivers/scsi/sd.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index e5e7d78..e6fd6fd 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1441,10 +1441,8 @@ got_data:
> sector_t mb = sz;
>
> blk_queue_hardsect_size(queue, hard_sector);
> - /* avoid 64-bit division on 32-bit platforms */
> - sector_div(sz, 625);
> - mb -= sz - 974;
> - sector_div(mb, 1950);
> + /* Convert to megabytes (/2048) */
> + mb = sz >> 11;
>
> sd_printk(KERN_NOTICE, sdkp,
> "%llu %d-byte hardware sectors (%llu MB)\n",
No, this is wrong. By mandated standards the manufacturers are allowed
to calculate MB by dividing by 10^6. This is a fiddle to allow them to
make their drives look slightly bigger. However, we want the printed
information to match that written on the drive, so in this printk, we
use the manufacturer standard for calculation (and then do everything
else in bytes so we don't have to bother with it ever again).
James
On Sat, Aug 30, 2008 at 12:24:50PM -0500, James Bottomley wrote:
> linux-scsi is the correct list for this, cc's added
>
> On Sat, 2008-08-30 at 15:08 +0100, Simon Arlott wrote:
> > The capacity printk'd in bytes is divided by 1000000,
> > whereas 1048576 would be more consistent with the rest
> > of the OS and disk-related utilities ('df' etc.).
> > - /* avoid 64-bit division on 32-bit platforms */
> > - sector_div(sz, 625);
> > - mb -= sz - 974;
> > - sector_div(mb, 1950);
> > + /* Convert to megabytes (/2048) */
> > + mb = sz >> 11;
> >
> > sd_printk(KERN_NOTICE, sdkp,
> > "%llu %d-byte hardware sectors (%llu MB)\n",
>
> No, this is wrong. By mandated standards the manufacturers are allowed
> to calculate MB by dividing by 10^6. This is a fiddle to allow them to
> make their drives look slightly bigger. However, we want the printed
> information to match that written on the drive, so in this printk, we
> use the manufacturer standard for calculation (and then do everything
> else in bytes so we don't have to bother with it ever again).
I was looking at this code recently because it looks really bizarre when
you create a half-petabyte filesystem:
$ sudo insmod drivers/ata/ata_ram.ko capacity=1099511627776 preallocate=0
[12095.028093] ata7.00: 1099511627776 sectors, multi 0: LBA48 NCQ (depth 31/32)
[12095.028093] ata7.00: configured for UDMA/133
[12095.041915] scsi 7:0:0:0: Direct-Access ATA Linux RAM Drive 0.01 PQ: 0 ANSI: 5
[12095.041915] sd 7:0:0:0: [sdc] Very big device. Trying to use READ CAPACITY(16).
[12095.041915] sd 7:0:0:0: [sdc] 1099511627776 512-byte hardware sectors (562949953 MB)
[12095.041915] sd 7:0:0:0: [sdc] Write Protect is off
[12095.041915] sd 7:0:0:0: [sdc] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
1. Avoiding 64-bit divisions is _so_ last decade. We have
linux/math64.h, we should use it.
2. We should report in GB or TB when appropriate. The exact definition
of 'appropriate' is going to vary from person to person. Might I
suggest that we should report between two and four significant digits.
eg 9543 MB is ok, 10543 MB should be 10 GB.
3. I hate myself for saying this ... but maybe we should be using the
horrific MiB/GiB/TiB instead of MB/GB/TB.
4. I've been far too busy to write said patch. Simon, would you mind
doing the honours?
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Sat, 30 Aug 2008 11:45:16 -0600
Matthew Wilcox <[email protected]> wrote:
>
> 2. We should report in GB or TB when appropriate. The exact definition
> of 'appropriate' is going to vary from person to person. Might I
> suggest that we should report between two and four significant digits.
> eg 9543 MB is ok, 10543 MB should be 10 GB.
>
I've been looking at doing something like this for the mmc_block
driver, so a generic helper is welcome. :)
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On 30/08/08 18:45, Matthew Wilcox wrote:
> On Sat, Aug 30, 2008 at 12:24:50PM -0500, James Bottomley wrote:
>> No, this is wrong. By mandated standards the manufacturers are allowed
>> to calculate MB by dividing by 10^6. This is a fiddle to allow them to
>> make their drives look slightly bigger. However, we want the printed
>> information to match that written on the drive, so in this printk, we
>> use the manufacturer standard for calculation (and then do everything
>> else in bytes so we don't have to bother with it ever again).
It's unlikely to match what's on the drive, "1000204886016" isn't 1TB
by any standard.
> I was looking at this code recently because it looks really bizarre when
> you create a half-petabyte filesystem:
>
> $ sudo insmod drivers/ata/ata_ram.ko capacity=1099511627776 preallocate=0
>
> [12095.028093] ata7.00: 1099511627776 sectors, multi 0: LBA48 NCQ (depth 31/32)
> [12095.028093] ata7.00: configured for UDMA/133
> [12095.041915] scsi 7:0:0:0: Direct-Access ATA Linux RAM Drive 0.01 PQ: 0 ANSI: 5
> [12095.041915] sd 7:0:0:0: [sdc] Very big device. Trying to use READ CAPACITY(16).
> [12095.041915] sd 7:0:0:0: [sdc] 1099511627776 512-byte hardware sectors (562949953 MB)
> [12095.041915] sd 7:0:0:0: [sdc] Write Protect is off
> [12095.041915] sd 7:0:0:0: [sdc] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
This looks useful for testing this... do you have an updated version?
> 1. Avoiding 64-bit divisions is _so_ last decade. We have
> linux/math64.h, we should use it.
>
> 2. We should report in GB or TB when appropriate. The exact definition
> of 'appropriate' is going to vary from person to person. Might I
> suggest that we should report between two and four significant digits.
> eg 9543 MB is ok, 10543 MB should be 10 GB.
I've gone with five digits, it switches to GB at ~98GB, and to TB
at ~98TB etc.
> 3. I hate myself for saying this ... but maybe we should be using the
> horrific MiB/GiB/TiB instead of MB/GB/TB.
Somehow this stuff got into net-tools (ifconfig) too, so I have a
patch to remove it from my systems.
> 4. I've been far too busy to write said patch. Simon, would you mind
> doing the honours?
Sure, patch will follow this email... it can only go as far as 8192EB
and then there's not enough space to store more than 2^64 512-byte
sectors.
(And if you only modify drivers/scsi/sd.c, the kernel make system
won't recompile sd.o!)
--
Simon Arlott
The capacity printk'd in bytes is divided by 1000000,
whereas 1048576 would be more consistent with the rest
of the OS and disk-related utilities ('df' etc.).
This change replaces the (sz - (sz/625 - 974))/1950
calculation with a simple right shift to output with
five significant digits the capacity in KB, MB, GB, TB,
PB, or EB. Anything beyond this becomes too large...
---
drivers/scsi/sd.c | 62 ++++++++++++++++++++++++++++++++++------------------
1 files changed, 40 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e5e7d78..c47a071 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1290,6 +1290,8 @@ sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer)
struct scsi_sense_hdr sshdr;
int sense_valid = 0;
struct scsi_device *sdp = sdkp->device;
+ unsigned long long orig_capacity, sz;
+ char *units;
repeat:
retries = 3;
@@ -1429,30 +1431,16 @@ got_data:
*/
sector_size = 512;
}
- {
- /*
- * The msdos fs needs to know the hardware sector size
- * So I have created this table. See ll_rw_blk.c
- * Jacques Gelinas ([email protected])
- */
- int hard_sector = sector_size;
- sector_t sz = (sdkp->capacity/2) * (hard_sector/256);
- struct request_queue *queue = sdp->request_queue;
- sector_t mb = sz;
-
- blk_queue_hardsect_size(queue, hard_sector);
- /* avoid 64-bit division on 32-bit platforms */
- sector_div(sz, 625);
- mb -= sz - 974;
- sector_div(mb, 1950);
- sd_printk(KERN_NOTICE, sdkp,
- "%llu %d-byte hardware sectors (%llu MB)\n",
- (unsigned long long)sdkp->capacity,
- hard_sector, (unsigned long long)mb);
- }
+ /*
+ * The msdos fs needs to know the hardware sector size
+ * So I have created this table. See ll_rw_blk.c
+ * Jacques Gelinas ([email protected])
+ */
+ blk_queue_hardsect_size(sdp->request_queue, sector_size);
/* Rescale capacity to 512-byte units */
+ orig_capacity = sdkp->capacity;
if (sector_size == 4096)
sdkp->capacity <<= 3;
else if (sector_size == 2048)
@@ -1463,6 +1451,36 @@ got_data:
sdkp->capacity >>= 1;
sdkp->device->sector_size = sector_size;
+
+ /* Output device size with at most 5 digits,
+ * this assumes sz is based on a 512-byte
+ * sector size.
+ */
+ sz = sdkp->capacity;
+ if (sz >= (unsigned long long)100000*((unsigned long long)2<<40)) {
+ // 100000PB
+ sz >>= 51;
+ units = "EB";
+ } else if (sz >= (unsigned long long)100000*(2<<30)) { // 100000TB
+ sz >>= 41;
+ units = "PB";
+ } else if (sz >= (unsigned long long)100000*(2<<20)) { // 100000GB
+ sz >>= 31;
+ units = "TB";
+ } else if (sz >= 100000*(2<<10)) { // 100000MB
+ sz >>= 21;
+ units = "GB";
+ } else if (sz >= 100000*2) { // 100000KB
+ sz >>= 11;
+ units = "MB";
+ } else {
+ sz >>= 1;
+ units = "KB";
+ }
+ sd_printk(KERN_NOTICE, sdkp,
+ "%llu %d-byte hardware sectors (%llu %s)\n",
+ (unsigned long long)orig_capacity, sector_size,
+ (unsigned long long)sz, units);
}
/* called with buffer of length 512 */
--
1.5.6.5
--
Simon Arlott
On Sat, 2008-08-30 at 22:59 +0200, Pierre Ossman wrote:
> On Sat, 30 Aug 2008 11:45:16 -0600
> Matthew Wilcox <[email protected]> wrote:
>
> >
> > 2. We should report in GB or TB when appropriate. The exact definition
> > of 'appropriate' is going to vary from person to person. Might I
> > suggest that we should report between two and four significant digits.
> > eg 9543 MB is ok, 10543 MB should be 10 GB.
> >
>
> I've been looking at doing something like this for the mmc_block
> driver, so a generic helper is welcome. :)
Does MMC have the 10^3 requirement like SCSI/ATA disks do, or can you
use 2^10 like standard computer stuff for capacities?
James
On Sat, Aug 30, 2008 at 10:02:10PM +0100, Simon Arlott wrote:
> On 30/08/08 18:45, Matthew Wilcox wrote:
> > On Sat, Aug 30, 2008 at 12:24:50PM -0500, James Bottomley wrote:
> >> No, this is wrong. By mandated standards the manufacturers are allowed
> >> to calculate MB by dividing by 10^6. This is a fiddle to allow them to
> >> make their drives look slightly bigger. However, we want the printed
> >> information to match that written on the drive, so in this printk, we
> >> use the manufacturer standard for calculation (and then do everything
> >> else in bytes so we don't have to bother with it ever again).
>
> It's unlikely to match what's on the drive, "1000204886016" isn't 1TB
> by any standard.
Hm. I bought a 500GB drive last year:
sd 1:0:0:0: [sda] 976773168 512-byte hardware sectors (500108 MB)
512 * 976773168
500107862016
512 * 976773168 / (1024 * 1024 * 1024)
465.76174163818359375000
If we report it as 465GB, we will get questions. Even pretending it's
1024 * 1000 * 1000 doesn't work:
512 * 976773168 / (1000 * 1000 * 1024)
488.38658400000000000000
I think we have to stick with dividing by multiples of 1000. It's what
the drive manufacturers do (and I do understand their reasons for doing
it).
> This looks useful for testing this... do you have an updated version?
Yes.
http://git.kernel.org/?p=linux/kernel/git/willy/ata.git;a=shortlog;h=ata-ram
> > 2. We should report in GB or TB when appropriate. The exact definition
> > of 'appropriate' is going to vary from person to person. Might I
> > suggest that we should report between two and four significant digits.
> > eg 9543 MB is ok, 10543 MB should be 10 GB.
>
> I've gone with five digits, it switches to GB at ~98GB, and to TB
> at ~98TB etc.
Reasonable minds can certainly disagree on this one. I respectfully
submit that reporting a 97415MB capacity is less useful than reporting a
97GB capacity. If you look at drive advertisements, they sell 1TB,
1.5TB, 80GB, 750GB, 360GB, ... we should be trying to match that. I'm a
little dubious about trying to match the 1.5TB; I think 1500GB is close
enough, but a 50GB drive shouldn't be reported as 50000MB. IMO, anyway.
> > 3. I hate myself for saying this ... but maybe we should be using the
> > horrific MiB/GiB/TiB instead of MB/GB/TB.
>
> Somehow this stuff got into net-tools (ifconfig) too, so I have a
> patch to remove it from my systems.
I understand why networking tools are particularly cautious about this.
The line rate (eg 1Gbps) is 1000,000,000 bps, but the amount of traffic
reported might well use either binary SI or decimal SI. Reporting the
wrong one makes a 7% difference at the GB/GiB level, and that's the kind
of amount that people can spend a week or more chasing.
> > 4. I've been far too busy to write said patch. Simon, would you mind
> > doing the honours?
>
> Sure, patch will follow this email... it can only go as far as 8192EB
> and then there's not enough space to store more than 2^64 512-byte
> sectors.
I think it'll be a while before we get drives of that capacity. ATA is
limited to 48 bits for the number of sectors, and while you can increase
the sector size (4k is currently planned), that still doesn't bring you
close. I suppose you could get ata_ram to have multiple drives and
raid-0 them together, but you'd still need to allocate 2^13 of them.
scsi_debug can probably go to the full 2^64 sectors. I haven't looked
into it.
> (And if you only modify drivers/scsi/sd.c, the kernel make system
> won't recompile sd.o!)
That sounds odd to me; what command are you using to rebuild?
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Sat, 30 Aug 2008 16:45:49 -0500
James Bottomley <[email protected]> wrote:
> On Sat, 2008-08-30 at 22:59 +0200, Pierre Ossman wrote:
> > On Sat, 30 Aug 2008 11:45:16 -0600
> > Matthew Wilcox <[email protected]> wrote:
> >
> > >
> > > 2. We should report in GB or TB when appropriate. The exact definition
> > > of 'appropriate' is going to vary from person to person. Might I
> > > suggest that we should report between two and four significant digits.
> > > eg 9543 MB is ok, 10543 MB should be 10 GB.
> > >
> >
> > I've been looking at doing something like this for the mmc_block
> > driver, so a generic helper is welcome. :)
>
> Does MMC have the 10^3 requirement like SCSI/ATA disks do, or can you
> use 2^10 like standard computer stuff for capacities?
>
Some do, some don't. I prefer the 2^10 and MiB variants as it avoids
ambiguity.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.
On 30/08/08 22:57, Matthew Wilcox wrote:
> On Sat, Aug 30, 2008 at 10:02:10PM +0100, Simon Arlott wrote:
>> On 30/08/08 18:45, Matthew Wilcox wrote:
>> > On Sat, Aug 30, 2008 at 12:24:50PM -0500, James Bottomley wrote:
>> >> No, this is wrong. By mandated standards the manufacturers are allowed
>> >> to calculate MB by dividing by 10^6. This is a fiddle to allow them to
>> >> make their drives look slightly bigger. However, we want the printed
>> >> information to match that written on the drive, so in this printk, we
>> >> use the manufacturer standard for calculation (and then do everything
>> >> else in bytes so we don't have to bother with it ever again).
>>
>> It's unlikely to match what's on the drive, "1000204886016" isn't 1TB
>> by any standard.
>
> Hm. I bought a 500GB drive last year:
>
> sd 1:0:0:0: [sda] 976773168 512-byte hardware sectors (500108 MB)
>
> 512 * 976773168
> 500107862016
>
> 512 * 976773168 / (1024 * 1024 * 1024)
> 465.76174163818359375000
>
> If we report it as 465GB, we will get questions. Even pretending it's
> 1024 * 1000 * 1000 doesn't work:
>
> 512 * 976773168 / (1000 * 1000 * 1024)
> 488.38658400000000000000
>
> I think we have to stick with dividing by multiples of 1000. It's what
> the drive manufacturers do (and I do understand their reasons for doing
> it).
>
I disagree. The difference between advertised and actual capacity is
only going to get worse when drive capacity increases further.
e.g. "4TB" will only be 3.6TB.
Who is going to be asking these questions about the kernel output, but
not about whatever else reports 465GB, from 'df' to a GUI showing disk
capacity? This actually makes the kernel more consistent with everything
else.
>> This looks useful for testing this... do you have an updated version?
>
> Yes.
> http://git.kernel.org/?p=linux/kernel/git/willy/ata.git;a=shortlog;h=ata-ram
>
>> > 2. We should report in GB or TB when appropriate. The exact definition
>> > of 'appropriate' is going to vary from person to person. Might I
>> > suggest that we should report between two and four significant digits.
>> > eg 9543 MB is ok, 10543 MB should be 10 GB.
>>
>> I've gone with five digits, it switches to GB at ~98GB, and to TB
>> at ~98TB etc.
>
> Reasonable minds can certainly disagree on this one. I respectfully
> submit that reporting a 97415MB capacity is less useful than reporting a
> 97GB capacity. If you look at drive advertisements, they sell 1TB,
> 1.5TB, 80GB, 750GB, 360GB, ... we should be trying to match that. I'm a
> little dubious about trying to match the 1.5TB; I think 1500GB is close
> enough, but a 50GB drive shouldn't be reported as 50000MB. IMO, anyway.
This really depends on whether or not you're going for matching advertised
capacity. I think the extra digit avoids losing precision too early.
If you're intending to divide by 1000 you may as well determined what the
advertised capacity would be and handle .5 xB (or even .25, .75, .1
through .9).
>> > 3. I hate myself for saying this ... but maybe we should be using the
>> > horrific MiB/GiB/TiB instead of MB/GB/TB.
>>
>> Somehow this stuff got into net-tools (ifconfig) too, so I have a
>> patch to remove it from my systems.
>
> I understand why networking tools are particularly cautious about this.
> The line rate (eg 1Gbps) is 1000,000,000 bps, but the amount of traffic
> reported might well use either binary SI or decimal SI. Reporting the
> wrong one makes a 7% difference at the GB/GiB level, and that's the kind
> of amount that people can spend a week or more chasing.
It's not actually a useful value... you'd need to use the byte value, which
is also displayed, to monitor actual usage over time.
>> > 4. I've been far too busy to write said patch. Simon, would you mind
>> > doing the honours?
>>
>> Sure, patch will follow this email... it can only go as far as 8192EB
>> and then there's not enough space to store more than 2^64 512-byte
>> sectors.
>
> I think it'll be a while before we get drives of that capacity. ATA is
> limited to 48 bits for the number of sectors, and while you can increase
> the sector size (4k is currently planned), that still doesn't bring you
> close. I suppose you could get ata_ram to have multiple drives and
> raid-0 them together, but you'd still need to allocate 2^13 of them.
The sector size can't currently be increased beyond 512 to get around this
because it's scaled to 512 to store capacity. (Which my patch then uses to
avoid computing the unit splits at runtime.)
> scsi_debug can probably go to the full 2^64 sectors. I haven't looked
> into it.
>
>> (And if you only modify drivers/scsi/sd.c, the kernel make system
>> won't recompile sd.o!)
>
> That sounds odd to me; what command are you using to rebuild?
>
Maybe I was imagining it then... it appeared to be doing that at times
while I found all the possible ways to mess up calculating 100000xB
and having "0 EB" devices each time.
--
Simon Arlott
On 30/08/08 23:13, Pierre Ossman wrote:
> On Sat, 30 Aug 2008 16:45:49 -0500
> James Bottomley <[email protected]> wrote:
>
>> On Sat, 2008-08-30 at 22:59 +0200, Pierre Ossman wrote:
>> > On Sat, 30 Aug 2008 11:45:16 -0600
>> > Matthew Wilcox <[email protected]> wrote:
>> >
>> > >
>> > > 2. We should report in GB or TB when appropriate. The exact definition
>> > > of 'appropriate' is going to vary from person to person. Might I
>> > > suggest that we should report between two and four significant digits.
>> > > eg 9543 MB is ok, 10543 MB should be 10 GB.
>> > >
>> >
>> > I've been looking at doing something like this for the mmc_block
>> > driver, so a generic helper is welcome. :)
>>
>> Does MMC have the 10^3 requirement like SCSI/ATA disks do, or can you
>> use 2^10 like standard computer stuff for capacities?
>>
>
> Some do, some don't. I prefer the 2^10 and MiB variants as it avoids
> ambiguity.
If there's a generic function to do this, there could be a Kconfig option
to select the variant (10^3 MB, 2^10 MiB, 2^10 MB ;)), as long as the byte
value is available from the same message.
--
Simon Arlott
On Sun, Aug 31, 2008 at 12:13:14AM +0200, Pierre Ossman wrote:
> On Sat, 30 Aug 2008 16:45:49 -0500
> James Bottomley <[email protected]> wrote:
> > Does MMC have the 10^3 requirement like SCSI/ATA disks do, or can you
> > use 2^10 like standard computer stuff for capacities?
>
> Some do, some don't. I prefer the 2^10 and MiB variants as it avoids
> ambiguity.
I think the most compelling reason for ensuring they're the same is
that you can plug these devices into both an MMC slot directly and a
card-reader and have them show up over USB. You don't want the reported
capacity to change when you do that.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Sat, 2008-08-30 at 22:03 +0100, Simon Arlott wrote:
> The capacity printk'd in bytes is divided by 1000000,
> whereas 1048576 would be more consistent with the rest
> of the OS and disk-related utilities ('df' etc.).
>
> This change replaces the (sz - (sz/625 - 974))/1950
> calculation with a simple right shift to output with
> five significant digits the capacity in KB, MB, GB, TB,
> PB, or EB. Anything beyond this becomes too large...
Well, still needs to be dividing by 1000 not 1024 for SCSI and ATA.
However, I'm afraid it needs to be a bit more sophisticated: for
instance, under these calculations, a 1.75TB disk will show up as 1TB.
Thus, I think we need to print the capacity to 3 significant figures to
cope with this case.
James
On Sat, Aug 30, 2008 at 08:59:07PM -0500, James Bottomley wrote:
> However, I'm afraid it needs to be a bit more sophisticated: for
> instance, under these calculations, a 1.75TB disk will show up as 1TB.
> Thus, I think we need to print the capacity to 3 significant figures to
> cope with this case.
Do you have an objection to my original suggestion of 1750GB in that
case? It saves faffing around with fractions and it's unlikely to
confuse the user.
BTW, I do appreciate Simon's point about df showing a different number.
How about we print:
sd 7:0:0:0: [sdc] 1099511627776 512-byte hardware sectors (563TB / 512TiB)
(or perhaps a more realistic number ...)
sd 7:0:0:0: [sdc] 976562500000 512-byte hardware sectors (500TB / 455TiB)
It's perhaps a more gentle way of informing our users that they may not
have quite as much capacity as they thought they had.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
Matthew Wilcox wrote:
> Reasonable minds can certainly disagree on this one. I respectfully
> submit that reporting a 97415MB capacity is less useful than reporting a
> 97GB capacity. If you look at drive advertisements, they sell 1TB,
> 1.5TB, 80GB, 750GB, 360GB, ... we should be trying to match that. I'm a
> little dubious about trying to match the 1.5TB; I think 1500GB is close
> enough, but a 50GB drive shouldn't be reported as 50000MB. IMO, anyway.
Since when did techies start paying attention to marketing statements ?
We should be doing what's natural and *consistent*, which is typically
dealing with power-of-2. Saying it's one thing at one level, and when
the natural use (how many 512 byte sectors get added up later) changes
that number in a different level, you've created even more confusion.
There's no consistency.
As far as user concern - they've seen this discrepancy in the PC/Windows
world for years now... Why should we be taking on the task to solve or
answer it now ? Throw in different overheads for filesystem metadata
loss, volume manager metadata, raid level loss, etc - you'll never be
able to explain it all to the user. And personally, I'd rather have
natural numbers so that if I do understand the uses, I can do
calculations without doing number-base conversions.
-- james s
Hi Matthew,
On Sunday 31 August 2008, Matthew Wilcox wrote:
> BTW, I do appreciate Simon's point about df showing a different number.
> How about we print:
>
> sd 7:0:0:0: [sdc] 1099511627776 512-byte hardware sectors (563TB / 512TiB)
>
> (or perhaps a more realistic number ...)
>
> sd 7:0:0:0: [sdc] 976562500000 512-byte hardware sectors (500TB / 455TiB)
>
> It's perhaps a more gentle way of informing our users that they may not
> have quite as much capacity as they thought they had.
Great idea! As a user/admin I would find this the best one of all.
1. All variants given.
2. Correct scientific units used.
I would ACK that one, if you provide a small helper for that printout
somehwere in block/{genhd,blk-core}.c
Best Regards
Ingo Oeser
On 31/08/08 15:25, Ingo Oeser wrote:
> Hi Matthew,
>
> On Sunday 31 August 2008, Matthew Wilcox wrote:
>> BTW, I do appreciate Simon's point about df showing a different number.
>> How about we print:
>>
>> sd 7:0:0:0: [sdc] 1099511627776 512-byte hardware sectors (563TB / 512TiB)
>>
>> (or perhaps a more realistic number ...)
>>
>> sd 7:0:0:0: [sdc] 976562500000 512-byte hardware sectors (500TB / 455TiB)
>>
>> It's perhaps a more gentle way of informing our users that they may not
>> have quite as much capacity as they thought they had.
>
> Great idea! As a user/admin I would find this the best one of all.
>
> 1. All variants given.
> 2. Correct scientific units used.
I oppose all the "iB" forms. Has anyone tried pronouncing these?
Pee-bee-bytes?
Alternatively we could just remove the part in brackets from the message...
--
Simon Arlott
On Sat, 2008-08-30 at 20:54 -0600, Matthew Wilcox wrote:
> On Sat, Aug 30, 2008 at 08:59:07PM -0500, James Bottomley wrote:
> > However, I'm afraid it needs to be a bit more sophisticated: for
> > instance, under these calculations, a 1.75TB disk will show up as 1TB.
> > Thus, I think we need to print the capacity to 3 significant figures to
> > cope with this case.
>
> Do you have an objection to my original suggestion of 1750GB in that
> case? It saves faffing around with fractions and it's unlikely to
> confuse the user.
>
> BTW, I do appreciate Simon's point about df showing a different number.
> How about we print:
>
> sd 7:0:0:0: [sdc] 1099511627776 512-byte hardware sectors (563TB / 512TiB)
>
> (or perhaps a more realistic number ...)
>
> sd 7:0:0:0: [sdc] 976562500000 512-byte hardware sectors (500TB / 455TiB)
>
> It's perhaps a more gentle way of informing our users that they may not
> have quite as much capacity as they thought they had.
OK, uncle. We're wasting far more time on this email thread than it
would take to code the damn thing. So, here it is as a generic helper:
both forms of calculation correctly to 3sf.
James
On 31/08/08 02:59, James Bottomley wrote:
> On Sat, 2008-08-30 at 22:03 +0100, Simon Arlott wrote:
>> The capacity printk'd in bytes is divided by 1000000,
>> whereas 1048576 would be more consistent with the rest
>> of the OS and disk-related utilities ('df' etc.).
>>
>> This change replaces the (sz - (sz/625 - 974))/1950
>> calculation with a simple right shift to output with
>> five significant digits the capacity in KB, MB, GB, TB,
>> PB, or EB. Anything beyond this becomes too large...
>
> Well, still needs to be dividing by 1000 not 1024 for SCSI and ATA.
> However, I'm afraid it needs to be a bit more sophisticated: for
> instance, under these calculations, a 1.75TB disk will show up as 1TB.
> Thus, I think we need to print the capacity to 3 significant figures to
> cope with this case.
Actually it'll show up as 1629GB, as my patch shows up to 5 digits
(not five significant digits, which would require outputting a
non-integer value).
Isn't outputting "1.75" unnecessarily complicated, and "1750" would
work better?
--
Simon Arlott
This patch adds the ability to print sizes in either units of 10^3 (SI)
or 2^10 (Binary) units. It rounds up to three significant figures and
can be used for either memory or storage capacities.
Oh, and I'm fully aware that 64 bits is only 16EiB ... the Zetta and
Yotta units are added for future proofing against the day we have 128
bit computers ...
Signed-off-by: James Bottomley <[email protected]>
---
include/linux/string_helpers.h | 10 ++++++
lib/Makefile | 3 +-
lib/string_helpers.c | 62 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+), 1 deletions(-)
create mode 100644 include/linux/string_helpers.h
create mode 100644 lib/string_helpers.c
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
new file mode 100644
index 0000000..6b827fb
--- /dev/null
+++ b/include/linux/string_helpers.h
@@ -0,0 +1,10 @@
+#include <linux/types.h>
+
+enum string_size_units {
+ STRING_UNITS_10,
+ STRING_UNITS_2,
+};
+
+int string_get_size(u64 size, enum string_size_units units,
+ char *buf, int len);
+
diff --git a/lib/Makefile b/lib/Makefile
index 3b1f94b..44001af 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -19,7 +19,8 @@ lib-$(CONFIG_SMP) += cpumask.o
lib-y += kobject.o kref.o klist.o
obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
- bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o
+ bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
+ string_helpers.o
ifeq ($(CONFIG_DEBUG_KOBJECT),y)
CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
new file mode 100644
index 0000000..3675eaa
--- /dev/null
+++ b/lib/string_helpers.c
@@ -0,0 +1,62 @@
+/*
+ * Helpers for formatting and printing strings
+ *
+ * Copyright 31 August 2008 James Bottomley
+ */
+#include <linux/kernel.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/string_helpers.h>
+
+/**
+ * string_get_size - get the size in the specified units
+ * @size: The size to be converted
+ * @units: units to use (powers of 1000 or 1024)
+ * @buf: buffer to format to
+ * @len: length of buffer
+ *
+ * This function returns a string formatted to 3 significant figures
+ * giving the size in the required units. Returns 0 on success or
+ * error on failure. @buf is always zero terminated.
+ *
+ */
+int string_get_size(u64 size, const enum string_size_units units,
+ char *buf, int len)
+{
+ const char *units_10[] = { "B", "KB", "MB", "GB", "TB", "PB",
+ "EB", "ZB", "YB", NULL};
+ const char *units_2[] = {"B", "Kib", "MiB", "GiB", "TiB", "PiB",
+ "EiB", "ZiB", "YiB", NULL };
+ const char **units_str[] = {
+ [STRING_UNITS_10] = units_10,
+ [STRING_UNITS_2] = units_2,
+ };
+ const int divisor[] = {
+ [STRING_UNITS_10] = 1000,
+ [STRING_UNITS_2] = 1024,
+ };
+ int i,j;
+ u64 remainder = 0, sf_cap;
+ char tmp[8];
+
+ tmp[0] = '\0';
+
+ for (i = 0; size > divisor[units] && units_str[units][i]; i++)
+ remainder = do_div(size, divisor[units]);
+
+ sf_cap = size;
+ for (j = 0; sf_cap*10 < 1000; j++)
+ sf_cap *= 10;
+
+ if (j) {
+ remainder *= 1000;
+ do_div(remainder, divisor[units]);
+ snprintf(tmp, sizeof(tmp), ".%03lld", remainder);
+ tmp[j+1] = '\0';
+ }
+
+ snprintf(buf, len, "%lld%s%s", size, tmp, units_str[units][i]);
+
+ return 0;
+}
+EXPORT_SYMBOL(string_get_size);
--
1.5.6.3
James
---
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e5e7d78..a9f919e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -47,6 +47,7 @@
#include <linux/blkpg.h>
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/string_helpers.h>
#include <asm/uaccess.h>
#include <scsi/scsi.h>
@@ -1429,29 +1430,8 @@ got_data:
*/
sector_size = 512;
}
- {
- /*
- * The msdos fs needs to know the hardware sector size
- * So I have created this table. See ll_rw_blk.c
- * Jacques Gelinas ([email protected])
- */
- int hard_sector = sector_size;
- sector_t sz = (sdkp->capacity/2) * (hard_sector/256);
- struct request_queue *queue = sdp->request_queue;
- sector_t mb = sz;
-
- blk_queue_hardsect_size(queue, hard_sector);
- /* avoid 64-bit division on 32-bit platforms */
- sector_div(sz, 625);
- mb -= sz - 974;
- sector_div(mb, 1950);
-
- sd_printk(KERN_NOTICE, sdkp,
- "%llu %d-byte hardware sectors (%llu MB)\n",
- (unsigned long long)sdkp->capacity,
- hard_sector, (unsigned long long)mb);
- }
-
+ blk_queue_hardsect_size(sdp->request_queue, sector_size);
+
/* Rescale capacity to 512-byte units */
if (sector_size == 4096)
sdkp->capacity <<= 3;
@@ -1463,6 +1443,21 @@ got_data:
sdkp->capacity >>= 1;
sdkp->device->sector_size = sector_size;
+ {
+ char cap_str_2[10], cap_str_10[10];
+ u64 sz = sdkp->capacity << 9;
+
+ string_get_size(sz, STRING_UNITS_2, cap_str_2,
+ sizeof(cap_str_2));
+ string_get_size(sz, STRING_UNITS_10, cap_str_10,
+ sizeof(cap_str_10));
+
+ sd_printk(KERN_NOTICE, sdkp,
+ "%llu %d-byte hardware sectors: (%s/%s)\n",
+ (unsigned long long)sdkp->capacity,
+ sector_size, cap_str_10, cap_str_2);
+ }
+
}
/* called with buffer of length 512 */
On 31/08/08 16:13, James Bottomley wrote:
> This patch adds the ability to print sizes in either units of 10^3 (SI)
> or 2^10 (Binary) units. It rounds up to three significant figures and
> can be used for either memory or storage capacities.
> + const char *units_2[] = {"B", "Kib", "MiB", "GiB", "TiB", "PiB",
I think this should have been "KiB".
I'd prefer an option to display these without the "i"...
Oh and you need to store the original capacity before it's re-scaled to
512 bytes. Outputting sdkp->capacity at that stage it'll be a count of
512-byte sectors.
--
Simon Arlott
On Sun, 2008-08-31 at 16:20 +0100, Simon Arlott wrote:
> On 31/08/08 16:13, James Bottomley wrote:
> > This patch adds the ability to print sizes in either units of 10^3 (SI)
> > or 2^10 (Binary) units. It rounds up to three significant figures and
> > can be used for either memory or storage capacities.
>
> > + const char *units_2[] = {"B", "Kib", "MiB", "GiB", "TiB", "PiB",
>
> I think this should have been "KiB".
Yes, shift slip.
> I'd prefer an option to display these without the "i"...
If anyone ever finds a use for that, possibly ... but with the i is the
NIST way now, so we follow the standards.
> Oh and you need to store the original capacity before it's re-scaled to
> 512 bytes. Outputting sdkp->capacity at that stage it'll be a count of
> 512-byte sectors.
Yes, just trying to avoid a multiplication. I should have just used ffz
instead.
James
---
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e5e7d78..ef1c06c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -47,6 +47,7 @@
#include <linux/blkpg.h>
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/string_helpers.h>
#include <asm/uaccess.h>
#include <scsi/scsi.h>
@@ -1429,27 +1430,21 @@ got_data:
*/
sector_size = 512;
}
+ blk_queue_hardsect_size(sdp->request_queue, sector_size);
+
{
- /*
- * The msdos fs needs to know the hardware sector size
- * So I have created this table. See ll_rw_blk.c
- * Jacques Gelinas ([email protected])
- */
- int hard_sector = sector_size;
- sector_t sz = (sdkp->capacity/2) * (hard_sector/256);
- struct request_queue *queue = sdp->request_queue;
- sector_t mb = sz;
+ char cap_str_2[10], cap_str_10[10];
+ u64 sz = sdkp->capacity << ffz(~sector_size);
- blk_queue_hardsect_size(queue, hard_sector);
- /* avoid 64-bit division on 32-bit platforms */
- sector_div(sz, 625);
- mb -= sz - 974;
- sector_div(mb, 1950);
+ string_get_size(sz, STRING_UNITS_2, cap_str_2,
+ sizeof(cap_str_2));
+ string_get_size(sz, STRING_UNITS_10, cap_str_10,
+ sizeof(cap_str_10));
sd_printk(KERN_NOTICE, sdkp,
- "%llu %d-byte hardware sectors (%llu MB)\n",
+ "%llu %d-byte hardware sectors: (%s/%s)\n",
(unsigned long long)sdkp->capacity,
- hard_sector, (unsigned long long)mb);
+ sector_size, cap_str_10, cap_str_2);
}
/* Rescale capacity to 512-byte units */
On Sun, Aug 31, 2008 at 10:13:54AM -0500, James Bottomley wrote:
> +int string_get_size(u64 size, const enum string_size_units units,
> + char *buf, int len)
> +{
> + const char *units_10[] = { "B", "KB", "MB", "GB", "TB", "PB",
> + "EB", "ZB", "YB", NULL};
> + const char *units_2[] = {"B", "Kib", "MiB", "GiB", "TiB", "PiB",
Typo for KiB?
Should this be another %p extension instead? %pB and %piB perhaps?
That would seem easier for the callers than futzing around with managing
their own string buffers.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
From: Pierre Ossman <[email protected]>
Signed-off-by: Pierre Ossman <[email protected]>
---
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 62a4c91..dad8edb 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -29,6 +29,7 @@
#include <linux/blkdev.h>
#include <linux/mutex.h>
#include <linux/scatterlist.h>
+#include <linux/string_helpers.h>
#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
@@ -496,6 +497,8 @@ static int mmc_blk_probe(struct mmc_card *card)
struct mmc_blk_data *md;
int err;
+ char cap_str[10];
+
/*
* Check that the card supports the command class(es) we need.
*/
@@ -510,10 +513,11 @@ static int mmc_blk_probe(struct mmc_card *card)
if (err)
goto out;
- printk(KERN_INFO "%s: %s %s %lluKiB %s\n",
+ string_get_size(get_capacity(md->disk) << 9, STRING_UNITS_2,
+ cap_str, sizeof(cap_str));
+ printk(KERN_INFO "%s: %s %s %s %s\n",
md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
- (unsigned long long)(get_capacity(md->disk) >> 1),
- md->read_only ? "(ro)" : "");
+ cap_str, md->read_only ? "(ro)" : "");
mmc_set_drvdata(card, md);
add_disk(md->disk);
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
WARNING: This correspondence is being monitored by the
Swedish government. Make sure your server uses encryption
for SMTP traffic and consider using PGP for end-to-end
encryption.