2014-10-26 05:22:48

by Li Xi

[permalink] [raw]
Subject: [v5 0/5] quota: add project quota support for ext4

The following patches propose an implementation of project quota
support for ext4. A project is an aggregate of unrelated inodes
which might scatter in different directories. Inodes belongs to a
project possesses a same identification i.e. 'project ID', just
like every inode has its user/group identification. The following
patches adds project quota as supplement to the former uer/group
quota types.

The semantics of ext4 project quota is consistent with XFS. Each
directory can have EXT4_INODE_PROJINHERIT flag set. When the
EXT4_INODE_PROJINHERIT flag of a parent directory is not set, a
newly created inode under that directory will have a default project
ID (i.e. 0). And its EXT4_INODE_PROJINHERIT flag is not set either.
When this flag is set on a directory, following rules will be kept:

1) The newly created inode under that directory will inherit both
the EXT4_INODE_PROJINHERIT flag and the project ID from its parent
directory.

2) Hard-linking a inode with different project ID into that directory
will fail with errno EXDEV.

3) Renaming a inode with different project ID into that directory
will fail with errno EXDEV. However, 'mv' command will detect this
failure and copy the renamed inode to a new inode in the directory.
Thus, this new inode will inherit both the project ID and
EXT4_INODE_PROJINHERIT flag.

4) If the project quota of that ID is being enforced, statfs() on
that directory will take the quotas as another upper limits along
with the capacity of the file system, i.e. the total block/inode
number will be the minimum of the quota limits and file system
capacity.

Changelog:
* v5 <- v4:
- Check project feature when set/get project ID;
- Do not check project feature for project quota;
- Add support of FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR.
* v4 <- v3:
- Do not check project feature when set/get project ID;
- Use EXT4_MAXQUOTAS instead of MAXQUOTAS in ext4 patches;
- Remove unnecessary change of fs/quota/dquot.c;
- Remove CONFIG_QUOTA_PROJECT.
* v3 <- v2:
- Add EXT4_INODE_PROJINHERIT semantics.
* v2 <- v1:
- Add ioctl interface for setting/getting project;
- Add EXT4_FEATURE_RO_COMPAT_PROJECT;
- Add get_projid() method in struct dquot_operations;
- Add error check of ext4_inode_projid_set/get().

v4: http://lwn.net/Articles/612972/
v3: http://www.spinics.net/lists/linux-ext4/msg45184.html
v2: http://www.spinics.net/lists/linux-ext4/msg44695.html
v1: http://article.gmane.org/gmane.comp.file-systems.ext4/45153

Any comments or feedbacks are appreciated.

Regards,
- Li Xi

Li Xi (5):
Adds general codes to enforces project quota limits
Adds project ID support for ext4
Adds project quota support for ext4
Adds ioctl interface support for ext4 project
Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

Documentation/filesystems/ext4.txt | 4 +
fs/ext4/ext4.h | 33 ++++-
fs/ext4/ialloc.c | 8 +
fs/ext4/inode.c | 31 +++++
fs/ext4/ioctl.c | 253 ++++++++++++++++++++++++++++++++++++
fs/ext4/namei.c | 10 ++
fs/ext4/super.c | 96 ++++++++++++--
fs/quota/dquot.c | 32 ++++-
fs/quota/quota.c | 10 +-
fs/quota/quotaio_v2.h | 6 +-
fs/xfs/xfs_fs.h | 17 +--
include/linux/quota.h | 2 +
include/uapi/linux/fs.h | 22 +++
include/uapi/linux/quota.h | 6 +-
14 files changed, 488 insertions(+), 42 deletions(-)


2014-10-26 05:25:42

by Li Xi

[permalink] [raw]
Subject: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
support for ext4. The interface is kept consistent with
XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.

Signed-off-by: Li Xi <[email protected]>
---
fs/ext4/ext4.h | 2 +
fs/ext4/ioctl.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/xfs_fs.h | 17 +-----
include/uapi/linux/fs.h | 22 +++++++
4 files changed, 177 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index f0e3e08..ebe40ad 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -619,6 +619,8 @@ enum {
#define EXT4_IOC_PRECACHE_EXTENTS _IO('f', 18)
#define EXT4_IOC_GETPROJECT _IOR('f', 19, long)
#define EXT4_IOC_SETPROJECT _IOW('f', 20, long)
+#define EXT4_IOC_FSGETXATTR FS_IOC_FSGETXATTR
+#define EXT4_IOC_FSSETXATTR FS_IOC_FSSETXATTR

#if defined(__KERNEL__) && defined(CONFIG_COMPAT)
/*
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index b685c42..21ec6fb 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -200,6 +200,112 @@ journal_err_out:
return err;
}

+static int ext4_ioctl_setflags(struct file *filp, unsigned int flags)
+{
+ struct inode *inode = file_inode(filp);
+ struct ext4_inode_info *ei = EXT4_I(inode);
+ handle_t *handle = NULL;
+ int err, migrate = 0;
+ struct ext4_iloc iloc;
+ unsigned int oldflags, mask, i;
+ unsigned int jflag;
+
+ if (!inode_owner_or_capable(inode))
+ return -EACCES;
+
+ err = mnt_want_write_file(filp);
+ if (err)
+ return err;
+
+ flags = ext4_mask_flags(inode->i_mode, flags);
+
+ err = -EPERM;
+ mutex_lock(&inode->i_mutex);
+ /* Is it quota file? Do not allow user to mess with it */
+ if (IS_NOQUOTA(inode))
+ goto flags_out;
+
+ oldflags = ei->i_flags;
+
+ /* The JOURNAL_DATA flag is modifiable only by root */
+ jflag = flags & EXT4_JOURNAL_DATA_FL;
+
+ /*
+ * The IMMUTABLE and APPEND_ONLY flags can only be changed by
+ * the relevant capability.
+ *
+ * This test looks nicer. Thanks to Pauline Middelink
+ */
+ if ((flags ^ oldflags) & (EXT4_APPEND_FL | EXT4_IMMUTABLE_FL)) {
+ if (!capable(CAP_LINUX_IMMUTABLE))
+ goto flags_out;
+ }
+
+ /*
+ * The JOURNAL_DATA flag can only be changed by
+ * the relevant capability.
+ */
+ if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL)) {
+ if (!capable(CAP_SYS_RESOURCE))
+ goto flags_out;
+ }
+ if ((flags ^ oldflags) & EXT4_EXTENTS_FL)
+ migrate = 1;
+ if (flags & EXT4_EOFBLOCKS_FL) {
+ /* we don't support adding EOFBLOCKS flag */
+ if (!(oldflags & EXT4_EOFBLOCKS_FL)) {
+ err = -EOPNOTSUPP;
+ goto flags_out;
+ }
+ } else if (oldflags & EXT4_EOFBLOCKS_FL)
+ ext4_truncate(inode);
+
+ handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
+ if (IS_ERR(handle)) {
+ err = PTR_ERR(handle);
+ goto flags_out;
+ }
+ if (IS_SYNC(inode))
+ ext4_handle_sync(handle);
+ err = ext4_reserve_inode_write(handle, inode, &iloc);
+ if (err)
+ goto flags_err;
+
+ for (i = 0, mask = 1; i < 32; i++, mask <<= 1) {
+ if (!(mask & EXT4_FL_USER_MODIFIABLE))
+ continue;
+ if (mask & flags)
+ ext4_set_inode_flag(inode, i);
+ else
+ ext4_clear_inode_flag(inode, i);
+ }
+
+ ext4_set_inode_flags(inode);
+ inode->i_ctime = ext4_current_time(inode);
+
+ err = ext4_mark_iloc_dirty(handle, inode, &iloc);
+flags_err:
+ ext4_journal_stop(handle);
+ if (err)
+ goto flags_out;
+
+ if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL))
+ err = ext4_change_inode_journal_flag(inode, jflag);
+ if (err)
+ goto flags_out;
+ if (migrate) {
+ if (flags & EXT4_EXTENTS_FL)
+ err = ext4_ext_migrate(inode);
+ else
+ err = ext4_ind_migrate(inode);
+ }
+
+flags_out:
+ mutex_unlock(&inode->i_mutex);
+ mnt_drop_write_file(filp);
+ return err;
+}
+
static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
{
struct inode *inode = file_inode(filp);
@@ -720,6 +826,51 @@ resizefs_out:

return ext4_ioctl_setproject(filp, projid);
}
+ case EXT4_IOC_FSGETXATTR:
+ {
+ struct fsxattr fa;
+
+ memset(&fa, 0, sizeof(struct fsxattr));
+
+ ext4_get_inode_flags(ei);
+ fa.fsx_xflags = ei->i_flags & EXT4_FL_USER_VISIBLE;
+ fa.fsx_valid |= FSX_XFLAGS;
+
+ if (EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
+ EXT4_FEATURE_RO_COMPAT_PROJECT)) {
+ fa.fsx_projid = (__u32)from_kprojid(&init_user_ns,
+ EXT4_I(inode)->i_projid);
+ fa.fsx_valid |= FSX_PROJID;
+ }
+
+ if (copy_to_user((struct fsxattr __user *)arg,
+ &fa, sizeof(fa)))
+ return -EFAULT;
+ return 0;
+ }
+ case EXT4_IOC_FSSETXATTR:
+ {
+ struct fsxattr fa;
+ int err;
+
+ if (copy_from_user(&fa, (struct fsxattr __user *)arg,
+ sizeof(fa)))
+ return -EFAULT;
+
+ if (fa.fsx_valid & FSX_XFLAGS) {
+ err = ext4_ioctl_setflags(filp, fa.fsx_xflags);
+ if (err)
+ return err;
+ }
+
+ if (fa.fsx_valid & FSX_PROJID) {
+ err = ext4_ioctl_setproject(filp, fa.fsx_projid);
+ if (err)
+ return err;
+ }
+
+ return 0;
+ }
default:
return -ENOTTY;
}
diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
index 18dc721..5dd6013 100644
--- a/fs/xfs/xfs_fs.h
+++ b/fs/xfs/xfs_fs.h
@@ -36,19 +36,6 @@ struct dioattr {
#endif

/*
- * Structure for XFS_IOC_FSGETXATTR[A] and XFS_IOC_FSSETXATTR.
- */
-#ifndef HAVE_FSXATTR
-struct fsxattr {
- __u32 fsx_xflags; /* xflags field value (get/set) */
- __u32 fsx_extsize; /* extsize field value (get/set)*/
- __u32 fsx_nextents; /* nextents field value (get) */
- __u32 fsx_projid; /* project identifier (get/set) */
- unsigned char fsx_pad[12];
-};
-#endif
-
-/*
* Flags for the bs_xflags/fsx_xflags field
* There should be a one-to-one correspondence between these flags and the
* XFS_DIFLAG_s.
@@ -503,8 +490,8 @@ typedef struct xfs_swapext
#define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
#define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
#define XFS_IOC_DIOINFO _IOR ('X', 30, struct dioattr)
-#define XFS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr)
-#define XFS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr)
+#define XFS_IOC_FSGETXATTR FS_IOC_FSGETXATTR
+#define XFS_IOC_FSSETXATTR FS_IOC_FSSETXATTR
#define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
#define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
#define XFS_IOC_GETBMAP _IOWR('X', 38, struct getbmap)
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index ca1a11b..89c6492 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -57,6 +57,26 @@ struct inodes_stat_t {
long dummy[5]; /* padding for sysctl ABI compatibility */
};

+/*
+ * Extend attribute flags. These should be or-ed together to figure out what
+ * is valid.
+ */
+#define FSX_XFLAGS (1 << 0)
+#define FSX_EXTSIZE (1 << 1)
+#define FSX_NEXTENTS (1 << 2)
+#define FSX_PROJID (1 << 3)
+
+/*
+ * Structure for FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR.
+ */
+struct fsxattr {
+ __u32 fsx_xflags; /* xflags field value (get/set) */
+ __u32 fsx_extsize; /* extsize field value (get/set)*/
+ __u32 fsx_nextents; /* nextents field value (get) */
+ __u32 fsx_projid; /* project identifier (get/set) */
+ __u32 fsx_valid;
+ unsigned char fsx_pad[8];
+};

#define NR_FILE 8192 /* this can well be larger on a larger system */

@@ -162,6 +182,8 @@ struct inodes_stat_t {
#define FS_IOC_GETVERSION _IOR('v', 1, long)
#define FS_IOC_SETVERSION _IOW('v', 2, long)
#define FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
+#define FS_IOC_FSGETXATTR _IOR('f', 31, long)
+#define FS_IOC_FSSETXATTR _IOW('f', 32, long)
#define FS_IOC32_GETFLAGS _IOR('f', 1, int)
#define FS_IOC32_SETFLAGS _IOW('f', 2, int)
#define FS_IOC32_GETVERSION _IOR('v', 1, int)
--
1.7.1


2014-10-26 21:56:26

by Dave Chinner

[permalink] [raw]
Subject: Re: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

On Sun, Oct 26, 2014 at 01:22:53PM +0800, Li Xi wrote:
> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> support for ext4. The interface is kept consistent with
> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.

What you haven't mentioned is that you also changed the fsxattr
interface structure to add functionality and new behaviours that
isn't supported by XFS or existing applications that use the
interface.

There is no need to modify the interface *at all* for ext4 to use
it. Fields that ext4 does not use can be zeroed on getxattr, and
ignored on setxattr - you do not need to add new fields to say what
fields are valid.

> +/*
> + * Extend attribute flags. These should be or-ed together to figure out what
> + * is valid.
> + */
> +#define FSX_XFLAGS (1 << 0)
> +#define FSX_EXTSIZE (1 << 1)
> +#define FSX_NEXTENTS (1 << 2)
> +#define FSX_PROJID (1 << 3)

They are not interface definition flags - these are internal XFS
flags to tell an internal shared function what the caller was
modifying. I actually have a series of patches that removes them
because the internal XFS code shouldn't be shared in this way. As
such, they shouldn't be propagated into the user interface for the
ioctls....

Cheers,

Dave.
--
Dave Chinner
[email protected]

2014-10-27 01:09:20

by Li Xi

[permalink] [raw]
Subject: Re: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

On Mon, Oct 27, 2014 at 5:56 AM, Dave Chinner <[email protected]> wrote:
> On Sun, Oct 26, 2014 at 01:22:53PM +0800, Li Xi wrote:
>> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
>> support for ext4. The interface is kept consistent with
>> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
>
> What you haven't mentioned is that you also changed the fsxattr
> interface structure to add functionality and new behaviours that
> isn't supported by XFS or existing applications that use the
> interface.
>
> There is no need to modify the interface *at all* for ext4 to use
> it. Fields that ext4 does not use can be zeroed on getxattr, and
> ignored on setxattr - you do not need to add new fields to say what
> fields are valid.
Sorry, I don't want to change the interfaces either. But, the problem
is that zero might be valid value for some fields. How can we
distinguish an unsupported attribute and an attribute whose value
is zero? It is common case the only part of the fields are supported.
So, for example, if we don't have valid flags, how can use space
application tell kernel which attributes should be skipped when it
tries to set only a part of atrributes?

Regards,
Li Xi

2014-10-27 22:40:58

by Dave Chinner

[permalink] [raw]
Subject: Re: [v5 5/5] Adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support for ext4

On Mon, Oct 27, 2014 at 09:09:19AM +0800, Li Xi wrote:
> On Mon, Oct 27, 2014 at 5:56 AM, Dave Chinner <[email protected]> wrote:
> > On Sun, Oct 26, 2014 at 01:22:53PM +0800, Li Xi wrote:
> >> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> >> support for ext4. The interface is kept consistent with
> >> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
> >
> > What you haven't mentioned is that you also changed the fsxattr
> > interface structure to add functionality and new behaviours that
> > isn't supported by XFS or existing applications that use the
> > interface.
> >
> > There is no need to modify the interface *at all* for ext4 to use
> > it. Fields that ext4 does not use can be zeroed on getxattr, and
> > ignored on setxattr - you do not need to add new fields to say what
> > fields are valid.
> Sorry, I don't want to change the interfaces either. But, the problem
> is that zero might be valid value for some fields. How can we
> distinguish an unsupported attribute and an attribute whose value
> is zero?

You don't. Userspace has no concept of what parts of the struct
fsxattr are valid or not, nor what are valid values the filesystem
will accept or reject.

> It is common case the only part of the fields are supported.
> So, for example, if we don't have valid flags, how can use space
> application tell kernel which attributes should be skipped when it
> tries to set only a part of atrributes?

It *doesn't*. Userspace requires the kernel to initialise the struct
fsxattr before it tries to modify anything, just like
fcntl(F_[GS]ETFL) and other similar "file flag change" syscall APIs.

IOWs, you have to initialise the struct fsxattr by calling
FS_IOC_FSGETXATTR before you call FS_IOC_FSSETXATTR. The
intialisation sets all the fields to the current (correct) values,
and hence when the set call is made all the fields then have the
same/correct values in them except for what the application changed.

E.g. this code from xfs_quota to clear the project ID on a given
file or directory:

if ((fd = open(path, O_RDONLY|O_NOCTTY)) == -1) {
// error handling ....
} else if (xfsctl(path, fd, XFS_IOC_FSGETXATTR, &fsx) < 0) {
// error handling ....
}

fsx.fsx_projid = 0;
fsx.fsx_xflags &= ~XFS_XFLAG_PROJINHERIT;
if (xfsctl(path, fd, XFS_IOC_FSSETXATTR, &fsx) < 0) {
// error handling ....
}
close(fd);

This ensures that *only* the project ID and the specific project ID
inheritance flag is cleared, and none of the other inode flags or
state are modified....

Cheers,

Dave.
--
Dave Chinner
[email protected]