On Wed, 2006-08-09 at 23:40 -0700, Andrew Morton wrote:
> On Wed, 09 Aug 2006 18:20:43 -0700
> Mingming Cao <[email protected]> wrote:
>
> >
> > Define SECTOR_FMT to print sector_t in proper format
> >
> > ...
> >
> > #define HAVE_SECTOR_T
> > typedef u64 sector_t;
> > +#define SECTOR_FMT "%llu"
>
> We've thus-far avoided doing this. In fact a similar construct in
> device-mapper was recently removed.
>
> Unlike many other attempts, this one appears to be correct (people usually
> get powerpc wrong, due to its u64=unsigned long).
>
> That being said, I'm not really sure we want to add this. It produces
> rather nasty-looking source code and thus far we've just used %llu and we've
> typecasted the sector_t to `unsigned long long'. That happens in a lot of
> places in the kernel and perhaps we don't want to start innovating in ext4
> ;)
>
> That also being said... does a 32-bit sector_t make any sense on a
> 48-bit-blocknumber filesystem? I'd have thought that we'd just make ext4
> depend on 64-bit sector_t and be done with it.
>
> Consequently, sector_t should largely vanish from ext4 and JBD2, except for
> those places where it interfaces with the VFS and the block layer.
> Internally it should just use 64-bit quantities. That could be u64, but
> I'd suggest that the fs simply open-code `unsigned long long' so that we
> don't need to play any gams at all when passing these things into printk.
>
I am fine with unsigned long long -- it does makes the printk a lot
simplier--- I think we had a debate about whether to use sector_t or
just unsigned long long on ext2-devel before, the argument at that time
is to avoid unnecssary memory for in-kernel block variables on 32 bit
machine ..I think that's the concern about using unsigned long long.
I intend to agree with you that the benefit is not so obvious. And since
the on-disk data block number and some metadata are 48bit all the time,
we do have to check the bits of the sector_t to make sure the in-kernel
block variable have enough room to store the blocks when read it from
disk. Not very pretty.
> Finally, perhaps the code is printing block numbers too much ;)
>
> <Notices E3FSBLK, wonders how that snuck through>
>
When we cleanup ext3 code to fix some "int" type block numbers to
"unsigned long" type to able to truely support 2^32 bit ext3 (otherwise
ext3 is limited to 2^31 blocks (8TB)), we had to go through a lot of
printk to modify the format string from %d to %lu. It's a pain.
At that time we are planning to have 48 bit ext3 (now ext4) too, so we
need to go through the same pain again: replace all %lu to %llu in all
the places where the blocks are being print out. Another huge chunk of
patch.
So the decision at that time is to do it once: identify all the printk
cases and use a micro to replace the format string. That is the reason
behind the E3FSBLK
I agree with you that this makes the code hard to read -- I'm fine to
remove it. Fortunately with the previous work, removing it is not so
hard, just simplely search/replace.
The only thing is that we need to type cast every block numbers to be
printed, if the block type is sector_t -- that's a pain. For this reason
I do like to use unsigned long long instead of sector_t.
> I'd suggest that "[patch] ext3: remove E3FSBLK" be written and merged
> before we clone ext4, too...
>
Mingming
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
