From: Joel Becker <Joel.Becker@oracle.com>
Subject: Re: [PATCH 1/3] ext3/ext4: Factor out disk
 addressability check
Date: Sun, 15 Aug 2010 19:54:37 -0700
Message-ID: <20100816025436.GA9061@mail.oracle.com>
References: <874ofr2myq.fsf@patl.com> <20100812174215.GC6561@mail.oracle.com>
	<1F3EDC08-AC93-4D4D-8F83-A13C418DFC88@dilger.ca>
	<20100812201534.GA22777@mail.oracle.com>
	<209AEA97-E284-4ADB-8774-50C2630606B9@dilger.ca>
	<20100812222949.GC22777@mail.oracle.com>
	<20100813163006.GB4329@quack.suse.cz> <4C6821A8.5080805@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: Andreas Dilger <adilger@dilger.ca>, "Ted Ts'o" <tytso@mit.edu>,
        linux-fsdevel@vger.kernel.org, Jan Kara <jack@suse.cz>,
        linux-ext4@vger.kernel.org, ocfs2-devel@oss.oracle.com
To: Eric Sandeen <sandeen@redhat.com>
Content-Disposition: inline
In-Reply-To: <4C6821A8.5080805@redhat.com>
Sender: ocfs2-devel-bounces@oss.oracle.com
Errors-To: ocfs2-devel-bounces@oss.oracle.com

On Sun, Aug 15, 2010 at 12:19:36PM -0500, Eric Sandeen wrote:
> >> +	    (last_fs_block >
> >> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> 
> Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
> 16T devices out there, I guess.
> 
> I went through the math again and also came up with:
> 
> total fs pages is blocks / (blocks per page)
> total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
> total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
> total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
> total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)
> 
> too big if total pages is > (pgoff_t)(~0ULL)
> too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)

	Why not stop here, which is what I put in my other email?
"blocks >> SHIFT-bits" is "how many pages do I need?".

> too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
> and to not overflow:
> too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

	This still overflows.  pgoff_t is a u64 on 64bit machines,
right?  So shift that left by anything and you wrap.

Joel

-- 

"You cannot bring about prosperity by discouraging thrift. You cannot
 strengthen the weak by weakening the strong. You cannot help the wage
 earner by pulling down the wage payer. You cannot further the
 brotherhood of man by encouraging class hatred. You cannot help the
 poor by destroying the rich. You cannot build character and courage by
 taking away a man's initiative and independence. You cannot help men
 permanently by doing for them what they could and should do for
 themselves."
	- Abraham Lincoln 

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127