Re: [patch 069/104] lib/string_helpers.c:string_get_size(): remove redundant prefixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2015-02-12 at 15:40 -0800, Andrew Morton wrote:
> On Thu, 12 Feb 2015 15:25:08 -0800 James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > On Thu, 2015-02-12 at 15:01 -0800, akpm@xxxxxxxxxxxxxxxxxxxx wrote:
> > > From: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> > > Subject: lib/string_helpers.c:string_get_size(): remove redundant prefixes
> > > 
> > > While 3c9f3681d0b4 "[SCSI] lib: add generic helper to print sizes rounded
> > > to the correct SI range" says that Z and Y are included in preparation for
> > > 128 bit computers, they just waste .text currently.  If and when we get
> > > u128, string_get_size needs updating anyway (and ISO needs to come up with
> > > four more prefixes).
> > 
> > This is rubbish. It's nothing to do with 128 bits.  This is to do with
> > disk sizes linux gets attached to.  The current largest device clusters
> > are Petabytes ... I think we may have some exabyte ones somewhere in the
> > Academic community, so it's by no means inconcievable we'll have
> > Zettabyte ones within a few years.  The SCSI standard, with 4k blocks
> > supports up to 2^76, which is well into Zettabytes.  We obviously run
> > off the mmap possibilities a lot sooner, because of the byte offsets,
> > but that's fixable.  Someone will probably start first by passing blocks
> > into that interface not bytes, so we'd like it not to be based on
> > assumptions that think 2^64 is the largest possible value.
> 
> I don't get it.  As the man says, this is presently dead code and
> string_get_size() will need to be changed to work for disks larger than
> 2^64 bytes.  That change may be to take a u128 or it may be as you
> suggest: replace the `u64 size' with `u64 size, u64 units' which is
> effectively the same thing.

The first thing someone's going to do is pass in blocks, because that's
the way the rest of block functions. If we're lucky the add "ZB" too,
but if not we run off the end in some obscure large cluster somewhere.
Don't set people up to make mistakes.

> > > Also there's no need to include and test for the NULL sentinel; once we
> > > reach "E" size is at most 18.  [The test is also wrong; it should be
> > > units_str[units][i+1]; if we've reached NULL we're already doomed.]
> > 
> > So fix the bug, don't set us up to run off the end of the array.  And
> > please consult the community which keeps track of this rather than
> > trying to get it into Linux without review.
> 
> That seems a bit harsh - you've been cc'ed on this every step of the way.

I think you need to check your scripts.  This is the first time I've
seen this patch, which is why I'm reacting this way.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux