sector_t format string

Define SECTOR_FMT to print sector_t in proper format

...

#define HAVE_SECTOR_T
typedef u64 sector_t;
+#define SECTOR_FMT "%llu"

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.

Hi,

On Wed, 9 Aug 2006, Andrew Morton wrote:

Quoted 3 lines

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.

Is this really necessary? There are a few features, which would make ext4 also interesting at the low end (e.g. extents). Storing 64bit values on disk is fine, but they should be converted to native values as soon as possible.

Quoted 6 lines · Andrew Morton

Quoted 4 lines

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.

Quoted 3 lines

Is this really necessary? There are a few features, which would make ext4 also interesting at the low end (e.g. extents). Storing 64bit values on disk is fine, but they should be converted to native values as soon as possible.

Consider what that means. "converted to native" means dealing with truncation issues...

Yes, it does, but I don't think it's that difficult - basically returning -EIO, it should be part of the basic error handling. Afterwards you don't have to waste cpu/memory on unused data anymore.

Quoted 3 lines · Roman Zippel

Yes, it does, but I don't think it's that difficult - basically returning -EIO, it should be part of the basic error handling. Afterwards you don't have to waste cpu/memory on unused data anymore.

Or you could just not bother, and leave everything as u64.

Hi,

Quoted 5 lines · Jeff Garzik

Quoted 3 lines · Roman Zippel

Yes, it does, but I don't think it's that difficult - basically returning -EIO, it should be part of the basic error handling. Afterwards you don't have to waste cpu/memory on unused data anymore.

Or you could just not bother, and leave everything as u64.

Why?

Quoted 3 lines

Quoted 1 line

Or you could just not bother, and leave everything as u64.

Why?

To eliminate needless complexity and keep things simple and obvious?

Quoted 3 lines · Jeff Garzik

Quoted 2 lines

Quoted 1 line

Or you could just not bother, and leave everything as u64.

Why?

To eliminate needless complexity and keep things simple and obvious?

Considering the amount of complexity we add for the high end, why is it suddenly a bad thing to add even a _little_ complexity for the other end?

Quoted 2 lines

Considering the amount of complexity we add for the high end, why is it suddenly a bad thing to add even a _little_ complexity for the other end?

This is ext4 not ext3 we're talking about. The next gen Linux filesystem should be tuned for modern machines -- 64bit, moving forward -- while still working just fine on 32bit.

If you force everyone to use 64bit sector numbers, I don't understand how you can claim "still working just fine on 32bit"?

At some point ext4 is probably going to be the de facto standard, which very many people want to use, because it has all the new features, which won't be ported to ext2/3. So I still don't understand, what's so wrong about a little tuning in both directions?

Quoted 2 lines · Roman Zippel

If you force everyone to use 64bit sector numbers, I don't understand how you can claim "still working just fine on 32bit"?

64bit sector numbers work just fine on 32-bit machines.

Quoted 4 lines

At some point ext4 is probably going to be the de facto standard, which very many people want to use, because it has all the new features, which won't be ported to ext2/3. So I still don't understand, what's so wrong about a little tuning in both directions?

Just seems like wasted effort to me.

I disagree.
Many developer still brag about how Linux runs on about everything, but it's little steps like this, which make it more and more a joke.

ext4 is being developed primarily to address scaling issues at the high end of the storage spectrum. If you're concerned about carrying 64-bit containers, just use ext3, and be happy with your 32-bit, < 16TB filesystems, I'd say.

Hi,

Quoted 3 lines · Eric Sandeen

ext4 is being developed primarily to address scaling issues at the high end of the storage spectrum. If you're concerned about carrying 64-bit containers, just use ext3, and be happy with your 32-bit, < 16TB filesystems, I'd say.

The problem being that it doesn't _exclusively_ address scaling issues, some new features may well be interesting to non high end users as well. If it's supposed to be a high end only fs, then please don't call ext4, otherwise it would mislead users about what it doesn't is - a general purpose fs.

Quoted 4 lines

The problem being that it doesn't _exclusively_ address scaling issues, some new features may well be interesting to non high end users as well. If it's supposed to be a high end only fs, then please don't call ext4, otherwise it would mislead users about what it doesn't is - a general purpose fs.

It will work just fine on 32-bit machines.

You're making a mountain out of a molehill.

Hi,

Quoted 4 lines · Jeff Garzik

Quoted 2 lines · Roman Zippel

If you force everyone to use 64bit sector numbers, I don't understand how you can claim "still working just fine on 32bit"?

64bit sector numbers work just fine on 32-bit machines.

Depends on the definition of "fine".

Quoted 5 lines

Quoted 4 lines

At some point ext4 is probably going to be the de facto standard, which very many people want to use, because it has all the new features, which won't be ported to ext2/3. So I still don't understand, what's so wrong about a little tuning in both directions?

Just seems like wasted effort to me.

I disagree.
Many developer still brag about how Linux runs on about everything, but it's little steps like this, which make it more and more a joke.

With CONFIG_LBD, 32-bit machines can already support large block devices.

If you feel that hardcoding u64 as sector numbers will mean ext4 suddenly fails on 32-bit, you misunderstand the situation completely.

Hi,

Quoted 5 lines · Jeff Garzik

With CONFIG_LBD, 32-bit machines can already support large block devices.

If you feel that hardcoding u64 as sector numbers will mean ext4 suddenly fails on 32-bit, you misunderstand the situation completely.

With CONFIG_LBD disabled you still had the truncation/complexity issues somewhere else, so you gain nothing, but waste memory in ext4.

Quoted 2 lines

With CONFIG_LBD disabled you still had the truncation/complexity issues somewhere else, so you gain nothing, but waste memory in ext4.

You gain simplicity and reduced number of code paths.

"waste memory" is hardly a significant argument. I doubt you will notice a difference.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Roman" == Roman Zippel <[email protected]> writes:

The problem as I see it, is that you want extents, but you don't want the RAM/DISK/ROM penalty of 64bit blocks, since embedded devices won't ever go past the existing ext3 sizes, right?

Is this a more clear statement of what you want?

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Roman" == Roman Zippel <[email protected]> writes:

Roman> If you force everyone to use 64bit sector numbers, I don't Roman> understand how you can claim "still working just fine on Roman> 32bit"? At some point ext4 is probably going to be the de Roman> facto standard, which very many people want to use, because it Roman> has all the new features, which won't be ported to ext2/3. So I Roman> still don't understand, what's so wrong about a little tuning Roman> in both directions?

The problem as I see it, is that you want extents, but you don't want the RAM/DISK/ROM penalty of 64bit blocks, since embedded devices won't ever go past the existing ext3 sizes, right?

For ext3 on x86:

CONFIG_LBD=y:

box:/usr/src/25> size fs/jbd/jbd.o fs/ext3/ext3.o
text data bss dec hex filename
51076 8 32 51116 c7ac fs/jbd/jbd.o
87466 1020 4 88490 159aa fs/ext3/ext3.o

CONFIG_LBD=n:

box:/usr/src/25> size fs/jbd/jbd.o fs/ext3/ext3.o
text data bss dec hex filename
51133 8 32 51173 c7e5 fs/jbd/jbd.o
87679 1020 4 88703 15a7f fs/ext3/ext3.o

That's a grand total of 270 bytes of text saved. aka 0.19%.

We'll save four bytes in the inode (unlikely to save anything due to slab packing).

Quoted 8 lines · 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.

Ext4 will support a 48-bit blocknumber format for extents, but I do want to make ext4 suitable as a general purpose filesystem, and 32-bit systems will be around for I fear far longer than people might wish. So while I agree that we shouldn't go _too_ far out of our way to make things efficient on 32-bit systems, if it's not that much work to support a 32-bit sector_t, we ought to do it.

So how about a compromise? We allow for a 32-bit sector_t in ext4, but we drop the SECTOR_FMT, and rely on %llu and typecasts in printk's. Then the only other extra hair in the filesystem code will be a mount-time check to make sure we don't try to mount a large filesystem on system with a 32-bit sector_t.

Hi,

Quoted 7 lines · Theodore Tso

Ext4 will support a 48-bit blocknumber format for extents, but I do want to make ext4 suitable as a general purpose filesystem, and 32-bit systems will be around for I fear far longer than people might wish. So while I agree that we shouldn't go _too_ far out of our way to make things efficient on 32-bit systems, if it's not that much work to support a 32-bit sector_t, we ought to do it.

Quoted 5 lines

So how about a compromise? We allow for a 32-bit sector_t in ext4, but we drop the SECTOR_FMT, and rely on %llu and typecasts in printk's. Then the only other extra hair in the filesystem code will be a mount-time check to make sure we don't try to mount a large filesystem on system with a 32-bit sector_t.

Thanks, that's what I was hoping for. :)
Disallowing to mount large fs without CONFIG_LBD is not a real problem and then also truncation is not an issue anymore (except maybe for e2fsck).

bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 1 line · On Wed, 09 Aug 2006 18:20:43 -0700 Mingming Cao <[email protected]> wrote:

Define SECTOR_FMT to print sector_t in proper format

We've thus-far avoided doing this. In fact a similar construct in device-mapper was recently removed.

Quoted 3 lines · Andrew Morton

On Wed, 09 Aug 2006 18:20:43 -0700
Mingming Cao <[email protected]> wrote:

Quoted 4 lines

Quoted 1 line

Define SECTOR_FMT to print sector_t in proper format

We've thus-far avoided doing this. In fact a similar construct in device-mapper was recently removed.

Yeah, OCFS2 had similar formats, and we were asked to change
them to naked casts before inclusion. Seems quite consistent with the rest of the kernel.

Quoted 4 lines · Joel Becker

Quoted 3 lines · Andrew Morton

On Wed, 09 Aug 2006 18:20:43 -0700
Mingming Cao <[email protected]> wrote:

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Define SECTOR_FMT to print sector_t in proper format

We've thus-far avoided doing this. In fact a similar construct in device-mapper was recently removed.

Yeah, OCFS2 had similar formats, and we were asked to change
them to naked casts before inclusion. Seems quite consistent with the rest of the kernel.

Will

	printk("%S", sector_t);

kill at least one kitten?

Quoted 18 lines · On Thu, 10 Aug 2006 23:44:40 +0400 Alexey Dobriyan <[email protected]> wrote:

Quoted 4 lines · Joel Becker

Quoted 3 lines · Andrew Morton

On Wed, 09 Aug 2006 18:20:43 -0700
Mingming Cao <[email protected]> wrote:

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Define SECTOR_FMT to print sector_t in proper format

We've thus-far avoided doing this. In fact a similar construct in device-mapper was recently removed.

Yeah, OCFS2 had similar formats, and we were asked to change
them to naked casts before inclusion. Seems quite consistent with the rest of the kernel.

Will

	printk("%S", sector_t);

Quoted 1 line

kill at least one kitten?

It would be really nice to be able to define local enhancements like this to printf. It would solve lots of these problems quite nicely.

Bus alas, there's no way (afaik) to teach __attribute__((format)) about them, so gcc will warn.

Quoted 10 lines · Joel Becker

Quoted 6 lines · Andrew Morton

Quoted 1 line · On Wed, 09 Aug 2006 18:20:43 -0700 Mingming Cao <[email protected]> wrote:

Define SECTOR_FMT to print sector_t in proper format

We've thus-far avoided doing this. In fact a similar construct in device-mapper was recently removed.

Yeah, OCFS2 had similar formats, and we were asked to change
them to naked casts before inclusion. Seems quite consistent with the rest of the kernel.

Will

	printk("%S", sector_t);

kill at least one kitten?

Quoted 5 lines

Will

	printk("%S", sector_t);

kill at least one kitten?

I like the general idea. I think that having to cast every time you want to print a sector number is pretty gross. I had something more like %Su in mind, though.

Quoted 9 lines

Quoted 5 lines

Will

	printk("%S", sector_t);

kill at least one kitten?

I like the general idea. I think that having to cast every time you want to print a sector number is pretty gross. I had something more like %Su in mind, though.

What will happen if you run out of %[a-z] ?

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Will

	printk("%S", sector_t);

kill at least one kitten?

Quoted 5 lines

Quoted 3 lines

I like the general idea. I think that having to cast every time you want to print a sector number is pretty gross. I had something more like %Su in mind, though.

What will happen if you run out of %[a-z] ?

My keyboard can produce a lot more characters (even if I ignore the non-ASCII ones).

Quoted 8 lines

Quoted 5 lines

Will

	printk("%S", sector_t);

kill at least one kitten?

I like the general idea. I think that having to cast every time you want to print a sector number is pretty gross. I had something more like %Su in mind, though.

What will happen if you run out of %[a-z] ?

Quoted 8 lines · 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.

Finally, perhaps the code is printing block numbers too much ;)

<Notices E3FSBLK, wonders how that snuck through>

I'd suggest that "[patch] ext3: remove E3FSBLK" be written and merged before we clone ext4, too...

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.

Quoted 28 lines · Andrew Morton

Quoted 8 lines · 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.

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

I'd suggest that "[patch] ext3: remove E3FSBLK" be written and merged before we clone ext4, too...