`On Thu, 18 May 2006, Linus Torvalds wrote:> On Thu, 18 May 2006, Andreas Dilger wrote:> >  	struct bio *bio;> > +	unsigned long long sector;> >  	int ret = 0;> > > >  	BUG_ON(!buffer_locked(bh));> >  	BUG_ON(!buffer_mapped(bh));> >  	BUG_ON(!bh->b_end_io);> > > > +	/* Check if we overflow sector_t when computing the sector offset.  */> > +	sector = (unsigned long long)bh->b_blocknr * (bh->b_size >> 9);> > Ok so far, looks fine.> > But what the heck is this:> > > +#if !defined(CONFIG_LBD) && BITS_PER_LONG == 32> > +	if (unlikely(sector != (sector_t)sector))> > +#else> > +	if (unlikely(((bh->b_blocknr >> 32) * (bh->b_size >> 9)) >=> > +		     0xffffffff00000000ULL))> > +#endif> > I don't understand the #ifdef at all.> > Why isn't that just a > > 	if (unlikely(sector != (sector_t)sector))> > and that's it? What does this have to do with CONFIG_LBD or BITS_PER_LONG, > or anything at all?> > If the sector number fits in a sector_t, we're all good.I think you missed that Andrewas said he is worried about 64-bit overflows as well.  And you would not catch those with the sector != (sector_t)sector test because you would be comparing two 64-bit values together so they always match...Hence why he shifts the value right by 32 bits then multiplies and tests the result for overflowing 32-bits which if it does it means it would overflow the 64-bit multiplication, too therefor your "sector" is truncated.Makes sense to me in a some very convoluted sickening way...  (-;Best regards,	Anton-- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)Unix Support, Computing Service, University of Cambridge, CB2 3QH, UKLinux NTFS maintainer, http://www.linux-ntfs.org/-To unsubscribe from this list: send the line "unsubscribe linux-kernel" inthe body of a message to majordomo@vger.kernel.orgMore majordomo info at  http://vger.kernel.org/majordomo-info.htmlPlease read the FAQ at  http://www.tux.org/lkml/`