scsi/sd: Fix size output in MB
Subsystems (4)
PATCH 1/1 scsi/sd: Fix size output in MB
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).
--- drivers/scsi/sd.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/sd.c +2 -4
@@ -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",
Signature · 4 lines
1.5.6.5 -- Simon Arlott
James Bottomley
maintainerlinux-scsi is the correct list for this, cc's added
Quoted 20 lines · 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 by 11 bits (/2048).
Signed-off-by Simon Arlott <[email protected]>--- drivers/scsi/sd.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)drivers/scsi/sd.c +0
-0@@ -1441,10 +1441,8 @@ got_data:sector_t mb = sz;
drivers/scsi/sd.c got_data: · line 1448
Quoted 10 lines
drivers/scsi/sd.c +2
-4blk_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
Quoted 14 lines · James Bottomley
linux-scsi is the correct list for this, cc's added
Quoted 11 lines · 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.).
drivers/scsi/sd.c +2
-4-/* 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 ;
Quoted 9 lines
drivers/scsi/sd.c got_data: · line 1448
Quoted 2 lines
drivers/scsi/sd.c +0
-0sd_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?
Signature · 4 lines
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."
Quoted 6 lines · 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. :)
Signature · 9 lines
-- 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.
James Bottomley
maintainerQuoted 4 lines · Pierre Ossman
Quoted 1 line · On Sat, 30 Aug 2008 11:45:16 -0600 Matthew Wilcox <[email protected]> wrote:
Quoted 8 lines
Quoted 5 lines
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_blockdriver, 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
Quoted 11 lines · On Sat, 30 Aug 2008 16:45:49 -0500 James Bottomley…
Quoted 10 lines · Pierre Ossman
Quoted 6 lines · 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.
Quoted 6 lines
Quoted 2 lines
I've been looking at doing something like this for the
mmc_blockdriver, 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
Signature · 9 lines
-- 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.
Quoted 17 lines · Pierre Ossman
Quoted 14 lines · On Sat, 30 Aug 2008 16:45:49 -0500 James Bottomley…
Quoted 12 lines · Pierre Ossman
Quoted 6 lines · 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_blockdriver, so a generic helper is welcome. :)
Quoted 6 lines
Quoted 3 lines
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.
Signature · 1 line
Simon Arlott
Quoted 7 lines · Pierre Ossman
Quoted 2 lines · On Sat, 30 Aug 2008 16:45:49 -0500 James Bottomley…
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.
Signature · 4 lines
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."
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · James Bottomley
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.
Quoted 5 lines
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
Quoted 7 lines
[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?
Quoted 7 lines
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.
Quoted 2 lines
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.
Quoted 2 lines
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!)
Signature · 1 line
Simon Arlott
PATCH 1/1 scsi/sd: Fix capacity output to show MB/GB/TB/...
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(-)
drivers/scsi/sd.c +39 -21
@@ -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 */
Signature · 4 lines
1.5.6.5 -- Simon Arlott
James Bottomley
maintainerRe: [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/...
Quoted 8 lines · 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...
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
Re: [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/...
Quoted 4 lines · James Bottomley
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.
Signature · 4 lines
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."
Re: [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/...
Hi Matthew,
Quoted 11 lines · Matthew Wilcox
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
Re: [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/...
Quoted 12 lines · Ingo Oeser
Hi Matthew,
Quoted 9 lines · Matthew Wilcox
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)
Quoted 7 lines
Quoted 2 lines
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...
Signature · 1 line
Simon Arlott
James Bottomley
maintainerRe: [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/...
In reply to Matthew WilcoxQuoted 13 lines · Matthew Wilcox
Quoted 4 lines · James Bottomley
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:
Quoted 8 lines
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
James Bottomley
maintainerPATCH 1/2 lib: add generic helper to print sizes rounded to the correct SI range
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 ...
--- 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
include/linux/string_helpers.h new file +10 -0
@@ -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); +
lib/Makefile +2 -1
@@ -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
lib/string_helpers.c new file +62 -0
62 lines not on this page · raw message
Signature · 1 line
1.5.6.3
Re: [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range
lib/string_helpers.c line 28
Quoted 5 lines · James Bottomley
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.
lib/string_helpers.c new file +1
-0+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.
Signature · 1 line
Simon Arlott
James Bottomley
maintainerRe: [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range
Quoted 8 lines · Simon Arlott
Review
Quoted 5 lines · James Bottomley
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.
(quoted diff) +1
-0+ const char *units_2[] = {"B", "Kib", "MiB", "GiB", "TiB", "PiB",I think this should have been "KiB".
Yes, shift slip.
Quoted 1 line
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.
Quoted 3 lines
Oh and you need to store the original capacity before it's re-scaled to 512 bytes. Outputting
sdkp->capacityat 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
---
drivers/scsi/sd.c +11 -16
@@ -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 */
Re: [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range
In reply to James Bottomleylib/string_helpers.c line 28
Quoted 6 lines · James Bottomley
lib/string_helpers.c new file +6
-0+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.
Signature · 4 lines
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."
PATCH 1/1 mmc_block: use generic helper to print capacities
In reply to James BottomleyFrom: Pierre Ossman <[email protected]>
---
drivers/mmc/card/block.c +7 -3
@@ -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);
Signature · 9 lines
-- 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.
James Bottomley
maintainerRe: [PATCH] mmc_block: use generic helper to print capacities
Quoted 3 lines · Pierre Ossman
From: Pierre Ossman <[email protected]>
Signed-off-by Pierre Ossman <[email protected]>
Did you want me to take this? It seems to depend on pieces of your mmc tree, so I'd have to carry it in my post-merge tree.
James
Re: [PATCH] mmc_block: use generic helper to print capacities
Quoted 6 lines · On Fri, 05 Sep 2008 15:09:00 -0500 James Bottomley…
Quoted 3 lines · Pierre Ossman
From: Pierre Ossman <[email protected]>
Signed-off-by Pierre Ossman <[email protected]>Did you want me to take this?
That was the idea since it needed your helper.
Quoted 3 lines
It seems to depend on pieces of your mmc tree, so I'd have to carry it in my post-merge tree.
Oh? There shouldn't be any changes around those chunks. What kind of problems are you seeing?
Rgds
Signature · 9 lines
-- 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.
James Bottomley
maintainerRe: [PATCH] mmc_block: use generic helper to print capacities
Quoted 10 lines · Pierre Ossman
Quoted 6 lines · On Fri, 05 Sep 2008 15:09:00 -0500 James Bottomley…
Quoted 3 lines · Pierre Ossman
From: Pierre Ossman <[email protected]>
Signed-off-by Pierre Ossman <[email protected]>Did you want me to take this?
Quoted 8 lines
That was the idea since it needed your helper.
Quoted 3 lines
It seems to depend on pieces of your mmc tree, so I'd have to carry it in my post-merge tree.
Oh? There shouldn't be any changes around those chunks. What kind of problems are you seeing?
Oh ... your inline attached patch is actually whitespace broken (it has spaces for tabs), which is why it doesn't apply. Sorry, that's the first thing I usually check, I just skipped it in your case. Could you resend it as an attachment?
Thanks,
James
Re: [PATCH] mmc_block: use generic helper to print capacities
Quoted 6 lines · On Fri, 05 Sep 2008 16:03:54 -0500 James Bottomley…
Oh ... your inline attached patch is actually whitespace broken (it has spaces for tabs), which is why it doesn't apply. Sorry, that's the first thing I usually check, I just skipped it in your case. Could you resend it as an attachment?
What the... It seems someone broke git-diff (or less). This one is properly undamaged:
drivers/mmc/card/block.c +7 -3
@@ -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);
Signature · 9 lines
-- 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.
Andrew Morton
maintainerRe: [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range
In reply to James BottomleyQuoted 8 lines · On Sun, 31 Aug 2008 10:13:54 -0500 James Bottomley…
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 ...
As Matthew said - a new %p thingy might be appropriate.
Quoted 15 lines
--- 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.cinclude/linux/string_helpers.h new file +0
-0
include/linux/string_helpers.h line 6
Quoted 6 lines
include/linux/string_helpers.h new file +6
-0@@ -0,0 +1,10 @@ +#include <linux/types.h> + +enum string_size_units { +STRING_UNITS_10 , +STRING_UNITS_2 , +};
Could do with some comments.
Quoted 50 lines
include/linux/string_helpers.h new file +3
-0+int string_get_size (u64 size,enum string_size_units units, +char *buf,int len); +lib/Makefile +2
-1@@ -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 += -DDEBUGlib/string_helpers.c new file +27
-0@@ -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 };
lib/string_helpers.c line 37
Quoted 10 lines
lib/string_helpers.c new file +10
-0+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 , + };
Formally these should be static, but I expect the compiler will work it out.
Quoted 11 lines
lib/string_helpers.c new file +11
-0+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++)
lib/string_helpers.c line 58
Quoted 10 lines
lib/string_helpers.c new file +10
-0+ 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]);
Can't print a u64 - we don't know what type it has. It must be cast to something, usually unsigned long long.
Quoted 3 lines
lib/string_helpers.c new file +3
-0+return 0 ; +} +EXPORT_SYMBOL (string_get_size);
James Bottomley
maintainerRe: [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range
Quoted 6 lines · Andrew Morton
Quoted 4 lines · On Sun, 31 Aug 2008 10:13:54 -0500 James Bottomley…
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.
Quoted 6 lines
Quoted 4 lines
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 ...
As Matthew said - a new %p thingy might be appropriate.
Yes ... I thought about that. Two things I don't like about the approach are:
1. It adds to the confetti of letters vsnprintf already accepts ... it's becoming like ls, and we're soon going to run out of alphabet
2. I'd probably have to plumb in precision and things, which make the code a bit nastier.
I'm not sure the advantages (use in all our prints) outweighs this.
Quoted 9 lines
Quoted 9 lines
--- 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
Quoted 14 lines
include/linux/string_helpers.h line 6
Quoted 12 lines
include/linux/string_helpers.h new file +6
-0@@ -0,0 +1,10 @@ +#include <linux/types.h> + +enum string_size_units { +STRING_UNITS_10 , +STRING_UNITS_2 , +};Could do with some comments.
Sure, will add.
Quoted 54 lines
Quoted 54 lines
include/linux/string_helpers.h new file +3
-0+int string_get_size (u64 size,enum string_size_units units, +char *buf,int len); +lib/Makefile +2
-1@@ -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 += -DDEBUGlib/string_helpers.c new file +31
-0@@ -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,
Quoted 8 lines
lib/string_helpers.c line 37
Quoted 6 lines
lib/string_helpers.c new file +6
-0+ [STRING_UNITS_2] = units_2, + }; +const int divisor[] = { + [STRING_UNITS_10 ] =1000 , + [STRING_UNITS_2 ] =1024 , + };Formally these should be static, but I expect the compiler will work it out.
As long as I have everything correctly const'd, the optimisations available should be similar, yes.
Quoted 17 lines
Quoted 17 lines
lib/string_helpers.c new file +16
-0+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]);
Quoted 8 lines
lib/string_helpers.c line 58
Quoted 5 lines
lib/string_helpers.c new file +5
-0+snprintf (tmp,sizeof (tmp),".%03lld" , remainder); + tmp[j+1 ] ='\0' ; + } + +snprintf (buf, len,"%lld%s%s" , size, tmp, units_str[units][i]);Can't print a u64 - we don't know what type it has. It must be cast to something, usually unsigned long long.
I thought there was a patch from Matthew to move u64 to unsigned long long on all architectures, thus obviating this annoying conversion ... is that still wandering upstream, or has it been dropped?
Quoted 3 lines
lib/string_helpers.c line 62
Quoted 3 lines
lib/string_helpers.c new file +3
-0+return 0 ; +} +EXPORT_SYMBOL (string_get_size);
James
Andrew Morton
maintainerRe: [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range
Quoted 8 lines · On Wed, 03 Sep 2008 09:32:34 -0500 James Bottomley…
Quoted 4 lines
lib/string_helpers.c line 58
Quoted 1 line
lib/string_helpers.c new file +1
-0+snprintf (buf, len,"%lld%s%s" , size, tmp, units_str[units][i]);Can't print a u64 - we don't know what type it has. It must be cast to something, usually unsigned long long.
I thought there was a patch from Matthew to move u64 to unsigned long long on all architectures, thus obviating this annoying conversion ... is that still wandering upstream, or has it been dropped?
It broke, and it needs vast numbers of conversions in the arch code.
James Bottomley
maintainerPATCH 2/2 sd: use generic helper to print capacities in both binary and SI
In reply to James BottomleyJames
---
drivers/scsi/sd.c +18 -23
@@ -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 */
Re: [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/...
In reply to James BottomleyQuoted 10 lines · James Bottomley
Quoted 8 lines · 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...
Quoted 5 lines
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?
Signature · 1 line
Simon Arlott
Quoted 11 lines · Simon Arlott
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · James Bottomley
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).
Quoted 1 line
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
Quoted 7 lines
Quoted 4 lines
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 6 lines
Quoted 2 lines
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.
Quoted 2 lines
(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?
Signature · 4 lines
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."
Quoted 26 lines · Matthew Wilcox
Quoted 11 lines · Simon Arlott
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · James Bottomley
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
500107862016512 * 976773168 / (1024 * 1024 * 1024)
465.76174163818359375000If we report it as 465GB, we will get questions. Even pretending it's 1024 * 1000 * 1000 doesn't work:
Quoted 7 lines
512 * 976773168 / (1000 * 1000 * 1024)
488.38658400000000000000I 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.
Quoted 13 lines
Quoted 1 line
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-ramQuoted 7 lines
Quoted 4 lines
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.
Quoted 6 lines
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).
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
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.
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
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.
Quoted 5 lines
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_ramto 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.)
Quoted 8 lines
scsi_debugcan probably go to the full 2^64 sectors. I haven't looked into it.Quoted 2 lines
(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.
Signature · 1 line
Simon Arlott
Quoted 6 lines · Matthew Wilcox
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