Forking ext4 filesystem from ext3 filesystem
Subsystems (2)
PATCH 1/5 Forking ext4 filesystem from ext3 filesystem
Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from ext3_xxx() to ext4_xxx().
---
linux-2.6.18-rc4-ming/fs/ext4/Makefile | 12 linux-2.6.18-rc4-ming/fs/ext4/acl.c | 551 +++ linux-2.6.18-rc4-ming/fs/ext4/acl.h | 81 linux-2.6.18-rc4-ming/fs/ext4/balloc.c | 1612 +++++++++++ linux-2.6.18-rc4-ming/fs/ext4/bitmap.c | 32 linux-2.6.18-rc4-ming/fs/ext4/dir.c | 516 +++ linux-2.6.18-rc4-ming/fs/ext4/file.c | 137 linux-2.6.18-rc4-ming/fs/ext4/fsync.c | 88 linux-2.6.18-rc4-ming/fs/ext4/hash.c | 152 + linux-2.6.18-rc4-ming/fs/ext4/ialloc.c | 759 +++++ linux-2.6.18-rc4-ming/fs/ext4/inode.c | 3217 +++++++++++++++++++++++ linux-2.6.18-rc4-ming/fs/ext4/ioctl.c | 254 + linux-2.6.18-rc4-ming/fs/ext4/namei.c | 2385 +++++++++++++++++ linux-2.6.18-rc4-ming/fs/ext4/namei.h | 8 linux-2.6.18-rc4-ming/fs/ext4/resize.c | 1024 +++++++ linux-2.6.18-rc4-ming/fs/ext4/super.c | 2710 +++++++++++++++++++ linux-2.6.18-rc4-ming/fs/ext4/symlink.c | 54 linux-2.6.18-rc4-ming/fs/ext4/xattr.c | 1317 +++++++++ linux-2.6.18-rc4-ming/fs/ext4/xattr.h | 145 + linux-2.6.18-rc4-ming/fs/ext4/xattr_security.c | 77 linux-2.6.18-rc4-ming/fs/ext4/xattr_trusted.c | 62 linux-2.6.18-rc4-ming/fs/ext4/xattr_user.c | 64 linux-2.6.18-rc4-ming/include/linux/ext4_fs.h | 862 ++++++ linux-2.6.18-rc4-ming/include/linux/ext4_fs_i.h | 147 + linux-2.6.18-rc4-ming/include/linux/ext4_fs_sb.h | 83 linux-2.6.18-rc4-ming/include/linux/ext4_jbd.h | 268 + 26 files changed, 16617 insertions(+)
fs/ext4/acl.c new file +551 -0
@@ -0,0 +1,551 @@ +/* +* linux/fs/ext4/acl.c +* +* Copyright (C) 2001-2003 Andreas Gruenbacher, <[email protected]> +*/ + +#include <linux/init.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/capability.h> +#include <linux/fs.h> +#include <linux/ext4_jbd.h> +#include <linux/ext4_fs.h> +#include "xattr.h" +#include "acl.h" + +/* +* Convert from filesystem to in-memory representation. +*/ +static struct posix_acl * +ext4_acl_from_disk (const void *value,size_t size) +{ +const char *end = (char *)value + size; +int n, count; +struct posix_acl *acl; + +if (!value) +return NULL ; +if (size <sizeof (ext4_acl_header)) +return ERR_PTR (-EINVAL ); +if (((ext4_acl_header *)value)->a_version != +cpu_to_le32 (EXT4_ACL_VERSION )) +return ERR_PTR (-EINVAL ); + value = (char *)value +sizeof (ext4_acl_header); + count =ext4_acl_count (size); +if (count <0 ) +return ERR_PTR (-EINVAL ); +if (count ==0 ) +return NULL ; + acl =posix_acl_alloc (count,GFP_KERNEL ); +if (!acl) +return ERR_PTR (-ENOMEM ); +for (n=0 ; n < count; n++) { +ext4_acl_entry *entry = + (ext4_acl_entry *)value; +if ((char *)value +sizeof (ext4_acl_entry_short) > end) + goto fail; + acl->a_entries[n].e_tag =le16_to_cpu (entry->e_tag); + acl->a_entries[n].e_perm =le16_to_cpu (entry->e_perm); +switch (acl->a_entries[n].e_tag) { +case ACL_USER_OBJ : +case ACL_GROUP_OBJ : +case ACL_MASK : +case ACL_OTHER : + value = (char *)value + +sizeof (ext4_acl_entry_short); + acl->a_entries[n].e_id =ACL_UNDEFINED_ID ; +break ; + +case ACL_USER : +case ACL_GROUP : + value = (char *)value +sizeof (ext4_acl_entry); +if ((char *)value > end) + goto fail; + acl->a_entries[n].e_id = +le32_to_cpu (entry->e_id); +break ; + +default : + goto fail; + } + } +if (value != end) + goto fail; +return acl; + +fail: +posix_acl_release (acl); +return ERR_PTR (-EINVAL ); +} + +/* +* Convert from in-memory to filesystem representation. +*/ +static void * +ext4_acl_to_disk (const struct posix_acl *acl,size_t *size) +{ +ext4_acl_header *ext_acl; +char *e; +size_t n; + + *size =ext4_acl_size (acl->a_count); + ext_acl = (ext4_acl_header *)kmalloc (sizeof (ext4_acl_header) + + acl->a_count *sizeof (ext4_acl_entry),GFP_KERNEL ); +if (!ext_acl) +return ERR_PTR (-ENOMEM ); + ext_acl->a_version =cpu_to_le32 (EXT4_ACL_VERSION ); + e = (char *)ext_acl +sizeof (ext4_acl_header); +for (n=0 ; n < acl->a_count; n++) { +ext4_acl_entry *entry = (ext4_acl_entry *)e; + entry->e_tag =cpu_to_le16 (acl->a_entries[n].e_tag); + entry->e_perm =cpu_to_le16 (acl->a_entries[n].e_perm); +switch (acl->a_entries[n].e_tag) { +case ACL_USER : +case ACL_GROUP : + entry->e_id = +cpu_to_le32 (acl->a_entries[n].e_id); + e +=sizeof (ext4_acl_entry); +break ; + +case ACL_USER_OBJ : +case ACL_GROUP_OBJ : +case ACL_MASK : +case ACL_OTHER : + e +=sizeof (ext4_acl_entry_short); +break ; + +default : + goto fail; + } + } +return (char *)ext_acl; + +fail: +kfree (ext_acl); +return ERR_PTR (-EINVAL ); +} + +static inline struct posix_acl * +ext4_iget_acl (struct inode *inode,struct posix_acl **i_acl) +{ +struct posix_acl *acl =EXT4_ACL_NOT_CACHED ; + +spin_lock (&inode->i_lock); +if (*i_acl !=EXT4_ACL_NOT_CACHED ) + acl =posix_acl_dup (*i_acl); +spin_unlock (&inode->i_lock); + +return acl; +} + +static inline void +ext4_iset_acl (struct inode *inode,struct posix_acl **i_acl, +struct posix_acl *acl) +{ +spin_lock (&inode->i_lock); +if (*i_acl !=EXT4_ACL_NOT_CACHED ) +posix_acl_release (*i_acl); + *i_acl =posix_acl_dup (acl); +spin_unlock (&inode->i_lock); +} + +/* +* Inode operation get_posix_acl(). +* +* inode->i_mutex: don't care +*/ +static struct posix_acl * +ext4_get_acl (struct inode *inode,int type) +{ +struct ext4_inode_info *ei =EXT4_I (inode); +int name_index; +char *value =NULL ; +struct posix_acl *acl; +int retval; + +if (!test_opt (inode->i_sb,POSIX_ACL )) +return NULL ; + +switch (type) { +case ACL_TYPE_ACCESS : + acl =ext4_iget_acl (inode, &ei->i_acl); +if (acl !=EXT4_ACL_NOT_CACHED ) +return acl; + name_index =EXT4_XATTR_INDEX_POSIX_ACL_ACCESS ; +break ; + +case ACL_TYPE_DEFAULT : + acl =ext4_iget_acl (inode, &ei->i_default_acl); +if (acl !=EXT4_ACL_NOT_CACHED ) +return acl; + name_index =EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT ; +break ; + +default : +return ERR_PTR (-EINVAL ); + } + retval =ext4_xattr_get (inode, name_index,"" ,NULL ,0 ); +if (retval >0 ) { + value =kmalloc (retval,GFP_KERNEL ); +if (!value) +return ERR_PTR (-ENOMEM ); + retval =ext4_xattr_get (inode, name_index,"" , value, retval); + } +if (retval >0 ) + acl =ext4_acl_from_disk (value, retval); +else if (retval == -ENODATA || retval == -ENOSYS ) + acl =NULL ; +else + acl =ERR_PTR (retval); +kfree (value); + +if (!IS_ERR (acl)) { +switch (type) { +case ACL_TYPE_ACCESS : +ext4_iset_acl (inode, &ei->i_acl, acl); +break ; + +case ACL_TYPE_DEFAULT : +ext4_iset_acl (inode, &ei->i_default_acl, acl); +break ; + } + } +return acl; +} + +/* +* Set the access or default ACL of an inode. +* +* inode->i_mutex: down unless called from ext4_new_inode +*/ +static int +ext4_set_acl (handle_t *handle,struct inode *inode,int type, +struct posix_acl *acl) +{ +struct ext4_inode_info *ei =EXT4_I (inode); +int name_index; +void *value =NULL ; +size_t size =0 ; +int error; + +if (S_ISLNK (inode->i_mode)) +return -EOPNOTSUPP ; + +switch (type) { +case ACL_TYPE_ACCESS : + name_index =EXT4_XATTR_INDEX_POSIX_ACL_ACCESS ; +if (acl) { +mode_t mode = inode->i_mode; + error =posix_acl_equiv_mode (acl, &mode); +if (error <0 ) +return error; +else { + inode->i_mode = mode; +ext4_mark_inode_dirty (handle, inode); +if (error ==0 ) + acl =NULL ; + } + } +break ; + +case ACL_TYPE_DEFAULT : + name_index =EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT ; +if (!S_ISDIR (inode->i_mode)) +return acl ? -EACCES :0 ; +break ; + +default : +return -EINVAL ; + } +if (acl) { + value =ext4_acl_to_disk (acl, &size); +if (IS_ERR (value)) +return (int )PTR_ERR (value); + } + + error =ext4_xattr_set_handle (handle, inode, name_index,"" , + value, size,0 ); + +kfree (value); +if (!error) { +switch (type) { +case ACL_TYPE_ACCESS : +ext4_iset_acl (inode, &ei->i_acl, acl); +break ; + +case ACL_TYPE_DEFAULT : +ext4_iset_acl (inode, &ei->i_default_acl, acl); +break ; + } + } +return error; +} + +static int +ext4_check_acl (struct inode *inode,int mask) +{ +struct posix_acl *acl =ext4_get_acl (inode,ACL_TYPE_ACCESS ); + +if (IS_ERR (acl)) +return PTR_ERR (acl); +if (acl) { +int error =posix_acl_permission (inode, acl, mask); +posix_acl_release (acl); +return error; + } + +return -EAGAIN ; +} + +int +ext4_permission (struct inode *inode,int mask,struct nameidata *nd) +{ +return generic_permission (inode, mask, ext4_check_acl); +} + +/* +* Initialize the ACLs of a new inode. Called from ext4_new_inode. +* +* dir->i_mutex: down +* inode->i_mutex: up (access to inode is still exclusive) +*/ +int +ext4_init_acl (handle_t *handle,struct inode *inode,struct inode *dir) +{ +struct posix_acl *acl =NULL ; +int error =0 ; + +if (!S_ISLNK (inode->i_mode)) { +if (test_opt (dir->i_sb,POSIX_ACL )) { + acl =ext4_get_acl (dir,ACL_TYPE_DEFAULT ); +if (IS_ERR (acl)) +return PTR_ERR (acl); + } +if (!acl) + inode->i_mode &= ~current->fs->umask; + } +if (test_opt (inode->i_sb,POSIX_ACL ) && acl) { +struct posix_acl *clone; +mode_t mode; + +if (S_ISDIR (inode->i_mode)) { + error =ext4_set_acl (handle, inode, +ACL_TYPE_DEFAULT , acl); +if (error) + goto cleanup; + } + clone =posix_acl_clone (acl,GFP_KERNEL ); + error = -ENOMEM ; +if (!clone) + goto cleanup; + + mode = inode->i_mode; + error =posix_acl_create_masq (clone, &mode); +if (error >=0 ) { + inode->i_mode = mode; +if (error >0 ) { +/* This is an extended ACL */ + error =ext4_set_acl (handle, inode, +ACL_TYPE_ACCESS , clone); + } + } +posix_acl_release (clone); + } +cleanup: +posix_acl_release (acl); +return error; +} + +/* +* Does chmod for an inode that may have an Access Control List. The +* inode->i_mode field must be updated to the desired value by the caller +* before calling this function. +* Returns 0 on success, or a negative error number. +* +* We change the ACL rather than storing some ACL entries in the file +* mode permission bits (which would be more efficient), because that +* would break once additional permissions (like ACL_APPEND, ACL_DELETE +* for directories) are added. There are no more bits available in the +* file mode. +* +* inode->i_mutex: down +*/ +int +ext4_acl_chmod (struct inode *inode) +{ +struct posix_acl *acl, *clone; +int error; + +if (S_ISLNK (inode->i_mode)) +return -EOPNOTSUPP ; +if (!test_opt (inode->i_sb,POSIX_ACL )) +return 0 ; + acl =ext4_get_acl (inode,ACL_TYPE_ACCESS ); +if (IS_ERR (acl) || !acl) +return PTR_ERR (acl); + clone =posix_acl_clone (acl,GFP_KERNEL ); +posix_acl_release (acl); +if (!clone) +return -ENOMEM ; + error =posix_acl_chmod_masq (clone, inode->i_mode); +if (!error) { +handle_t *handle; +int retries =0 ; + + retry: + handle =ext4_journal_start (inode, +EXT4_DATA_TRANS_BLOCKS (inode->i_sb)); +if (IS_ERR (handle)) { + error =PTR_ERR (handle); +ext4_std_error (inode->i_sb, error); + goto out; + } + error =ext4_set_acl (handle, inode,ACL_TYPE_ACCESS , clone); +ext4_journal_stop (handle); +if (error == -ENOSPC && +ext4_should_retry_alloc (inode->i_sb, &retries)) + goto retry; + } +out: +posix_acl_release (clone); +return error; +} + +/* +* Extended attribute handlers +*/ +static size_t +ext4_xattr_list_acl_access (struct inode *inode,char *list,size_t list_len, +const char *name,size_t name_len) +{ +const size_t size =sizeof (POSIX_ACL_XATTR_ACCESS ); + +if (!test_opt (inode->i_sb,POSIX_ACL )) +return 0 ; +if (list && size <= list_len) +memcpy (list,POSIX_ACL_XATTR_ACCESS , size); +return size; +} + +static size_t +ext4_xattr_list_acl_default (struct inode *inode,char *list,size_t list_len, +const char *name,size_t name_len) +{ +const size_t size =sizeof (POSIX_ACL_XATTR_DEFAULT ); + +if (!test_opt (inode->i_sb,POSIX_ACL )) +return 0 ; +if (list && size <= list_len) +memcpy (list,POSIX_ACL_XATTR_DEFAULT , size); +return size; +} + +static int +ext4_xattr_get_acl (struct inode *inode,int type,void *buffer,size_t size) +{ +struct posix_acl *acl; +int error; + +if (!test_opt (inode->i_sb,POSIX_ACL )) +return -EOPNOTSUPP ; + + acl =ext4_get_acl (inode, type); +if (IS_ERR (acl)) +return PTR_ERR (acl); +if (acl ==NULL ) +return -ENODATA ; + error =posix_acl_to_xattr (acl, buffer, size); +posix_acl_release (acl); + +return error; +} + +static int +ext4_xattr_get_acl_access (struct inode *inode,const char *name, +void *buffer,size_t size) +{ +if (strcmp (name,"" ) !=0 ) +return -EINVAL ; +return ext4_xattr_get_acl (inode,ACL_TYPE_ACCESS , buffer, size); +} + +static int +ext4_xattr_get_acl_default (struct inode *inode,const char *name, +void *buffer,size_t size) +{ +if (strcmp (name,"" ) !=0 ) +return -EINVAL ; +return ext4_xattr_get_acl (inode,ACL_TYPE_DEFAULT , buffer, size); +} + +static int +ext4_xattr_set_acl (struct inode *inode,int type,const void *value, +size_t size) +{ +handle_t *handle; +struct posix_acl *acl; +int error, retries =0 ; + +if (!test_opt (inode->i_sb,POSIX_ACL )) +return -EOPNOTSUPP ; +if ((current->fsuid != inode->i_uid) && !capable (CAP_FOWNER )) +return -EPERM ; + +if (value) { + acl =posix_acl_from_xattr (value, size); +if (IS_ERR (acl)) +return PTR_ERR (acl); +else if (acl) { + error =posix_acl_valid (acl); +if (error) + goto release_and_out; + } + }else + acl =NULL ; + +retry: + handle =ext4_journal_start (inode,EXT4_DATA_TRANS_BLOCKS (inode->i_sb)); +if (IS_ERR (handle)) +return PTR_ERR (handle); + error =ext4_set_acl (handle, inode, type, acl); +ext4_journal_stop (handle); +if (error == -ENOSPC &&ext4_should_retry_alloc (inode->i_sb, &retries)) + goto retry; + +release_and_out: +posix_acl_release (acl); +return error; +} + +static int +ext4_xattr_set_acl_access (struct inode *inode,const char *name, +const void *value,size_t size,int flags) +{ +if (strcmp (name,"" ) !=0 ) +return -EINVAL ; +return ext4_xattr_set_acl (inode,ACL_TYPE_ACCESS , value, size); +} + +static int +ext4_xattr_set_acl_default (struct inode *inode,const char *name, +const void *value,size_t size,int flags) +{ +if (strcmp (name,"" ) !=0 ) +return -EINVAL ; +return ext4_xattr_set_acl (inode,ACL_TYPE_DEFAULT , value, size); +} + +struct xattr_handler ext4_xattr_acl_access_handler = { + .prefix =POSIX_ACL_XATTR_ACCESS , + .list = ext4_xattr_list_acl_access, + .get = ext4_xattr_get_acl_access, + .set = ext4_xattr_set_acl_access, +}; + +struct xattr_handler ext4_xattr_acl_default_handler = { + .prefix =POSIX_ACL_XATTR_DEFAULT , + .list = ext4_xattr_list_acl_default, + .get = ext4_xattr_get_acl_default, + .set = ext4_xattr_set_acl_default, +};
fs/ext4/acl.h new file +81 -0
81 lines not on this page · raw message
fs/ext4/balloc.c new file +1612 -0
1,612 lines not on this page · raw message
fs/ext4/bitmap.c new file +32 -0
@@ -0,0 +1,32 @@ +/* +* linux/fs/ext4/bitmap.c +* +* Copyright (C) 1992, 1993, 1994, 1995 +* Remy Card ([email protected]) +* Laboratoire MASI - Institut Blaise Pascal +* Universite Pierre et Marie Curie (Paris VI) +*/ + +#include <linux/buffer_head.h> +#include <linux/jbd.h> +#include <linux/ext4_fs.h> + +#ifdef EXT4FS_DEBUG + +static int nibblemap[] = {4 ,3 ,3 ,2 ,3 ,2 ,2 ,1 ,3 ,2 ,2 ,1 ,2 ,1 ,1 ,0 }; + +unsigned long ext4_count_free (struct buffer_head * map,unsigned int numchars) +{ +unsigned int i; +unsigned long sum =0 ; + +if (!map) +return (0 ); +for (i =0 ; i < numchars; i++) + sum += nibblemap[map->b_data[i] &0xf ] + + nibblemap[(map->b_data[i] >>4 ) &0xf ]; +return (sum); +} + +#endif /* EXT4FS_DEBUG */ +
fs/ext4/dir.c new file +516 -0
516 lines not on this page · raw message
fs/ext4/file.c new file +137 -0
137 lines not on this page · raw message
fs/ext4/fsync.c new file +88 -0
71 lines not on this page · raw message
fs/ext4/hash.c new file +152 -0
152 lines not on this page · raw message
fs/ext4/ialloc.c new file +759 -0
759 lines not on this page · raw message
fs/ext4/inode.c new file +3217 -0
3,217 lines not on this page · raw message
fs/ext4/ioctl.c new file +254 -0
254 lines not on this page · raw message
fs/ext4/namei.c new file +2385 -0
2,385 lines not on this page · raw message
fs/ext4/resize.c new file +1024 -0
1,024 lines not on this page · raw message
fs/ext4/super.c new file +2710 -0
2,710 lines not on this page · raw message
fs/ext4/symlink.c new file +54 -0
54 lines not on this page · raw message
fs/ext4/xattr.c new file +1317 -0
1,317 lines not on this page · raw message
fs/ext4/xattr.h new file +145 -0
145 lines not on this page · raw message
fs/ext4/xattr_security.c new file +77 -0
77 lines not on this page · raw message
fs/ext4/xattr_trusted.c new file +62 -0
62 lines not on this page · raw message
fs/ext4/xattr_user.c new file +64 -0
64 lines not on this page · raw message
include/linux/ext4_fs.h new file +862 -0
862 lines not on this page · raw message
include/linux/ext4_fs_i.h new file +147 -0
147 lines not on this page · raw message
include/linux/ext4_fs_sb.h new file +83 -0
83 lines not on this page · raw message
include/linux/ext4_jbd.h new file +268 -0
268 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
_
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Part of the diff is not shown here. View the raw message.
Quoted 1 line · On Wed, 09 Aug 2006 18:17:02 -0700 Mingming Cao <[email protected]> wrote:
Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from
ext3_xxx()toext4_xxx().
It would have been nice to spend a few hours cleaning up ext3 and JBD before doing this. The code isn't toooo bad, but there are number of coding style problems, whitespace screwups, incorrect comments, missing comments, poorly-chosen variable names and all of that sort of thing.
One the fs has been copied-and-pasted, it's much harder to address these
things: either need to do it twice, or allow the filesystems to diverge, or not do it.
Also, -mm presently has two patches pending against fs/jbd/ and nine pending against fs/ext3/. We should get all those things merged before taking the copy.
Also, JBD is presently feeding into submit_bh() buffer_heads which span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 7 lines · Andrew Morton
Quoted 1 line · On Wed, 09 Aug 2006 18:17:02 -0700 Mingming Cao <[email protected]> wrote:
Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from
ext3_xxx()toext4_xxx().
Quoted 9 lines
It would have been nice to spend a few hours cleaning up ext3 and JBD before doing this. The code isn't toooo bad, but there are number of coding style problems, whitespace screwups, incorrect comments, missing comments, poorly-chosen variable names and all of that sort of thing.
One the fs has been copied-and-pasted, it's much harder to address these
things: either need to do it twice, or allow the filesystems to diverge, or not do it.
Andrew, thanks for taking a close look this series of changes.
I agree with you that the timing is right, to do the clean up now rather than later. I would give it a try. If I could get more help from more code reviewer, it probably makes the effort a lot easier. For those issues you pointed out : coding style problem,incorrect comments, poorly-named variables -- do you have any specific examples in your mind?
Quoted 4 lines
Also, -mm presently has two patches pending against
fs/jbd/and nine pending againstfs/ext3/. We should get all those things merged before taking the copy.
So probably the right thing to do is keep the ext4 patches against mm tree instead of rc three?
Quoted 3 lines
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..
Okay, I will look at it.
Thanks,
Mingming
-
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 6 lines · Mingming Cao
I agree with you that the timing is right, to do the clean up now rather than later. I would give it a try. If I could get more help from more code reviewer, it probably makes the effort a lot easier. For those issues you pointed out : coding style problem??incorrect comments, poorly-named variables -- do you have any specific examples in your mind?
For whitespace damage, you can try the following regex:
/\s\+$\| \+\ze\t/
Or if you use vim as an editor, you can add this to your vimrc:
highlight RedundantSpaces ctermbg=red guibg=red
match RedundantSpaces /\s\+$\| \+\ze\t/
For example, it will show 4 cases of trailing whitespace in the quoted part of your email above. :)
Jörn
Signature · 8 lines
You ain't got no problem, Jules. I'm on the motherfucker. Go back in there, chill them niggers out and wait for the Wolf, who should be coming directly. -- Marsellus Wallace - 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 18 lines · On Thu, 10 Aug 2006 09:41:59 -0700 Mingming Cao <[email protected]> wrote:
Quoted 7 lines · Andrew Morton
Quoted 1 line · On Wed, 09 Aug 2006 18:17:02 -0700 Mingming Cao <[email protected]> wrote:
Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from
ext3_xxx()toext4_xxx().Quoted 9 lines
It would have been nice to spend a few hours cleaning up ext3 and JBD before doing this. The code isn't toooo bad, but there are number of coding style problems, whitespace screwups, incorrect comments, missing comments, poorly-chosen variable names and all of that sort of thing.
One the fs has been copied-and-pasted, it's much harder to address these
things: either need to do it twice, or allow the filesystems to diverge, or not do it.
Quoted 7 lines
Andrew, thanks for taking a close look this series of changes.
I agree with you that the timing is right, to do the clean up now rather than later. I would give it a try. If I could get more help from more code reviewer, it probably makes the effort a lot easier. For those issues you pointed out : coding style
problem___incorrectcomments, poorly-named variables -- do you have any specific examples in your mind?
Not really, apart from the few things I identified elsewhere (such as the brelse thing).
It's just that now is the right time for a general spring-cleaning, if we ever want to do that.
Quoted 6 lines
Quoted 4 lines
Also, -mm presently has two patches pending against
fs/jbd/and nine pending againstfs/ext3/. We should get all those things merged before taking the copy.So probably the right thing to do is keep the ext4 patches against mm tree instead of rc three?
That would drive everyone nuts, I think. What I would suggest is:
- get ext3 into a ready-to-copy state (merge bugfixes, spring-clean, etc)
- do the big copy-n-rename operation in Linus's tree.
- run a quilt or git tree against the resulting Linus tree.
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 38 lines · Andrew Morton
Quoted 17 lines · On Thu, 10 Aug 2006 09:41:59 -0700 Mingming Cao <[email protected]> wrote:
Quoted 6 lines · Andrew Morton
Quoted 1 line · On Wed, 09 Aug 2006 18:17:02 -0700 Mingming Cao <[email protected]> wrote:
Fork(copy) ext4 filesystem from ext3 filesystem. Rename all functions in ext4 from
ext3_xxx()toext4_xxx().Quoted 9 lines
It would have been nice to spend a few hours cleaning up ext3 and JBD before doing this. The code isn't toooo bad, but there are number of coding style problems, whitespace screwups, incorrect comments, missing comments, poorly-chosen variable names and all of that sort of thing.
One the fs has been copied-and-pasted, it's much harder to address these
things: either need to do it twice, or allow the filesystems to diverge, or not do it.Quoted 7 lines
Andrew, thanks for taking a close look this series of changes.
I agree with you that the timing is right, to do the clean up now rather than later. I would give it a try. If I could get more help from more code reviewer, it probably makes the effort a lot easier. For those issues you pointed out : coding style
problem___incorrectcomments, poorly-named variables -- do you have any specific examples in your mind?Not really, apart from the few things I identified elsewhere (such as the brelse thing).
It's just that now is the right time for a general spring-cleaning, if we ever want to do that.
Quoted 4 lines
Quoted 4 lines
Also, -mm presently has two patches pending against
fs/jbd/and nine pending againstfs/ext3/. We should get all those things merged before taking the copy.
Quoted 6 lines
Quoted 2 lines
So probably the right thing to do is keep the ext4 patches against mm tree instead of rc three?
That would drive everyone nuts, I think. What I would suggest is:
- get ext3 into a ready-to-copy state (merge bugfixes, spring-clean, etc)
Presumably bug fixes should go in immediately, regardless of whether it's before or after "cp -a ext3 ext4".
I strongly disagree that ext3 should be subject to a spring cleaning. Comments, whitespace, very very minor things, sure. Trying to get rid of brelse() when _many_ other filesystems also use it? ext4 material.
That detracts from the idea that its the stable counterpart to the devel filesystem (ext4).
Jeff
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 3 lines · On Thu, 10 Aug 2006 16:22:26 -0400 Jeff Garzik <[email protected]> wrote:
I strongly disagree that ext3 should be subject to a spring cleaning. Comments, whitespace, very very minor things, sure. Trying to get rid of
brelse()when _many_ other filesystems also use it? ext4 material.
We should seek to minimise the difficulty of cross-porting bugfixes and enhancements. Putting cleanups in only ext4 works against that.
ext3 will be around for many years yet. We cannot just let it rot due to some false belief that performing routine maintenance against it will for some magical reason cause it to break.
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 11 lines · Andrew Morton
Quoted 1 line · On Thu, 10 Aug 2006 16:22:26 -0400 Jeff Garzik <[email protected]> wrote:
I strongly disagree that ext3 should be subject to a spring cleaning. Comments, whitespace, very very minor things, sure. Trying to get rid of
brelse()when _many_ other filesystems also use it? ext4 material.We should seek to minimise the difficulty of cross-porting bugfixes and enhancements. Putting cleanups in only ext4 works against that.
ext3 will be around for many years yet. We cannot just let it rot due to some false belief that performing routine maintenance against it will for some magical reason cause it to break.
Because ext4 is impending, you want to push a bunch of cleanups into ext3 over a short span of time. That's not routine maintenance at all.
We're not talking about routine maintenance. In your words, we are
talking about spring cleaning.
Why not let the devel/stable system work its magic? If the cleanups are viable, proving that first in ext4 should give us more confidence to put them into ext3.
Cross-porting bugfixes and cleanups will _obviously_ be quite easy, during the first few months of ext4's life.
Just look at ext2->ext3 history. Regardless of when you make the split, there will be a bunch of stuff people wish to backport after the split occurs. Given that, it makes more sense to testbed the changes in ext4 first.
Jeff
Theodore Tso
maintainerQuoted 7 lines · Andrew Morton
One the fs has been copied-and-pasted, it's much harder to address these
things: either need to do it twice, or allow the filesystems to diverge, or not do it.Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..
Well, it's harder no matter when we do it, right? Whether we do it before we submit or after, we still before us have the choice of whether to let the filesystems diverge (and make it harder to do maintenance), or not. But Linus and others have spoken pretty clearly that they don't want ext3 to be touched, and it's not even clear that people would be happy with cleanups.
If we are going to do the cleanups in both, it would actually be better to fix up ext3 *first*, and then do the copy...
- Ted
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 4 lines · Andrew Morton
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..
Andrew,
I looked at this few days ago. I am not sure how we end up having multiple pages (especially, why we end up having buffers with bh_size > pagesize) ? Do you know why ?
Easiest fix would be to fix submit_bh() to deal with multiple vecs - which is vetoed by Jens and I agree with him :(
Thanks,
Badari
Quoted 11 lines · On Thu, 10 Aug 2006 11:51:34 -0700 Badari Pulavarty <[email protected]> wrote:
Quoted 4 lines · Andrew Morton
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..Andrew,
I looked at this few days ago. I am not sure how we end up having multiple pages (especially,
why we end up having buffers withbh_size> pagesize) ? Do you know why ?
It's one or both of the jbd_kmalloc(bh->b_size) calls in fs/jbd/transaction.c. Here we're allocating data to attach to a bh which later gets fed into submit_bh().
Problem is, with CONFIG_DEBUG_SLAB=y, the data which kmalloc() returns can be offset by 4 bytes due to redzoning.
Example: if the fs is using a 1k blocksize and we have a 4k pagesize, the data coming back from kmalloc may have an address of 0xnnnnxc04, so the data which we later feed into submit_bh() will span two pages.
A simple fix would be to replace kmalloc() with a call to alloc_page(). We'd need to work out how much memory that will worst-case-waste. If "not much" then OK.
If "quite a lot in the worst case" then we'd need something more elaborate.
I'd suggest that ext3 implement ext3-private slab caches of size 1024,
2048, 4096 and perhaps 8192. Those caches should be kmem_cache_create()d on-demand at mount-time. They should be created with appropriate slab options to defeat the redzoning. The transaction.c code should use the appropriate slab (based on b_size) rather than using kmalloc(). The up-to-four private slab caches should be destroyed on ext3 rmmod.
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 26 lines · Andrew Morton
Quoted 11 lines · On Thu, 10 Aug 2006 11:51:34 -0700 Badari Pulavarty <[email protected]> wrote:
Quoted 4 lines · Andrew Morton
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..Andrew,
I looked at this few days ago. I am not sure how we end up having multiple pages (especially,
why we end up having buffers withbh_size> pagesize) ? Do you know why ?It's one or both of the
jbd_kmalloc(bh->b_size)calls infs/jbd/transaction.c. Here we're allocating data to attach to a bh which later gets fed intosubmit_bh().Problem is, with
CONFIG_DEBUG_SLAB=y, the data whichkmalloc()returns can be offset by 4 bytes due to redzoning.Example: if the fs is using a 1k blocksize and we have a 4k pagesize, the data coming back from kmalloc may have an address of 0xnnnnxc04, so the data which we later feed into
submit_bh()will span two pages.
Quoted 5 lines
A simple fix would be to replace
kmalloc()with a call toalloc_page(). We'd need to work out how much memory that will worst-case-waste. If "not much" then OK.If "quite a lot in the worst case" then we'd need something more elaborate.
Would some like this be okay:
#ifdef CONFIG_DEBUG_SLAB
return alloc_page(...
#else
return kmalloc(...
#endif
This keeps it simple, and should be still be efficient in the non-DEBUG_SLAB case.
Quoted 6 lines
I'd suggest that ext3 implement ext3-private slab caches of size 1024,
2048, 4096 and perhaps 8192. Those caches should be kmem_cache_create()d on-demand at mount-time. They should be created with appropriate slab options to defeat the redzoning. Thetransaction.ccode should use the appropriate slab (based onb_size) rather than usingkmalloc(). The up-to-four private slab caches should be destroyed on ext3 rmmod.
Signature · 9 lines
David Kleikamp IBM Linux Technology Center ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 35 lines · On Thu, 10 Aug 2006 14:36:10 -0500 Dave Kleikamp <[email protected]> wrote:
Quoted 26 lines · Andrew Morton
Quoted 11 lines · On Thu, 10 Aug 2006 11:51:34 -0700 Badari Pulavarty <[email protected]> wrote:
Quoted 4 lines · Andrew Morton
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..Andrew,
I looked at this few days ago. I am not sure how we end up having multiple pages (especially,
why we end up having buffers withbh_size> pagesize) ? Do you know why ?It's one or both of the
jbd_kmalloc(bh->b_size)calls infs/jbd/transaction.c. Here we're allocating data to attach to a bh which later gets fed intosubmit_bh().Problem is, with
CONFIG_DEBUG_SLAB=y, the data whichkmalloc()returns can be offset by 4 bytes due to redzoning.Example: if the fs is using a 1k blocksize and we have a 4k pagesize, the data coming back from kmalloc may have an address of 0xnnnnxc04, so the data which we later feed into
submit_bh()will span two pages.Quoted 5 lines
A simple fix would be to replace
kmalloc()with a call toalloc_page(). We'd need to work out how much memory that will worst-case-waste. If "not much" then OK.If "quite a lot in the worst case" then we'd need something more elaborate.
Would some like this be okay:
Quoted 9 lines
#ifdef
CONFIG_DEBUG_SLAB
return alloc_page(...
#else
return kmalloc(...
#endifThis keeps it simple, and should be still be efficient in the non-DEBUG_SLAB case.
I guess that would work OK. It does appear that a lot of people build and distribute CONFIG_DEBUG_SLAB kernels though. Fedora, for one.
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 10 lines · Andrew Morton
Quoted 7 lines · On Thu, 10 Aug 2006 11:51:34 -0700 Badari Pulavarty <[email protected]> wrote:
Quoted 4 lines · Andrew Morton
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..Andrew,
Quoted 8 lines
Quoted 4 lines
I looked at this few days ago. I am not sure how we end up having multiple pages (especially,
why we end up having buffers withbh_size> pagesize) ? Do you know why ?It's one or both of the
jbd_kmalloc(bh->b_size)calls infs/jbd/transaction.c. Here we're allocating data to attach to a bh which later gets fed intosubmit_bh().
[...]
I respectfully submit that this sort of urge to clean up all the narstiness (a local term) in ext3 is _precisely_ the reason why we need ext4 in the tree ASAP.
Once people start pushing a large volume of changes into ext[34], the "obvious cleanups" start popping up. Look at the ratholes we are _already_ diving down, in this thread, trying to clean up ext3 before the copy.
ext4 will exist precisely to enable these sort of rapid cleanups. If obvious stuff starts popping up, the cleanups can go in immediately without worry of destabilization.
Just let ext3 sit. Other filesystems use submit_bh(), brelse(), etc. If ext3 is to stop using those, let it be when others stop using them as well.
ext4 is the devel tree. ext3 is the stable tree. Just go ahead and "cp fs/ext3 fs/ext4" immediately, otherwise the cleanups will pile up, and the branch will take place a year from now.
Jeff, still waiting for submit_bio() to be used in fs's :)
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 7 lines · Badari Pulavarty
Quoted 4 lines · Andrew Morton
Also, JBD is presently feeding into
submit_bh()buffer_headswhich span two
machine pages, and some device drivers spit the dummy. It'd be better to fix that once, rather than twice..Andrew,
Quoted 7 lines
I looked at this few days ago. I am not sure how we end up having multiple pages (especially,
why we end up having buffers withbh_size> pagesize) ? Do you know why ?Easiest fix would be to fix
submit_bh()to deal with multiple vecs - which is vetoed by
Jens and I agree with him :(
Yep. The sooner we kill buffer heads and use submit_bio(), the better :)
Jeff
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 1 line · On Thu, 10 Aug 2006 16:13:33 -0400 Jeff Garzik <[email protected]> wrote:
The sooner we kill buffer heads and use
submit_bio(), the better :)
A buffer_head is a caching entity and a bio is an IO container. They're quite separate concepts.
A buffer_head is the kernel's sole abstraction of a disk block. Filesystems use disk blocks a lot, and they need such an abstraction.
If one was to replace buffer_heads with direct-to-BIO operations then the filesytem would need to internally track the mapping from
page+offset+length -> disk block
and it would need to internally track the page+offset+length<->disk block coherency state and it would need to internally perform serialisation of access to each page+offset+length hunk of pagecache and etc and etc and etc. Create a data structure with which to do all that and voila, buffer_heads reinvented.
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 7 lines · Andrew Morton
Quoted 1 line · On Thu, 10 Aug 2006 16:13:33 -0400 Jeff Garzik <[email protected]> wrote:
The sooner we kill buffer heads and use
submit_bio(), the better :)A
buffer_headis a caching entity and a bio is an IO container. They're quite separate concepts.
Yeah, sorry, I meant direct pagecache I/O. I forgot that bio doesn't handle caching.
Quoted 2 lines
A
buffer_headis the kernel's sole abstraction of a disk block. Filesystems use disk blocks a lot, and they need such an abstraction.
IMO Al Viro work has shown that you can do pagecache I/O without needing such a heavyweight system.
Jeff
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Jeff Garzik (JG) writes:
A
buffer_headis the kernel's sole abstraction of a disk block. Filesystems use disk blocks a lot, and they need such an abstraction.
JG> IMO Al Viro work has shown that you can do pagecache I/O without needing JG> such a heavyweight system.
in delayed allocation patch for ext3 (ontop of extents) we do use bio's for data.
thanks, Alex
Quoted 8 lines · On Fri, 11 Aug 2006 01:11:23 +0400 Alex Tomas <[email protected]> wrote:
Quoted 2 lines
Quoted 2 lines
A
buffer_headis the kernel's sole abstraction of a disk block. Filesystems use disk blocks a lot, and they need such an abstraction.JG> IMO Al Viro work has shown that you can do pagecache I/O without needing JG> such a heavyweight system.
in delayed allocation patch for ext3 (ontop of extents) we do use bio's for data.
As does ext3 with `-o writeback,nobh'.