Re: Request for direction on changes required in e2fsprog.
I've cc'ed the linux-ext4 mailing list since a lot of this is about
code cleanliness and coding style of e2fsprogs. Yes, some of this
probably should be written up in Documentation/CodingStyle file in
e2fsprogs, since in general it's much like the kernel CodingStyle,
except that I care a lot more about ABI and API backwards
compatibility, since e2fsprogs exports a userspace shared library.
- Ted
On Fri, Jul 20, 2007 at 11:25:49AM -0500, Jose R. Santos wrote:
As you mentioned in on of the interlock meeting a couple of weeks ago,
you said that you don't have that big of a problem changing parts of
the libe2fs API/ABI as long as only break it once.
I said that we can break it at _most_ once. But because I believe in
doing incremental coding, I'd much rather try not to break it at all,
and then in the worst case, break things only once. Let me quote from
Linus Torvalds from a recent posting he made on the git mailing list.
LT (Most of the time I actually try to get it right the first time. It's
LT actually become a challenge to me to notice when some change needs a
LT cleanup first in order to make the later changes much easier, so I really
LT *like* trying to actually do the actual development in a logical order:
LT first re-organize the code, and verify that the re-organized code works
LT identically to the old one, then commit that, then start actually working
LT on the new feature with the now cleaner code-base).
LT
LT And no, I didn't start out programming that way. But when you get used to
LT looking at changes as a nice series of independent commits in emails, you
LT really start _working_ that way yourself. And I'm 100% convinced that it
LT actually makes you a better programmer too.
This is why I **really** dislike mongo patches such as the 64-bit
patches from that have come out so far. They change way too much, and
afterwards it's very hard to see what the heck the patch actually
*does*. I am convinced that if we do a better job of breaking up
patches both for e2fsprogs and for the ext4 patch queue, it will make
it a lot easier for people to review patches. Each patch should in
the ideal world only do one thing.
So for example, take a look at some of the patches which I just
commited into the git master branch last night (Sunday night). What
you are seeing there are all cleanup patches. Each of them are
relatively small; none of them change the ABI/API; and after each of
them e2fsprogs passes the make check regression test suite, so the
whole thing is git bisectable.
All of these changes was to move any 32-bit bitmap knowledge out of
inline functions, and into the file gen_bitmap.c. Of course, I had to
preserve any functions that had been previously called by inline
functions, as well as anything that had been exported as part of the
ABI. But that's easy to do.
The next step, which I haven't done yet (and probably won't have time
to do for at least a day or two thanks to my needing to do very Unfun
things like Fall Plan stuff, as opposed to Fun stuff like e2fsprogs
hacking :-) is to create a new set of interfaces that look somewhat
like this:
int ext2fs_mark_block_nbitmap(ext2fs_block_bitmap bitmap, blk64_t block);
int ext2fs_unmark_block_nbitmap(ext2fs_block_bitmap bitmap, blk64_t block);
int ext2fs_test_block_nbitmap(ext2fs_block_bitmap bitmap, blk64_t block);
int ext2fs_mark_inode_nbitmap(ext2fs_block_bitmap bitmap, ino64_t inode);
int ext2fs_unmark_inode_nbitmap(ext2fs_block_bitmap bitmap, ino64_t inode);
int ext2fs_test_inode_nbitmap(ext2fs_block_bitmap bitmap, ino64_t inode);
And then rearrage the structure definitions like so:
in ext2fs.h:
/* Redefined from original values in ext2fs.h */
typedef struct ext2fs_struct_nbitmap *ext2fs_generic_bitmap;
typedef struct ext2fs_struct_inode_bitmap *ext2fs_inode_bitmap;
typedef struct ext2fs_struct_block_bitmap *ext2fs_block_bitmap;
(No, we never define ext2fs_struct_block_bitmap and
ext2fs_struct_inode_bitmap; after doing the appropriate structure
magic number checking, we cast it to ext2fs_struct_nbitmap and then
use the nbitmap functions for common handling of the inode and block
bitmaps. The two different structures are there just to allow the
compiler to enforce proper type-checking.)
And in ext2fsP.h we add:
#define EXT2_NBITMAP_TYPE_JBOB 1 /* Just a Bunch of Bits */
#define EXT2_NBITMAP_TYPE_RBTREE 2 /* Red-Black Tree */
struct ext2fs_struct_nbitmap {
errcode_tmagic;
ext2_filsys bm_fs;
__u64bm_start, bm_end;
__u64bm_real_end;
char *bm_description;
errcode_tbm_base_errcode;
int bm_type;
void *bm_private;
__u32bm_reserved[7];
};
Now, if bm_type is EXT2_NBITMAP_TYPE_JBOB, then bm_private will just
be a pointer to a memory
Re: Patch to support LUKS UUIDs in libblkid
Theodore Tso schrieb: Thanks, I've applied the blkid LUKS patch to the e2fsprogs SCM (modulo a minor whitespace breakage which I fixed up). I've encountered a problem with this patch, consider the following setup: in /etc/fstab: UUID=some LUKS uuid / ext3 defaults 0 0 During bootup the init scripts run 'fsck -A', fsck reads in /etc/fstab and converts the LUKS-uuid to a device name. It then uses this devicename to populate filesys_info and sets the type to ext3 (from fstab). It then tries to check the underlying device instead of the /dev/mapper/ LUKS device. I'm not sure how to work around this, adding crypt_luks to ignored_types won't work because fsck thinks the device has an ext3 FS. Is create_fs_device() the right place to add a check if this is a LUKS device or would it be better to do that in parse_fstab_line() around the blkid_get_devname() call ? Karsten -- Karsten Hopp| Mail: [EMAIL PROTECTED] Red Hat Deutschland | Tel: +49-711-96437-0 Hauptstaetterstr.58 | Fax: +49-711-613590 D-70178 Stuttgart | http://www.redhat.de - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Patch to support LUKS UUIDs in libblkid
On Mon, Jul 23, 2007 at 05:00:02PM +0200, Karsten Hopp wrote: I've encountered a problem with this patch, consider the following setup: in /etc/fstab: UUID=some LUKS uuid / ext3 defaults 0 0 Well, the problem is that what you should be putting in /etc/fstab is: UUID=uuid of the ext3 filesystem / ext3 defaults 0 0 If you put UUID=some LUKS uuid, then of course you will get the underlying device; that's what you asked for --- what device has the UUID set to some LUKS uuid --- and the computer gave you what you asked for, which is what not what you wanted. :-) If you put the UUID of the ext3 filesystem, then the blkid library will search through the /dev/mapper devices and find the correct /dev/mapper device for the underlying filesystem, which is as it should be. This of course assumes that someone has correctly activated the LUKS filesystem (and presumably passed LUKS the correct crypto information) so that it appears as one of the /dev/mapper filesystems. If that is why you put the UUID of LUKS partition in /etc/fstab, then we have a fundamental conflict of what /etc/fstab is for. /etc/fstab's job is to identify the actual devices for mounting a filesystem. - Ted - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fix inode_table test in ext234_check_descriptors
ext[234]_check_descriptors sanity checks block group descriptor geometry
at mount time, testing whether the block bitmap, inode bitmap, and
inode table reside wholly within the blockgroup. However, the inode
table test is off by one so that if the last block in the inode table
resides on the last block of the block group, the test incorrectly
fails. This is because it tests the last block as (start + length)
rather than (start + length - 1).
This can be seen by trying to mount a filesystem made such as:
mkfs.ext2 -F -b 1024 -m 0 -g 256 -N 3744 fsfile 1024
which yields:
EXT2-fs error (device loop0): ext2_check_descriptors: Inode table for group 0
not in group (block 101)!
EXT2-fs: group descriptors corrupted!
There is a similar bug in e2fsprogs, patch already sent for that.
(I wonder if inside(), outside(), and/or in_range() should someday be
used in this and other tests throughout the ext filesystems...)
Signed-off-by: Eric Sandeen [EMAIL PROTECTED]
Index: linux-2.6.22-rc4/fs/ext2/super.c
===
--- linux-2.6.22-rc4.orig/fs/ext2/super.c
+++ linux-2.6.22-rc4/fs/ext2/super.c
@@ -579,7 +579,7 @@ static int ext2_check_descriptors (struc
return 0;
}
if (le32_to_cpu(gdp-bg_inode_table) first_block ||
- le32_to_cpu(gdp-bg_inode_table) + sbi-s_itb_per_group
+ le32_to_cpu(gdp-bg_inode_table) + sbi-s_itb_per_group - 1
last_block)
{
ext2_error (sb, ext2_check_descriptors,
Index: linux-2.6.22-rc4/fs/ext3/super.c
===
--- linux-2.6.22-rc4.orig/fs/ext3/super.c
+++ linux-2.6.22-rc4/fs/ext3/super.c
@@ -1211,7 +1211,7 @@ static int ext3_check_descriptors (struc
return 0;
}
if (le32_to_cpu(gdp-bg_inode_table) first_block ||
- le32_to_cpu(gdp-bg_inode_table) + sbi-s_itb_per_group
+ le32_to_cpu(gdp-bg_inode_table) + sbi-s_itb_per_group - 1
last_block)
{
ext3_error (sb, ext3_check_descriptors,
Index: linux-2.6.22-rc4/fs/ext4/super.c
===
--- linux-2.6.22-rc4.orig/fs/ext4/super.c
+++ linux-2.6.22-rc4/fs/ext4/super.c
@@ -1269,7 +1269,7 @@ static int ext4_check_descriptors (struc
}
inode_table = ext4_inode_table(sb, gdp);
if (inode_table first_block ||
- inode_table + sbi-s_itb_per_group last_block)
+ inode_table + sbi-s_itb_per_group - 1 last_block)
{
ext4_error (sb, ext4_check_descriptors,
Inode table for group %d
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] remove unused bh in calls to ext234_get_group_desc
ext[234]_get_group_desc never tests the bh argument, and only sets
it if it is passed in; it is perfectly happy with a NULL bh argument.
But, many callers send one in and never use it. May as well call with
NULL like other callers who don't use the bh.
Signed-off-by: Eric Sandeen [EMAIL PROTECTED]
Index: linux-2.6.22-rc4/fs/ext3/ialloc.c
===
--- linux-2.6.22-rc4.orig/fs/ext3/ialloc.c
+++ linux-2.6.22-rc4/fs/ext3/ialloc.c
@@ -204,14 +204,13 @@ static int find_group_dir(struct super_b
int ngroups = EXT3_SB(sb)-s_groups_count;
unsigned int freei, avefreei;
struct ext3_group_desc *desc, *best_desc = NULL;
- struct buffer_head *bh;
int group, best_group = -1;
freei =
percpu_counter_read_positive(EXT3_SB(sb)-s_freeinodes_counter);
avefreei = freei / ngroups;
for (group = 0; group ngroups; group++) {
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (!desc || !desc-bg_free_inodes_count)
continue;
if (le16_to_cpu(desc-bg_free_inodes_count) avefreei)
@@ -269,7 +268,6 @@ static int find_group_orlov(struct super
ext3_grpblk_t min_blocks;
int group = -1, i;
struct ext3_group_desc *desc;
- struct buffer_head *bh;
freei = percpu_counter_read_positive(sbi-s_freeinodes_counter);
avefreei = freei / ngroups;
@@ -286,7 +284,7 @@ static int find_group_orlov(struct super
parent_group = (unsigned)group % ngroups;
for (i = 0; i ngroups; i++) {
group = (parent_group + i) % ngroups;
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (!desc || !desc-bg_free_inodes_count)
continue;
if (le16_to_cpu(desc-bg_used_dirs_count) = best_ndir)
@@ -319,7 +317,7 @@ static int find_group_orlov(struct super
for (i = 0; i ngroups; i++) {
group = (parent_group + i) % ngroups;
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (!desc || !desc-bg_free_inodes_count)
continue;
if (le16_to_cpu(desc-bg_used_dirs_count) = max_dirs)
@@ -334,7 +332,7 @@ static int find_group_orlov(struct super
fallback:
for (i = 0; i ngroups; i++) {
group = (parent_group + i) % ngroups;
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (!desc || !desc-bg_free_inodes_count)
continue;
if (le16_to_cpu(desc-bg_free_inodes_count) = avefreei)
@@ -358,14 +356,13 @@ static int find_group_other(struct super
int parent_group = EXT3_I(parent)-i_block_group;
int ngroups = EXT3_SB(sb)-s_groups_count;
struct ext3_group_desc *desc;
- struct buffer_head *bh;
int group, i;
/*
* Try to place the inode in its parent directory
*/
group = parent_group;
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (desc le16_to_cpu(desc-bg_free_inodes_count)
le16_to_cpu(desc-bg_free_blocks_count))
return group;
@@ -389,7 +386,7 @@ static int find_group_other(struct super
group += i;
if (group = ngroups)
group -= ngroups;
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (desc le16_to_cpu(desc-bg_free_inodes_count)
le16_to_cpu(desc-bg_free_blocks_count))
return group;
@@ -403,7 +400,7 @@ static int find_group_other(struct super
for (i = 0; i ngroups; i++) {
if (++group = ngroups)
group = 0;
- desc = ext3_get_group_desc (sb, group, bh);
+ desc = ext3_get_group_desc (sb, group, NULL);
if (desc le16_to_cpu(desc-bg_free_inodes_count))
return group;
}
Index: linux-2.6.22-rc4/fs/ext2/ialloc.c
===
--- linux-2.6.22-rc4.orig/fs/ext2/ialloc.c
+++ linux-2.6.22-rc4/fs/ext2/ialloc.c
@@ -177,7 +177,6 @@ static void ext2_preread_inode(struct in
unsigned long block_group;
unsigned long offset;
unsigned long block;
- struct buffer_head *bh;
struct ext2_group_desc * gdp;
struct backing_dev_info *bdi;
@@ -188,7 +187,7 @@ static void ext2_preread_inode(struct in
Re: [PATCH e2fsprogs] properly calculate overhead in ext2fs_initialize()
Theodore Tso wrote: On Fri, Jul 20, 2007 at 04:47:14PM -0500, Eric Sandeen wrote: For some odd geometries*, mkfs will try to allocate inode tables off the end of the block group and fail, rather than warning that too many inodes have been requested. This is because when ext2fs_initialize calculates metadata overhead, it is only adding in group descriptor blocks and the superblock if the *last* bg contains them - but the first bg also has all of the various metadata bits taking up space. Unconditionally adding those counts into the overhead seems to fix it properly. Yes, but it screws up the test right after that. We need to calculate the overheads for both the first and last block group. When I respun your patch and applied it to git, I fixed this. Yep, I missed that. Thanks for catching I also added an explicit Addresses-Red-Hat-Bugzilla: entry to the commit log. (This is mainly for your convenience when you're cherry-picking patches.) Thanks, I appreciate that. Finally, your patch didn't apply because you apparently nuked a newline in the patch here: Urhg, sorry. Not sure how that happened. Thanks, -Eric - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] ext2: show all mount options
From: Miklos Szeredi [EMAIL PROTECTED]
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux-2.6/fs/ext2/super.c
===
--- linux-2.6.orig/fs/ext2/super.c 2007-07-23 17:41:29.0 +0200
+++ linux-2.6/fs/ext2/super.c 2007-07-23 18:02:38.0 +0200
@@ -203,10 +203,66 @@ static void ext2_clear_inode(struct inod
static int ext2_show_options(struct seq_file *seq, struct vfsmount *vfs)
{
- struct ext2_sb_info *sbi = EXT2_SB(vfs-mnt_sb);
+ struct super_block *sb = vfs-mnt_sb;
+ struct ext2_sb_info *sbi = EXT2_SB(sb);
+ struct ext2_super_block *es = sbi-s_es;
+ unsigned long def_mount_opts;
+
+ def_mount_opts = le32_to_cpu(es-s_default_mount_opts);
- if (sbi-s_mount_opt EXT2_MOUNT_GRPID)
+ if (sbi-s_sb_block != 1)
+ seq_printf(seq, ,sb=%lu, sbi-s_sb_block);
+ if (test_opt(sb, MINIX_DF))
+ seq_puts(seq, ,minixdf);
+ if (test_opt(sb, GRPID))
seq_puts(seq, ,grpid);
+ if (!test_opt(sb, GRPID) (def_mount_opts EXT2_DEFM_BSDGROUPS))
+ seq_puts(seq, ,nogrpid);
+ if (sbi-s_resuid != EXT2_DEF_RESUID ||
+ le16_to_cpu(es-s_def_resuid) != EXT2_DEF_RESUID) {
+ seq_printf(seq, ,resuid=%u, sbi-s_resuid);
+ }
+ if (sbi-s_resgid != EXT2_DEF_RESGID ||
+ le16_to_cpu(es-s_def_resgid) != EXT2_DEF_RESGID) {
+ seq_printf(seq, ,resgid=%u, sbi-s_resgid);
+ }
+ if (test_opt(sb, ERRORS_CONT)) {
+ int def_errors = le16_to_cpu(es-s_errors);
+
+ if (def_errors == EXT2_ERRORS_PANIC ||
+ def_errors == EXT2_ERRORS_RO) {
+ seq_puts(seq, ,errors=continue);
+ }
+ }
+ if (test_opt(sb, ERRORS_RO))
+ seq_puts(seq, ,errors=remount-ro);
+ if (test_opt(sb, ERRORS_PANIC))
+ seq_puts(seq, ,errors=panic);
+ if (test_opt(sb, NO_UID32))
+ seq_puts(seq, ,nouid32);
+ if (test_opt(sb, DEBUG))
+ seq_puts(seq, ,debug);
+ if (test_opt(sb, OLDALLOC))
+ seq_puts(seq, ,oldalloc);
+
+#ifdef CONFIG_EXT2_FS_XATTR
+ if (test_opt(sb, XATTR_USER))
+ seq_puts(seq, ,user_xattr);
+ if (!test_opt(sb, XATTR_USER)
+ (def_mount_opts EXT2_DEFM_XATTR_USER)) {
+ seq_puts(seq, ,nouser_xattr);
+ }
+#endif
+
+#ifdef CONFIG_EXT2_FS_POSIX_ACL
+ if (test_opt(sb, POSIX_ACL))
+ seq_puts(seq, ,acl);
+ if (!test_opt(sb, POSIX_ACL) (def_mount_opts EXT2_DEFM_ACL))
+ seq_puts(seq, ,noacl);
+#endif
+
+ if (test_opt(sb, NOBH))
+ seq_puts(seq, ,nobh);
#if defined(CONFIG_QUOTA)
if (sbi-s_mount_opt EXT2_MOUNT_USRQUOTA)
@@ -658,6 +714,7 @@ static int ext2_fill_super(struct super_
if (!sbi)
return -ENOMEM;
sb-s_fs_info = sbi;
+ sbi-s_sb_block = sb_block;
/*
* See what the current blocksize for the device is, and
Index: linux-2.6/include/linux/ext2_fs_sb.h
===
--- linux-2.6.orig/include/linux/ext2_fs_sb.h 2007-07-23 17:41:29.0
+0200
+++ linux-2.6/include/linux/ext2_fs_sb.h2007-07-23 18:01:40.0
+0200
@@ -39,6 +39,7 @@ struct ext2_sb_info {
struct ext2_super_block * s_es; /* Pointer to the super block in the
buffer */
struct buffer_head ** s_group_desc;
unsigned long s_mount_opt;
+ unsigned long s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
unsigned short s_mount_state;
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] ext4: show all mount options
From: Miklos Szeredi [EMAIL PROTECTED]
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux-2.6/fs/ext4/super.c
===
--- linux-2.6.orig/fs/ext4/super.c 2007-07-20 12:22:35.0 +0200
+++ linux-2.6/fs/ext4/super.c 2007-07-23 21:02:47.0 +0200
@@ -596,9 +596,80 @@ static inline void ext4_show_quota_optio
#endif
}
+/*
+ * Show an option if
+ * - it's set to a non-default value OR
+ * - if the per-sb default is different from the global default
+ */
static int ext4_show_options(struct seq_file *seq, struct vfsmount *vfs)
{
struct super_block *sb = vfs-mnt_sb;
+ struct ext4_sb_info *sbi = EXT4_SB(sb);
+ struct ext4_super_block *es = sbi-s_es;
+ unsigned long def_mount_opts;
+
+ def_mount_opts = le32_to_cpu(es-s_default_mount_opts);
+
+ if (sbi-s_sb_block != 1)
+ seq_printf(seq, ,sb=%llu, sbi-s_sb_block);
+ if (test_opt(sb, MINIX_DF))
+ seq_puts(seq, ,minixdf);
+ if (test_opt(sb, GRPID))
+ seq_puts(seq, ,grpid);
+ if (!test_opt(sb, GRPID) (def_mount_opts EXT4_DEFM_BSDGROUPS))
+ seq_puts(seq, ,nogrpid);
+ if (sbi-s_resuid != EXT4_DEF_RESUID ||
+ le16_to_cpu(es-s_def_resuid) != EXT4_DEF_RESUID) {
+ seq_printf(seq, ,resuid=%u, sbi-s_resuid);
+ }
+ if (sbi-s_resgid != EXT4_DEF_RESGID ||
+ le16_to_cpu(es-s_def_resgid) != EXT4_DEF_RESGID) {
+ seq_printf(seq, ,resgid=%u, sbi-s_resgid);
+ }
+ if (test_opt(sb, ERRORS_CONT)) {
+ int def_errors = le16_to_cpu(es-s_errors);
+
+ if (def_errors == EXT4_ERRORS_PANIC ||
+ def_errors == EXT4_ERRORS_RO) {
+ seq_puts(seq, ,errors=continue);
+ }
+ }
+ if (test_opt(sb, ERRORS_RO))
+ seq_puts(seq, ,errors=remount-ro);
+ if (test_opt(sb, ERRORS_PANIC))
+ seq_puts(seq, ,errors=panic);
+ if (test_opt(sb, NO_UID32))
+ seq_puts(seq, ,nouid32);
+ if (test_opt(sb, DEBUG))
+ seq_puts(seq, ,debug);
+ if (test_opt(sb, OLDALLOC))
+ seq_puts(seq, ,oldalloc);
+#ifdef CONFIG_EXT4_FS_XATTR
+ if (test_opt(sb, XATTR_USER))
+ seq_puts(seq, ,user_xattr);
+ if (!test_opt(sb, XATTR_USER)
+ (def_mount_opts EXT4_DEFM_XATTR_USER)) {
+ seq_puts(seq, ,nouser_xattr);
+ }
+#endif
+#ifdef CONFIG_EXT4_FS_POSIX_ACL
+ if (test_opt(sb, POSIX_ACL))
+ seq_puts(seq, ,acl);
+ if (!test_opt(sb, POSIX_ACL) (def_mount_opts EXT4_DEFM_ACL))
+ seq_puts(seq, ,noacl);
+#endif
+ if (!test_opt(sb, RESERVATION))
+ seq_puts(seq, ,noreservation);
+ if (sbi-s_commit_interval) {
+ seq_printf(seq, ,commit=%u,
+ (unsigned) (sbi-s_commit_interval / HZ));
+ }
+ if (test_opt(sb, BARRIER))
+ seq_puts(seq, ,barrier=1);
+ if (test_opt(sb, NOBH))
+ seq_puts(seq, ,nobh);
+ if (!test_opt(sb, EXTENTS))
+ seq_puts(seq, ,noextents);
if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)
seq_puts(seq, ,data=journal);
@@ -1487,6 +1558,7 @@ static int ext4_fill_super (struct super
sbi-s_mount_opt = 0;
sbi-s_resuid = EXT4_DEF_RESUID;
sbi-s_resgid = EXT4_DEF_RESGID;
+ sbi-s_sb_block = sb_block;
unlock_kernel();
Index: linux-2.6/include/linux/ext4_fs_sb.h
===
--- linux-2.6.orig/include/linux/ext4_fs_sb.h 2007-07-20 12:22:40.0
+0200
+++ linux-2.6/include/linux/ext4_fs_sb.h2007-07-23 20:55:28.0
+0200
@@ -45,6 +45,7 @@ struct ext4_sb_info {
struct ext4_super_block * s_es; /* Pointer to the super block in the
buffer */
struct buffer_head ** s_group_desc;
unsigned long s_mount_opt;
+ ext4_fsblk_t s_sb_block;
uid_t s_resuid;
gid_t s_resgid;
unsigned short s_mount_state;
-
To unsubscribe from this list: send the line unsubscribe linux-ext4 in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ext2: show all mount options
On Mon, 23 Jul 2007 22:12:54 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: From: Miklos Szeredi [EMAIL PROTECTED] Signed-off-by: Miklos Szeredi [EMAIL PROTECTED] Could we have changelogs for these patches, please? ie: what's wrong with the existing code, what change does this patch make? - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ext2: show all mount options
On Mon, Jul 23, 2007 at 01:56:42PM -0700, Andrew Morton wrote: On Mon, 23 Jul 2007 22:12:54 +0200 Miklos Szeredi [EMAIL PROTECTED] wrote: From: Miklos Szeredi [EMAIL PROTECTED] Signed-off-by: Miklos Szeredi [EMAIL PROTECTED] Could we have changelogs for these patches, please? ie: what's wrong with the existing code, what change does this patch make? The idea is to allow /proc/mounts to display all of the mount options that were used to mount a particular filesystem. This is useful if you want /proc/mounts to replace /etc/mtab. I keep thinking that this is really the wrong approach, though, since it means adding a lot of coding to every single filesystem to reconstruct the mount options, and in some cases it will still never be enough to reconstruct exactly what was in /etc/mtab. (For example, the fully qualified domain named passed into some remote filesystem.) It seems to me the right answer would be to enhance the mount(2) system call with a new mount operation which would allow the user space mount command can stash exactly the options used to mount the filesystem. That we the kernel can store the exact ascii string in allocated memory and regurgitate it for the benefit of /proc/mounts, instead of adding a lot of code into ext2, ext3, ext4, et. al in an attempt partially reconstruct the mount options. - Ted - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] readahead drop behind and size adjustment
On Tue, 24 Jul 2007 08:47:28 +0800
Fengguang Wu [EMAIL PROTECTED] wrote:
Subject: convert ill defined log2() to ilog2()
It's *wrong* to have
#define log2(n) ffz(~(n))
It should be *reversed*:
#define log2(n) flz(~(n))
or
#define log2(n) fls(n)
or just use
ilog2(n) defined in linux/log2.h.
This patch follows the last solution, recommended by Andrew Morton.
//Or are they simply the wrong naming, and is in fact correct in behavior?
Cc: linux-ext4@vger.kernel.org
Cc: Mingming Cao [EMAIL PROTECTED]
Cc: Bjorn Helgaas [EMAIL PROTECTED]
Cc: Chris Ahna [EMAIL PROTECTED]
Cc: David Mosberger-Tang [EMAIL PROTECTED]
Cc: Kyle McMartin [EMAIL PROTECTED]
Signed-off-by: Fengguang Wu [EMAIL PROTECTED]
---
drivers/char/agp/hp-agp.c |9 +++--
drivers/char/agp/i460-agp.c |5 ++---
drivers/char/agp/parisc-agp.c |7 ++-
fs/ext4/super.c |6 ++
4 files changed, 9 insertions(+), 18 deletions(-)
hm, yes, there is a risk that the code was accidentally correct. Or the
code has only ever dealt with power-of-2 inputs, in which case it happens
to work either way.
David(s) and ext4-people: could we please have a close review of these
changes?
Thanks.
--- linux-2.6.22-rc6-mm1.orig/drivers/char/agp/hp-agp.c
+++ linux-2.6.22-rc6-mm1/drivers/char/agp/hp-agp.c
@@ -14,15 +14,12 @@
#include linux/pci.h
#include linux/init.h
#include linux/agp_backend.h
+#include linux/log2.h
#include asm/acpi-ext.h
#include agp.h
-#ifndef log2
-#define log2(x) ffz(~(x))
-#endif
-
#define HP_ZX1_IOC_OFFSET0x1000 /* ACPI reports SBA, we want IOC */
/* HP ZX1 IOC registers */
@@ -256,7 +253,7 @@ hp_zx1_configure (void)
readl(hp-ioc_regs+HP_ZX1_IMASK);
writel(hp-iova_base|1, hp-ioc_regs+HP_ZX1_IBASE);
readl(hp-ioc_regs+HP_ZX1_IBASE);
- writel(hp-iova_base|log2(HP_ZX1_IOVA_SIZE),
hp-ioc_regs+HP_ZX1_PCOM);
+ writel(hp-iova_base|ilog2(HP_ZX1_IOVA_SIZE),
hp-ioc_regs+HP_ZX1_PCOM);
readl(hp-ioc_regs+HP_ZX1_PCOM);
}
@@ -284,7 +281,7 @@ hp_zx1_tlbflush (struct agp_memory *mem)
{
struct _hp_private *hp = hp_private;
- writeq(hp-gart_base | log2(hp-gart_size), hp-ioc_regs+HP_ZX1_PCOM);
+ writeq(hp-gart_base | ilog2(hp-gart_size), hp-ioc_regs+HP_ZX1_PCOM);
readq(hp-ioc_regs+HP_ZX1_PCOM);
}
--- linux-2.6.22-rc6-mm1.orig/drivers/char/agp/i460-agp.c
+++ linux-2.6.22-rc6-mm1/drivers/char/agp/i460-agp.c
@@ -13,6 +13,7 @@
#include linux/string.h
#include linux/slab.h
#include linux/agp_backend.h
+#include linux/log2.h
#include agp.h
@@ -59,8 +60,6 @@
*/
#define WR_FLUSH_GATT(index) RD_GATT(index)
-#define log2(x) ffz(~(x))
-
static struct {
void *gatt; /* ioremap'd GATT area */
@@ -148,7 +147,7 @@ static int i460_fetch_size (void)
* values[i].size.
*/
values[i].num_entries = (values[i].size 8)
(I460_IO_PAGE_SHIFT - 12);
- values[i].page_order = log2((sizeof(u32)*values[i].num_entries)
PAGE_SHIFT);
+ values[i].page_order =
ilog2((sizeof(u32)*values[i].num_entries) PAGE_SHIFT);
}
for (i = 0; i agp_bridge-driver-num_aperture_sizes; i++) {
--- linux-2.6.22-rc6-mm1.orig/drivers/char/agp/parisc-agp.c
+++ linux-2.6.22-rc6-mm1/drivers/char/agp/parisc-agp.c
@@ -18,6 +18,7 @@
#include linux/init.h
#include linux/klist.h
#include linux/agp_backend.h
+#include linux/log2.h
#include asm-parisc/parisc-device.h
#include asm-parisc/ropes.h
@@ -27,10 +28,6 @@
#define DRVNAME quicksilver
#define DRVPFX DRVNAME :
-#ifndef log2
-#define log2(x) ffz(~(x))
-#endif
-
#define AGP8X_MODE_BIT 3
#define AGP8X_MODE (1 AGP8X_MODE_BIT)
@@ -92,7 +89,7 @@ parisc_agp_tlbflush(struct agp_memory *m
{
struct _parisc_agp_info *info = parisc_agp_info;
- writeq(info-gart_base | log2(info-gart_size),
info-ioc_regs+IOC_PCOM);
+ writeq(info-gart_base | ilog2(info-gart_size),
info-ioc_regs+IOC_PCOM);
readq(info-ioc_regs+IOC_PCOM); /* flush */
}
--- linux-2.6.22-rc6-mm1.orig/fs/ext4/super.c
+++ linux-2.6.22-rc6-mm1/fs/ext4/super.c
@@ -1433,8 +1433,6 @@ static void ext4_orphan_cleanup (struct
sb-s_flags = s_flags; /* Restore MS_RDONLY status */
}
-#define log2(n) ffz(~(n))
-
/*
* Maximal file size. There is a direct, and {,double-,triple-}indirect
* block limit, and also a limit of (2^32 - 1) 512-byte sectors in i_blocks.
@@ -1706,8 +1704,8 @@ static int ext4_fill_super (struct super
sbi-s_desc_per_block = blocksize / EXT4_DESC_SIZE(sb);
sbi-s_sbh = bh;
sbi-s_mount_state = le16_to_cpu(es-s_state);
-
+ convert-ill-defined-log2-to-ilog2.patch added to -mm tree
The patch titled
convert ill defined log2() to ilog2()
has been added to the -mm tree. Its filename is
convert-ill-defined-log2-to-ilog2.patch
*** Remember to use Documentation/SubmitChecklist when testing your code ***
See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this
--
Subject: convert ill defined log2() to ilog2()
From: Fengguang Wu [EMAIL PROTECTED]
It's *wrong* to have
#define log2(n) ffz(~(n))
It should be *reversed*:
#define log2(n) flz(~(n))
or
#define log2(n) fls(n)
or just use
ilog2(n) defined in linux/log2.h.
This patch follows the last solution, recommended by Andrew Morton.
Or are they simply the wrong naming, and is in fact correct in behavior?
Cc: linux-ext4@vger.kernel.org
Cc: Mingming Cao [EMAIL PROTECTED]
Cc: Bjorn Helgaas [EMAIL PROTECTED]
Cc: Chris Ahna [EMAIL PROTECTED]
Cc: David Mosberger-Tang [EMAIL PROTECTED]
Cc: Kyle McMartin [EMAIL PROTECTED]
Cc: Dave Airlie [EMAIL PROTECTED]
Cc: Dave Jones [EMAIL PROTECTED]
Signed-off-by: Fengguang Wu [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---
drivers/char/agp/hp-agp.c |9 +++--
drivers/char/agp/i460-agp.c |5 ++---
drivers/char/agp/parisc-agp.c |7 ++-
fs/ext4/super.c |6 ++
4 files changed, 9 insertions(+), 18 deletions(-)
diff -puN drivers/char/agp/hp-agp.c~convert-ill-defined-log2-to-ilog2
drivers/char/agp/hp-agp.c
--- a/drivers/char/agp/hp-agp.c~convert-ill-defined-log2-to-ilog2
+++ a/drivers/char/agp/hp-agp.c
@@ -14,15 +14,12 @@
#include linux/pci.h
#include linux/init.h
#include linux/agp_backend.h
+#include linux/log2.h
#include asm/acpi-ext.h
#include agp.h
-#ifndef log2
-#define log2(x)ffz(~(x))
-#endif
-
#define HP_ZX1_IOC_OFFSET 0x1000 /* ACPI reports SBA, we want IOC */
/* HP ZX1 IOC registers */
@@ -256,7 +253,7 @@ hp_zx1_configure (void)
readl(hp-ioc_regs+HP_ZX1_IMASK);
writel(hp-iova_base|1, hp-ioc_regs+HP_ZX1_IBASE);
readl(hp-ioc_regs+HP_ZX1_IBASE);
- writel(hp-iova_base|log2(HP_ZX1_IOVA_SIZE),
hp-ioc_regs+HP_ZX1_PCOM);
+ writel(hp-iova_base|ilog2(HP_ZX1_IOVA_SIZE),
hp-ioc_regs+HP_ZX1_PCOM);
readl(hp-ioc_regs+HP_ZX1_PCOM);
}
@@ -284,7 +281,7 @@ hp_zx1_tlbflush (struct agp_memory *mem)
{
struct _hp_private *hp = hp_private;
- writeq(hp-gart_base | log2(hp-gart_size), hp-ioc_regs+HP_ZX1_PCOM);
+ writeq(hp-gart_base | ilog2(hp-gart_size), hp-ioc_regs+HP_ZX1_PCOM);
readq(hp-ioc_regs+HP_ZX1_PCOM);
}
diff -puN drivers/char/agp/i460-agp.c~convert-ill-defined-log2-to-ilog2
drivers/char/agp/i460-agp.c
--- a/drivers/char/agp/i460-agp.c~convert-ill-defined-log2-to-ilog2
+++ a/drivers/char/agp/i460-agp.c
@@ -13,6 +13,7 @@
#include linux/string.h
#include linux/slab.h
#include linux/agp_backend.h
+#include linux/log2.h
#include agp.h
@@ -59,8 +60,6 @@
*/
#define WR_FLUSH_GATT(index) RD_GATT(index)
-#define log2(x)ffz(~(x))
-
static struct {
void *gatt; /* ioremap'd GATT area */
@@ -148,7 +147,7 @@ static int i460_fetch_size (void)
* values[i].size.
*/
values[i].num_entries = (values[i].size 8)
(I460_IO_PAGE_SHIFT - 12);
- values[i].page_order = log2((sizeof(u32)*values[i].num_entries)
PAGE_SHIFT);
+ values[i].page_order =
ilog2((sizeof(u32)*values[i].num_entries) PAGE_SHIFT);
}
for (i = 0; i agp_bridge-driver-num_aperture_sizes; i++) {
diff -puN drivers/char/agp/parisc-agp.c~convert-ill-defined-log2-to-ilog2
drivers/char/agp/parisc-agp.c
--- a/drivers/char/agp/parisc-agp.c~convert-ill-defined-log2-to-ilog2
+++ a/drivers/char/agp/parisc-agp.c
@@ -18,6 +18,7 @@
#include linux/init.h
#include linux/klist.h
#include linux/agp_backend.h
+#include linux/log2.h
#include asm-parisc/parisc-device.h
#include asm-parisc/ropes.h
@@ -27,10 +28,6 @@
#define DRVNAMEquicksilver
#define DRVPFX DRVNAME :
-#ifndef log2
-#define log2(x)ffz(~(x))
-#endif
-
#define AGP8X_MODE_BIT 3
#define AGP8X_MODE (1 AGP8X_MODE_BIT)
@@ -92,7 +89,7 @@ parisc_agp_tlbflush(struct agp_memory *m
{
struct _parisc_agp_info *info = parisc_agp_info;
- writeq(info-gart_base | log2(info-gart_size),
info-ioc_regs+IOC_PCOM);
+ writeq(info-gart_base | ilog2(info-gart_size),
info-ioc_regs+IOC_PCOM);
readq(info-ioc_regs+IOC_PCOM); /* flush */
}
diff -puN fs/ext4/super.c~convert-ill-defined-log2-to-ilog2 fs/ext4/super.c
--- a/fs/ext4/super.c~convert-ill-defined-log2-to-ilog2
+++ a/fs/ext4/super.c
@@ -1415,8 +1415,6 @@ static void ext4_orphan_cleanup
[GIT PULL] ext[234] bugfixes/cleanup
Hi Linus, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git for_linus Many thanks!! - Ted Eric Sandeen (2): Fix fencepost error in ext[234]_check_descriptors Remove unused bh in calls to ext[234]_get_group_desc Mingming Cao (1): Some cleanups in ext4/JBD2 to follow the naming rules b/fs/ext2/ialloc.c | 24 b/fs/ext2/inode.c |2 +- b/fs/ext2/super.c |2 +- b/fs/ext3/ialloc.c | 17 +++-- b/fs/ext3/super.c |2 +- b/fs/ext4/extents.c |2 +- b/fs/ext4/ialloc.c | 17 +++-- b/fs/ext4/super.c |2 +- b/fs/jbd2/commit.c |2 +- b/fs/jbd2/journal.c | 26 +- b/fs/jbd2/recovery.c|2 +- b/fs/jbd2/revoke.c |4 ++-- b/include/linux/ext4_jbd2.h |6 +++--- b/include/linux/jbd2.h | 30 +++--- b/include/linux/poison.h|3 ++- fs/ext4/super.c |2 +- 16 files changed, 65 insertions(+), 78 deletions(-) - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ext2: show all mount options
The idea is to allow /proc/mounts to display all of the mount options that were used to mount a particular filesystem. This is useful if you want /proc/mounts to replace /etc/mtab. I keep thinking that this is really the wrong approach, though, since it means adding a lot of coding to every single filesystem to reconstruct the mount options, and in some cases it will still never be enough to reconstruct exactly what was in /etc/mtab. (For example, the fully qualified domain named passed into some remote filesystem.) It seems to me the right answer would be to enhance the mount(2) system call with a new mount operation which would allow the user space mount command can stash exactly the options used to mount the filesystem. Well, that would be good for emulating /etc/mtab. But the problem is that that's wrong a lot of times. For example in ext* mount -oremount,opt doesn't reset all options, it just changes opt. And this is different from fs to fs, so there's no easy way to handle it from generic code. I think it shouln't be hard to keep the shown mount options in sync with the parsed options. The problem is just that people have been lazy to do that, because /etc/mtab was good enough. Miklos - To unsubscribe from this list: send the line unsubscribe linux-ext4 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
