[PATCH 00/19] fs cleanup: remove duplicated code on inode init

[PATCH 00/19] fs cleanup: remove duplicated code on inode init

[Dmitry Monakhov]

Each filesystem init uid,gid,mode on inode creation.
gid inheritance is obey to posix rules. Usually this code is
copy-pasted. Let's move this logic to separate function.

In some filesystems it is not easy to replace the code, so i've
simply skipped such fs.
Skipped fs: xfs, hugetlbfs, gfs2, cifs, affs

Some filesystems require less trivial code replacement
so i've split the patch in to per-fs patch-set.

[PATCH 01/19] vfs: Add inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/namei.c         |   19 +++++++++++++++++++
 include/linux/fs.h |    3 ++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d62fdc8..bbe9578 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1435,6 +1435,25 @@ void unlock_rename(struct dentry *p1, struct dentry *p2)
 		mutex_unlock(&p1->d_inode->i_sb->s_vfs_rename_mutex);
 	}
 }
+/**
+ * Init uid,gid,mode for new inode according to posix standards
+ * @inode: New inode
+ * @dir: Directory inode
+ * @mode: mode of the new inode
+ */
+inline void inode_init_owner(struct inode *inode, const struct inode *dir,
+			int mode)
+{
+	inode->i_uid = current_fsuid();
+	if (dir && dir->i_mode & S_ISGID) {
+		inode->i_gid = dir->i_gid;
+		if (S_ISDIR(mode))
+			mode |= S_ISGID;
+	} else
+		inode->i_gid = current_fsgid();
+	inode->i_mode = mode;
+}
+EXPORT_SYMBOL(inode_init_owner);
 
 int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
 		struct nameidata *nd)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b1bcb27..b2d7cb3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1426,7 +1426,8 @@ extern void dentry_unhash(struct dentry *dentry);
  * VFS file helper functions.
  */
 extern int file_permission(struct file *, int);
-
+extern void inode_init_owner(struct inode *inode, const struct inode *dir,
+			int mode);
 /*
  * VFS FS_IOC_FIEMAP helper definitions.
  */
-- 
1.6.6

Re: [PATCH 01/19] vfs: Add inode uid,gid,mode initialization with helper function

[James Morris]

On Wed, 17 Feb 2010, Dmitry Monakhov wrote:

> + */
> +inline void inode_init_owner(struct inode *inode, const struct inode *dir,
> +			int mode)
> +{

You should only try and inline functions which are smaller than about 100 
bytes or which have fewer than four lines of code.  Also, the compiler can 
generally figure out which functions to inline. 

> +	inode->i_uid = current_fsuid();
> +	if (dir && dir->i_mode & S_ISGID) {
> +		inode->i_gid = dir->i_gid;
> +		if (S_ISDIR(mode))
> +			mode |= S_ISGID;
> +	} else
> +		inode->i_gid = current_fsgid();
> +	inode->i_mode = mode;
> +}
> +EXPORT_SYMBOL(inode_init_owner);

The rest of them look ok to me, although probably best to get acks from 
each fs maintainer.


-- 
James Morris
<jmorris@namei.org>

[PATCH 01/19] vfs: Add inode uid,gid,mode init helper v2

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

James Morris <jmorris@namei.org> writes:

> On Wed, 17 Feb 2010, Dmitry Monakhov wrote:
>
>> + */
>> +inline void inode_init_owner(struct inode *inode, const struct inode *dir,
>> +			int mode)
>> +{
>
> You should only try and inline functions which are smaller than about 100 
> bytes or which have fewer than four lines of code.  Also, the compiler can 
> generally figure out which functions to inline. 
Indeed.

[-- Attachment #2: 0001-vfs-Add-inode-uid-gid-mode-init-helper-v2.patch --]
[-- Type: text/plain, Size: 1688 bytes --]

>From 38b401c4bf5e29f9fa8afa90b17ab0adc3f362d8 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:46:36 +0300
Subject: [PATCH 01/19] vfs: Add inode uid,gid,mode init helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/namei.c         |   19 +++++++++++++++++++
 include/linux/fs.h |    3 ++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d62fdc8..d3ba44b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1435,6 +1435,25 @@ void unlock_rename(struct dentry *p1, struct dentry *p2)
 		mutex_unlock(&p1->d_inode->i_sb->s_vfs_rename_mutex);
 	}
 }
+/**
+ * Init uid,gid,mode for new inode according to posix standards
+ * @inode: New inode
+ * @dir: Directory inode
+ * @mode: mode of the new inode
+ */
+void inode_init_owner(struct inode *inode, const struct inode *dir,
+			int mode)
+{
+	inode->i_uid = current_fsuid();
+	if (dir && dir->i_mode & S_ISGID) {
+		inode->i_gid = dir->i_gid;
+		if (S_ISDIR(mode))
+			mode |= S_ISGID;
+	} else
+		inode->i_gid = current_fsgid();
+	inode->i_mode = mode;
+}
+EXPORT_SYMBOL(inode_init_owner);
 
 int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
 		struct nameidata *nd)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b1bcb27..b2d7cb3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1426,7 +1426,8 @@ extern void dentry_unhash(struct dentry *dentry);
  * VFS file helper functions.
  */
 extern int file_permission(struct file *, int);
-
+extern void inode_init_owner(struct inode *inode, const struct inode *dir,
+			int mode);
 /*
  * VFS FS_IOC_FIEMAP helper definitions.
  */
-- 
1.6.6

[PATCH 02/19] 9p: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/9p/vfs_inode.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 9d03d1e..037b67e 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -252,9 +252,7 @@ struct inode *v9fs_get_inode(struct super_block *sb, int mode)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	inode->i_gid = current_fsgid();
+	inode_init_owner(inode, NULL, mode);
 	inode->i_blocks = 0;
 	inode->i_rdev = 0;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
-- 
1.6.6

[PATCH 03/19] bfs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

- Also by unknown reason S_ISGID wasn't inherented from parent dir,
  change this to obey posix standards.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/bfs/dir.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index 1e41aad..8f73841 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -105,14 +105,12 @@ static int bfs_create(struct inode *dir, struct dentry *dentry, int mode,
 	}
 	set_bit(ino, info->si_imap);
 	info->si_freei--;
-	inode->i_uid = current_fsuid();
-	inode->i_gid = (dir->i_mode & S_ISGID) ? dir->i_gid : current_fsgid();
+	inode_init_owner(inode, dir, mode);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
 	inode->i_blocks = 0;
 	inode->i_op = &bfs_file_inops;
 	inode->i_fop = &bfs_file_operations;
 	inode->i_mapping->a_ops = &bfs_aops;
-	inode->i_mode = mode;
 	inode->i_ino = ino;
 	BFS_I(inode)->i_dsk_ino = ino;
 	BFS_I(inode)->i_sblock = 0;
-- 
1.6.6

[PATCH 04/19] btrfs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/btrfs/inode.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 4deb280..ae4f64c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4145,16 +4145,7 @@ static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans,
 	if (ret != 0)
 		goto fail;
 
-	inode->i_uid = current_fsuid();
-
-	if (dir && (dir->i_mode & S_ISGID)) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-
-	inode->i_mode = mode;
+	inode_init_owner(inode, dir, mode);
 	inode->i_ino = objectid;
 	inode_set_bytes(inode, 0);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-- 
1.6.6

[PATCH 05/19] exofs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/exofs/inode.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
index 2afbceb..466ad0d 100644
--- a/fs/exofs/inode.c
+++ b/fs/exofs/inode.c
@@ -1083,16 +1083,7 @@ struct inode *exofs_new_inode(struct inode *dir, int mode)
 	sbi = sb->s_fs_info;
 
 	sb->s_dirt = 1;
-	inode->i_uid = current->cred->fsuid;
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else {
-		inode->i_gid = current->cred->fsgid;
-	}
-	inode->i_mode = mode;
-
+	inode_init_owner(inode, dir, mode);
 	inode->i_ino = sbi->s_nextid++;
 	inode->i_blkbits = EXOFS_BLKSHIFT;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-- 
1.6.6

Re: [PATCH 05/19] exofs: replace inode uid,gid,mode initialization with helper function

[Boaz Harrosh]

On 02/17/2010 10:39 AM, Dmitry Monakhov wrote:
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Ack-by: Boaz Harrosh <bharrosh@panasas.com>

> ---
>  fs/exofs/inode.c |   11 +----------
>  1 files changed, 1 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
> index 2afbceb..466ad0d 100644
> --- a/fs/exofs/inode.c
> +++ b/fs/exofs/inode.c
> @@ -1083,16 +1083,7 @@ struct inode *exofs_new_inode(struct inode *dir, int mode)
>  	sbi = sb->s_fs_info;
>  
>  	sb->s_dirt = 1;
> -	inode->i_uid = current->cred->fsuid;
> -	if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else {
> -		inode->i_gid = current->cred->fsgid;
> -	}
> -	inode->i_mode = mode;
> -
> +	inode_init_owner(inode, dir, mode);
>  	inode->i_ino = sbi->s_nextid++;
>  	inode->i_blkbits = EXOFS_BLKSHIFT;
>  	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;

[PATCH 06/19] ext2: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext2/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index 15387c9..4a4b132 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -550,16 +550,12 @@ got:
 
 	sb->s_dirt = 1;
 	mark_buffer_dirty(bh2);
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
+	if (test_opt (sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	inode->i_blocks = 0;
-- 
1.6.6

Re: [PATCH 06/19] ext2: replace inode uid,gid,mode initialization with helper function

[Jan Kara]

> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/ext2/ialloc.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
> index 15387c9..4a4b132 100644
> --- a/fs/ext2/ialloc.c
> +++ b/fs/ext2/ialloc.c
> @@ -550,16 +550,12 @@ got:
>  
>  	sb->s_dirt = 1;
>  	mark_buffer_dirty(bh2);
> -	inode->i_uid = current_fsuid();
> -	if (test_opt (sb, GRPID))
> +	if (test_opt (sb, GRPID)) {
  As Andreas wrote, please don't use space after a function name.
Otherwise the patch looks fine.

							Honza

> +		inode->i_mode = mode;
> +		inode->i_uid = current_fsuid();
>  		inode->i_gid = dir->i_gid;
> -	else if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
>  	} else
> -		inode->i_gid = current_fsgid();
> -	inode->i_mode = mode;
> +		inode_init_owner(inode, dir, mode);
>  
>  	inode->i_ino = ino;
>  	inode->i_blocks = 0;
> -- 
> 1.6.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

[PATCH 06/19] ext2: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext2/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index 15387c9..2c484c6 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -550,16 +550,12 @@ got:
 
 	sb->s_dirt = 1;
 	mark_buffer_dirty(bh2);
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	inode->i_blocks = 0;
-- 
1.6.6

Re: [PATCH 06/19] ext2: replace inode uid,gid,mode init with helper v2

[Jan Kara]

On Thu 18-02-10 10:00:03, Dmitry Monakhov wrote:
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
  Looks good.
Acked-by: Jan Kara <jack@suse.cz>

							Honza
> ---
>  fs/ext2/ialloc.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
> index 15387c9..2c484c6 100644
> --- a/fs/ext2/ialloc.c
> +++ b/fs/ext2/ialloc.c
> @@ -550,16 +550,12 @@ got:
>  
>  	sb->s_dirt = 1;
>  	mark_buffer_dirty(bh2);
> -	inode->i_uid = current_fsuid();
> -	if (test_opt (sb, GRPID))
> +	if (test_opt(sb, GRPID)) {
> +		inode->i_mode = mode;
> +		inode->i_uid = current_fsuid();
>  		inode->i_gid = dir->i_gid;
> -	else if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
>  	} else
> -		inode->i_gid = current_fsgid();
> -	inode->i_mode = mode;
> +		inode_init_owner(inode, dir, mode);
>  
>  	inode->i_ino = ino;
>  	inode->i_blocks = 0;
> -- 
> 1.6.6
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 07/19] ext3: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext3/ialloc.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index b399912..1516e6a 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -538,16 +538,13 @@ got:
 	if (S_ISDIR(mode))
 		percpu_counter_inc(&sbi->s_dirs_counter);
 
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
-		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
+
+	if (test_opt (sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

[PATCH 07/19] ext3: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 67 bytes --]

changes from v1:
 update code style according to Andreas comments


[-- Attachment #2: 0007-ext3-replace-inode-uid-gid-mode-init-with-helper-v2.patch --]
[-- Type: text/plain, Size: 1105 bytes --]

>From bd7bd775d7c1f939797ddb5df61aa3c4f50ba7b0 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:44:32 +0300
Subject: [PATCH 07/19] ext3: replace inode uid,gid,mode init with helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext3/ialloc.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index b399912..1a48294 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -538,16 +538,13 @@ got:
 	if (S_ISDIR(mode))
 		percpu_counter_inc(&sbi->s_dirs_counter);
 
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
-		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
+
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

[PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index f3624ea..1b33417 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -985,16 +985,12 @@ got:
 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
 	}
 
-	inode->i_uid = current_fsuid();
-	if (test_opt(sb, GRPID))
+	if (test_opt (sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino + group * EXT4_INODES_PER_GROUP(sb);
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

Re: [PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[Andreas Dilger]

On 2010-02-17, at 11:40, Dmitry Monakhov wrote:
> @@ -985,16 +985,12 @@ got:
> 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
> 	}
>
> +	if (test_opt (sb, GRPID)) {

(style) please don't add the space after "test_opt" back into the code.


> +		inode->i_mode = mode;
> +		inode->i_uid = current_fsuid();
> 		inode->i_gid = dir->i_gid;
> 	} else
> +		inode_init_owner(inode, dir, mode);


This code is a bit misleading, since it implies that "i_uid" and  
"i_mode" are set this way due to the GRPID option, even though with  
further investigation it is just set the same way in both cases.

Ted,
what do you think of just removing the "GRPID" mount option?  I  
believe this was around for ages, due to the lack of BSD setgid-on- 
parent functionality in Linux.  I don't think it is needed anymore,  
since the BSD functionality is much more flexible (it can be set on a  
per-directory basis instead of for the whole filesystem).

I suppose one way to find out positively would be to add a printk() to  
case Opt_grpid: in the option parsing that this option is deprecated,  
and to contact linux-ext4@vger.kernel.org if you are still using it.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[PATCH 08/19] ext4: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

Andreas Dilger <adilger@sun.com> writes:

> On 2010-02-17, at 11:40, Dmitry Monakhov wrote:
>> @@ -985,16 +985,12 @@ got:
>> 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
>> 	}
>>
>> +	if (test_opt (sb, GRPID)) {
>
> (style) please don't add the space after "test_opt" back into the code.
Ok, updated, new version attached.
>
>
>> +		inode->i_mode = mode;
>> +		inode->i_uid = current_fsuid();
>> 		inode->i_gid = dir->i_gid;
>> 	} else
>> +		inode_init_owner(inode, dir, mode);
>
>
> This code is a bit misleading, since it implies that "i_uid" and
> "i_mode" are set this way due to the GRPID option, even though with
> further investigation it is just set the same way in both cases.
>
> Ted,
> what do you think of just removing the "GRPID" mount option?  I
> believe this was around for ages, due to the lack of BSD setgid-on- 
> parent functionality in Linux.  I don't think it is needed anymore,
> since the BSD functionality is much more flexible (it can be set on a
> per-directory basis instead of for the whole filesystem).
>
> I suppose one way to find out positively would be to add a printk() to
> case Opt_grpid: in the option parsing that this option is deprecated,
> and to contact linux-ext4@vger.kernel.org if you are still using it.
Let's post this as separate patch because this change existing behavior.
>
> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.

[-- Attachment #2: 0008-ext4-replace-inode-uid-gid-mode-init-with-helper-v2.patch --]
[-- Type: text/plain, Size: 1135 bytes --]

>From a1336d4e57352423a7eb0f7b95ed20e78dcefc8c Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:44:01 +0300
Subject: [PATCH 08/19] ext4: replace inode uid,gid,mode init with helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index f3624ea..a2a35d3 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -985,16 +985,12 @@ got:
 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
 	}
 
-	inode->i_uid = current_fsuid();
-	if (test_opt(sb, GRPID))
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino + group * EXT4_INODES_PER_GROUP(sb);
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

Re: [PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[tytso]

On Wed, Feb 17, 2010 at 04:39:54PM -0700, Andreas Dilger wrote:
> 
> Ted,
> what do you think of just removing the "GRPID" mount option?  I
> believe this was around for ages, due to the lack of BSD setgid-on-
> parent functionality in Linux.  I don't think it is needed anymore,
> since the BSD functionality is much more flexible (it can be set on
> a per-directory basis instead of for the whole filesystem).
> 
> I suppose one way to find out positively would be to add a printk()
> to case Opt_grpid: in the option parsing that this option is
> deprecated, and to contact linux-ext4@vger.kernel.org if you are
> still using it.

There are whole raft of options I'd love to deprecate:

* bsddf/minixdf
* grpid/bsdgroups/nogrpiid/sysvgroups

I also wonder whether it's time that we just enable acls and xattrs
all the time, instead of making them be explicit mount options.

So anyone wants to send me a patch, feel free....  (or I may get to it
maybe this weekend.)

					- Ted

Re: [PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

tytso@mit.edu writes:

> On Wed, Feb 17, 2010 at 04:39:54PM -0700, Andreas Dilger wrote:
>> 
>> Ted,
>> what do you think of just removing the "GRPID" mount option?  I
>> believe this was around for ages, due to the lack of BSD setgid-on-
>> parent functionality in Linux.  I don't think it is needed anymore,
>> since the BSD functionality is much more flexible (it can be set on
>> a per-directory basis instead of for the whole filesystem).
>> 
>> I suppose one way to find out positively would be to add a printk()
>> to case Opt_grpid: in the option parsing that this option is
>> deprecated, and to contact linux-ext4@vger.kernel.org if you are
>> still using it.
>
> There are whole raft of options I'd love to deprecate:
>
> * bsddf/minixdf
> * grpid/bsdgroups/nogrpiid/sysvgroups
>
> I also wonder whether it's time that we just enable acls and xattrs
> all the time, instead of making them be explicit mount options.
>
> So anyone wants to send me a patch, feel free....  (or I may get to it
I'll handle this, because currently there are not free bits left in
mount_flags.
> maybe this weekend.)
>
> 					- Ted

[PATCH] ext4: deprecate obsoleted mount options

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

This patch deprecate some obsoleted functions.
It is not obvious what should we do in case of deprecated options on mount.
Just printk and continue or fail the mount, i've implemented the last one.
BTW: Do we need similar patch for e2fslib?


[-- Attachment #2: 0001-ext4-deprecate-obsoleted-mount-options.patch --]
[-- Type: text/plain, Size: 7711 bytes --]

>From 72649dbf903705df6f20c330e0fcecabf24e426e Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Fri, 19 Feb 2010 17:20:36 +0300
Subject: [PATCH] ext4: deprecate obsoleted mount options

Disable following list of obsoleted mount options:
 - bsddf, miniddf
 - grpid, bsdgroups, nogrpid, sysvgroups

Disable following list of obsoleted default mount options
 - bsdgroups

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ext4.h   |    4 +-
 fs/ext4/ialloc.c |    4 +--
 fs/ext4/super.c  |   96 ++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 68 insertions(+), 36 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 67859fa..1fd55a7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -731,12 +731,10 @@ struct ext4_inode_info {
  * Mount flags
  */
 #define EXT4_MOUNT_OLDALLOC		0x00002  /* Don't use the new Orlov allocator */
-#define EXT4_MOUNT_GRPID		0x00004	/* Create files with directory's group */
 #define EXT4_MOUNT_DEBUG		0x00008	/* Some debugging messages */
 #define EXT4_MOUNT_ERRORS_CONT		0x00010	/* Continue on errors */
 #define EXT4_MOUNT_ERRORS_RO		0x00020	/* Remount fs ro on errors */
 #define EXT4_MOUNT_ERRORS_PANIC		0x00040	/* Panic on errors */
-#define EXT4_MOUNT_MINIX_DF		0x00080	/* Mimics the Minix statfs */
 #define EXT4_MOUNT_NOLOAD		0x00100	/* Don't use existing journal*/
 #define EXT4_MOUNT_DATA_FLAGS		0x00C00	/* Mode for data writes: */
 #define EXT4_MOUNT_JOURNAL_DATA		0x00400	/* Write data to journal */
@@ -1185,6 +1183,8 @@ static inline void ext4_clear_inode_state(struct inode *inode, int bit)
 #define EXT4_DEFM_JMODE_DATA	0x0020
 #define EXT4_DEFM_JMODE_ORDERED	0x0040
 #define EXT4_DEFM_JMODE_WBACK	0x0060
+/* Deprecated default mount options mask */
+#define EXT4_DEFM_DEPRECATED	EXT4_DEFM_BSDGROUPS
 
 /*
  * Default journal batch times
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 2fab5ad..0bccf0d 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -986,9 +986,7 @@ got:
 	}
 
 	inode->i_uid = current_fsuid();
-	if (test_opt(sb, GRPID))
-		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
+	if (dir->i_mode & S_ISGID) {
 		inode->i_gid = dir->i_gid;
 		if (S_ISDIR(mode))
 			mode |= S_ISGID;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9e45e62..7fb9e77 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -823,12 +823,6 @@ static int ext4_show_options(struct seq_file *seq, struct vfsmount *vfs)
 
 	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) && !(def_mount_opts & EXT4_DEFM_BSDGROUPS))
-		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);
@@ -1096,7 +1090,6 @@ static const struct export_operations ext4_export_ops = {
 };
 
 enum {
-	Opt_bsd_df, Opt_minix_df, Opt_grpid, Opt_nogrpid,
 	Opt_resgid, Opt_resuid, Opt_sb, Opt_err_cont, Opt_err_panic, Opt_err_ro,
 	Opt_nouid32, Opt_debug, Opt_oldalloc, Opt_orlov,
 	Opt_user_xattr, Opt_nouser_xattr, Opt_acl, Opt_noacl,
@@ -1114,16 +1107,16 @@ enum {
 	Opt_block_validity, Opt_noblock_validity,
 	Opt_inode_readahead_blks, Opt_journal_ioprio,
 	Opt_dioread_nolock, Opt_dioread_lock,
-	Opt_discard, Opt_nodiscard,
+	Opt_discard, Opt_nodiscard, Opt_deprecated,
 };
 
 static const match_table_t tokens = {
-	{Opt_bsd_df, "bsddf"},
-	{Opt_minix_df, "minixdf"},
-	{Opt_grpid, "grpid"},
-	{Opt_grpid, "bsdgroups"},
-	{Opt_nogrpid, "nogrpid"},
-	{Opt_nogrpid, "sysvgroups"},
+	{Opt_deprecated, "bsddf"},
+	{Opt_deprecated, "minixdf"},
+	{Opt_deprecated, "grpid"},
+	{Opt_deprecated, "bsdgroups"},
+	{Opt_deprecated, "nogrpid"},
+	{Opt_deprecated, "sysvgroups"},
 	{Opt_resgid, "resgid=%u"},
 	{Opt_resuid, "resuid=%u"},
 	{Opt_sb, "sb=%u"},
@@ -1188,6 +1181,18 @@ static const match_table_t tokens = {
 	{Opt_err, NULL},
 };
 
+static const match_table_t default_mountopt = {
+	{EXT4_DEFM_DEBUG, "debug"},
+	{EXT4_DEFM_BSDGROUPS, "bsdgroups"},
+	{EXT4_DEFM_XATTR_USER, "user_xattr"},
+	{EXT4_DEFM_ACL, "acl"},
+	{EXT4_DEFM_UID16, "uid16"},
+	{EXT4_DEFM_JMODE_WBACK, "journal_data_writeback"},
+	{EXT4_DEFM_JMODE_DATA, "journal_data"},
+	{EXT4_DEFM_JMODE_ORDERED, "journal_data_ordered"},
+	{0, NULL},
+};
+
 static ext4_fsblk_t get_sb_block(void **data)
 {
 	ext4_fsblk_t	sb_block;
@@ -1238,18 +1243,6 @@ static int parse_options(char *options, struct super_block *sb,
 
 		token = match_token(p, tokens, args);
 		switch (token) {
-		case Opt_bsd_df:
-			clear_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
-		case Opt_minix_df:
-			set_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
-		case Opt_grpid:
-			set_opt(sbi->s_mount_opt, GRPID);
-			break;
-		case Opt_nogrpid:
-			clear_opt(sbi->s_mount_opt, GRPID);
-			break;
 		case Opt_resuid:
 			if (match_int(&args[0], &option))
 				return 0;
@@ -1622,6 +1615,12 @@ set_qf_format:
 		case Opt_dioread_lock:
 			clear_opt(sbi->s_mount_opt, DIOREAD_NOLOCK);
 			break;
+		case Opt_deprecated:
+			ext4_msg(sb, KERN_ERR,
+				"Deprecated mount option \"%s\". "
+				"Please contact linux-ext4@vger.kernel.org if "
+				"you are still using it.", p);
+			return 0;
 		default:
 			ext4_msg(sb, KERN_ERR,
 			       "Unrecognized mount option \"%s\" "
@@ -2358,7 +2357,42 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 	}
 	return 1;
 }
+static int handle_deprecated_defmopt(struct super_block *sb, unsigned int opt)
+{
+	int first = 1;
+	/*
+	 * When deprecated options are found they are not cleared from
+	 * super block by default. Just print error message and let
+	 * user clear it manually.
+	 */
+	printk(KERN_ERR "EXT4-fs (%s) Deprecated default mount options:",
+		sb->s_id);
+	/*
+	 * It is impossible to use simple bit traversing because,
+	 * some options use shared bits.
+	 */
+	opt &= EXT4_DEFM_DEPRECATED;
+	while (opt) {
+		const struct match_token *mt = default_mountopt;
+		while (mt->pattern != NULL) {
+			if ((opt & mt->token) == mt->token)
+				break;
+			mt++;
+		}
+		if (mt->pattern == NULL) {
+			printk("%s unknown", !first ? "," : "");
+			break;
+		} else {
+			printk("%s %s", !first ? "," : "", mt->pattern);
+			opt &= ~mt->token;
+		}
+		first = 0;
+	}
+	printk(". Please use tune2fs to disable it, or contact "
+		"linux-ext4@vger.kernel.org if you are still need it.");
 
+	return 1;
+}
 static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 				__releases(kernel_lock)
 				__acquires(kernel_lock)
@@ -2443,10 +2477,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* Set defaults before we parse the mount options */
 	def_mount_opts = le32_to_cpu(es->s_default_mount_opts);
+	if (def_mount_opts & EXT4_DEFM_DEPRECATED) {
+		if (handle_deprecated_defmopt(sb, def_mount_opts))
+			goto failed_mount;
+	}
 	if (def_mount_opts & EXT4_DEFM_DEBUG)
 		set_opt(sbi->s_mount_opt, DEBUG);
-	if (def_mount_opts & EXT4_DEFM_BSDGROUPS)
-		set_opt(sbi->s_mount_opt, GRPID);
 	if (def_mount_opts & EXT4_DEFM_UID16)
 		set_opt(sbi->s_mount_opt, NO_UID32);
 #ifdef CONFIG_EXT4_FS_XATTR
@@ -3668,9 +3704,7 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct ext4_super_block *es = sbi->s_es;
 	u64 fsid;
 
-	if (test_opt(sb, MINIX_DF)) {
-		sbi->s_overhead_last = 0;
-	} else if (sbi->s_blocks_last != ext4_blocks_count(es)) {
+	if (sbi->s_blocks_last != ext4_blocks_count(es)) {
 		ext4_group_t i, ngroups = ext4_get_groups_count(sb);
 		ext4_fsblk_t overhead = 0;
 
-- 
1.6.6

Re: [PATCH] ext4: deprecate obsoleted mount options

[Andreas Dilger]

On 2010-02-19, at 07:39, Dmitry Monakhov wrote:
> This patch deprecate some obsoleted functions.
> It is not obvious what should we do in case of deprecated options on  
> mount.
> Just printk and continue or fail the mount, i've implemented the  
> last one.
> BTW: Do we need similar patch for e2fslib?

I think deprecating an option is not the same as removing it  
entirely.  Even though I don't think these options are in wide usage,  
I'd still prefer to add a printk() to the parsing code first, leave it  
for a year, then remove them entirely after that.

> #define EXT4_MOUNT_OLDALLOC		0x00002  /* Don't use the new Orlov  
> allocator */
> -#define EXT4_MOUNT_GRPID		0x00004	/* Create files with directory's  
> group */
> #define EXT4_MOUNT_DEBUG		0x00008	/* Some debugging messages */
> #define EXT4_MOUNT_ERRORS_CONT		0x00010	/* Continue on errors */
> #define EXT4_MOUNT_ERRORS_RO		0x00020	/* Remount fs ro on errors */
> #define EXT4_MOUNT_ERRORS_PANIC		0x00040	/* Panic on errors */
> -#define EXT4_MOUNT_MINIX_DF		0x00080	/* Mimics the Minix statfs */
> #define EXT4_MOUNT_NOLOAD		0x00100	/* Don't use existing journal*/
> #define EXT4_MOUNT_DATA_FLAGS		0x00C00	/* Mode for data writes: */
> #define EXT4_MOUNT_JOURNAL_DATA		0x00400	/* Write data to journal */

I'd prefer to leave a comment like "/* was EXT4_MOUNT_GRPID -  
2010/02/21 */" so that it is clear that these values were previously  
in use, and should not be re-used until other options are exhausted...

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[PATCH] ext4: deprecate obsoleted mount options v2

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

Andreas Dilger <adilger@sun.com> writes:

> On 2010-02-19, at 07:39, Dmitry Monakhov wrote:
>> This patch deprecate some obsoleted functions.
>> It is not obvious what should we do in case of deprecated options on
>> mount.
>> Just printk and continue or fail the mount, i've implemented the
>> last one.
>> BTW: Do we need similar patch for e2fslib?
>
> I think deprecating an option is not the same as removing it entirely.
Ohh.. I've hoped to reuse freed bits for new crap.
> Even though I don't think these options are in wide usage,  I'd still
> prefer to add a printk() to the parsing code first, leave it  for a
> year, then remove them entirely after that.
So option deprecation result in code boosting instead of code shrinkage.
Indeed the second law of thermodynamics is absolutely true.
New version attached.


[-- Attachment #2: 0001-ext4-deprecate-obsoleted-mount-options-v2.patch --]
[-- Type: text/plain, Size: 5021 bytes --]

>From 0d12e733a6dd87ed3bc4dea27126f6e814b5586b Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Tue, 23 Feb 2010 22:11:47 +0300
Subject: [PATCH] ext4: deprecate obsoleted mount options v2

Declare following list of mount options as deprecated:
 - bsddf, miniddf
 - grpid, bsdgroups, nogrpid, sysvgroups

Declare following list of default mount options as deprecated:
 - bsdgroups

Changes from v1
 - Mark options as deprecated instead of disabling it completely,
   they will be disabled after exportation period.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ext4.h  |    2 +
 fs/ext4/super.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 67859fa..66ed482 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1185,6 +1185,8 @@ static inline void ext4_clear_inode_state(struct inode *inode, int bit)
 #define EXT4_DEFM_JMODE_DATA	0x0020
 #define EXT4_DEFM_JMODE_ORDERED	0x0040
 #define EXT4_DEFM_JMODE_WBACK	0x0060
+/* Deprecated default mount options mask */
+#define EXT4_DEFM_DEPRECATED	EXT4_DEFM_BSDGROUPS
 
 /*
  * Default journal batch times
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d5596ca..d07f506 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1114,7 +1114,7 @@ enum {
 	Opt_block_validity, Opt_noblock_validity,
 	Opt_inode_readahead_blks, Opt_journal_ioprio,
 	Opt_dioread_nolock, Opt_dioread_lock,
-	Opt_discard, Opt_nodiscard,
+	Opt_discard, Opt_nodiscard, Opt_deprecated, Opt_disabled,
 };
 
 static const match_table_t tokens = {
@@ -1188,6 +1188,18 @@ static const match_table_t tokens = {
 	{Opt_err, NULL},
 };
 
+static const match_table_t default_mountopt = {
+	{EXT4_DEFM_DEBUG, "debug"},
+	{EXT4_DEFM_BSDGROUPS, "bsdgroups"},
+	{EXT4_DEFM_XATTR_USER, "user_xattr"},
+	{EXT4_DEFM_ACL, "acl"},
+	{EXT4_DEFM_UID16, "uid16"},
+	{EXT4_DEFM_JMODE_WBACK, "journal_data_writeback"},
+	{EXT4_DEFM_JMODE_DATA, "journal_data"},
+	{EXT4_DEFM_JMODE_ORDERED, "journal_data_ordered"},
+	{0, NULL},
+};
+
 static ext4_fsblk_t get_sb_block(void **data)
 {
 	ext4_fsblk_t	sb_block;
@@ -1240,16 +1252,16 @@ static int parse_options(char *options, struct super_block *sb,
 		switch (token) {
 		case Opt_bsd_df:
 			clear_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
+			goto deprecated;
 		case Opt_minix_df:
 			set_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
+			goto deprecated;
 		case Opt_grpid:
 			set_opt(sbi->s_mount_opt, GRPID);
-			break;
+			goto deprecated;
 		case Opt_nogrpid:
 			clear_opt(sbi->s_mount_opt, GRPID);
-			break;
+			goto deprecated;
 		case Opt_resuid:
 			if (match_int(&args[0], &option))
 				return 0;
@@ -1622,6 +1634,21 @@ set_qf_format:
 		case Opt_dioread_lock:
 			clear_opt(sbi->s_mount_opt, DIOREAD_NOLOCK);
 			break;
+
+		case Opt_deprecated:
+deprecated:
+			ext4_msg(sb, KERN_ERR,
+				"Deprecated mount option \"%s\". Will be "
+				"removed soon. Please contact "
+				"linux-ext4@vger.kernel.org if you are still "
+				"using it.", p);
+			break;
+
+		case Opt_disabled:
+			ext4_msg(sb, KERN_ERR,
+				"Deprecated mount option \"%s\". And not "
+				"longer supported.", p);
+			return 0;
 		default:
 			ext4_msg(sb, KERN_ERR,
 			       "Unrecognized mount option \"%s\" "
@@ -2358,7 +2385,42 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 	}
 	return 1;
 }
+static int handle_deprecated_defmopt(struct super_block *sb, unsigned int opt)
+{
+	int first = 1;
+	/*
+	 * When deprecated options are found they are not cleared from
+	 * super block by default. Just print error message and let
+	 * user clear it manually.
+	 */
+	printk(KERN_ERR "EXT4-fs (%s) Deprecated default mount options:",
+		sb->s_id);
+	/*
+	 * It is impossible to use simple bit traversing because,
+	 * some options use shared bits.
+	 */
+	opt &= EXT4_DEFM_DEPRECATED;
+	while (opt) {
+		const struct match_token *mt = default_mountopt;
+		while (mt->pattern != NULL) {
+			if ((opt & mt->token) == mt->token)
+				break;
+			mt++;
+		}
+		if (mt->pattern == NULL) {
+			printk("%s unknown", !first ? "," : "");
+			break;
+		} else {
+			printk("%s %s", !first ? "," : "", mt->pattern);
+			opt &= ~mt->token;
+		}
+		first = 0;
+	}
+	printk(". Please use tune2fs to disable it, or contact "
+		"linux-ext4@vger.kernel.org if you are still need it.");
 
+	return 0;
+}
 static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 				__releases(kernel_lock)
 				__acquires(kernel_lock)
@@ -2443,6 +2505,10 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* Set defaults before we parse the mount options */
 	def_mount_opts = le32_to_cpu(es->s_default_mount_opts);
+	if (def_mount_opts & EXT4_DEFM_DEPRECATED) {
+		if (handle_deprecated_defmopt(sb, def_mount_opts))
+			goto failed_mount;
+	}
 	if (def_mount_opts & EXT4_DEFM_DEBUG)
 		set_opt(sbi->s_mount_opt, DEBUG);
 	if (def_mount_opts & EXT4_DEFM_BSDGROUPS)
-- 
1.6.6

Re: [PATCH] ext4: deprecate obsoleted mount options v2

[tytso]

On Tue, Feb 23, 2010 at 10:23:45PM +0300, Dmitry Monakhov wrote:
> >
> > I think deprecating an option is not the same as removing it entirely.
> Ohh.. I've hoped to reuse freed bits for new crap.

What "new crap" are you hoping to will need mount options?  One of the
things I want to do long term is to try to reduce/remove mount options
in general.

If we get general agreement that it's time to just turn on acl's and
xattr's by default, we can change the default, and in that case
removing the "noacl/noxattr" might be something that we might not need
to keep for as long, or maybe at all.  But for things like
bsddf/minixdf, we do need some kind of deprecation schedule.

The use of Opt_deprecated and Opt_disabled seems a little pointless;
nothing is using now, and nothing needs it.  All I'd probably do is
something like this:

static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
       "Contact linux-ext4@vger.kernel.org if you think we should keep it.\n"

And then in each option that we want to deprecate, just add:

    ext4_msg(sb, KERN_WARN, deprecated_msg, "bsddf", "2.6.39");

The extra opt_discard, and goto deprecated, etc., seems way more
complicated than what is necessary.

						- Ted

[PATCH] ext4: deprecate obsoleted mount options v3

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

tytso@mit.edu writes:

> On Tue, Feb 23, 2010 at 10:23:45PM +0300, Dmitry Monakhov wrote:
>> >
>> > I think deprecating an option is not the same as removing it entirely.
>> Ohh.. I've hoped to reuse freed bits for new crap.
>
> What "new crap" are you hoping to will need mount options? 
I want two bits in mount flags. But only one is available
for now. Don't you mind to extend mount flags to 64 bits?
>One of the
> things I want to do long term is to try to reduce/remove mount options
> in general.
>
> If we get general agreement that it's time to just turn on acl's and
> xattr's by default, we can change the default, and in that case
> removing the "noacl/noxattr" might be something that we might not need
> to keep for as long, or maybe at all.  But for things like
> bsddf/minixdf, we do need some kind of deprecation schedule.
>
> The use of Opt_deprecated and Opt_disabled seems a little pointless;
> nothing is using now, and nothing needs it.  All I'd probably do is
> something like this:
>
> static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
>        "Contact linux-ext4@vger.kernel.org if you think we should keep it.\n"
>
> And then in each option that we want to deprecate, just add:
>
>     ext4_msg(sb, KERN_WARN, deprecated_msg, "bsddf", "2.6.39");
Yess. Definitely this look nicer.


[-- Attachment #2: 0001-ext4-deprecate-obsoleted-mount-options-v3.patch --]
[-- Type: text/plain, Size: 2515 bytes --]

>From 92ce6a649e6f9a99c9aa34b1650a7a85c8ee5b82 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Wed, 24 Feb 2010 21:00:49 +0300
Subject: [PATCH] ext4: deprecate obsoleted mount options v3

Declare following list of mount options as deprecated:
 - bsddf, miniddf
 - grpid, bsdgroups, nogrpid, sysvgroups

Declare following list of default mount options as deprecated:
 - bsdgroups

Changes from v1
 - Mark options as deprecated instead of disabling it completely,
   they will be disabled after exportation period.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/super.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d5596ca..2b413c5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1212,6 +1212,8 @@ static ext4_fsblk_t get_sb_block(void **data)
 }
 
 #define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))
+static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
+	"Contact linux-ext4@vger.kernel.org if you think we should keep it.\n";
 
 static int parse_options(char *options, struct super_block *sb,
 			 unsigned long *journal_devnum,
@@ -1239,16 +1241,23 @@ static int parse_options(char *options, struct super_block *sb,
 		token = match_token(p, tokens, args);
 		switch (token) {
 		case Opt_bsd_df:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			clear_opt(sbi->s_mount_opt, MINIX_DF);
 			break;
 		case Opt_minix_df:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			set_opt(sbi->s_mount_opt, MINIX_DF);
+
 			break;
 		case Opt_grpid:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			set_opt(sbi->s_mount_opt, GRPID);
+
 			break;
 		case Opt_nogrpid:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			clear_opt(sbi->s_mount_opt, GRPID);
+
 			break;
 		case Opt_resuid:
 			if (match_int(&args[0], &option))
@@ -2445,8 +2454,11 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	def_mount_opts = le32_to_cpu(es->s_default_mount_opts);
 	if (def_mount_opts & EXT4_DEFM_DEBUG)
 		set_opt(sbi->s_mount_opt, DEBUG);
-	if (def_mount_opts & EXT4_DEFM_BSDGROUPS)
+	if (def_mount_opts & EXT4_DEFM_BSDGROUPS) {
+		ext4_msg(sb, KERN_WARNING, deprecated_msg, "bsdgroups",
+			"2.6.39");
 		set_opt(sbi->s_mount_opt, GRPID);
+	}
 	if (def_mount_opts & EXT4_DEFM_UID16)
 		set_opt(sbi->s_mount_opt, NO_UID32);
 #ifdef CONFIG_EXT4_FS_XATTR
-- 
1.6.6

Re: [PATCH] ext4: deprecate obsoleted mount options v3

[tytso]

I've added this to the ext4 patch queue, thanks.

     	   	       	    	  	 - Ted

[PATCH 09/19] jfs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/jfs/jfs_inode.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/fs/jfs/jfs_inode.c b/fs/jfs/jfs_inode.c
index dc0e021..841a8d5 100644
--- a/fs/jfs/jfs_inode.c
+++ b/fs/jfs/jfs_inode.c
@@ -98,14 +98,7 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 		goto fail_unlock;
 	}
 
-	inode->i_uid = current_fsuid();
-	if (parent->i_mode & S_ISGID) {
-		inode->i_gid = parent->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-
+	inode_init_owner(inode, parent, mode);
 	/*
 	 * New inodes need to save sane values on disk when
 	 * uid & gid mount options are used
@@ -121,7 +114,6 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 		goto fail_drop;
 	}
 
-	inode->i_mode = mode;
 	/* inherit flags from parent */
 	jfs_inode->mode2 = JFS_IP(parent)->mode2 & JFS_FL_INHERIT;
 
-- 
1.6.6

Re: [PATCH 09/19] jfs: replace inode uid,gid,mode initialization with helper function

[Dave Kleikamp]

Acked-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

One minor nit though.  jfs maintains jfs_inode->mode2 which is a
superset of inode->i_mode.  Since inode_init_owner() may set S_ISGID,
the following change should also be made.  (I don't think these
duplicate bits in mode2 are really important, but we may as well be
consistent.)

diff -Nurp linux/fs/jfs/jfs_inode.c linux.new/fs/jfs/jfs_inode.c
--- linux/fs/jfs/jfs_inode.c	2010-02-17 15:46:31.000000000 -0600
+++ linux.new/fs/jfs/jfs_inode.c	2010-02-17 15:47:58.000000000 -0600
@@ -126,7 +126,7 @@ struct inode *ialloc(struct inode *paren
 		if (S_ISLNK(mode))
 			jfs_inode->mode2 &= ~(JFS_IMMUTABLE_FL|JFS_APPEND_FL);
 	}
-	jfs_inode->mode2 |= mode;
+	jfs_inode->mode2 |= inode->i_mode;
 
 	inode->i_blocks = 0;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;

Thanks,
Shaggy

On Wed, 2010-02-17 at 21:41 +0300, Dmitry Monakhov wrote:
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/jfs/jfs_inode.c |   10 +---------
>  1 files changed, 1 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/jfs/jfs_inode.c b/fs/jfs/jfs_inode.c
> index dc0e021..841a8d5 100644
> --- a/fs/jfs/jfs_inode.c
> +++ b/fs/jfs/jfs_inode.c
> @@ -98,14 +98,7 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
>  		goto fail_unlock;
>  	}
> 
> -	inode->i_uid = current_fsuid();
> -	if (parent->i_mode & S_ISGID) {
> -		inode->i_gid = parent->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else
> -		inode->i_gid = current_fsgid();
> -
> +	inode_init_owner(inode, parent, mode);
>  	/*
>  	 * New inodes need to save sane values on disk when
>  	 * uid & gid mount options are used
> @@ -121,7 +114,6 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
>  		goto fail_drop;
>  	}
> 
> -	inode->i_mode = mode;
>  	/* inherit flags from parent */
>  	jfs_inode->mode2 = JFS_IP(parent)->mode2 & JFS_FL_INHERIT;
> 
-- 
David Kleikamp
IBM Linux Technology Center

[PATCH 09/19] jfs: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

Dave Kleikamp <shaggy@linux.vnet.ibm.com> writes:

> Acked-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>
> One minor nit though.  jfs maintains jfs_inode->mode2 which is a
> superset of inode->i_mode.  Since inode_init_owner() may set S_ISGID,
> the following change should also be made.  (I don't think these
> duplicate bits in mode2 are really important, but we may as well be
> consistent.)
Folded in to one patch.

[-- Attachment #2: 0009-jfs-replace-inode-uid-gid-mode-init-with-helper-v2.patch --]
[-- Type: text/plain, Size: 1499 bytes --]

>From fae29b54bd3735a9723499bc6110c533089b79fe Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:43:31 +0300
Subject: [PATCH 09/19] jfs: replace inode uid,gid,mode init with helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/jfs/jfs_inode.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/fs/jfs/jfs_inode.c b/fs/jfs/jfs_inode.c
index dc0e021..705646d 100644
--- a/fs/jfs/jfs_inode.c
+++ b/fs/jfs/jfs_inode.c
@@ -98,14 +98,7 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 		goto fail_unlock;
 	}
 
-	inode->i_uid = current_fsuid();
-	if (parent->i_mode & S_ISGID) {
-		inode->i_gid = parent->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-
+	inode_init_owner(inode, parent, mode);
 	/*
 	 * New inodes need to save sane values on disk when
 	 * uid & gid mount options are used
@@ -121,7 +114,6 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 		goto fail_drop;
 	}
 
-	inode->i_mode = mode;
 	/* inherit flags from parent */
 	jfs_inode->mode2 = JFS_IP(parent)->mode2 & JFS_FL_INHERIT;
 
@@ -134,7 +126,7 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 		if (S_ISLNK(mode))
 			jfs_inode->mode2 &= ~(JFS_IMMUTABLE_FL|JFS_APPEND_FL);
 	}
-	jfs_inode->mode2 |= mode;
+	jfs_inode->mode2 |= inode->i_mode;
 
 	inode->i_blocks = 0;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-- 
1.6.6

[PATCH 10/19] minix: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

- also redesign minix_new_inode interface

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/minix/bitmap.c |    5 ++---
 fs/minix/minix.h  |    3 ++-
 fs/minix/namei.c  |   11 +++--------
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/minix/bitmap.c b/fs/minix/bitmap.c
index 6ac693f..dc7f625 100644
--- a/fs/minix/bitmap.c
+++ b/fs/minix/bitmap.c
@@ -221,7 +221,7 @@ void minix_free_inode(struct inode * inode)
 	clear_inode(inode);		/* clear in-memory copy */
 }
 
-struct inode * minix_new_inode(const struct inode * dir, int * error)
+struct inode * minix_new_inode(const struct inode * dir, int mode, int * error)
 {
 	struct super_block *sb = dir->i_sb;
 	struct minix_sb_info *sbi = minix_sb(sb);
@@ -263,8 +263,7 @@ struct inode * minix_new_inode(const struct inode * dir, int * error)
 		iput(inode);
 		return NULL;
 	}
-	inode->i_uid = current_fsuid();
-	inode->i_gid = (dir->i_mode & S_ISGID) ? dir->i_gid : current_fsgid();
+	inode_init_owner(inode, dir, mode);
 	inode->i_ino = j;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
 	inode->i_blocks = 0;
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index 9dcf95b..b4355b1 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -46,7 +46,8 @@ struct minix_sb_info {
 extern struct inode *minix_iget(struct super_block *, unsigned long);
 extern struct minix_inode * minix_V1_raw_inode(struct super_block *, ino_t, struct buffer_head **);
 extern struct minix2_inode * minix_V2_raw_inode(struct super_block *, ino_t, struct buffer_head **);
-extern struct inode * minix_new_inode(const struct inode * dir, int * error);
+extern struct inode * minix_new_inode(const struct inode * dir, int mode,
+				int * error);
 extern void minix_free_inode(struct inode * inode);
 extern unsigned long minix_count_free_inodes(struct minix_sb_info *sbi);
 extern int minix_new_block(struct inode * inode);
diff --git a/fs/minix/namei.c b/fs/minix/namei.c
index 32b131c..e20ee85 100644
--- a/fs/minix/namei.c
+++ b/fs/minix/namei.c
@@ -46,10 +46,9 @@ static int minix_mknod(struct inode * dir, struct dentry *dentry, int mode, dev_
 	if (!old_valid_dev(rdev))
 		return -EINVAL;
 
-	inode = minix_new_inode(dir, &error);
+	inode = minix_new_inode(dir, mode, &error);
 
 	if (inode) {
-		inode->i_mode = mode;
 		minix_set_inode(inode, rdev);
 		mark_inode_dirty(inode);
 		error = add_nondir(dentry, inode);
@@ -73,11 +72,10 @@ static int minix_symlink(struct inode * dir, struct dentry *dentry,
 	if (i > dir->i_sb->s_blocksize)
 		goto out;
 
-	inode = minix_new_inode(dir, &err);
+	inode = minix_new_inode(dir, S_IFLNK | 0777, &err);
 	if (!inode)
 		goto out;
 
-	inode->i_mode = S_IFLNK | 0777;
 	minix_set_inode(inode, 0);
 	err = page_symlink(inode, symname, i);
 	if (err)
@@ -117,13 +115,10 @@ static int minix_mkdir(struct inode * dir, struct dentry *dentry, int mode)
 
 	inode_inc_link_count(dir);
 
-	inode = minix_new_inode(dir, &err);
+	inode = minix_new_inode(dir, mode, &err);
 	if (!inode)
 		goto out_dir;
 
-	inode->i_mode = S_IFDIR | mode;
-	if (dir->i_mode & S_ISGID)
-		inode->i_mode |= S_ISGID;
 	minix_set_inode(inode, 0);
 
 	inode_inc_link_count(inode);
-- 
1.6.6

[PATCH 11/19] nilfs2: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/nilfs2/inode.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 7868cc1..5d8e866 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -279,16 +279,7 @@ struct inode *nilfs_new_inode(struct inode *dir, int mode)
 	/* reference count of i_bh inherits from nilfs_mdt_read_block() */
 
 	atomic_inc(&sbi->s_inodes_count);
-
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-
-	inode->i_mode = mode;
+	inode_init_owner(inode, dir, mode);
 	inode->i_ino = ino;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 
-- 
1.6.6

Re: [PATCH 11/19] nilfs2: replace inode uid,gid,mode initialization with helper function

[Ryusuke Konishi]

On Wed, 17 Feb 2010 21:42:48 +0300, Dmitry Monakhov <dmonakhov@openvz.org> wrote:
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/nilfs2/inode.c |   11 +----------
>  1 files changed, 1 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 7868cc1..5d8e866 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -279,16 +279,7 @@ struct inode *nilfs_new_inode(struct inode *dir, int mode)
>  	/* reference count of i_bh inherits from nilfs_mdt_read_block() */
>  
>  	atomic_inc(&sbi->s_inodes_count);
> -
> -	inode->i_uid = current_fsuid();
> -	if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else
> -		inode->i_gid = current_fsgid();
> -
> -	inode->i_mode = mode;
> +	inode_init_owner(inode, dir, mode);
>  	inode->i_ino = ino;
>  	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
>  
> -- 
> 1.6.6

Looks fine to me.

Acked-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>

[PATCH 12/19] ocfs2: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ocfs2/dlm/dlmfs.c |   11 +----------
 fs/ocfs2/namei.c     |    9 +--------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmfs.c b/fs/ocfs2/dlm/dlmfs.c
index 02bf178..248df45 100644
--- a/fs/ocfs2/dlm/dlmfs.c
+++ b/fs/ocfs2/dlm/dlmfs.c
@@ -364,9 +364,7 @@ static struct inode *dlmfs_get_inode(struct inode *parent,
 	if (!inode)
 		return NULL;
 
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	inode->i_gid = current_fsgid();
+	inode_init_owner(inode, parent, mode);
 	inode->i_mapping->backing_dev_info = &dlmfs_backing_dev_info;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 
@@ -403,13 +401,6 @@ static struct inode *dlmfs_get_inode(struct inode *parent,
 		inc_nlink(inode);
 		break;
 	}
-
-	if (parent->i_mode & S_ISGID) {
-		inode->i_gid = parent->i_gid;
-		if (S_ISDIR(mode))
-			inode->i_mode |= S_ISGID;
-	}
-
 	return inode;
 }
 
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 50fb26a..2877404 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -204,14 +204,7 @@ static struct inode *ocfs2_get_init_inode(struct inode *dir, int mode)
 		inode->i_nlink = 2;
 	else
 		inode->i_nlink = 1;
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+	inode_init_owner(inode, dir, mode);
 	vfs_dq_init(inode);
 	return inode;
 }
-- 
1.6.6

[Ocfs2-devel] [PATCH 12/19] ocfs2: replace inode uid, gid, mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ocfs2/dlm/dlmfs.c |   11 +----------
 fs/ocfs2/namei.c     |    9 +--------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmfs.c b/fs/ocfs2/dlm/dlmfs.c
index 02bf178..248df45 100644
--- a/fs/ocfs2/dlm/dlmfs.c
+++ b/fs/ocfs2/dlm/dlmfs.c
@@ -364,9 +364,7 @@ static struct inode *dlmfs_get_inode(struct inode *parent,
 	if (!inode)
 		return NULL;
 
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	inode->i_gid = current_fsgid();
+	inode_init_owner(inode, parent, mode);
 	inode->i_mapping->backing_dev_info = &dlmfs_backing_dev_info;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 
@@ -403,13 +401,6 @@ static struct inode *dlmfs_get_inode(struct inode *parent,
 		inc_nlink(inode);
 		break;
 	}
-
-	if (parent->i_mode & S_ISGID) {
-		inode->i_gid = parent->i_gid;
-		if (S_ISDIR(mode))
-			inode->i_mode |= S_ISGID;
-	}
-
 	return inode;
 }
 
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 50fb26a..2877404 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -204,14 +204,7 @@ static struct inode *ocfs2_get_init_inode(struct inode *dir, int mode)
 		inode->i_nlink = 2;
 	else
 		inode->i_nlink = 1;
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+	inode_init_owner(inode, dir, mode);
 	vfs_dq_init(inode);
 	return inode;
 }
-- 
1.6.6

Re: [PATCH 12/19] ocfs2: replace inode uid,gid,mode initialization with helper function

[Joel Becker]

On Wed, Feb 17, 2010 at 09:43:26PM +0300, Dmitry Monakhov wrote:
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Acked-by: Joel Becker <joel.becker@oracle.com>

-- 

"Not everything that can be counted counts, and not everything
 that counts can be counted."
        - Albert Einstein 

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 12/19] ocfs2: replace inode uid, gid, mode initialization with helper function

[Joel Becker]

On Wed, Feb 17, 2010 at 09:43:26PM +0300, Dmitry Monakhov wrote:
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Acked-by: Joel Becker <joel.becker@oracle.com>

-- 

"Not everything that can be counted counts, and not everything
 that counts can be counted."
        - Albert Einstein 

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[PATCH 13/19] omfs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/omfs/inode.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/fs/omfs/inode.c b/fs/omfs/inode.c
index f3b7c15..6240ea1 100644
--- a/fs/omfs/inode.c
+++ b/fs/omfs/inode.c
@@ -36,9 +36,7 @@ struct inode *omfs_new_inode(struct inode *dir, int mode)
 		goto fail;
 
 	inode->i_ino = new_block;
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	inode->i_gid = current_fsgid();
+	inode_init_owner(inode, NULL, mode);
 	inode->i_mapping->a_ops = &omfs_aops;
 
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
-- 
1.6.6

[PATCH 14/19] ramfs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

- seems what ramfs_get_inode is only locally, make it static.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ramfs/inode.c      |   20 ++++++--------------
 include/linux/ramfs.h |    1 -
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/fs/ramfs/inode.c b/fs/ramfs/inode.c
index a6090aa..ac55307 100644
--- a/fs/ramfs/inode.c
+++ b/fs/ramfs/inode.c
@@ -51,14 +51,13 @@ static struct backing_dev_info ramfs_backing_dev_info = {
 			  BDI_CAP_READ_MAP | BDI_CAP_WRITE_MAP | BDI_CAP_EXEC_MAP,
 };
 
-struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev)
+static struct inode *ramfs_get_inode(struct super_block *sb,
+				const struct inode *dir, int mode, dev_t dev)
 {
 	struct inode * inode = new_inode(sb);
 
 	if (inode) {
-		inode->i_mode = mode;
-		inode->i_uid = current_fsuid();
-		inode->i_gid = current_fsgid();
+		inode_init_owner(inode, dir, mode);
 		inode->i_mapping->a_ops = &ramfs_aops;
 		inode->i_mapping->backing_dev_info = &ramfs_backing_dev_info;
 		mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
@@ -94,15 +93,10 @@ struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev)
 static int
 ramfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 {
-	struct inode * inode = ramfs_get_inode(dir->i_sb, mode, dev);
+	struct inode * inode = ramfs_get_inode(dir->i_sb, dir, mode, dev);
 	int error = -ENOSPC;
 
 	if (inode) {
-		if (dir->i_mode & S_ISGID) {
-			inode->i_gid = dir->i_gid;
-			if (S_ISDIR(mode))
-				inode->i_mode |= S_ISGID;
-		}
 		d_instantiate(dentry, inode);
 		dget(dentry);	/* Extra count - pin the dentry in core */
 		error = 0;
@@ -129,13 +123,11 @@ static int ramfs_symlink(struct inode * dir, struct dentry *dentry, const char *
 	struct inode *inode;
 	int error = -ENOSPC;
 
-	inode = ramfs_get_inode(dir->i_sb, S_IFLNK|S_IRWXUGO, 0);
+	inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK|S_IRWXUGO, 0);
 	if (inode) {
 		int l = strlen(symname)+1;
 		error = page_symlink(inode, symname, l);
 		if (!error) {
-			if (dir->i_mode & S_ISGID)
-				inode->i_gid = dir->i_gid;
 			d_instantiate(dentry, inode);
 			dget(dentry);
 			dir->i_mtime = dir->i_ctime = CURRENT_TIME;
@@ -240,7 +232,7 @@ static int ramfs_fill_super(struct super_block * sb, void * data, int silent)
 	sb->s_op		= &ramfs_ops;
 	sb->s_time_gran		= 1;
 
-	inode = ramfs_get_inode(sb, S_IFDIR | fsi->mount_opts.mode, 0);
+	inode = ramfs_get_inode(sb, NULL, S_IFDIR | fsi->mount_opts.mode, 0);
 	if (!inode) {
 		err = -ENOMEM;
 		goto fail;
diff --git a/include/linux/ramfs.h b/include/linux/ramfs.h
index 4e768dd..6ee2bb9 100644
--- a/include/linux/ramfs.h
+++ b/include/linux/ramfs.h
@@ -1,7 +1,6 @@
 #ifndef _LINUX_RAMFS_H
 #define _LINUX_RAMFS_H
 
-struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev);
 extern int ramfs_get_sb(struct file_system_type *fs_type,
 	 int flags, const char *dev_name, void *data, struct vfsmount *mnt);
 
-- 
1.6.6

[PATCH 15/19] reiserfs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/reiserfs/namei.c |   17 ++++-------------
 1 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 9d4dcf0..e138e0a 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -561,22 +561,13 @@ static int drop_new_inode(struct inode *inode)
 static int new_inode_init(struct inode *inode, struct inode *dir, int mode)
 {
 
-	/* the quota init calls have to know who to charge the quota to, so
-	 ** we have to set uid and gid here
-	 */
-	inode->i_uid = current_fsuid();
-	inode->i_mode = mode;
 	/* Make inode invalid - just in case we are going to drop it before
 	 * the initialization happens */
 	INODE_PKEY(inode)->k_objectid = 0;
-
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			inode->i_mode |= S_ISGID;
-	} else {
-		inode->i_gid = current_fsgid();
-	}
+	/* the quota init calls have to know who to charge the quota to, so
+	 ** we have to set uid and gid here
+	 */
+	inode_init_owner(inode, dir, mode);
 	vfs_dq_init(inode);
 	return 0;
 }
-- 
1.6.6

[PATCH 16/19] sysv: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/sysv/ialloc.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/fs/sysv/ialloc.c b/fs/sysv/ialloc.c
index 241e976..bbd69bd 100644
--- a/fs/sysv/ialloc.c
+++ b/fs/sysv/ialloc.c
@@ -159,15 +159,7 @@ struct inode * sysv_new_inode(const struct inode * dir, mode_t mode)
 	*sbi->s_sb_fic_count = cpu_to_fs16(sbi, count);
 	fs16_add(sbi, sbi->s_sb_total_free_inodes, -1);
 	dirty_sb(sb);
-	
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-
-	inode->i_uid = current_fsuid();
+	inode_init_owner(inode, dir, mode);
 	inode->i_ino = fs16_to_cpu(sbi, ino);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
 	inode->i_blocks = 0;
@@ -176,7 +168,6 @@ struct inode * sysv_new_inode(const struct inode * dir, mode_t mode)
 	insert_inode_hash(inode);
 	mark_inode_dirty(inode);
 
-	inode->i_mode = mode;		/* for sysv_write_inode() */
 	sysv_write_inode(inode, 0);	/* ensure inode not allocated again */
 	mark_inode_dirty(inode);	/* cleared by sysv_write_inode() */
 	/* That's it. */
-- 
1.6.6

[PATCH 17/19] ubifs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ubifs/dir.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 552fb01..5a357bf 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -104,14 +104,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
 	 */
 	inode->i_flags |= (S_NOCMTIME);
 
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+	inode_init_owner(inode, dir, mode);
 	inode->i_mtime = inode->i_atime = inode->i_ctime =
 			 ubifs_current_time(inode);
 	inode->i_mapping->nrpages = 0;
-- 
1.6.6

[PATCH 17/19] ubifs: replace inode uid, gid, mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ubifs/dir.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 552fb01..5a357bf 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -104,14 +104,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
 	 */
 	inode->i_flags |= (S_NOCMTIME);
 
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+	inode_init_owner(inode, dir, mode);
 	inode->i_mtime = inode->i_atime = inode->i_ctime =
 			 ubifs_current_time(inode);
 	inode->i_mapping->nrpages = 0;
-- 
1.6.6

Re: [PATCH 17/19] ubifs: replace inode uid,gid,mode initialization with helper function

[Artem Bityutskiy]

On Wed, 2010-02-17 at 21:46 +0300, Dmitry Monakhov wrote:
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Acked-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

> ---
>  fs/ubifs/dir.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 552fb01..5a357bf 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -104,14 +104,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
>  	 */
>  	inode->i_flags |= (S_NOCMTIME);
>  
> -	inode->i_uid = current_fsuid();
> -	if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else
> -		inode->i_gid = current_fsgid();
> -	inode->i_mode = mode;
> +	inode_init_owner(inode, dir, mode);
>  	inode->i_mtime = inode->i_atime = inode->i_ctime =
>  			 ubifs_current_time(inode);
>  	inode->i_mapping->nrpages = 0;


-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/19] ubifs: replace inode uid,gid,mode initialization with helper function

[Artem Bityutskiy]

On Wed, 2010-02-17 at 21:46 +0300, Dmitry Monakhov wrote:
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>

Acked-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

> ---
>  fs/ubifs/dir.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 552fb01..5a357bf 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -104,14 +104,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
>  	 */
>  	inode->i_flags |= (S_NOCMTIME);
>  
> -	inode->i_uid = current_fsuid();
> -	if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else
> -		inode->i_gid = current_fsgid();
> -	inode->i_mode = mode;
> +	inode_init_owner(inode, dir, mode);
>  	inode->i_mtime = inode->i_atime = inode->i_ctime =
>  			 ubifs_current_time(inode);
>  	inode->i_mapping->nrpages = 0;


-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCH 18/19] udf: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/udf/ialloc.c |   11 ++---------
 fs/udf/namei.c  |    3 ---
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index c10fa39..52d6e4d 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -124,15 +124,8 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
 		udf_updated_lvid(sb);
 	}
 	mutex_unlock(&sbi->s_alloc_mutex);
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else {
-		inode->i_gid = current_fsgid();
-	}
+
+	inode_init_owner(inode, dir, mode);
 
 	iinfo->i_location.logicalBlockNum = block;
 	iinfo->i_location.partitionReferenceNum =
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index cd21150..9069fc2 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -691,9 +691,6 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 			FID_FILE_CHAR_DIRECTORY | FID_FILE_CHAR_PARENT;
 	udf_write_fi(inode, &cfi, fi, &fibh, NULL, NULL);
 	brelse(fibh.sbh);
-	inode->i_mode = S_IFDIR | mode;
-	if (dir->i_mode & S_ISGID)
-		inode->i_mode |= S_ISGID;
 	mark_inode_dirty(inode);
 
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
-- 
1.6.6

Re: [PATCH 18/19] udf: replace inode uid,gid,mode initialization with helper function

[Jan Kara]

On Wed 17-02-10 21:47:06, Dmitry Monakhov wrote:
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/udf/ialloc.c |   11 ++---------
>  fs/udf/namei.c  |    3 ---
>  2 files changed, 2 insertions(+), 12 deletions(-)
  Hmm, this patch looks kind of half baked. There are several calls to
udf_new_inode() in udf/namei.c. From your patch I'd expect they all should
pass correct final mode to udf_new_inode which they currently don't to (the
case of udf_mkdir and udf_symlink). Otherwise I'm fine with this change.

								Honza

> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index c10fa39..52d6e4d 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -124,15 +124,8 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
>  		udf_updated_lvid(sb);
>  	}
>  	mutex_unlock(&sbi->s_alloc_mutex);
> -	inode->i_mode = mode;
> -	inode->i_uid = current_fsuid();
> -	if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else {
> -		inode->i_gid = current_fsgid();
> -	}
> +
> +	inode_init_owner(inode, dir, mode);
>  
>  	iinfo->i_location.logicalBlockNum = block;
>  	iinfo->i_location.partitionReferenceNum =
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index cd21150..9069fc2 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -691,9 +691,6 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>  			FID_FILE_CHAR_DIRECTORY | FID_FILE_CHAR_PARENT;
>  	udf_write_fi(inode, &cfi, fi, &fibh, NULL, NULL);
>  	brelse(fibh.sbh);
> -	inode->i_mode = S_IFDIR | mode;
> -	if (dir->i_mode & S_ISGID)
> -		inode->i_mode |= S_ISGID;
>  	mark_inode_dirty(inode);
>  
>  	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
> -- 
> 1.6.6
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 18/19] udf: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

Jan Kara <jack@suse.cz> writes:

> On Wed 17-02-10 21:47:06, Dmitry Monakhov wrote:
>> 
>> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
>> ---
>>  fs/udf/ialloc.c |   11 ++---------
>>  fs/udf/namei.c  |    3 ---
>>  2 files changed, 2 insertions(+), 12 deletions(-)
>   Hmm, this patch looks kind of half baked. There are several calls to
> udf_new_inode() in udf/namei.c. From your patch I'd expect they all should
> pass correct final mode to udf_new_inode which they currently don't to (the
> case of udf_mkdir and udf_symlink). Otherwise I'm fine with this change.
Yeah, you right. I was to lazy to check all callers :(.
Correct version are follows:

[-- Attachment #2: 0018-udf-replace-inode-uid-gid-mode-init-with-helper-v2.patch --]
[-- Type: text/plain, Size: 2657 bytes --]

>From 612e46abea261e9fdaea5f3285c5deee4f86f162 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:42:52 +0300
Subject: [PATCH 18/19] udf: replace inode uid,gid,mode init with helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/udf/ialloc.c |   11 ++---------
 fs/udf/namei.c  |    8 +-------
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index c10fa39..52d6e4d 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -124,15 +124,8 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
 		udf_updated_lvid(sb);
 	}
 	mutex_unlock(&sbi->s_alloc_mutex);
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else {
-		inode->i_gid = current_fsgid();
-	}
+
+	inode_init_owner(inode, dir, mode);
 
 	iinfo->i_location.logicalBlockNum = block;
 	iinfo->i_location.partitionReferenceNum =
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index cd21150..5a59b97 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -577,7 +577,6 @@ static int udf_create(struct inode *dir, struct dentry *dentry, int mode,
 		inode->i_data.a_ops = &udf_aops;
 	inode->i_op = &udf_file_inode_operations;
 	inode->i_fop = &udf_file_operations;
-	inode->i_mode = mode;
 	mark_inode_dirty(inode);
 
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
@@ -623,7 +622,6 @@ static int udf_mknod(struct inode *dir, struct dentry *dentry, int mode,
 		goto out;
 
 	iinfo = UDF_I(inode);
-	inode->i_uid = current_fsuid();
 	init_special_inode(inode, mode, rdev);
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
 	if (!fi) {
@@ -691,9 +689,6 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 			FID_FILE_CHAR_DIRECTORY | FID_FILE_CHAR_PARENT;
 	udf_write_fi(inode, &cfi, fi, &fibh, NULL, NULL);
 	brelse(fibh.sbh);
-	inode->i_mode = S_IFDIR | mode;
-	if (dir->i_mode & S_ISGID)
-		inode->i_mode |= S_ISGID;
 	mark_inode_dirty(inode);
 
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
@@ -900,7 +895,7 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 	struct udf_inode_info *iinfo;
 
 	lock_kernel();
-	inode = udf_new_inode(dir, S_IFLNK, &err);
+	inode = udf_new_inode(dir, S_IFLNK | S_IRWXUGO, &err);
 	if (!inode)
 		goto out;
 
@@ -911,7 +906,6 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 	}
 
 	iinfo = UDF_I(inode);
-	inode->i_mode = S_IFLNK | S_IRWXUGO;
 	inode->i_data.a_ops = &udf_symlink_aops;
 	inode->i_op = &page_symlink_inode_operations;
 
-- 
1.6.6

Re: [PATCH 18/19] udf: replace inode uid,gid,mode init with helper v2

[Jan Kara]

On Thu 18-02-10 10:18:32, Dmitry Monakhov wrote:
> Jan Kara <jack@suse.cz> writes:
> 
> > On Wed 17-02-10 21:47:06, Dmitry Monakhov wrote:
> >> 
> >> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> >> ---
> >>  fs/udf/ialloc.c |   11 ++---------
> >>  fs/udf/namei.c  |    3 ---
> >>  2 files changed, 2 insertions(+), 12 deletions(-)
> >   Hmm, this patch looks kind of half baked. There are several calls to
> > udf_new_inode() in udf/namei.c. From your patch I'd expect they all should
> > pass correct final mode to udf_new_inode which they currently don't to (the
> > case of udf_mkdir and udf_symlink). Otherwise I'm fine with this change.
> Yeah, you right. I was to lazy to check all callers :(.
> Correct version are follows:

> From 612e46abea261e9fdaea5f3285c5deee4f86f162 Mon Sep 17 00:00:00 2001
> From: Dmitry Monakhov <dmonakhov@openvz.org>
> Date: Thu, 18 Feb 2010 09:42:52 +0300
> Subject: [PATCH 18/19] udf: replace inode uid,gid,mode init with helper v2
> 
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/udf/ialloc.c |   11 ++---------
>  fs/udf/namei.c  |    8 +-------
>  2 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index c10fa39..52d6e4d 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -691,9 +689,6 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>  			FID_FILE_CHAR_DIRECTORY | FID_FILE_CHAR_PARENT;
>  	udf_write_fi(inode, &cfi, fi, &fibh, NULL, NULL);
>  	brelse(fibh.sbh);
> -	inode->i_mode = S_IFDIR | mode;
> -	if (dir->i_mode & S_ISGID)
> -		inode->i_mode |= S_ISGID;
>  	mark_inode_dirty(inode);
>  
>  	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
  I think you're still missing to call udf_new_inode with S_IFDIR | mode in
this function.

										Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 18/19] udf: replace inode uid,gid,mode init with helper v3

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 241 bytes --]

Jan Kara <jack@suse.cz> writes:

>   I think you're still missing to call udf_new_inode with S_IFDIR | mode in
> this function.
Ohh, crap... i dont know what am i thinking about. Really sorry for waisting
your time. Correct patch attached.


[-- Attachment #2: 0018-udf-replace-inode-uid-gid-mode-init-with-helper-v3.patch --]
[-- Type: text/plain, Size: 2907 bytes --]

>From 42ae3fb0a4f1ac3c1b3338a6d3107363c8c40757 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 22:37:09 +0300
Subject: [PATCH 18/19] udf: replace inode uid,gid,mode init with helper v3


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/udf/ialloc.c |   11 ++---------
 fs/udf/namei.c  |   10 ++--------
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index c10fa39..52d6e4d 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -124,15 +124,8 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
 		udf_updated_lvid(sb);
 	}
 	mutex_unlock(&sbi->s_alloc_mutex);
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
-	} else {
-		inode->i_gid = current_fsgid();
-	}
+
+	inode_init_owner(inode, dir, mode);
 
 	iinfo->i_location.logicalBlockNum = block;
 	iinfo->i_location.partitionReferenceNum =
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index cd21150..6297432 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -577,7 +577,6 @@ static int udf_create(struct inode *dir, struct dentry *dentry, int mode,
 		inode->i_data.a_ops = &udf_aops;
 	inode->i_op = &udf_file_inode_operations;
 	inode->i_fop = &udf_file_operations;
-	inode->i_mode = mode;
 	mark_inode_dirty(inode);
 
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
@@ -623,7 +622,6 @@ static int udf_mknod(struct inode *dir, struct dentry *dentry, int mode,
 		goto out;
 
 	iinfo = UDF_I(inode);
-	inode->i_uid = current_fsuid();
 	init_special_inode(inode, mode, rdev);
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
 	if (!fi) {
@@ -668,7 +666,7 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 		goto out;
 
 	err = -EIO;
-	inode = udf_new_inode(dir, S_IFDIR, &err);
+	inode = udf_new_inode(dir, S_IFDIR | mode, &err);
 	if (!inode)
 		goto out;
 
@@ -691,9 +689,6 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 			FID_FILE_CHAR_DIRECTORY | FID_FILE_CHAR_PARENT;
 	udf_write_fi(inode, &cfi, fi, &fibh, NULL, NULL);
 	brelse(fibh.sbh);
-	inode->i_mode = S_IFDIR | mode;
-	if (dir->i_mode & S_ISGID)
-		inode->i_mode |= S_ISGID;
 	mark_inode_dirty(inode);
 
 	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
@@ -900,7 +895,7 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 	struct udf_inode_info *iinfo;
 
 	lock_kernel();
-	inode = udf_new_inode(dir, S_IFLNK, &err);
+	inode = udf_new_inode(dir, S_IFLNK | S_IRWXUGO, &err);
 	if (!inode)
 		goto out;
 
@@ -911,7 +906,6 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
 	}
 
 	iinfo = UDF_I(inode);
-	inode->i_mode = S_IFLNK | S_IRWXUGO;
 	inode->i_data.a_ops = &udf_symlink_aops;
 	inode->i_op = &page_symlink_inode_operations;
 
-- 
1.6.6

Re: [PATCH 18/19] udf: replace inode uid,gid,mode init with helper v3

[Jan Kara]

On Thu 18-02-10 22:55:43, Dmitry Monakhov wrote:
> Jan Kara <jack@suse.cz> writes:
> 
> >   I think you're still missing to call udf_new_inode with S_IFDIR | mode in
> > this function.
> Ohh, crap... i dont know what am i thinking about. Really sorry for waisting
> your time. Correct patch attached.
  No problem. Thanks for the patch. This time it looks OK so you can add
Acked-by: Jan Kara <jack@suse.cz>

  Since the patch depends on the generic patch, I expect you'll merge it
together with it.

									Honza
> 

> From 42ae3fb0a4f1ac3c1b3338a6d3107363c8c40757 Mon Sep 17 00:00:00 2001
> From: Dmitry Monakhov <dmonakhov@openvz.org>
> Date: Thu, 18 Feb 2010 22:37:09 +0300
> Subject: [PATCH 18/19] udf: replace inode uid,gid,mode init with helper v3
> 
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/udf/ialloc.c |   11 ++---------
>  fs/udf/namei.c  |   10 ++--------
>  2 files changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index c10fa39..52d6e4d 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -124,15 +124,8 @@ struct inode *udf_new_inode(struct inode *dir, int mode, int *err)
>  		udf_updated_lvid(sb);
>  	}
>  	mutex_unlock(&sbi->s_alloc_mutex);
> -	inode->i_mode = mode;
> -	inode->i_uid = current_fsuid();
> -	if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
> -	} else {
> -		inode->i_gid = current_fsgid();
> -	}
> +
> +	inode_init_owner(inode, dir, mode);
>  
>  	iinfo->i_location.logicalBlockNum = block;
>  	iinfo->i_location.partitionReferenceNum =
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index cd21150..6297432 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -577,7 +577,6 @@ static int udf_create(struct inode *dir, struct dentry *dentry, int mode,
>  		inode->i_data.a_ops = &udf_aops;
>  	inode->i_op = &udf_file_inode_operations;
>  	inode->i_fop = &udf_file_operations;
> -	inode->i_mode = mode;
>  	mark_inode_dirty(inode);
>  
>  	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
> @@ -623,7 +622,6 @@ static int udf_mknod(struct inode *dir, struct dentry *dentry, int mode,
>  		goto out;
>  
>  	iinfo = UDF_I(inode);
> -	inode->i_uid = current_fsuid();
>  	init_special_inode(inode, mode, rdev);
>  	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
>  	if (!fi) {
> @@ -668,7 +666,7 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>  		goto out;
>  
>  	err = -EIO;
> -	inode = udf_new_inode(dir, S_IFDIR, &err);
> +	inode = udf_new_inode(dir, S_IFDIR | mode, &err);
>  	if (!inode)
>  		goto out;
>  
> @@ -691,9 +689,6 @@ static int udf_mkdir(struct inode *dir, struct dentry *dentry, int mode)
>  			FID_FILE_CHAR_DIRECTORY | FID_FILE_CHAR_PARENT;
>  	udf_write_fi(inode, &cfi, fi, &fibh, NULL, NULL);
>  	brelse(fibh.sbh);
> -	inode->i_mode = S_IFDIR | mode;
> -	if (dir->i_mode & S_ISGID)
> -		inode->i_mode |= S_ISGID;
>  	mark_inode_dirty(inode);
>  
>  	fi = udf_add_entry(dir, dentry, &fibh, &cfi, &err);
> @@ -900,7 +895,7 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
>  	struct udf_inode_info *iinfo;
>  
>  	lock_kernel();
> -	inode = udf_new_inode(dir, S_IFLNK, &err);
> +	inode = udf_new_inode(dir, S_IFLNK | S_IRWXUGO, &err);
>  	if (!inode)
>  		goto out;
>  
> @@ -911,7 +906,6 @@ static int udf_symlink(struct inode *dir, struct dentry *dentry,
>  	}
>  
>  	iinfo = UDF_I(inode);
> -	inode->i_mode = S_IFLNK | S_IRWXUGO;
>  	inode->i_data.a_ops = &udf_symlink_aops;
>  	inode->i_op = &page_symlink_inode_operations;
>  
> -- 
> 1.6.6
> 

-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 19/19] ufs: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ufs/ialloc.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/fs/ufs/ialloc.c b/fs/ufs/ialloc.c
index 3527c00..59b8315 100644
--- a/fs/ufs/ialloc.c
+++ b/fs/ufs/ialloc.c
@@ -303,15 +303,7 @@ cg_found:
 	sb->s_dirt = 1;
 
 	inode->i_ino = cg * uspi->s_ipg + bit;
-	inode->i_mode = mode;
-	inode->i_uid = current_fsuid();
-	if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			inode->i_mode |= S_ISGID;
-	} else
-		inode->i_gid = current_fsgid();
-
+	inode_init_owner(inode, dir, mode);
 	inode->i_blocks = 0;
 	inode->i_generation = 0;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
-- 
1.6.6

[PATCH 00/19] fs cleanup: remove duplicated code on inode init v2

[Dmitry Monakhov]

Al Please take care of the first patch(the vfs helper).
But it will be good if you also take the others.

Each filesystem init uid,gid,mode on inode creation.
gid inheritance is obey to posix rules. Usually this code is
copy-pasted. Some times it was done in wrong way.
Let's move this logic to separate function.

In some filesystems it is not easy to replace the code, so i've
simply skipped such fs.
Skipped fs: xfs, hugetlbfs, gfs2, cifs, affs

Some filesystems require less trivial code replacement
so i've split the patch on to per-fs basis.

Patch prepared against vfs/untested tree, but it also may
be applied on top of linux-next-2010-03-04.
There was some rejects from ocfs2's patch on linux-next tree
because of dlm changes. So i've skipp dlm changes in ocfs2's patch.
I'll send dlm changes to ocfs2 mainainers directly after
core vfs helpers changes appears in linux-next tree.

[PATCH 06/19] ext2: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

Acked-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext2/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index 15387c9..2c484c6 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -550,16 +550,12 @@ got:
 
 	sb->s_dirt = 1;
 	mark_buffer_dirty(bh2);
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	inode->i_blocks = 0;
-- 
1.6.6