public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <matthew@wil.cx>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Simon Arlott <simon@fire.lp0.eu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH] scsi/sd: Fix size output in MB
Date: Sat, 30 Aug 2008 11:45:16 -0600	[thread overview]
Message-ID: <20080830174516.GD1239@parisc-linux.org> (raw)
In-Reply-To: <1220117091.3615.3.camel@localhost.localdomain>

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

  reply	other threads:[~2008-08-30 17:45 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-30 14:08 [PATCH] scsi/sd: Fix size output in MB Simon Arlott
2008-08-30 17:24 ` James Bottomley
2008-08-30 17:45   ` Matthew Wilcox [this message]
2008-08-30 20:59     ` Pierre Ossman
2008-08-30 21:45       ` James Bottomley
2008-08-30 22:13         ` Pierre Ossman
2008-08-30 22:24           ` Simon Arlott
2008-08-30 22:36           ` Matthew Wilcox
2008-08-30 21:02     ` Simon Arlott
2008-08-30 21:03       ` [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/ Simon Arlott
2008-08-31  1:59         ` James Bottomley
2008-08-31  2:54           ` Matthew Wilcox
2008-08-31 14:25             ` Ingo Oeser
2008-08-31 15:04               ` Simon Arlott
2008-08-31 15:08             ` James Bottomley
2008-08-31 15:13               ` [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range James Bottomley
2008-08-31 15:20                 ` Simon Arlott
2008-08-31 15:41                   ` James Bottomley
2008-08-31 15:51                 ` Matthew Wilcox
2008-08-31 18:54                 ` [PATCH] mmc_block: use generic helper to print capacities Pierre Ossman
2008-09-05 20:09                   ` James Bottomley
2008-09-05 20:52                     ` Pierre Ossman
2008-09-05 21:03                       ` James Bottomley
2008-09-06  8:57                         ` Pierre Ossman
2008-09-03  3:39                 ` [PATCH 1/2] lib: add generic helper to print sizes rounded to the correct SI range Andrew Morton
2008-09-03 14:32                   ` James Bottomley
2008-09-03 15:58                     ` Andrew Morton
2008-08-31 15:15               ` [PATCH 2/2] sd: use generic helper to print capacities in both binary and SI James Bottomley
2008-08-31 15:08           ` [PATCH] scsi/sd: Fix capacity output to show MB/GB/TB/ Simon Arlott
2008-08-30 21:57       ` [PATCH] scsi/sd: Fix size output in MB Matthew Wilcox
2008-08-30 22:22         ` Simon Arlott
2008-08-31 12:27         ` James Smart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080830174516.GD1239@parisc-linux.org \
    --to=matthew@wil.cx \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=simon@fire.lp0.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox