[PATCH v2 00/35] Add quota supporting in ubifs

[PATCH v2 00/35] Add quota supporting in ubifs

[Dongsheng Yang]

Hi all,
	This is the V2 for adding quota in ubifs.

Changelog:
	v1->v2:
		- remove the duplication in block_dev and char_dev
		- rename read_block to ubifs_read_block
		- drop patch for ui->budgeted
		- budget in ubifs_dirty_inode() if necessary

TEST:
	With a patchset [xfstests: Introduce ubifs to xfstests],
we can run xfstests for ubifs. And the result is shown below:

 # ./check -ubifs
FSTYP         -- ubifs
PLATFORM      -- Linux/x86_64 atest-guest 4.1.0+
MKFS_OPTIONS  -- /dev/ubi0_1
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/ubi0_1 /mnt/scratch

generic/001 7s ... 7s
generic/002 1s ... 1s

[...]

Passed all 70 tests

 # ./check -ubifs -g quota
FSTYP         -- ubifs
PLATFORM      -- Linux/x86_64 atest-guest 4.1.0+
MKFS_OPTIONS  -- /dev/ubi0_1
MOUNT_OPTIONS -- -o context=system_u:object_r:nfs_t:s0 /dev/ubi0_1 /mnt/scratch

generic/055	 [not run] ubifs does not support shutdown
generic/082 1s ... 0s
generic/219 23s ... [not run] O_DIRECT is not supported
generic/230 13s ... 13s
generic/231 51s ... 50s
generic/232 3s ... 3s
generic/233 4s ... 5s
generic/234 10s ... 9s
generic/235 1s ... 1s
generic/270	 [not run] Filesystem ubifs not supported in _scratch_mkfs_sized
generic/280	 [not run] ubifs does not support freezing
Ran: generic/082 generic/230 generic/231 generic/232 generic/233 generic/234 generic/235
Not run: generic/055 generic/219 generic/270 generic/280
Passed all 7 tests

You can get the code by:
ubifs:  https://github.com/yangdongsheng/linux.git ubifs_quota_v2
xfstests: https://github.com/yangdongsheng/xfstests.git ubifs_v1

NOTE: the branch of ubifs_quota_v2 contains one patch about atime and one patch about get/setxattr.
These patches can make xfstests all pass and there would be some conflict between them and quota
patchset. So I keep them in this branch.

NOTE2: I found Jan has post his patch about "quota: Propagate errors when creating quota entry",
if that went to mainline, I will send a patch to catch the ret-value of dquot_initialize() in ubifs.

Any test or review is welcom :)

Thanx
Yang

Dongsheng Yang (35):
  fs: introduce a ->s_cdev field into struct super_block
  fs: cleanup: remove the blank line before EXPORT_SYMBOL
  fs: super: cleanup: make the comment of each function aligned
  fs: super: consolidate the get_super class functions
  fs: super: introduce a get_super_cdev to get super by a cdev reference
  fs: super: introduce a get_super_cdev_thawed to get sb by cdev
    reference
  fs: char_dev: introduce cd_acquire function to acquire cdev
  fs: introduce a __lookup_dev for internal using
  fs: char_dev: introduce lookup_cdev to get cdev by pathname
  fs: dquot: skip invalidate_bdev if bdev is NULL
  fs: quota: make quota support fs which is running on char dev
  ubi: introduce a interface to get cdev in ubi_volume
  ubifs: fix a typo in comment of ubifs_budget_req
  ubifs: extend budget for blocks
  ubifs: fill sb->s_cdev in ubifs_fill_super()
  ubifs: fill ->s_dev in ubifs_fill_super
  ubifs: export read_block() from file.c
  ubifs: introduce quota related mount options
  ubifs: budget for inode in ubifs_dirty_inode if necessary
  ubifs: implement IO functions for quota files
  ubifs: disable quota in ubifs_put_super
  ubifs: write quota back in ubifs_sync
  ubifs: set/clear MS_RDONLY properly in ubifs_remount
  ubifs: suspend & resume quota properly in ubifs_remount
  ubifs: record quota information about inode in ubifs_new_inode
  ubifs: free quota inode information in ubifs_evict_inode
  ubifs: alloc quota space in ubifs_write_begin
  ubifs: free quota space in do_truncation
  ubifs: free quota space when deleting a file
  ubifs: adapt quota space informatin in do_setattr
  ubifs: transfer quota information in changing owner or group
  ubifs: write inode in ubifs_quota_write if we are appending
  fs: introduce a get_qsize() to file_operations
  ubifs: implement ubifs_get_qsize to get quota size in ubifs
  ubifs: make ubifs to support quota

 drivers/mtd/ubi/kapi.c  |   6 +
 fs/Makefile             |   2 +-
 fs/block_dev.c          |  26 +---
 fs/char_dev.c           |  48 ++++++++
 fs/dev.c                |  77 ++++++++++++
 fs/internal.h           |  14 +++
 fs/ioctl.c              |  31 +++--
 fs/quota/dquot.c        |   3 +-
 fs/quota/quota.c        |  30 +++--
 fs/super.c              | 284 +++++++++++++++++++++++--------------------
 fs/ubifs/budget.c       |   4 +
 fs/ubifs/debug.c        |   2 +
 fs/ubifs/dir.c          |  24 +++-
 fs/ubifs/file.c         |  78 +++++++++++-
 fs/ubifs/journal.c      |   6 +
 fs/ubifs/super.c        | 317 +++++++++++++++++++++++++++++++++++++++++++++++-
 fs/ubifs/ubifs.h        |  19 ++-
 include/linux/fs.h      |   6 +
 include/linux/mtd/ubi.h |   1 +
 19 files changed, 792 insertions(+), 186 deletions(-)
 create mode 100644 fs/dev.c

-- 
1.8.4.2

[PATCH v2 01/35] fs: introduce a ->s_cdev field into struct super_block

[Dongsheng Yang]

There are some filesystems are running on char devs, such
as ubifs. So we need a field in super_block to hold a
reference to the char device.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 include/linux/fs.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 35ec87e..2f1d9499 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1284,6 +1284,7 @@ struct super_block {
 	struct hlist_bl_head	s_anon;		/* anonymous dentries for (nfs) exporting */
 	struct list_head	s_mounts;	/* list of mounts; _not_ for fs use */
 	struct block_device	*s_bdev;
+	struct cdev		*s_cdev;
 	struct backing_dev_info *s_bdi;
 	struct mtd_info		*s_mtd;
 	struct hlist_node	s_instances;
-- 
1.8.4.2

[PATCH v2 02/35] fs: cleanup: remove the blank line before EXPORT_SYMBOL

[Dongsheng Yang]

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/super.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 928c20f..b48077b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -299,7 +299,6 @@ void deactivate_locked_super(struct super_block *s)
 		up_write(&s->s_umount);
 	}
 }
-
 EXPORT_SYMBOL(deactivate_locked_super);
 
 /**
@@ -317,7 +316,6 @@ void deactivate_super(struct super_block *s)
 		deactivate_locked_super(s);
 	}
 }
-
 EXPORT_SYMBOL(deactivate_super);
 
 /**
@@ -423,7 +421,6 @@ void generic_shutdown_super(struct super_block *sb)
 	spin_unlock(&sb_lock);
 	up_write(&sb->s_umount);
 }
-
 EXPORT_SYMBOL(generic_shutdown_super);
 
 /**
@@ -484,7 +481,6 @@ retry:
 	register_shrinker(&s->s_shrink);
 	return s;
 }
-
 EXPORT_SYMBOL(sget);
 
 void drop_super(struct super_block *sb)
@@ -492,7 +488,6 @@ void drop_super(struct super_block *sb)
 	up_read(&sb->s_umount);
 	put_super(sb);
 }
-
 EXPORT_SYMBOL(drop_super);
 
 /**
@@ -562,7 +557,6 @@ void iterate_supers_type(struct file_system_type *type,
 		__put_super(p);
 	spin_unlock(&sb_lock);
 }
-
 EXPORT_SYMBOL(iterate_supers_type);
 
 /**
@@ -602,7 +596,6 @@ rescan:
 	spin_unlock(&sb_lock);
 	return NULL;
 }
-
 EXPORT_SYMBOL(get_super);
 
 /**
@@ -870,7 +863,6 @@ int set_anon_super(struct super_block *s, void *data)
 {
 	return get_anon_bdev(&s->s_dev);
 }
-
 EXPORT_SYMBOL(set_anon_super);
 
 void kill_anon_super(struct super_block *sb)
@@ -879,7 +871,6 @@ void kill_anon_super(struct super_block *sb)
 	generic_shutdown_super(sb);
 	free_anon_bdev(dev);
 }
-
 EXPORT_SYMBOL(kill_anon_super);
 
 void kill_litter_super(struct super_block *sb)
@@ -888,7 +879,6 @@ void kill_litter_super(struct super_block *sb)
 		d_genocide(sb->s_root);
 	kill_anon_super(sb);
 }
-
 EXPORT_SYMBOL(kill_litter_super);
 
 static int ns_test_super(struct super_block *sb, void *data)
@@ -924,7 +914,6 @@ struct dentry *mount_ns(struct file_system_type *fs_type, int flags,
 
 	return dget(sb->s_root);
 }
-
 EXPORT_SYMBOL(mount_ns);
 
 #ifdef CONFIG_BLOCK
@@ -1034,7 +1023,6 @@ void kill_block_super(struct super_block *sb)
 	WARN_ON_ONCE(!(mode & FMODE_EXCL));
 	blkdev_put(bdev, mode | FMODE_EXCL);
 }
-
 EXPORT_SYMBOL(kill_block_super);
 #endif
 
-- 
1.8.4.2

[PATCH v2 03/35] fs: super: cleanup: make the comment of each function aligned

[Dongsheng Yang]

Originally, some of the comments start with "*\t" but some of them
start with "* ". This commit make them all start with "* "

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/super.c | 180 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 90 insertions(+), 90 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index b48077b..fadb677 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -136,10 +136,10 @@ static unsigned long super_cache_count(struct shrinker *shrink,
 }
 
 /**
- *	destroy_super	-	frees a superblock
- *	@s: superblock to free
+ * destroy_super	-	frees a superblock
+ * @s: superblock to free
  *
- *	Frees a superblock.
+ * Frees a superblock.
  */
 static void destroy_super(struct super_block *s)
 {
@@ -156,12 +156,12 @@ static void destroy_super(struct super_block *s)
 }
 
 /**
- *	alloc_super	-	create new superblock
- *	@type:	filesystem type superblock should belong to
- *	@flags: the mount flags
+ * alloc_super	-	create new superblock
+ * @type:	filesystem type superblock should belong to
+ * @flags: the mount flags
  *
- *	Allocates and initializes a new &struct super_block.  alloc_super()
- *	returns a pointer new superblock or %NULL if allocation had failed.
+ * Allocates and initializes a new &struct super_block.  alloc_super()
+ * returns a pointer new superblock or %NULL if allocation had failed.
  */
 static struct super_block *alloc_super(struct file_system_type *type, int flags)
 {
@@ -252,11 +252,11 @@ static void __put_super(struct super_block *sb)
 }
 
 /**
- *	put_super	-	drop a temporary reference to superblock
- *	@sb: superblock in question
+ * put_super	-	drop a temporary reference to superblock
+ * @sb: superblock in question
  *
- *	Drops a temporary reference, frees superblock if there's no
- *	references left.
+ * Drops a temporary reference, frees superblock if there's no
+ * references left.
  */
 static void put_super(struct super_block *sb)
 {
@@ -267,15 +267,15 @@ static void put_super(struct super_block *sb)
 
 
 /**
- *	deactivate_locked_super	-	drop an active reference to superblock
- *	@s: superblock to deactivate
+ * deactivate_locked_super	-	drop an active reference to superblock
+ * @s: superblock to deactivate
  *
- *	Drops an active reference to superblock, converting it into a temprory
- *	one if there is no other active references left.  In that case we
- *	tell fs driver to shut it down and drop the temporary reference we
- *	had just acquired.
+ * Drops an active reference to superblock, converting it into a temprory
+ * one if there is no other active references left.  In that case we
+ * tell fs driver to shut it down and drop the temporary reference we
+ * had just acquired.
  *
- *	Caller holds exclusive lock on superblock; that lock is released.
+ * Caller holds exclusive lock on superblock; that lock is released.
  */
 void deactivate_locked_super(struct super_block *s)
 {
@@ -302,12 +302,12 @@ void deactivate_locked_super(struct super_block *s)
 EXPORT_SYMBOL(deactivate_locked_super);
 
 /**
- *	deactivate_super	-	drop an active reference to superblock
- *	@s: superblock to deactivate
+ * deactivate_super	-	drop an active reference to superblock
+ * @s: superblock to deactivate
  *
- *	Variant of deactivate_locked_super(), except that superblock is *not*
- *	locked by caller.  If we are going to drop the final active reference,
- *	lock will be acquired prior to that.
+ * Variant of deactivate_locked_super(), except that superblock is *not*
+ * locked by caller.  If we are going to drop the final active reference,
+ * lock will be acquired prior to that.
  */
 void deactivate_super(struct super_block *s)
 {
@@ -319,17 +319,17 @@ void deactivate_super(struct super_block *s)
 EXPORT_SYMBOL(deactivate_super);
 
 /**
- *	grab_super - acquire an active reference
- *	@s: reference we are trying to make active
+ * grab_super - acquire an active reference
+ * @s: reference we are trying to make active
  *
- *	Tries to acquire an active reference.  grab_super() is used when we
- * 	had just found a superblock in super_blocks or fs_type->fs_supers
- *	and want to turn it into a full-blown active reference.  grab_super()
- *	is called with sb_lock held and drops it.  Returns 1 in case of
- *	success, 0 if we had failed (superblock contents was already dead or
- *	dying when grab_super() had been called).  Note that this is only
- *	called for superblocks not in rundown mode (== ones still on ->fs_supers
- *	of their type), so increment of ->s_count is OK here.
+ * Tries to acquire an active reference.  grab_super() is used when we
+ * had just found a superblock in super_blocks or fs_type->fs_supers
+ * and want to turn it into a full-blown active reference.  grab_super()
+ * is called with sb_lock held and drops it.  Returns 1 in case of
+ * success, 0 if we had failed (superblock contents was already dead or
+ * dying when grab_super() had been called).  Note that this is only
+ * called for superblocks not in rundown mode (== ones still on ->fs_supers
+ * of their type), so increment of ->s_count is OK here.
  */
 static int grab_super(struct super_block *s) __releases(sb_lock)
 {
@@ -346,21 +346,21 @@ static int grab_super(struct super_block *s) __releases(sb_lock)
 }
 
 /*
- *	trylock_super - try to grab ->s_umount shared
- *	@sb: reference we are trying to grab
+ * trylock_super - try to grab ->s_umount shared
+ * @sb: reference we are trying to grab
  *
- *	Try to prevent fs shutdown.  This is used in places where we
- *	cannot take an active reference but we need to ensure that the
- *	filesystem is not shut down while we are working on it. It returns
- *	false if we cannot acquire s_umount or if we lose the race and
- *	filesystem already got into shutdown, and returns true with the s_umount
- *	lock held in read mode in case of success. On successful return,
- *	the caller must drop the s_umount lock when done.
+ * Try to prevent fs shutdown.  This is used in places where we
+ * cannot take an active reference but we need to ensure that the
+ * filesystem is not shut down while we are working on it. It returns
+ * false if we cannot acquire s_umount or if we lose the race and
+ * filesystem already got into shutdown, and returns true with the s_umount
+ * lock held in read mode in case of success. On successful return,
+ * the caller must drop the s_umount lock when done.
  *
- *	Note that unlike get_super() et.al. this one does *not* bump ->s_count.
- *	The reason why it's safe is that we are OK with doing trylock instead
- *	of down_read().  There's a couple of places that are OK with that, but
- *	it's very much not a general-purpose interface.
+ * Note that unlike get_super() et.al. this one does *not* bump ->s_count.
+ * The reason why it's safe is that we are OK with doing trylock instead
+ * of down_read().  There's a couple of places that are OK with that, but
+ * it's very much not a general-purpose interface.
  */
 bool trylock_super(struct super_block *sb)
 {
@@ -375,18 +375,18 @@ bool trylock_super(struct super_block *sb)
 }
 
 /**
- *	generic_shutdown_super	-	common helper for ->kill_sb()
- *	@sb: superblock to kill
+ * generic_shutdown_super	-	common helper for ->kill_sb()
+ * @sb: superblock to kill
  *
- *	generic_shutdown_super() does all fs-independent work on superblock
- *	shutdown.  Typical ->kill_sb() should pick all fs-specific objects
- *	that need destruction out of superblock, call generic_shutdown_super()
- *	and release aforementioned objects.  Note: dentries and inodes _are_
- *	taken care of and do not need specific handling.
+ * generic_shutdown_super() does all fs-independent work on superblock
+ * shutdown.  Typical ->kill_sb() should pick all fs-specific objects
+ * that need destruction out of superblock, call generic_shutdown_super()
+ * and release aforementioned objects.  Note: dentries and inodes _are_
+ * taken care of and do not need specific handling.
  *
- *	Upon calling this function, the filesystem may no longer alter or
- *	rearrange the set of dentries belonging to this super_block, nor may it
- *	change the attachments of dentries to inodes.
+ * Upon calling this function, the filesystem may no longer alter or
+ * rearrange the set of dentries belonging to this super_block, nor may it
+ * change the attachments of dentries to inodes.
  */
 void generic_shutdown_super(struct super_block *sb)
 {
@@ -424,12 +424,12 @@ void generic_shutdown_super(struct super_block *sb)
 EXPORT_SYMBOL(generic_shutdown_super);
 
 /**
- *	sget	-	find or create a superblock
- *	@type:	filesystem type superblock should belong to
- *	@test:	comparison callback
- *	@set:	setup callback
- *	@flags:	mount flags
- *	@data:	argument to each of them
+ * sget	-	find or create a superblock
+ * @type:	filesystem type superblock should belong to
+ * @test:	comparison callback
+ * @set:	setup callback
+ * @flags:	mount flags
+ * @data:	argument to each of them
  */
 struct super_block *sget(struct file_system_type *type,
 			int (*test)(struct super_block *,void *),
@@ -491,12 +491,12 @@ void drop_super(struct super_block *sb)
 EXPORT_SYMBOL(drop_super);
 
 /**
- *	iterate_supers - call function for all active superblocks
- *	@f: function to call
- *	@arg: argument to pass to it
+ * iterate_supers - call function for all active superblocks
+ * @f: function to call
+ * @arg: argument to pass to it
  *
- *	Scans the superblock list and calls given function, passing it
- *	locked superblock and given argument.
+ * Scans the superblock list and calls given function, passing it
+ * locked superblock and given argument.
  */
 void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
 {
@@ -525,13 +525,13 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
 }
 
 /**
- *	iterate_supers_type - call function for superblocks of given type
- *	@type: fs type
- *	@f: function to call
- *	@arg: argument to pass to it
+ * iterate_supers_type - call function for superblocks of given type
+ * @type: fs type
+ * @f: function to call
+ * @arg: argument to pass to it
  *
- *	Scans the superblock list and calls given function, passing it
- *	locked superblock and given argument.
+ * Scans the superblock list and calls given function, passing it
+ * locked superblock and given argument.
  */
 void iterate_supers_type(struct file_system_type *type,
 	void (*f)(struct super_block *, void *), void *arg)
@@ -560,11 +560,11 @@ void iterate_supers_type(struct file_system_type *type,
 EXPORT_SYMBOL(iterate_supers_type);
 
 /**
- *	get_super - get the superblock of a device
- *	@bdev: device to get the superblock for
- *	
- *	Scans the superblock list and finds the superblock of the file system
- *	mounted on the device given. %NULL is returned if no match is found.
+ * get_super - get the superblock of a device
+ * @bdev: device to get the superblock for
+ * 
+ * Scans the superblock list and finds the superblock of the file system
+ * mounted on the device given. %NULL is returned if no match is found.
  */
 
 struct super_block *get_super(struct block_device *bdev)
@@ -599,13 +599,13 @@ rescan:
 EXPORT_SYMBOL(get_super);
 
 /**
- *	get_super_thawed - get thawed superblock of a device
- *	@bdev: device to get the superblock for
+ * get_super_thawed - get thawed superblock of a device
+ * @bdev: device to get the superblock for
  *
- *	Scans the superblock list and finds the superblock of the file system
- *	mounted on the device. The superblock is returned once it is thawed
- *	(or immediately if it was not frozen). %NULL is returned if no match
- *	is found.
+ * Scans the superblock list and finds the superblock of the file system
+ * mounted on the device. The superblock is returned once it is thawed
+ * (or immediately if it was not frozen). %NULL is returned if no match
+ * is found.
  */
 struct super_block *get_super_thawed(struct block_device *bdev)
 {
@@ -680,13 +680,13 @@ rescan:
 }
 
 /**
- *	do_remount_sb - asks filesystem to change mount options.
- *	@sb:	superblock in question
- *	@flags:	numeric part of options
- *	@data:	the rest of options
+ * do_remount_sb - asks filesystem to change mount options.
+ * @sb:	superblock in question
+ * @flags:	numeric part of options
+ * @data:	the rest of options
  *      @force: whether or not to force the change
  *
- *	Alters the mount options of a mounted file system.
+ * Alters the mount options of a mounted file system.
  */
 int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
 {
-- 
1.8.4.2

[PATCH v2 04/35] fs: super: consolidate the get_super class functions

[Dongsheng Yang]

There is a lot of duplicated code in get_super and user_get_super.
This commit introduc a __get_super as a common function to reduce
the duplication of it.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/super.c | 64 ++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index fadb677..46fd1f1 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -559,19 +559,20 @@ void iterate_supers_type(struct file_system_type *type,
 }
 EXPORT_SYMBOL(iterate_supers_type);
 
-/**
- * get_super - get the superblock of a device
- * @bdev: device to get the superblock for
- * 
- * Scans the superblock list and finds the superblock of the file system
- * mounted on the device given. %NULL is returned if no match is found.
+/*
+ * __get_super - helper function for the other functions in get_super class
+ * @compare: function pointer to compare sb in list and key
+ * @key: key of the sb you want
+ *
+ * Scans the superblock list and finds the superblock you want. %NULL is
+ * returned if no match is found.
  */
-
-struct super_block *get_super(struct block_device *bdev)
+static struct super_block *__get_super(
+		int (*compare)(struct super_block *, void *), void *key)
 {
 	struct super_block *sb;
 
-	if (!bdev)
+	if (!key)
 		return NULL;
 
 	spin_lock(&sb_lock);
@@ -579,7 +580,7 @@ rescan:
 	list_for_each_entry(sb, &super_blocks, s_list) {
 		if (hlist_unhashed(&sb->s_instances))
 			continue;
-		if (sb->s_bdev == bdev) {
+		if (compare(sb, key)) {
 			sb->s_count++;
 			spin_unlock(&sb_lock);
 			down_read(&sb->s_umount);
@@ -596,6 +597,20 @@ rescan:
 	spin_unlock(&sb_lock);
 	return NULL;
 }
+
+static int bdev_compare(struct super_block *sb, void *key)
+{
+	return (sb->s_bdev == (struct block_device *)key);
+}
+
+/**
+ * get_super - get the superblock of a device
+ * @bdev: device to get the superblock for
+ */
+struct super_block *get_super(struct block_device *bdev)
+{
+	return __get_super(bdev_compare, bdev);
+}
 EXPORT_SYMBOL(get_super);
 
 /**
@@ -651,32 +666,15 @@ restart:
 	spin_unlock(&sb_lock);
 	return NULL;
 }
+
+static int user_compare(struct super_block *sb, void *key)
+{
+	return (sb->s_dev == *(dev_t *)key);
+}
  
 struct super_block *user_get_super(dev_t dev)
 {
-	struct super_block *sb;
-
-	spin_lock(&sb_lock);
-rescan:
-	list_for_each_entry(sb, &super_blocks, s_list) {
-		if (hlist_unhashed(&sb->s_instances))
-			continue;
-		if (sb->s_dev ==  dev) {
-			sb->s_count++;
-			spin_unlock(&sb_lock);
-			down_read(&sb->s_umount);
-			/* still alive? */
-			if (sb->s_root && (sb->s_flags & MS_BORN))
-				return sb;
-			up_read(&sb->s_umount);
-			/* nope, got unmounted */
-			spin_lock(&sb_lock);
-			__put_super(sb);
-			goto rescan;
-		}
-	}
-	spin_unlock(&sb_lock);
-	return NULL;
+	return __get_super(user_compare, &dev);
 }
 
 /**
-- 
1.8.4.2

Re: [PATCH v2 04/35] fs: super: consolidate the get_super class functions

[Jan Kara]

On Thu 30-07-15 13:48:00, Dongsheng Yang wrote:
> There is a lot of duplicated code in get_super and user_get_super.
> This commit introduc a __get_super as a common function to reduce
> the duplication of it.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>

The patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.com>

								Honza

> ---
>  fs/super.c | 64 ++++++++++++++++++++++++++++++--------------------------------
>  1 file changed, 31 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index fadb677..46fd1f1 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -559,19 +559,20 @@ void iterate_supers_type(struct file_system_type *type,
>  }
>  EXPORT_SYMBOL(iterate_supers_type);
>  
> -/**
> - * get_super - get the superblock of a device
> - * @bdev: device to get the superblock for
> - * 
> - * Scans the superblock list and finds the superblock of the file system
> - * mounted on the device given. %NULL is returned if no match is found.
> +/*
> + * __get_super - helper function for the other functions in get_super class
> + * @compare: function pointer to compare sb in list and key
> + * @key: key of the sb you want
> + *
> + * Scans the superblock list and finds the superblock you want. %NULL is
> + * returned if no match is found.
>   */
> -
> -struct super_block *get_super(struct block_device *bdev)
> +static struct super_block *__get_super(
> +		int (*compare)(struct super_block *, void *), void *key)
>  {
>  	struct super_block *sb;
>  
> -	if (!bdev)
> +	if (!key)
>  		return NULL;
>  
>  	spin_lock(&sb_lock);
> @@ -579,7 +580,7 @@ rescan:
>  	list_for_each_entry(sb, &super_blocks, s_list) {
>  		if (hlist_unhashed(&sb->s_instances))
>  			continue;
> -		if (sb->s_bdev == bdev) {
> +		if (compare(sb, key)) {
>  			sb->s_count++;
>  			spin_unlock(&sb_lock);
>  			down_read(&sb->s_umount);
> @@ -596,6 +597,20 @@ rescan:
>  	spin_unlock(&sb_lock);
>  	return NULL;
>  }
> +
> +static int bdev_compare(struct super_block *sb, void *key)
> +{
> +	return (sb->s_bdev == (struct block_device *)key);
> +}
> +
> +/**
> + * get_super - get the superblock of a device
> + * @bdev: device to get the superblock for
> + */
> +struct super_block *get_super(struct block_device *bdev)
> +{
> +	return __get_super(bdev_compare, bdev);
> +}
>  EXPORT_SYMBOL(get_super);
>  
>  /**
> @@ -651,32 +666,15 @@ restart:
>  	spin_unlock(&sb_lock);
>  	return NULL;
>  }
> +
> +static int user_compare(struct super_block *sb, void *key)
> +{
> +	return (sb->s_dev == *(dev_t *)key);
> +}
>   
>  struct super_block *user_get_super(dev_t dev)
>  {
> -	struct super_block *sb;
> -
> -	spin_lock(&sb_lock);
> -rescan:
> -	list_for_each_entry(sb, &super_blocks, s_list) {
> -		if (hlist_unhashed(&sb->s_instances))
> -			continue;
> -		if (sb->s_dev ==  dev) {
> -			sb->s_count++;
> -			spin_unlock(&sb_lock);
> -			down_read(&sb->s_umount);
> -			/* still alive? */
> -			if (sb->s_root && (sb->s_flags & MS_BORN))
> -				return sb;
> -			up_read(&sb->s_umount);
> -			/* nope, got unmounted */
> -			spin_lock(&sb_lock);
> -			__put_super(sb);
> -			goto rescan;
> -		}
> -	}
> -	spin_unlock(&sb_lock);
> -	return NULL;
> +	return __get_super(user_compare, &dev);
>  }
>  
>  /**
> -- 
> 1.8.4.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v2 05/35] fs: super: introduce a get_super_cdev to get super by a cdev reference

[Dongsheng Yang]

As we have cdev in superblock now, we can provide get_super_cdev
to get super_block by a cdev reference, similar with get_super
which is working only for block_device.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/super.c         | 15 +++++++++++++++
 include/linux/fs.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index 46fd1f1..4c56dff 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -613,6 +613,21 @@ struct super_block *get_super(struct block_device *bdev)
 }
 EXPORT_SYMBOL(get_super);
 
+static int cdev_compare(struct super_block *sb, void *key)
+{
+	return (sb->s_cdev == (struct cdev *)key);
+}
+
+/**
+ * get_super_cdev - get the superblock of a cdev
+ * @cdev: char device to get the superblock for
+ */
+struct super_block *get_super_cdev(struct cdev *cdev)
+{
+	return __get_super(cdev_compare, cdev);
+}
+EXPORT_SYMBOL(get_super_cdev);
+
 /**
  * get_super_thawed - get thawed superblock of a device
  * @bdev: device to get the superblock for
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2f1d9499..e5ea425a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2744,6 +2744,7 @@ extern void get_filesystem(struct file_system_type *fs);
 extern void put_filesystem(struct file_system_type *fs);
 extern struct file_system_type *get_fs_type(const char *name);
 extern struct super_block *get_super(struct block_device *);
+extern struct super_block *get_super_cdev(struct cdev *);
 extern struct super_block *get_super_thawed(struct block_device *);
 extern struct super_block *get_active_super(struct block_device *bdev);
 extern void drop_super(struct super_block *sb);
-- 
1.8.4.2

Re: [PATCH v2 05/35] fs: super: introduce a get_super_cdev to get super by a cdev reference

[Jan Kara]

On Thu 30-07-15 13:48:01, Dongsheng Yang wrote:
> As we have cdev in superblock now, we can provide get_super_cdev
> to get super_block by a cdev reference, similar with get_super
> which is working only for block_device.

The patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.com>

								Honza

> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/super.c         | 15 +++++++++++++++
>  include/linux/fs.h |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 46fd1f1..4c56dff 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -613,6 +613,21 @@ struct super_block *get_super(struct block_device *bdev)
>  }
>  EXPORT_SYMBOL(get_super);
>  
> +static int cdev_compare(struct super_block *sb, void *key)
> +{
> +	return (sb->s_cdev == (struct cdev *)key);
> +}
> +
> +/**
> + * get_super_cdev - get the superblock of a cdev
> + * @cdev: char device to get the superblock for
> + */
> +struct super_block *get_super_cdev(struct cdev *cdev)
> +{
> +	return __get_super(cdev_compare, cdev);
> +}
> +EXPORT_SYMBOL(get_super_cdev);
> +
>  /**
>   * get_super_thawed - get thawed superblock of a device
>   * @bdev: device to get the superblock for
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2f1d9499..e5ea425a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2744,6 +2744,7 @@ extern void get_filesystem(struct file_system_type *fs);
>  extern void put_filesystem(struct file_system_type *fs);
>  extern struct file_system_type *get_fs_type(const char *name);
>  extern struct super_block *get_super(struct block_device *);
> +extern struct super_block *get_super_cdev(struct cdev *);
>  extern struct super_block *get_super_thawed(struct block_device *);
>  extern struct super_block *get_active_super(struct block_device *bdev);
>  extern void drop_super(struct super_block *sb);
> -- 
> 1.8.4.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v2 06/35] fs: super: introduce a get_super_cdev_thawed to get sb by cdev reference

[Dongsheng Yang]

This patch refactors the original get_super_thawed to get a common
helper function of __get_super_thawed. Then take the use of
__get_super_thawed, we can implement get_super_cdev_thawed easily.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/super.c         | 29 +++++++++++++++++++++++++----
 include/linux/fs.h |  1 +
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 4c56dff..0e11412 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -629,18 +629,20 @@ struct super_block *get_super_cdev(struct cdev *cdev)
 EXPORT_SYMBOL(get_super_cdev);
 
 /**
- * get_super_thawed - get thawed superblock of a device
- * @bdev: device to get the superblock for
+ * __get_super_thawed - get thawed superblock
+ * @func: function pointer to get superblock by key
+ * @key: key to find superblock
  *
  * Scans the superblock list and finds the superblock of the file system
  * mounted on the device. The superblock is returned once it is thawed
  * (or immediately if it was not frozen). %NULL is returned if no match
  * is found.
  */
-struct super_block *get_super_thawed(struct block_device *bdev)
+static struct super_block *__get_super_thawed(
+		struct super_block *(*func)(void *), void *key)
 {
 	while (1) {
-		struct super_block *s = get_super(bdev);
+		struct super_block *s = func(key);
 		if (!s || s->s_writers.frozen == SB_UNFROZEN)
 			return s;
 		up_read(&s->s_umount);
@@ -649,9 +651,28 @@ struct super_block *get_super_thawed(struct block_device *bdev)
 		put_super(s);
 	}
 }
+
+/**
+ * get_super_thawed - get thawed superblock of a device
+ * @bdev: device to get the superblock for
+ */
+struct super_block *get_super_thawed(struct block_device *bdev)
+{
+	return __get_super_thawed((struct super_block *(*)(void *))get_super, bdev);
+}
 EXPORT_SYMBOL(get_super_thawed);
 
 /**
+ * get_super_cdev_thawed - get thawed superblock of a char device
+ * @cdev: device to get the superblock for
+ */
+struct super_block *get_super_cdev_thawed(struct cdev *cdev)
+{
+	return __get_super_thawed((struct super_block *(*)(void *))get_super_cdev, cdev);
+}
+EXPORT_SYMBOL(get_super_cdev_thawed);
+
+/**
  * get_active_super - get an active reference to the superblock of a device
  * @bdev: device to get the superblock for
  *
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e5ea425a..5c7d789 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2746,6 +2746,7 @@ extern struct file_system_type *get_fs_type(const char *name);
 extern struct super_block *get_super(struct block_device *);
 extern struct super_block *get_super_cdev(struct cdev *);
 extern struct super_block *get_super_thawed(struct block_device *);
+extern struct super_block *get_super_cdev_thawed(struct cdev *);
 extern struct super_block *get_active_super(struct block_device *bdev);
 extern void drop_super(struct super_block *sb);
 extern void iterate_supers(void (*)(struct super_block *, void *), void *);
-- 
1.8.4.2

Re: [PATCH v2 06/35] fs: super: introduce a get_super_cdev_thawed to get sb by cdev reference

[Jan Kara]

On Thu 30-07-15 13:48:02, Dongsheng Yang wrote:
> This patch refactors the original get_super_thawed to get a common
> helper function of __get_super_thawed. Then take the use of
> __get_super_thawed, we can implement get_super_cdev_thawed easily.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/super.c         | 29 +++++++++++++++++++++++++----
>  include/linux/fs.h |  1 +
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 4c56dff..0e11412 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -629,18 +629,20 @@ struct super_block *get_super_cdev(struct cdev *cdev)
>  EXPORT_SYMBOL(get_super_cdev);
>  
>  /**
> - * get_super_thawed - get thawed superblock of a device
> - * @bdev: device to get the superblock for
> + * __get_super_thawed - get thawed superblock
> + * @func: function pointer to get superblock by key
> + * @key: key to find superblock
>   *
>   * Scans the superblock list and finds the superblock of the file system
>   * mounted on the device. The superblock is returned once it is thawed
>   * (or immediately if it was not frozen). %NULL is returned if no match
>   * is found.
>   */
> -struct super_block *get_super_thawed(struct block_device *bdev)
> +static struct super_block *__get_super_thawed(
> +		struct super_block *(*func)(void *), void *key)
>  {
>  	while (1) {
> -		struct super_block *s = get_super(bdev);
> +		struct super_block *s = func(key);

It would seem more obvious (and consistent with get_super() class of
functions) if you passed in a comparison function and a key and then call
from here:

s = __get_super(compare, key);

Also you would avoid some of the ugly typecasting below.

								Honza

>  		if (!s || s->s_writers.frozen == SB_UNFROZEN)
>  			return s;
>  		up_read(&s->s_umount);
> @@ -649,9 +651,28 @@ struct super_block *get_super_thawed(struct block_device *bdev)
>  		put_super(s);
>  	}
>  }
> +
> +/**
> + * get_super_thawed - get thawed superblock of a device
> + * @bdev: device to get the superblock for
> + */
> +struct super_block *get_super_thawed(struct block_device *bdev)
> +{
> +	return __get_super_thawed((struct super_block *(*)(void *))get_super, bdev);
> +}
>  EXPORT_SYMBOL(get_super_thawed);
>  
>  /**
> + * get_super_cdev_thawed - get thawed superblock of a char device
> + * @cdev: device to get the superblock for
> + */
> +struct super_block *get_super_cdev_thawed(struct cdev *cdev)
> +{
> +	return __get_super_thawed((struct super_block *(*)(void *))get_super_cdev, cdev);
> +}
> +EXPORT_SYMBOL(get_super_cdev_thawed);
> +
> +/**
>   * get_active_super - get an active reference to the superblock of a device
>   * @bdev: device to get the superblock for
>   *
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e5ea425a..5c7d789 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2746,6 +2746,7 @@ extern struct file_system_type *get_fs_type(const char *name);
>  extern struct super_block *get_super(struct block_device *);
>  extern struct super_block *get_super_cdev(struct cdev *);
>  extern struct super_block *get_super_thawed(struct block_device *);
> +extern struct super_block *get_super_cdev_thawed(struct cdev *);
>  extern struct super_block *get_active_super(struct block_device *bdev);
>  extern void drop_super(struct super_block *sb);
>  extern void iterate_supers(void (*)(struct super_block *, void *), void *);
> -- 
> 1.8.4.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v2 07/35] fs: char_dev: introduce cd_acquire function to acquire cdev

[Dongsheng Yang]

cd_acquire() works like bd_acquire() to get cdev by an inode,
this commit is a preparation for lookup_cdev().

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/char_dev.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index ea06a3d..e818faa 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -439,6 +439,41 @@ static int exact_lock(dev_t dev, void *data)
 	return cdev_get(p) ? 0 : -1;
 }
 
+struct cdev *cd_acquire(struct inode *inode)
+{
+	struct cdev *cdev;
+	struct cdev *new = NULL;
+
+	spin_lock(&cdev_lock);
+	cdev = inode->i_cdev;
+	if (!cdev) {
+		struct kobject *kobj;
+		int idx;
+		spin_unlock(&cdev_lock);
+		kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx);
+		if (!kobj) {
+			cdev = NULL;
+			goto out;
+		}
+		new = container_of(kobj, struct cdev, kobj);
+		spin_lock(&cdev_lock);
+		/* Check i_cdev again in case somebody beat us to it while
+		   we dropped the lock. */
+		cdev = inode->i_cdev;
+		if (!cdev) {
+			inode->i_cdev = cdev = new;
+			list_add(&inode->i_devices, &cdev->list);
+		}
+	}
+
+	if (!cdev_get(cdev))
+		cdev = NULL;
+	spin_unlock(&cdev_lock);
+
+out:
+	return cdev;
+}
+
 /**
  * cdev_add() - add a char device to the system
  * @p: the cdev structure for the device
-- 
1.8.4.2

[PATCH v2 08/35] fs: introduce a __lookup_dev for internal using

[Dongsheng Yang]

This commit introduce a file of fs/dev.c at first. This is
a internal file shared by block_dev and char_dev. There is
only one function in it __lookup_dev which will be wrapped
to be lookup_bdev and loopup_cdev.

We will put more code in this file which is shared by
block_dev and char_dev.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/Makefile    |  2 +-
 fs/block_dev.c | 26 ++------------------
 fs/dev.c       | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/internal.h  | 14 +++++++++++
 4 files changed, 94 insertions(+), 25 deletions(-)
 create mode 100644 fs/dev.c

diff --git a/fs/Makefile b/fs/Makefile
index cb92fd4..d2f12b6 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -5,7 +5,7 @@
 # Rewritten to use lists instead of if-statements.
 # 
 
-obj-y :=	open.o read_write.o file_table.o super.o \
+obj-y :=	open.o read_write.o file_table.o super.o dev.o\
 		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
 		ioctl.o readdir.o select.o dcache.o inode.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o \
diff --git a/fs/block_dev.c b/fs/block_dev.c
index c7e4163..07e434a 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -654,7 +654,7 @@ void bdput(struct block_device *bdev)
 
 EXPORT_SYMBOL(bdput);
  
-static struct block_device *bd_acquire(struct inode *inode)
+struct block_device *bd_acquire(struct inode *inode)
 {
 	struct block_device *bdev;
 
@@ -1706,34 +1706,12 @@ EXPORT_SYMBOL(ioctl_by_bdev);
 struct block_device *lookup_bdev(const char *pathname)
 {
 	struct block_device *bdev;
-	struct inode *inode;
-	struct path path;
 	int error;
 
-	if (!pathname || !*pathname)
-		return ERR_PTR(-EINVAL);
-
-	error = kern_path(pathname, LOOKUP_FOLLOW, &path);
+	error = __lookup_dev(pathname, NULL, &bdev);
 	if (error)
 		return ERR_PTR(error);
-
-	inode = d_backing_inode(path.dentry);
-	error = -ENOTBLK;
-	if (!S_ISBLK(inode->i_mode))
-		goto fail;
-	error = -EACCES;
-	if (path.mnt->mnt_flags & MNT_NODEV)
-		goto fail;
-	error = -ENOMEM;
-	bdev = bd_acquire(inode);
-	if (!bdev)
-		goto fail;
-out:
-	path_put(&path);
 	return bdev;
-fail:
-	bdev = ERR_PTR(error);
-	goto out;
 }
 EXPORT_SYMBOL(lookup_bdev);
 
diff --git a/fs/dev.c b/fs/dev.c
new file mode 100644
index 0000000..5356a47
--- /dev/null
+++ b/fs/dev.c
@@ -0,0 +1,77 @@
+/*
+ * This file contains some functions shared by block_dev and char_dev
+ */
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/mount.h>
+
+#include "internal.h"
+
+/**
+ * __lookup_dev  - lookup a block_device or cdev by name
+ * @pathname:	special file representing the device
+ * @cdevp:	cdev would be returned by cdevp
+ * @bdevp:	block_device would be returned by bdevp
+ *
+ * Get a reference to the block_deivce or cdev at @pathname in
+ * the current namespace if possible and return it.
+ */
+int __lookup_dev(const char *pathname, struct cdev **cdevp,
+	       struct block_device **bdevp)
+{
+	struct inode *inode;
+	struct path path;
+	int error = 0;
+
+	if (!pathname || !*pathname)
+		return -EINVAL;
+
+	error = kern_path(pathname, LOOKUP_FOLLOW, &path);
+	if (error)
+		return error;
+
+	inode = d_backing_inode(path.dentry);
+
+	/**
+	 * We need at least one of bdevp and cdevp to be NULL,
+	 * but cdevp and bdevp can not be both NULL.
+	 */
+	error = -EINVAL;
+	if (!(cdevp || bdevp) || (cdevp && bdevp))
+		goto out;
+
+	if (cdevp) {
+		if (!S_ISCHR(inode->i_mode)) {
+			error = -EINVAL;
+			goto out;
+		}
+	} else {
+		if (!S_ISBLK(inode->i_mode)) {
+			error = -ENOTBLK;
+			goto out;
+		}
+	}
+	error = -EACCES;
+	if (path.mnt->mnt_flags & MNT_NODEV)
+		goto out;
+	error = -ENXIO;
+	if (S_ISCHR(inode->i_mode)) {
+		struct cdev *cdev;
+
+		cdev = cd_acquire(inode);
+		if (!cdev)
+			goto out;
+		*cdevp = cdev;
+	} else {
+		struct block_device *bdev;
+
+		bdev = bd_acquire(inode);
+		if (!bdev)
+			goto out;
+		*bdevp = bdev;
+	}
+	error = 0;
+out:
+	path_put(&path);
+	return error;
+}
diff --git a/fs/internal.h b/fs/internal.h
index 01dce1d..1e48f9e 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -17,6 +17,12 @@ struct mount;
 struct shrink_control;
 
 /*
+ * dev.c
+ */
+extern int __lookup_dev(const char *pathname, struct cdev **cdevp,
+			struct block_device **bdevp);
+
+/*
  * block_dev.c
  */
 #ifdef CONFIG_BLOCK
@@ -24,6 +30,8 @@ extern void __init bdev_cache_init(void);
 
 extern int __sync_blockdev(struct block_device *bdev, int wait);
 
+extern struct block_device *bd_acquire(struct inode *inode);
+
 #else
 static inline void bdev_cache_init(void)
 {
@@ -33,6 +41,11 @@ static inline int __sync_blockdev(struct block_device *bdev, int wait)
 {
 	return 0;
 }
+
+static struct block_device *bd_acquire(struct inode *inode)
+{
+	return NULL;
+}
 #endif
 
 /*
@@ -44,6 +57,7 @@ extern void guard_bio_eod(int rw, struct bio *bio);
  * char_dev.c
  */
 extern void __init chrdev_init(void);
+extern struct cdev *cd_acquire(struct inode *inode);
 
 /*
  * namei.c
-- 
1.8.4.2

Re: [PATCH v2 08/35] fs: introduce a __lookup_dev for internal using

[Jan Kara]

On Thu 30-07-15 13:48:04, Dongsheng Yang wrote:
> This commit introduce a file of fs/dev.c at first. This is
> a internal file shared by block_dev and char_dev. There is
> only one function in it __lookup_dev which will be wrapped
> to be lookup_bdev and loopup_cdev.
> 
> We will put more code in this file which is shared by
> block_dev and char_dev.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/Makefile    |  2 +-
>  fs/block_dev.c | 26 ++------------------
>  fs/dev.c       | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/internal.h  | 14 +++++++++++
>  4 files changed, 94 insertions(+), 25 deletions(-)
>  create mode 100644 fs/dev.c
> 
...
> +/**
> + * __lookup_dev  - lookup a block_device or cdev by name
> + * @pathname:	special file representing the device
> + * @cdevp:	cdev would be returned by cdevp
> + * @bdevp:	block_device would be returned by bdevp
> + *
> + * Get a reference to the block_deivce or cdev at @pathname in
> + * the current namespace if possible and return it.
> + */
> +int __lookup_dev(const char *pathname, struct cdev **cdevp,
> +	       struct block_device **bdevp)
> +{
> +	struct inode *inode;
> +	struct path path;
> +	int error = 0;
> +
> +	if (!pathname || !*pathname)
> +		return -EINVAL;
> +
> +	error = kern_path(pathname, LOOKUP_FOLLOW, &path);
> +	if (error)
> +		return error;
> +
> +	inode = d_backing_inode(path.dentry);
> +
> +	/**
> +	 * We need at least one of bdevp and cdevp to be NULL,
> +	 * but cdevp and bdevp can not be both NULL.
> +	 */
> +	error = -EINVAL;
> +	if (!(cdevp || bdevp) || (cdevp && bdevp))
> +		goto out;

Why don't you allow both cdevp and bdevp to be set and in that case accept
both block & character device and just set appropriate pointer and the
other one to NULL? Then quota code wouldn't have to search twice...

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

Re: [PATCH v2 08/35] fs: introduce a __lookup_dev for internal using

[Jan Kara]

On Mon 03-08-15 22:08:06, Jan Kara wrote:
> On Thu 30-07-15 13:48:04, Dongsheng Yang wrote:
> > This commit introduce a file of fs/dev.c at first. This is
> > a internal file shared by block_dev and char_dev. There is
> > only one function in it __lookup_dev which will be wrapped
> > to be lookup_bdev and loopup_cdev.
> > 
> > We will put more code in this file which is shared by
> > block_dev and char_dev.
> > 
> > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> > ---
> >  fs/Makefile    |  2 +-
> >  fs/block_dev.c | 26 ++------------------
> >  fs/dev.c       | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/internal.h  | 14 +++++++++++
> >  4 files changed, 94 insertions(+), 25 deletions(-)
> >  create mode 100644 fs/dev.c
> > 
> ...
> > +/**
> > + * __lookup_dev  - lookup a block_device or cdev by name
> > + * @pathname:	special file representing the device
> > + * @cdevp:	cdev would be returned by cdevp
> > + * @bdevp:	block_device would be returned by bdevp
> > + *
> > + * Get a reference to the block_deivce or cdev at @pathname in
> > + * the current namespace if possible and return it.
> > + */
> > +int __lookup_dev(const char *pathname, struct cdev **cdevp,
> > +	       struct block_device **bdevp)
> > +{
> > +	struct inode *inode;
> > +	struct path path;
> > +	int error = 0;
> > +
> > +	if (!pathname || !*pathname)
> > +		return -EINVAL;
> > +
> > +	error = kern_path(pathname, LOOKUP_FOLLOW, &path);
> > +	if (error)
> > +		return error;
> > +
> > +	inode = d_backing_inode(path.dentry);
> > +
> > +	/**
> > +	 * We need at least one of bdevp and cdevp to be NULL,
> > +	 * but cdevp and bdevp can not be both NULL.
> > +	 */
> > +	error = -EINVAL;
> > +	if (!(cdevp || bdevp) || (cdevp && bdevp))
> > +		goto out;
> 
> Why don't you allow both cdevp and bdevp to be set and in that case accept
> both block & character device and just set appropriate pointer and the
> other one to NULL? Then quota code wouldn't have to search twice...

On a second thought it's probably not worth the hassle with using the
function directly from quota code. So OK. You can add:

Reviewed-by: Jan Kara <jack@suse.com>

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

[PATCH v2 09/35] fs: char_dev: introduce lookup_cdev to get cdev by pathname

[Dongsheng Yang]

Function lookup_cdev works similarly with lookup_bdev, we can get
a cdev instance by lookup_cdev with a parameter of dev name.

This function will be used in quotactl to get a cdev by a dev name.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/char_dev.c      | 13 +++++++++++++
 include/linux/fs.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index e818faa..12c1bd5 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -474,6 +474,18 @@ out:
 	return cdev;
 }
 
+struct cdev *lookup_cdev(const char *pathname)
+{
+	struct cdev *cdev;
+	int error;
+
+	error = __lookup_dev(pathname, &cdev, NULL);
+	if (error)
+		return ERR_PTR(error);
+
+	return cdev;
+}
+
 /**
  * cdev_add() - add a char device to the system
  * @p: the cdev structure for the device
@@ -596,6 +608,7 @@ void __init chrdev_init(void)
 EXPORT_SYMBOL(register_chrdev_region);
 EXPORT_SYMBOL(unregister_chrdev_region);
 EXPORT_SYMBOL(alloc_chrdev_region);
+EXPORT_SYMBOL(lookup_cdev);
 EXPORT_SYMBOL(cdev_init);
 EXPORT_SYMBOL(cdev_alloc);
 EXPORT_SYMBOL(cdev_del);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5c7d789..860b235 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2308,6 +2308,8 @@ extern void __unregister_chrdev(unsigned int major, unsigned int baseminor,
 				unsigned int count, const char *name);
 extern void unregister_chrdev_region(dev_t, unsigned);
 extern void chrdev_show(struct seq_file *,off_t);
+extern struct cdev *lookup_cdev(const char *);
+
 
 static inline int register_chrdev(unsigned int major, const char *name,
 				  const struct file_operations *fops)
-- 
1.8.4.2

Re: [PATCH v2 09/35] fs: char_dev: introduce lookup_cdev to get cdev by pathname

[Jan Kara]

On Thu 30-07-15 13:48:05, Dongsheng Yang wrote:
> Function lookup_cdev works similarly with lookup_bdev, we can get
> a cdev instance by lookup_cdev with a parameter of dev name.
> 
> This function will be used in quotactl to get a cdev by a dev name.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.com>

								Honza

> ---
>  fs/char_dev.c      | 13 +++++++++++++
>  include/linux/fs.h |  2 ++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/fs/char_dev.c b/fs/char_dev.c
> index e818faa..12c1bd5 100644
> --- a/fs/char_dev.c
> +++ b/fs/char_dev.c
> @@ -474,6 +474,18 @@ out:
>  	return cdev;
>  }
>  
> +struct cdev *lookup_cdev(const char *pathname)
> +{
> +	struct cdev *cdev;
> +	int error;
> +
> +	error = __lookup_dev(pathname, &cdev, NULL);
> +	if (error)
> +		return ERR_PTR(error);
> +
> +	return cdev;
> +}
> +
>  /**
>   * cdev_add() - add a char device to the system
>   * @p: the cdev structure for the device
> @@ -596,6 +608,7 @@ void __init chrdev_init(void)
>  EXPORT_SYMBOL(register_chrdev_region);
>  EXPORT_SYMBOL(unregister_chrdev_region);
>  EXPORT_SYMBOL(alloc_chrdev_region);
> +EXPORT_SYMBOL(lookup_cdev);
>  EXPORT_SYMBOL(cdev_init);
>  EXPORT_SYMBOL(cdev_alloc);
>  EXPORT_SYMBOL(cdev_del);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 5c7d789..860b235 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2308,6 +2308,8 @@ extern void __unregister_chrdev(unsigned int major, unsigned int baseminor,
>  				unsigned int count, const char *name);
>  extern void unregister_chrdev_region(dev_t, unsigned);
>  extern void chrdev_show(struct seq_file *,off_t);
> +extern struct cdev *lookup_cdev(const char *);
> +
>  
>  static inline int register_chrdev(unsigned int major, const char *name,
>  				  const struct file_operations *fops)
> -- 
> 1.8.4.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v2 10/35] fs: dquot: skip invalidate_bdev if bdev is NULL

[Dongsheng Yang]

There would be a NULL pointer dereference if bdev is NULL

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/quota/dquot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 20d1f74..140397a 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2208,7 +2208,8 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
 		 * of the cache could fail because of other unrelated dirty
 		 * data */
 		sync_filesystem(sb);
-		invalidate_bdev(sb->s_bdev);
+		if (sb->s_bdev)
+			invalidate_bdev(sb->s_bdev);
 	}
 	mutex_lock(&dqopt->dqonoff_mutex);
 	if (sb_has_quota_loaded(sb, type)) {
-- 
1.8.4.2

Re: [PATCH v2 10/35] fs: dquot: skip invalidate_bdev if bdev is NULL

[Jan Kara]

On Thu 30-07-15 13:48:06, Dongsheng Yang wrote:
> There would be a NULL pointer dereference if bdev is NULL
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.com>

								Honza

> ---
>  fs/quota/dquot.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 20d1f74..140397a 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2208,7 +2208,8 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
>  		 * of the cache could fail because of other unrelated dirty
>  		 * data */
>  		sync_filesystem(sb);
> -		invalidate_bdev(sb->s_bdev);
> +		if (sb->s_bdev)
> +			invalidate_bdev(sb->s_bdev);
>  	}
>  	mutex_lock(&dqopt->dqonoff_mutex);
>  	if (sb_has_quota_loaded(sb, type)) {
> -- 
> 1.8.4.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v2 11/35] fs: quota: make quota support fs which is running on char dev

[Dongsheng Yang]

So far, we have done all preparation. Then let's add some
code to support char dev fs in quotactl.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/quota/quota.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 86ded73..3f9acea 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -17,6 +17,7 @@
 #include <linux/quotaops.h>
 #include <linux/types.h>
 #include <linux/writeback.h>
+#include <linux/cdev.h>
 
 static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
 				     qid_t id)
@@ -726,29 +727,40 @@ static int quotactl_cmd_write(int cmd)
  */
 static struct super_block *quotactl_block(const char __user *special, int cmd)
 {
+	struct super_block *sb = NULL;
+	struct filename *tmp = getname(special);
+	struct cdev *cdev = NULL;
+
 #ifdef CONFIG_BLOCK
 	struct block_device *bdev;
-	struct super_block *sb;
-	struct filename *tmp = getname(special);
 
 	if (IS_ERR(tmp))
 		return ERR_CAST(tmp);
 	bdev = lookup_bdev(tmp->name);
-	putname(tmp);
 	if (IS_ERR(bdev))
-		return ERR_CAST(bdev);
+		goto not_block;
 	if (quotactl_cmd_write(cmd))
 		sb = get_super_thawed(bdev);
 	else
 		sb = get_super(bdev);
 	bdput(bdev);
-	if (!sb)
-		return ERR_PTR(-ENODEV);
+	goto out;
 
-	return sb;
-#else
-	return ERR_PTR(-ENODEV);
+not_block:
 #endif
+	cdev = lookup_cdev(tmp->name);
+	if (IS_ERR(cdev))
+		goto out;
+	if (quotactl_cmd_write(cmd))
+		sb = get_super_cdev_thawed(cdev);
+	else
+		sb = get_super_cdev(cdev);
+	cdev_put(cdev);
+out:
+	putname(tmp);
+	if (sb)
+		return sb;
+	return ERR_PTR(-ENODEV);
 }
 
 /*
-- 
1.8.4.2

[PATCH v2 12/35] ubi: introduce a interface to get cdev in ubi_volume

[Dongsheng Yang]

We want to fill the sb->s_cdev in ubifs_fill_super(),
so, we need to add a interface in ubi to get the cdev
of an ubi_volume.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 drivers/mtd/ubi/kapi.c  | 6 ++++++
 include/linux/mtd/ubi.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
index e844887..d3d1982 100644
--- a/drivers/mtd/ubi/kapi.c
+++ b/drivers/mtd/ubi/kapi.c
@@ -862,3 +862,9 @@ int ubi_unregister_volume_notifier(struct notifier_block *nb)
 	return blocking_notifier_chain_unregister(&ubi_notifiers, nb);
 }
 EXPORT_SYMBOL_GPL(ubi_unregister_volume_notifier);
+
+inline struct cdev *ubi_get_volume_cdev(struct ubi_volume_desc *desc)
+{
+	return &(desc->vol->cdev);
+}
+EXPORT_SYMBOL_GPL(ubi_get_volume_cdev);
diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
index 1e271cb..301c6e9 100644
--- a/include/linux/mtd/ubi.h
+++ b/include/linux/mtd/ubi.h
@@ -261,6 +261,7 @@ int ubi_leb_map(struct ubi_volume_desc *desc, int lnum);
 int ubi_is_mapped(struct ubi_volume_desc *desc, int lnum);
 int ubi_sync(int ubi_num);
 int ubi_flush(int ubi_num, int vol_id, int lnum);
+struct cdev *ubi_get_volume_cdev(struct ubi_volume_desc *desc);
 
 /*
  * This function is the same as the 'ubi_leb_read()' function, but it does not
-- 
1.8.4.2

Re: [PATCH v2 12/35] ubi: introduce a interface to get cdev in ubi_volume

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We want to fill the sb->s_cdev in ubifs_fill_super(),
> so, we need to add a interface in ubi to get the cdev
> of an ubi_volume.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  drivers/mtd/ubi/kapi.c  | 6 ++++++
>  include/linux/mtd/ubi.h | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/ubi/kapi.c b/drivers/mtd/ubi/kapi.c
> index e844887..d3d1982 100644
> --- a/drivers/mtd/ubi/kapi.c
> +++ b/drivers/mtd/ubi/kapi.c
> @@ -862,3 +862,9 @@ int ubi_unregister_volume_notifier(struct notifier_block *nb)
>  	return blocking_notifier_chain_unregister(&ubi_notifiers, nb);
>  }
>  EXPORT_SYMBOL_GPL(ubi_unregister_volume_notifier);
> +
> +inline struct cdev *ubi_get_volume_cdev(struct ubi_volume_desc *desc)
> +{
> +	return &(desc->vol->cdev);
> +}
> +EXPORT_SYMBOL_GPL(ubi_get_volume_cdev);
> diff --git a/include/linux/mtd/ubi.h b/include/linux/mtd/ubi.h
> index 1e271cb..301c6e9 100644
> --- a/include/linux/mtd/ubi.h
> +++ b/include/linux/mtd/ubi.h
> @@ -261,6 +261,7 @@ int ubi_leb_map(struct ubi_volume_desc *desc, int lnum);
>  int ubi_is_mapped(struct ubi_volume_desc *desc, int lnum);
>  int ubi_sync(int ubi_num);
>  int ubi_flush(int ubi_num, int vol_id, int lnum);
> +struct cdev *ubi_get_volume_cdev(struct ubi_volume_desc *desc);
>  
>  /*
>   * This function is the same as the 'ubi_leb_read()' function, but it does not
> 

Acked-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

[PATCH v2 13/35] ubifs: fix a typo in comment of ubifs_budget_req

[Dongsheng Yang]

s/now/how

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/ubifs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 216ba87..3b5e932 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -858,9 +858,9 @@ struct ubifs_compressor {
  * @mod_dent: non-zero if the operation removes or modifies an existing
  *            directory entry
  * @new_ino: non-zero if the operation adds a new inode
- * @new_ino_d: now much data newly created inode contains
+ * @new_ino_d: how much data newly created inode contains
  * @dirtied_ino: how many inodes the operation makes dirty
- * @dirtied_ino_d: now much data dirtied inode contains
+ * @dirtied_ino_d: how much data dirtied inode contains
  * @idx_growth: how much the index will supposedly grow
  * @data_growth: how much new data the operation will supposedly add
  * @dd_growth: how much data that makes other data dirty the operation will
-- 
1.8.4.2

Re: [PATCH v2 13/35] ubifs: fix a typo in comment of ubifs_budget_req

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> s/now/how
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/ubifs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 216ba87..3b5e932 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -858,9 +858,9 @@ struct ubifs_compressor {
>   * @mod_dent: non-zero if the operation removes or modifies an existing
>   *            directory entry
>   * @new_ino: non-zero if the operation adds a new inode
> - * @new_ino_d: now much data newly created inode contains
> + * @new_ino_d: how much data newly created inode contains
>   * @dirtied_ino: how many inodes the operation makes dirty
> - * @dirtied_ino_d: now much data dirtied inode contains
> + * @dirtied_ino_d: how much data dirtied inode contains
>   * @idx_growth: how much the index will supposedly grow
>   * @data_growth: how much new data the operation will supposedly add
>   * @dd_growth: how much data that makes other data dirty the operation will

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

Re: [PATCH v2 13/35] ubifs: fix a typo in comment of ubifs_budget_req

[Artem Bityutskiy]

On Thu, 2015-07-30 at 13:48 +0800, Dongsheng Yang wrote:
> s/now/how
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/ubifs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Picked this trivial one to 'linux-ubifs.git', thanks!

[PATCH v2 14/35] ubifs: extend budget for blocks

[Dongsheng Yang]

Currently, budget subsystem in ubifs are working on budgeting
page-by-page. But sometimes we want to budget a space for one
block, e.g for quota file writing. So this commit extend budget
subsystem to support blocks budgeting and releasing.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/budget.c | 4 ++++
 fs/ubifs/debug.c  | 2 ++
 fs/ubifs/super.c  | 1 +
 fs/ubifs/ubifs.h  | 5 +++++
 4 files changed, 12 insertions(+)

diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index 11a11b3..ba4e530 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -397,6 +397,8 @@ static int calc_data_growth(const struct ubifs_info *c,
 		data_growth += c->bi.page_budget;
 	if (req->new_dent)
 		data_growth += c->bi.dent_budget;
+	if (req->new_block_num)
+		data_growth += c->bi.block_budget * req->new_block_num;
 	data_growth += req->new_ino_d;
 	return data_growth;
 }
@@ -418,6 +420,8 @@ static int calc_dd_growth(const struct ubifs_info *c,
 		dd_growth += c->bi.inode_budget << (req->dirtied_ino - 1);
 	if (req->mod_dent)
 		dd_growth += c->bi.dent_budget;
+	if (req->dirtied_block_num)
+		dd_growth += c->bi.block_budget * req->dirtied_block_num;
 	dd_growth += req->dirtied_ino_d;
 	return dd_growth;
 }
diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 4c46a98..cafd592 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -556,6 +556,8 @@ void ubifs_dump_budget_req(const struct ubifs_budget_req *req)
 	       req->new_page, req->dirtied_page);
 	pr_err("\tnew_dent    %d, mod_dent     %d\n",
 	       req->new_dent, req->mod_dent);
+	pr_err("\tnew_block   %d, dirtied_block %d\n",
+	       req->new_block_num, req->dirtied_block_num);
 	pr_err("\tidx_growth  %d\n", req->idx_growth);
 	pr_err("\tdata_growth %d dd_growth     %d\n",
 	       req->data_growth, req->dd_growth);
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index c643261..eb04e42 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -698,6 +698,7 @@ static int init_constants_sb(struct ubifs_info *c)
 	c->bi.page_budget = UBIFS_MAX_DATA_NODE_SZ * UBIFS_BLOCKS_PER_PAGE;
 	c->bi.inode_budget = UBIFS_INO_NODE_SZ;
 	c->bi.dent_budget = UBIFS_MAX_DENT_NODE_SZ;
+	c->bi.block_budget = UBIFS_MAX_DATA_NODE_SZ;
 
 	/*
 	 * When the amount of flash space used by buds becomes
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 3b5e932..71b79b5 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -861,6 +861,8 @@ struct ubifs_compressor {
  * @new_ino_d: how much data newly created inode contains
  * @dirtied_ino: how many inodes the operation makes dirty
  * @dirtied_ino_d: how much data dirtied inode contains
+ * @new_block_num: how many new blocks
+ * @dirtied_block_num: how many dirtied blocks
  * @idx_growth: how much the index will supposedly grow
  * @data_growth: how much new data the operation will supposedly add
  * @dd_growth: how much data that makes other data dirty the operation will
@@ -902,6 +904,8 @@ struct ubifs_budget_req {
 	unsigned int dirtied_ino;
 	unsigned int dirtied_ino_d;
 #endif
+	unsigned int new_block_num;
+	unsigned int dirtied_block_num;
 	int idx_growth;
 	int data_growth;
 	int dd_growth;
@@ -983,6 +987,7 @@ struct ubifs_budg_info {
 	int page_budget;
 	int inode_budget;
 	int dent_budget;
+	int block_budget;
 };
 
 struct ubifs_debug_info;
-- 
1.8.4.2

Re: [PATCH v2 14/35] ubifs: extend budget for blocks

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> Currently, budget subsystem in ubifs are working on budgeting
> page-by-page. But sometimes we want to budget a space for one
> block, e.g for quota file writing. So this commit extend budget
> subsystem to support blocks budgeting and releasing.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/budget.c | 4 ++++
>  fs/ubifs/debug.c  | 2 ++
>  fs/ubifs/super.c  | 1 +
>  fs/ubifs/ubifs.h  | 5 +++++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
> index 11a11b3..ba4e530 100644
> --- a/fs/ubifs/budget.c
> +++ b/fs/ubifs/budget.c
> @@ -397,6 +397,8 @@ static int calc_data_growth(const struct ubifs_info *c,
>  		data_growth += c->bi.page_budget;
>  	if (req->new_dent)
>  		data_growth += c->bi.dent_budget;
> +	if (req->new_block_num)
> +		data_growth += c->bi.block_budget * req->new_block_num;
>  	data_growth += req->new_ino_d;
>  	return data_growth;
>  }
> @@ -418,6 +420,8 @@ static int calc_dd_growth(const struct ubifs_info *c,
>  		dd_growth += c->bi.inode_budget << (req->dirtied_ino - 1);
>  	if (req->mod_dent)
>  		dd_growth += c->bi.dent_budget;
> +	if (req->dirtied_block_num)
> +		dd_growth += c->bi.block_budget * req->dirtied_block_num;
>  	dd_growth += req->dirtied_ino_d;
>  	return dd_growth;
>  }
> diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
> index 4c46a98..cafd592 100644
> --- a/fs/ubifs/debug.c
> +++ b/fs/ubifs/debug.c
> @@ -556,6 +556,8 @@ void ubifs_dump_budget_req(const struct ubifs_budget_req *req)
>  	       req->new_page, req->dirtied_page);
>  	pr_err("\tnew_dent    %d, mod_dent     %d\n",
>  	       req->new_dent, req->mod_dent);
> +	pr_err("\tnew_block   %d, dirtied_block %d\n",
> +	       req->new_block_num, req->dirtied_block_num);
>  	pr_err("\tidx_growth  %d\n", req->idx_growth);
>  	pr_err("\tdata_growth %d dd_growth     %d\n",
>  	       req->data_growth, req->dd_growth);
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index c643261..eb04e42 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -698,6 +698,7 @@ static int init_constants_sb(struct ubifs_info *c)
>  	c->bi.page_budget = UBIFS_MAX_DATA_NODE_SZ * UBIFS_BLOCKS_PER_PAGE;
>  	c->bi.inode_budget = UBIFS_INO_NODE_SZ;
>  	c->bi.dent_budget = UBIFS_MAX_DENT_NODE_SZ;
> +	c->bi.block_budget = UBIFS_MAX_DATA_NODE_SZ;
>  
>  	/*
>  	 * When the amount of flash space used by buds becomes
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 3b5e932..71b79b5 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -861,6 +861,8 @@ struct ubifs_compressor {
>   * @new_ino_d: how much data newly created inode contains
>   * @dirtied_ino: how many inodes the operation makes dirty
>   * @dirtied_ino_d: how much data dirtied inode contains
> + * @new_block_num: how many new blocks
> + * @dirtied_block_num: how many dirtied blocks

What are the semantics of these new fields?
e.g. is it allowed to set both new_block_num and new_dent?

>   * @idx_growth: how much the index will supposedly grow
>   * @data_growth: how much new data the operation will supposedly add
>   * @dd_growth: how much data that makes other data dirty the operation will
> @@ -902,6 +904,8 @@ struct ubifs_budget_req {
>  	unsigned int dirtied_ino;
>  	unsigned int dirtied_ino_d;
>  #endif
> +	unsigned int new_block_num;
> +	unsigned int dirtied_block_num;

Why are these not under UBIFS_DEBUG?
I like the overflow checks.

Thanks,
//richard

Re: [PATCH v2 14/35] ubifs: extend budget for blocks

[Dongsheng Yang]

On 08/04/2015 04:56 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> Currently, budget subsystem in ubifs are working on budgeting

[...]
>>   #endif
>> +	unsigned int new_block_num;
>> +	unsigned int dirtied_block_num;
>
> Why are these not under UBIFS_DEBUG?
> I like the overflow checks.

Sorry for the late reply.

I did not find the overflow checks in my reading.
Could you help to explain what kind of the check
is it? and why we define in different way with
UBIFS_DEBUG defined or not.

And, Where did we define the UBIFS_DEBUG? I did not
get the design of this macro. :(

Yang
>
> Thanks,
> //richard
> .
>

Re: [PATCH v2 14/35] ubifs: extend budget for blocks

[Richard Weinberger]

Am 21.08.2015 um 07:59 schrieb Dongsheng Yang:
> On 08/04/2015 04:56 AM, Richard Weinberger wrote:
>> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>>> Currently, budget subsystem in ubifs are working on budgeting
> 
> [...]
>>>   #endif
>>> +    unsigned int new_block_num;
>>> +    unsigned int dirtied_block_num;
>>
>> Why are these not under UBIFS_DEBUG?
>> I like the overflow checks.
> 
> Sorry for the late reply.
> 
> I did not find the overflow checks in my reading.
> Could you help to explain what kind of the check
> is it? and why we define in different way with
> UBIFS_DEBUG defined or not.

AFAICT the idea is that you see it from the value
from a crash dump.
i.e. if new_page is > 2 an overflow happened.

I don't know that Artem's original plan was.
But we could also automate this checks.

> And, Where did we define the UBIFS_DEBUG? I did not
> get the design of this macro. :(

You have define the macro yourself.

Thanks,
//richard

Re: [PATCH v2 14/35] ubifs: extend budget for blocks

[Dongsheng Yang]

On 08/21/2015 03:12 PM, Richard Weinberger wrote:
> Am 21.08.2015 um 07:59 schrieb Dongsheng Yang:
>> On 08/04/2015 04:56 AM, Richard Weinberger wrote:
>>> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>>>> Currently, budget subsystem in ubifs are working on budgeting
>>
>> [...]
>>>>    #endif
>>>> +    unsigned int new_block_num;
>>>> +    unsigned int dirtied_block_num;
>>>
>>> Why are these not under UBIFS_DEBUG?
>>> I like the overflow checks.
>>
>> Sorry for the late reply.
>>
>> I did not find the overflow checks in my reading.
>> Could you help to explain what kind of the check
>> is it? and why we define in different way with
>> UBIFS_DEBUG defined or not.
>
> AFAICT the idea is that you see it from the value
> from a crash dump.
> i.e. if new_page is > 2 an overflow happened.

Thanx, on my second thought, the new_block could
be unsigned int :1. Because there should be no
reading size larger than one block size. Okey,
thanx for your good suggestion here. I will
update it in next version. :)
>
> I don't know that Artem's original plan was.
> But we could also automate this checks.
>
>> And, Where did we define the UBIFS_DEBUG? I did not
>> get the design of this macro. :(
>
> You have define the macro yourself.

But what is the purpose of UBIFS_DEBUG? I mean, why
we want to define the new_page as unsigned int rather than
bit field of unsigned int :1 in UBIFS_DEBUG mode?

Okey, defining it in bit mode is for overflow checking, I agree.
But why we define it in non-bit mode when UBIFS_DEBUG defined.

It's confusing to me. :(


Thanx
Yang
>
> Thanks,
> //richard
> .
>

[PATCH v2 15/35] ubifs: fill sb->s_cdev in ubifs_fill_super()

[Dongsheng Yang]

We need to fill the sb->s_cdev to tell others, this sb
is for which ubi_volume. quotactl will use this connection
to find the super_block by the device.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index eb04e42..27e1739 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2032,6 +2032,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		goto out_bdi;
 
 	sb->s_bdi = &c->bdi;
+	sb->s_cdev = ubi_get_volume_cdev(c->ubi);
 	sb->s_fs_info = c;
 	sb->s_magic = UBIFS_SUPER_MAGIC;
 	sb->s_blocksize = UBIFS_BLOCK_SIZE;
-- 
1.8.4.2

Re: [PATCH v2 15/35] ubifs: fill sb->s_cdev in ubifs_fill_super()

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We need to fill the sb->s_cdev to tell others, this sb
> is for which ubi_volume. quotactl will use this connection
> to find the super_block by the device.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index eb04e42..27e1739 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -2032,6 +2032,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
>  		goto out_bdi;
>  
>  	sb->s_bdi = &c->bdi;
> +	sb->s_cdev = ubi_get_volume_cdev(c->ubi);
>  	sb->s_fs_info = c;
>  	sb->s_magic = UBIFS_SUPER_MAGIC;
>  	sb->s_blocksize = UBIFS_BLOCK_SIZE;
> 

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

[PATCH v2 16/35] ubifs: fill ->s_dev in ubifs_fill_super

[Dongsheng Yang]

Some tools in userspace, such as quota-tools, are using
stat() to get the st_dev of a mount point then to find
out the device mounted on this mount point.

So we have to fill ->s_dev here to tell them which device
is mounted on this point.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 27e1739..161b1a6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -36,6 +36,7 @@
 #include <linux/mount.h>
 #include <linux/math64.h>
 #include <linux/writeback.h>
+#include <linux/cdev.h>
 #include "ubifs.h"
 
 /*
@@ -2033,6 +2034,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_bdi = &c->bdi;
 	sb->s_cdev = ubi_get_volume_cdev(c->ubi);
+	sb->s_dev = sb->s_cdev->dev;
 	sb->s_fs_info = c;
 	sb->s_magic = UBIFS_SUPER_MAGIC;
 	sb->s_blocksize = UBIFS_BLOCK_SIZE;
-- 
1.8.4.2

Re: [PATCH v2 16/35] ubifs: fill ->s_dev in ubifs_fill_super

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> Some tools in userspace, such as quota-tools, are using
> stat() to get the st_dev of a mount point then to find
> out the device mounted on this mount point.
> 
> So we have to fill ->s_dev here to tell them which device
> is mounted on this point.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 27e1739..161b1a6 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -36,6 +36,7 @@
>  #include <linux/mount.h>
>  #include <linux/math64.h>
>  #include <linux/writeback.h>
> +#include <linux/cdev.h>
>  #include "ubifs.h"
>  
>  /*
> @@ -2033,6 +2034,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	sb->s_bdi = &c->bdi;
>  	sb->s_cdev = ubi_get_volume_cdev(c->ubi);
> +	sb->s_dev = sb->s_cdev->dev;
>  	sb->s_fs_info = c;
>  	sb->s_magic = UBIFS_SUPER_MAGIC;
>  	sb->s_blocksize = UBIFS_BLOCK_SIZE;
> 

As long userspace is fine with the fact that UBI device nodes are dynamically allocated it should work. :)

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

[PATCH v2 17/35] ubifs: export read_block() from file.c

[Dongsheng Yang]

We want to use read_block to read quota file bypass the page cache.
So export it from file.c, then we can use it in somewhere else.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/file.c  | 4 ++--
 fs/ubifs/ubifs.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 554d7b9..f307e86 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -54,7 +54,7 @@
 #include <linux/namei.h>
 #include <linux/slab.h>
 
-static int read_block(struct inode *inode, void *addr, unsigned int block,
+int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
 		      struct ubifs_data_node *dn)
 {
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
@@ -141,7 +141,7 @@ static int do_readpage(struct page *page)
 			err = -ENOENT;
 			memset(addr, 0, UBIFS_BLOCK_SIZE);
 		} else {
-			ret = read_block(inode, addr, block, dn);
+			ret = ubifs_read_block(inode, addr, block, dn);
 			if (ret) {
 				err = ret;
 				if (err != -ENOENT)
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 71b79b5..c48fb41 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1749,6 +1749,8 @@ const struct ubifs_lprops *ubifs_fast_find_frdi_idx(struct ubifs_info *c);
 int ubifs_calc_dark(const struct ubifs_info *c, int spc);
 
 /* file.c */
+int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
+		      struct ubifs_data_node *dn);
 int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync);
 int ubifs_setattr(struct dentry *dentry, struct iattr *attr);
 int ubifs_update_time(struct inode *inode, struct timespec *time, int flags);
-- 
1.8.4.2

Re: [PATCH v2 17/35] ubifs: export read_block() from file.c

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We want to use read_block to read quota file bypass the page cache.
> So export it from file.c, then we can use it in somewhere else.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/file.c  | 4 ++--
>  fs/ubifs/ubifs.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 554d7b9..f307e86 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -54,7 +54,7 @@
>  #include <linux/namei.h>
>  #include <linux/slab.h>
>  
> -static int read_block(struct inode *inode, void *addr, unsigned int block,
> +int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
>  		      struct ubifs_data_node *dn)
>  {
>  	struct ubifs_info *c = inode->i_sb->s_fs_info;
> @@ -141,7 +141,7 @@ static int do_readpage(struct page *page)
>  			err = -ENOENT;
>  			memset(addr, 0, UBIFS_BLOCK_SIZE);
>  		} else {
> -			ret = read_block(inode, addr, block, dn);
> +			ret = ubifs_read_block(inode, addr, block, dn);
>  			if (ret) {
>  				err = ret;
>  				if (err != -ENOENT)
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 71b79b5..c48fb41 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -1749,6 +1749,8 @@ const struct ubifs_lprops *ubifs_fast_find_frdi_idx(struct ubifs_info *c);
>  int ubifs_calc_dark(const struct ubifs_info *c, int spc);
>  
>  /* file.c */
> +int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
> +		      struct ubifs_data_node *dn);
>  int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync);
>  int ubifs_setattr(struct dentry *dentry, struct iattr *attr);
>  int ubifs_update_time(struct inode *inode, struct timespec *time, int flags);

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

Re: [PATCH v2 17/35] ubifs: export read_block() from file.c

[Richard Weinberger]

Am 03.08.2015 um 23:13 schrieb Richard Weinberger:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> We want to use read_block to read quota file bypass the page cache.
>> So export it from file.c, then we can use it in somewhere else.
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>  fs/ubifs/file.c  | 4 ++--
>>  fs/ubifs/ubifs.h | 2 ++
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
>> index 554d7b9..f307e86 100644
>> --- a/fs/ubifs/file.c
>> +++ b/fs/ubifs/file.c
>> @@ -54,7 +54,7 @@
>>  #include <linux/namei.h>
>>  #include <linux/slab.h>
>>  
>> -static int read_block(struct inode *inode, void *addr, unsigned int block,
>> +int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
>>  		      struct ubifs_data_node *dn)

Forgot one minor comment, as the function is now public please document the
parameters.

Thanks,
//richard

Re: [PATCH v2 17/35] ubifs: export read_block() from file.c

[Dongsheng Yang]

On 08/04/2015 05:29 AM, Richard Weinberger wrote:
> Am 03.08.2015 um 23:13 schrieb Richard Weinberger:
>> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>>> We want to use read_block to read quota file bypass the page cache.
>>> So export it from file.c, then we can use it in somewhere else.
>>>
>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>>> ---
>>>   fs/ubifs/file.c  | 4 ++--
>>>   fs/ubifs/ubifs.h | 2 ++
>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
>>> index 554d7b9..f307e86 100644
>>> --- a/fs/ubifs/file.c
>>> +++ b/fs/ubifs/file.c
>>> @@ -54,7 +54,7 @@
>>>   #include <linux/namei.h>
>>>   #include <linux/slab.h>
>>>
>>> -static int read_block(struct inode *inode, void *addr, unsigned int block,
>>> +int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
>>>   		      struct ubifs_data_node *dn)
>
> Forgot one minor comment, as the function is now public please document the
> parameters.

Thanx, will update it in next verison.

Yang
>
> Thanks,
> //richard
> .
>

[PATCH v2 18/35] ubifs: introduce quota related mount options

[Dongsheng Yang]

This commit introduce mount options of quota, noquota,
usrquota and grpquota. These options are used to make
ubifs support quota. But with this commit, quota will
not working on ubifs actually. We just introduce options
here and will make ubifs support quota later.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 30 ++++++++++++++++++++++++++++++
 fs/ubifs/ubifs.h |  7 +++++++
 2 files changed, 37 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 161b1a6..2491fff 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -36,6 +36,7 @@
 #include <linux/mount.h>
 #include <linux/math64.h>
 #include <linux/writeback.h>
+#include <linux/quotaops.h>
 #include <linux/cdev.h>
 #include "ubifs.h"
 
@@ -439,6 +440,12 @@ static int ubifs_show_options(struct seq_file *s, struct dentry *root)
 	else if (c->mount_opts.chk_data_crc == 1)
 		seq_puts(s, ",no_chk_data_crc");
 
+	if (c->usrquota)
+		seq_puts(s, ",usrquota");
+
+	if (c->grpquota)
+		seq_puts(s, ",grpquota");
+
 	if (c->mount_opts.override_compr) {
 		seq_printf(s, ",compr=%s",
 			   ubifs_compr_name(c->mount_opts.compr_type));
@@ -930,6 +937,10 @@ enum {
 	Opt_chk_data_crc,
 	Opt_no_chk_data_crc,
 	Opt_override_compr,
+	Opt_ignore,
+	Opt_quota,
+	Opt_usrquota,
+	Opt_grpquota,
 	Opt_err,
 };
 
@@ -941,6 +952,10 @@ static const match_table_t tokens = {
 	{Opt_chk_data_crc, "chk_data_crc"},
 	{Opt_no_chk_data_crc, "no_chk_data_crc"},
 	{Opt_override_compr, "compr=%s"},
+	{Opt_ignore, "noquota"},
+	{Opt_quota, "quota"},
+	{Opt_usrquota, "usrquota"},
+	{Opt_grpquota, "grpquota"},
 	{Opt_err, NULL},
 };
 
@@ -1041,6 +1056,21 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 			c->default_compr = c->mount_opts.compr_type;
 			break;
 		}
+#ifdef CONFIG_QUOTA
+		case Opt_quota:
+		case Opt_usrquota:
+			c->usrquota = 1;
+			break;
+		case Opt_grpquota:
+			c->grpquota = 1;
+			break;
+#else
+		case Opt_quota:
+		case Opt_usrquota:
+		case Opt_grpquota:
+			ubifs_err(c, "quota operations not supported");
+			break;
+#endif
 		default:
 		{
 			unsigned long flag;
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index c48fb41..3b5a204 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -418,6 +418,9 @@ struct ubifs_inode {
 	loff_t synced_i_size;
 	loff_t ui_size;
 	int flags;
+#ifdef CONFIG_QUOTA
+	struct dquot *i_dquot[MAXQUOTAS];
+#endif
 	pgoff_t last_page_read;
 	pgoff_t read_in_a_row;
 	int data_len;
@@ -1039,6 +1042,8 @@ struct ubifs_debug_info;
  * @bulk_read: enable bulk-reads
  * @default_compr: default compression algorithm (%UBIFS_COMPR_LZO, etc)
  * @rw_incompat: the media is not R/W compatible
+ * @usrquota: enable usrquota
+ * @grpquota: enable grpquota
  *
  * @tnc_mutex: protects the Tree Node Cache (TNC), @zroot, @cnext, @enext, and
  *             @calc_idx_sz
@@ -1280,6 +1285,8 @@ struct ubifs_info {
 	unsigned int bulk_read:1;
 	unsigned int default_compr:2;
 	unsigned int rw_incompat:1;
+	unsigned int usrquota:1;
+	unsigned int grpquota:1;
 
 	struct mutex tnc_mutex;
 	struct ubifs_zbranch zroot;
-- 
1.8.4.2

Re: [PATCH v2 18/35] ubifs: introduce quota related mount options

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> This commit introduce mount options of quota, noquota,
> usrquota and grpquota. These options are used to make
> ubifs support quota. But with this commit, quota will
> not working on ubifs actually. We just introduce options
> here and will make ubifs support quota later.

Please reorder your commits such that this commit is the last one.
Then it can be the "Let's wire up quota support commit". :-)

> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 30 ++++++++++++++++++++++++++++++
>  fs/ubifs/ubifs.h |  7 +++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 161b1a6..2491fff 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -36,6 +36,7 @@
>  #include <linux/mount.h>
>  #include <linux/math64.h>
>  #include <linux/writeback.h>
> +#include <linux/quotaops.h>
>  #include <linux/cdev.h>
>  #include "ubifs.h"
>  
> @@ -439,6 +440,12 @@ static int ubifs_show_options(struct seq_file *s, struct dentry *root)
>  	else if (c->mount_opts.chk_data_crc == 1)
>  		seq_puts(s, ",no_chk_data_crc");
>  
> +	if (c->usrquota)
> +		seq_puts(s, ",usrquota");
> +
> +	if (c->grpquota)
> +		seq_puts(s, ",grpquota");
> +
>  	if (c->mount_opts.override_compr) {
>  		seq_printf(s, ",compr=%s",
>  			   ubifs_compr_name(c->mount_opts.compr_type));
> @@ -930,6 +937,10 @@ enum {
>  	Opt_chk_data_crc,
>  	Opt_no_chk_data_crc,
>  	Opt_override_compr,
> +	Opt_ignore,

Opt_noquota please.

> +	Opt_quota,
> +	Opt_usrquota,
> +	Opt_grpquota,
>  	Opt_err,
>  };
>  
> @@ -941,6 +952,10 @@ static const match_table_t tokens = {
>  	{Opt_chk_data_crc, "chk_data_crc"},
>  	{Opt_no_chk_data_crc, "no_chk_data_crc"},
>  	{Opt_override_compr, "compr=%s"},
> +	{Opt_ignore, "noquota"},
> +	{Opt_quota, "quota"},
> +	{Opt_usrquota, "usrquota"},
> +	{Opt_grpquota, "grpquota"},
>  	{Opt_err, NULL},
>  };
>  
> @@ -1041,6 +1056,21 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>  			c->default_compr = c->mount_opts.compr_type;
>  			break;
>  		}
> +#ifdef CONFIG_QUOTA
> +		case Opt_quota:
> +		case Opt_usrquota:
> +			c->usrquota = 1;
> +			break;
> +		case Opt_grpquota:
> +			c->grpquota = 1;
> +			break;
> +#else
> +		case Opt_quota:
> +		case Opt_usrquota:
> +		case Opt_grpquota:
> +			ubifs_err(c, "quota operations not supported");
> +			break;
> +#endif
>  		default:
>  		{
>  			unsigned long flag;
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index c48fb41..3b5a204 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -418,6 +418,9 @@ struct ubifs_inode {
>  	loff_t synced_i_size;
>  	loff_t ui_size;
>  	int flags;
> +#ifdef CONFIG_QUOTA
> +	struct dquot *i_dquot[MAXQUOTAS];
> +#endif
>  	pgoff_t last_page_read;
>  	pgoff_t read_in_a_row;
>  	int data_len;
> @@ -1039,6 +1042,8 @@ struct ubifs_debug_info;
>   * @bulk_read: enable bulk-reads
>   * @default_compr: default compression algorithm (%UBIFS_COMPR_LZO, etc)
>   * @rw_incompat: the media is not R/W compatible
> + * @usrquota: enable usrquota
> + * @grpquota: enable grpquota
>   *
>   * @tnc_mutex: protects the Tree Node Cache (TNC), @zroot, @cnext, @enext, and
>   *             @calc_idx_sz
> @@ -1280,6 +1285,8 @@ struct ubifs_info {
>  	unsigned int bulk_read:1;
>  	unsigned int default_compr:2;
>  	unsigned int rw_incompat:1;
> +	unsigned int usrquota:1;
> +	unsigned int grpquota:1;
>  
>  	struct mutex tnc_mutex;
>  	struct ubifs_zbranch zroot;
> 

Re: [PATCH v2 18/35] ubifs: introduce quota related mount options

[Dongsheng Yang]

On 08/04/2015 05:13 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> This commit introduce mount options of quota, noquota,
>> usrquota and grpquota. These options are used to make
>> ubifs support quota. But with this commit, quota will
>> not working on ubifs actually. We just introduce options
>> here and will make ubifs support quota later.
>
> Please reorder your commits such that this commit is the last one.
> Then it can be the "Let's wire up quota support commit". :-)

Hmmm, yes, agree.
>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/super.c | 30 ++++++++++++++++++++++++++++++
>>   fs/ubifs/ubifs.h |  7 +++++++
>>   2 files changed, 37 insertions(+)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index 161b1a6..2491fff 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -36,6 +36,7 @@
>>   #include <linux/mount.h>
>>   #include <linux/math64.h>
>>   #include <linux/writeback.h>
>> +#include <linux/quotaops.h>
>>   #include <linux/cdev.h>
>>   #include "ubifs.h"
>>
>> @@ -439,6 +440,12 @@ static int ubifs_show_options(struct seq_file *s, struct dentry *root)
>>   	else if (c->mount_opts.chk_data_crc == 1)
>>   		seq_puts(s, ",no_chk_data_crc");
>>
>> +	if (c->usrquota)
>> +		seq_puts(s, ",usrquota");
>> +
>> +	if (c->grpquota)
>> +		seq_puts(s, ",grpquota");
>> +
>>   	if (c->mount_opts.override_compr) {
>>   		seq_printf(s, ",compr=%s",
>>   			   ubifs_compr_name(c->mount_opts.compr_type));
>> @@ -930,6 +937,10 @@ enum {
>>   	Opt_chk_data_crc,
>>   	Opt_no_chk_data_crc,
>>   	Opt_override_compr,
>> +	Opt_ignore,
>
> Opt_noquota please.

Good opinion.

Thanx
Yang
>
>> +	Opt_quota,
>> +	Opt_usrquota,
>> +	Opt_grpquota,
>>   	Opt_err,
>>   };
>>
>> @@ -941,6 +952,10 @@ static const match_table_t tokens = {
>>   	{Opt_chk_data_crc, "chk_data_crc"},
>>   	{Opt_no_chk_data_crc, "no_chk_data_crc"},
>>   	{Opt_override_compr, "compr=%s"},
>> +	{Opt_ignore, "noquota"},
>> +	{Opt_quota, "quota"},
>> +	{Opt_usrquota, "usrquota"},
>> +	{Opt_grpquota, "grpquota"},
>>   	{Opt_err, NULL},
>>   };
>>
>> @@ -1041,6 +1056,21 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
>>   			c->default_compr = c->mount_opts.compr_type;
>>   			break;
>>   		}
>> +#ifdef CONFIG_QUOTA
>> +		case Opt_quota:
>> +		case Opt_usrquota:
>> +			c->usrquota = 1;
>> +			break;
>> +		case Opt_grpquota:
>> +			c->grpquota = 1;
>> +			break;
>> +#else
>> +		case Opt_quota:
>> +		case Opt_usrquota:
>> +		case Opt_grpquota:
>> +			ubifs_err(c, "quota operations not supported");
>> +			break;
>> +#endif
>>   		default:
>>   		{
>>   			unsigned long flag;
>> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
>> index c48fb41..3b5a204 100644
>> --- a/fs/ubifs/ubifs.h
>> +++ b/fs/ubifs/ubifs.h
>> @@ -418,6 +418,9 @@ struct ubifs_inode {
>>   	loff_t synced_i_size;
>>   	loff_t ui_size;
>>   	int flags;
>> +#ifdef CONFIG_QUOTA
>> +	struct dquot *i_dquot[MAXQUOTAS];
>> +#endif
>>   	pgoff_t last_page_read;
>>   	pgoff_t read_in_a_row;
>>   	int data_len;
>> @@ -1039,6 +1042,8 @@ struct ubifs_debug_info;
>>    * @bulk_read: enable bulk-reads
>>    * @default_compr: default compression algorithm (%UBIFS_COMPR_LZO, etc)
>>    * @rw_incompat: the media is not R/W compatible
>> + * @usrquota: enable usrquota
>> + * @grpquota: enable grpquota
>>    *
>>    * @tnc_mutex: protects the Tree Node Cache (TNC), @zroot, @cnext, @enext, and
>>    *             @calc_idx_sz
>> @@ -1280,6 +1285,8 @@ struct ubifs_info {
>>   	unsigned int bulk_read:1;
>>   	unsigned int default_compr:2;
>>   	unsigned int rw_incompat:1;
>> +	unsigned int usrquota:1;
>> +	unsigned int grpquota:1;
>>
>>   	struct mutex tnc_mutex;
>>   	struct ubifs_zbranch zroot;
>>
> .
>

[PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Dongsheng Yang]

In ubifs, we have to do a budget for inode before marking
it as dirty. But sometimes, we would call dirty_inode in vfs
which will not do a budget for inode. In this case, we have
to do a budget in ubifs_dirty_inode() by ourselvies.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 2491fff..bc57685 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -383,15 +383,38 @@ done:
 	clear_inode(inode);
 }
 
+/*
+ * In theory, ubifs should take the full control of dirty<->clean
+ * of an inode with ui->ui_mutex. But there are callers of
+ * ubifs_dirty_inode in vfs without holding ui->ui_mutex and
+ * budgeting. So when we found the ui_mutex is not locked, we have
+ * to lock ui->ui_mutex by itself and do a budget by itself.
+ */
 static void ubifs_dirty_inode(struct inode *inode, int flags)
 {
         struct ubifs_inode *ui = ubifs_inode(inode);
+	int locked = mutex_is_locked(&ui->ui_mutex);
+	struct ubifs_info *c = inode->i_sb->s_fs_info;
+	int ret = 0;
+
+	if (!locked)
+		mutex_lock(&ui->ui_mutex);
 
-	ubifs_assert(mutex_is_locked(&ui->ui_mutex));
 	if (!ui->dirty) {
+		if (!locked) {
+			struct ubifs_budget_req req = { .dirtied_ino = 1,
+				   .dirtied_ino_d = ALIGN(ui->data_len, 8) };
+			ret = ubifs_budget_space(c, &req);
+			if (ret)
+				goto out;
+		}
 		ui->dirty = 1;
 		dbg_gen("inode %lu",  inode->i_ino);
 	}
+out:
+	if (!locked)
+		mutex_unlock(&ui->ui_mutex);
+	return;
 }
 
 static int ubifs_statfs(struct dentry *dentry, struct kstatfs *buf)
-- 
1.8.4.2

Re: [PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> In ubifs, we have to do a budget for inode before marking
> it as dirty. But sometimes, we would call dirty_inode in vfs
> which will not do a budget for inode. In this case, we have
> to do a budget in ubifs_dirty_inode() by ourselvies.

How is this commit related to quota support?

> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 2491fff..bc57685 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -383,15 +383,38 @@ done:
>  	clear_inode(inode);
>  }
>  
> +/*
> + * In theory, ubifs should take the full control of dirty<->clean
> + * of an inode with ui->ui_mutex. But there are callers of
> + * ubifs_dirty_inode in vfs without holding ui->ui_mutex and
> + * budgeting. So when we found the ui_mutex is not locked, we have
> + * to lock ui->ui_mutex by itself and do a budget by itself.
> + */
>  static void ubifs_dirty_inode(struct inode *inode, int flags)
>  {
>          struct ubifs_inode *ui = ubifs_inode(inode);
> +	int locked = mutex_is_locked(&ui->ui_mutex);
> +	struct ubifs_info *c = inode->i_sb->s_fs_info;
> +	int ret = 0;
> +
> +	if (!locked)
> +		mutex_lock(&ui->ui_mutex);
>  
> -	ubifs_assert(mutex_is_locked(&ui->ui_mutex));

So, currently this assert can be reached? How?

Thanks,
//richard

Re: [PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Dongsheng Yang]

Hi Richard,
	Thanx for your review. I decide to drop it in next version. :)

Thanx
Yang

On 08/04/2015 05:13 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> In ubifs, we have to do a budget for inode before marking
>> it as dirty. But sometimes, we would call dirty_inode in vfs
>> which will not do a budget for inode. In this case, we have
>> to do a budget in ubifs_dirty_inode() by ourselvies.
>
> How is this commit related to quota support?
>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/super.c | 25 ++++++++++++++++++++++++-
>>   1 file changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index 2491fff..bc57685 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -383,15 +383,38 @@ done:
>>   	clear_inode(inode);
>>   }
>>
>> +/*
>> + * In theory, ubifs should take the full control of dirty<->clean
>> + * of an inode with ui->ui_mutex. But there are callers of
>> + * ubifs_dirty_inode in vfs without holding ui->ui_mutex and
>> + * budgeting. So when we found the ui_mutex is not locked, we have
>> + * to lock ui->ui_mutex by itself and do a budget by itself.
>> + */
>>   static void ubifs_dirty_inode(struct inode *inode, int flags)
>>   {
>>           struct ubifs_inode *ui = ubifs_inode(inode);
>> +	int locked = mutex_is_locked(&ui->ui_mutex);
>> +	struct ubifs_info *c = inode->i_sb->s_fs_info;
>> +	int ret = 0;
>> +
>> +	if (!locked)
>> +		mutex_lock(&ui->ui_mutex);
>>
>> -	ubifs_assert(mutex_is_locked(&ui->ui_mutex));
>
> So, currently this assert can be reached? How?
>
> Thanks,
> //richard
> .
>

Re: [PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Artem Bityutskiy]

On Thu, 2015-07-30 at 13:48 +0800, Dongsheng Yang wrote:
> In ubifs, we have to do a budget for inode before marking
> it as dirty. But sometimes, we would call dirty_inode in vfs
> which will not do a budget for inode. In this case, we have
> to do a budget in ubifs_dirty_inode() by ourselvies.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>

Could you please explain some more the problem you are trying to solve.
Locking looks confusing and broken. It looks like what you are
expressing is that the 'ui_mutex' is optional, and this smells fishy. 

>  static void ubifs_dirty_inode(struct inode *inode, int flags)
>  {
>          struct ubifs_inode *ui = ubifs_inode(inode);
> +	int locked = mutex_is_locked(&ui->ui_mutex);

Suppose another process has it locked, so 'locked' is set to 1 here.

> +	struct ubifs_info *c = inode->i_sb->s_fs_info;
> +	int ret = 0;
> +
> +	if (!locked)

So we skip this.
> +		mutex_lock(&ui->ui_mutex);

And if the other process has released the lock by this time, we do not
mind, right? Therefore, ui_mutex is "optional"?

> -	ubifs_assert(mutex_is_locked(&ui->ui_mutex));
>  	if (!ui->dirty) {
> +		if (!locked) {

And similar here, we do not run this code because 'locked' is 1.
> +			struct ubifs_budget_req req = { .dirtied_ino 
> = 1,
> +				   .dirtied_ino_d = ALIGN(ui
> ->data_len, 8) };
> +			ret = ubifs_budget_space(c, &req);
> +			if (ret)
> +				goto out;
> +		}

But the other process has already released it, and we do not mind?


Please, try to explain what you want to achieve some more. I am not
sure I understand the end goal.

Artem.

Re: [PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Dongsheng Yang]

On 08/05/2015 04:11 PM, Artem Bityutskiy wrote:
> On Thu, 2015-07-30 at 13:48 +0800, Dongsheng Yang wrote:
>> In ubifs, we have to do a budget for inode before marking
>> it as dirty. But sometimes, we would call dirty_inode in vfs
>> which will not do a budget for inode. In this case, we have
>> to do a budget in ubifs_dirty_inode() by ourselvies.
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>
> Could you please explain some more the problem you are trying to solve.
> Locking looks confusing and broken. It looks like what you are
> expressing is that the 'ui_mutex' is optional, and this smells fishy.

Oh, yes, that's TRUE. This patch makes the locking broken. I am sorry
about it.
>
[...]
>
> Please, try to explain what you want to achieve some more. I am not
> sure I understand the end goal.

Okey, what I want here is to doing a budget for the inode in
.dirty_inode called by vfs. Currently, the all work is under the full
control of ubifs as the comment of @ui_mutex said. But the
dquot_disable() is doing a dirty_inode() without asking ubifs is that
allowed. So I want to do the budget in ubifs_dirty_inode() itself here.

But, that's INCORRECT. Yes, my bad. Thanx for your comment.

And I found another solution for it. To introduce a callback in quota
to allow filesystem to dirty inode in dquot_disable(). I believe that
works.

Anyway, I agree that this patch is a fishy one here. I will drop it
in next version and send a better solution for it. Of course, with
some more description for what I am doing. :)

Thanx a lot, Richard and Atem

Yang
>
> Artem.
> .
>

Re: [PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Artem Bityutskiy]

On Thu, 2015-08-06 at 14:46 +0800, Dongsheng Yang wrote:
> On 08/05/2015 04:11 PM, Artem Bityutskiy wrote:
> > On Thu, 2015-07-30 at 13:48 +0800, Dongsheng Yang wrote:
> > > In ubifs, we have to do a budget for inode before marking
> > > it as dirty. But sometimes, we would call dirty_inode in vfs
> > > which will not do a budget for inode. In this case, we have
> > > to do a budget in ubifs_dirty_inode() by ourselvies.
> > > 
> > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> > 
> > Could you please explain some more the problem you are trying to 
> > solve.
> > Locking looks confusing and broken. It looks like what you are
> > expressing is that the 'ui_mutex' is optional, and this smells 
> > fishy.
> 
> Oh, yes, that's TRUE. This patch makes the locking broken. I am sorry
> about it.
> > 
> [...]
> > 
> > Please, try to explain what you want to achieve some more. I am not
> > sure I understand the end goal.
> 
> Okey, what I want here is to doing a budget for the inode in
> .dirty_inode called by vfs. Currently, the all work is under the full
> control of ubifs as the comment of @ui_mutex said. But the
> dquot_disable() is doing a dirty_inode() without asking ubifs is that
> allowed. So I want to do the budget in ubifs_dirty_inode() itself 
> here.But, that's INCORRECT. Yes, my bad. Thanx for your comment.
> 
> And I found another solution for it. To introduce a callback in quota
> to allow filesystem to dirty inode in dquot_disable(). I believe that
> works.

Yes, the high-level picture is this.

1. Dirtying and inode implies liability - the system becomes liable to
write it to the media.

2. Most file-systems have no problems with this, because there is
always space allocated for the inode.

3. Some file-systems like UBIFS may have no space for writing dirty
inodes. Often UBIFS can product this space, but it will involve forcing
write-back, and there are locking issues when doing write-back from the
write-back path.

4. So the approach UBIFS took is to allocate space in advance for every
dirty inode.

5. To do that, UBIFS requires VFS avoid dirtying inodes directly, but
instead, dirty them via the file-system. The file-system then has a
chance to do all the necessary things related to dirtying the inode.
The FS may even refuse to dirty the inode if in knows that it is
impossible to do (the FS is 100% full, for example).

So yes, if the generic quota code could dirty its inodes via the FS,
not directly, that would be great.

But please, check that the generic quota code can handle errors,
because ubifs_dirty_inode() may return -ENOSPC or other errors.

Artem.

Re: [PATCH v2 19/35] ubifs: budget for inode in ubifs_dirty_inode if necessary

[Dongsheng Yang]

On 08/06/2015 03:26 PM, Artem Bityutskiy wrote:
> On Thu, 2015-08-06 at 14:46 +0800, Dongsheng Yang wrote:
>> On 08/05/2015 04:11 PM, Artem Bityutskiy wrote:
>>> On Thu, 2015-07-30 at 13:48 +0800, Dongsheng Yang wrote:
>>>> In ubifs, we have to do a budget for inode before marking
>>>> it as dirty. But sometimes, we would call dirty_inode in vfs
>>>> which will not do a budget for inode. In this case, we have
>>>> to do a budget in ubifs_dirty_inode() by ourselvies.
>>>>
>>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>>>
>>> Could you please explain some more the problem you are trying to
>>> solve.
>>> Locking looks confusing and broken. It looks like what you are
>>> expressing is that the 'ui_mutex' is optional, and this smells
>>> fishy.
>>
>> Oh, yes, that's TRUE. This patch makes the locking broken. I am sorry
>> about it.
>>>
>> [...]
>>>
>>> Please, try to explain what you want to achieve some more. I am not
>>> sure I understand the end goal.
>>
>> Okey, what I want here is to doing a budget for the inode in
>> .dirty_inode called by vfs. Currently, the all work is under the full
>> control of ubifs as the comment of @ui_mutex said. But the
>> dquot_disable() is doing a dirty_inode() without asking ubifs is that
>> allowed. So I want to do the budget in ubifs_dirty_inode() itself
>> here.But, that's INCORRECT. Yes, my bad. Thanx for your comment.
>>
>> And I found another solution for it. To introduce a callback in quota
>> to allow filesystem to dirty inode in dquot_disable(). I believe that
>> works.
>
> Yes, the high-level picture is this.
>
> 1. Dirtying and inode implies liability - the system becomes liable to
> write it to the media.
>
> 2. Most file-systems have no problems with this, because there is
> always space allocated for the inode.
>
> 3. Some file-systems like UBIFS may have no space for writing dirty
> inodes. Often UBIFS can product this space, but it will involve forcing
> write-back, and there are locking issues when doing write-back from the
> write-back path.
>
> 4. So the approach UBIFS took is to allocate space in advance for every
> dirty inode.
>
> 5. To do that, UBIFS requires VFS avoid dirtying inodes directly, but
> instead, dirty them via the file-system. The file-system then has a
> chance to do all the necessary things related to dirtying the inode.
> The FS may even refuse to dirty the inode if in knows that it is
> impossible to do (the FS is 100% full, for example).

Hi Atem,
	Thanx for your detail. Yes, I figured this out by reading code
and comments in last days. Maybe I should have asked it to you earlier. :)
>
> So yes, if the generic quota code could dirty its inodes via the FS,
> not directly, that would be great.
>
> But please, check that the generic quota code can handle errors,
> because ubifs_dirty_inode() may return -ENOSPC or other errors.

Yes, of course. I will do that. And It would also be helpful to
other FS, such as btrfs which works in cow.

Thanx
Yang
>
> Artem.
> .
>

[PATCH v2 20/35] ubifs: implement IO functions for quota files

[Dongsheng Yang]

We have to implement 3 functions to support quota in ubifs:
ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas()

ubifs_quota_read():
	callback to read quota file.
ubifs_quota_write():
	callback to write data to quota file.
ubifs_get_quotas():
	callback to get quota instance for inode.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 219 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index bc57685..3609d7b 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -940,6 +940,220 @@ static int check_volume_empty(struct ubifs_info *c)
 	return 0;
 }
 
+#ifdef CONFIG_QUOTA
+static ssize_t ubifs_quota_read(struct super_block *sb, int type, char *data,
+			       size_t len, loff_t off)
+{
+	struct inode *inode = sb_dqopt(sb)->files[type];
+	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
+	int offset = off & (sb->s_blocksize - 1);
+	int tocopy = 0;
+	size_t toread;
+	char *block_buf;
+	struct ubifs_data_node *dn;
+	int ret, err = 0;
+
+	toread = len;
+
+	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
+	if (!dn) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	block_buf = kmalloc(sb->s_blocksize, GFP_NOFS);
+	if (!block_buf) {
+		err = -ENOMEM;
+		goto free_dn;
+	}
+
+	if (offset) {
+		/* Read the un-aligned data in first block */
+		tocopy = sb->s_blocksize - offset;
+		if (toread < tocopy)
+			tocopy = toread;
+
+		ret = ubifs_read_block(inode, block_buf, block, dn);
+		if (ret) {
+			if (ret != -ENOENT) {
+				err = ret;
+				goto free_buf;
+			}
+		}
+
+		memcpy(data, block_buf + offset, tocopy);
+
+		block++;
+		toread -= tocopy;
+		data += tocopy;
+		tocopy = 0;
+	}
+
+	while (toread > 0) {
+		tocopy = sb->s_blocksize < toread ?
+				sb->s_blocksize : toread;
+
+		/* Break to read the last block */
+		if (tocopy < sb->s_blocksize)
+			break;
+
+		ret = ubifs_read_block(inode, data, block, dn);
+		if (ret) {
+			if (ret != -ENOENT) {
+				err = ret;
+				goto free_buf;
+			}
+		}
+		block++;;
+		toread -= tocopy;
+		data += tocopy;
+		tocopy = 0;
+	}
+
+	if (tocopy) {
+		/* Read the data in last block */
+		ret = ubifs_read_block(inode, block_buf, block, dn);
+		if (ret) {
+			if (ret != -ENOENT) {
+				err = ret;
+				goto free_buf;
+			}
+		}
+
+		memcpy(data, block_buf, tocopy);
+	}
+free_buf:
+	kfree(block_buf);
+free_dn:
+	kfree(dn);
+out:
+	if (!err)
+		return len;
+	return err;
+}
+
+static ssize_t ubifs_quota_write(struct super_block *sb, int type,
+				const char *data, size_t len, loff_t off)
+{
+	struct inode *inode = sb_dqopt(sb)->files[type];
+	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
+	struct ubifs_info *c = inode->i_sb->s_fs_info;
+	int offset = off & (sb->s_blocksize - 1);
+	union ubifs_key key;
+	int tocopy = 0;
+	size_t towrite = len;
+	int ret, err = 0;
+	struct ubifs_budget_req req = {};
+	int new_block = 0;
+	struct ubifs_data_node *dn;
+	char *block_buf;
+
+	/* There are dirtied blocks and new blockes, but we call them all
+	 * new blocks here, the budgeting is same for them.
+	 */
+	new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize);
+	if (offset)
+		new_block += 1;
+
+	req.new_block_num = new_block;
+
+	err = ubifs_budget_space(c, &req);
+	if (err)
+		goto out;
+
+	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
+	if (!dn) {
+		err = -ENOMEM;
+		goto release_budget;
+	}
+
+	block_buf = kmalloc(sb->s_blocksize, GFP_NOFS);
+	if (!block_buf) {
+		err = -ENOMEM;
+		goto free_dn;
+	}
+
+	if (offset) {
+		/* Write the un-aligned data in first block */
+		tocopy = sb->s_blocksize - offset;
+		if (towrite < tocopy)
+			tocopy = towrite;
+
+		ret = ubifs_read_block(inode, block_buf, block, dn);
+		if (ret) {
+			if (ret != -ENOENT) {
+				err = ret;
+				goto free_buf;
+			}
+			memset(block_buf, 0, sb->s_blocksize);
+		}
+
+		memcpy(block_buf + offset, data, tocopy);
+
+		data_key_init(c, &key, inode->i_ino, block);
+		err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize);
+		if (err) {
+			goto free_buf;
+		}
+		block++;
+		towrite -= tocopy;
+		data += tocopy;
+		tocopy = 0;
+	}
+
+	while (towrite > 0) {
+		tocopy = sb->s_blocksize < towrite ?
+				sb->s_blocksize : towrite;
+
+		/* Break to read the last block */
+		if (tocopy < sb->s_blocksize)
+			break;
+
+		data_key_init(c, &key, inode->i_ino, block);
+		err = ubifs_jnl_write_data(c, inode, &key, data, sb->s_blocksize);
+		if (err)
+			goto free_buf;
+		block++;;
+		towrite -= tocopy;
+		data += tocopy;
+		tocopy = 0;
+	}
+
+	if (tocopy) {
+		/* Read the data in last block */
+		ret = ubifs_read_block(inode, block_buf, block, dn);
+		if (ret) {
+			if (ret != -ENOENT) {
+				err = ret;
+				goto free_buf;
+			}
+			memset(block_buf, 0, sb->s_blocksize);
+		}
+
+		memcpy(block_buf, data, tocopy);
+		data_key_init(c, &key, inode->i_ino, block);
+		err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize);
+		if (err)
+			goto free_buf;
+	}
+free_buf:
+	kfree(block_buf);
+free_dn:
+	kfree(dn);
+release_budget:
+	ubifs_release_budget(c, &req);
+out:
+	if (!err)
+		return len;
+	return err;
+}
+
+static struct dquot **ubifs_get_dquots(struct inode *inode)
+{
+	return ubifs_inode(inode)->i_dquot;
+}
+#endif
+
 /*
  * UBIFS mount options.
  *
@@ -1939,6 +2153,11 @@ const struct super_operations ubifs_super_operations = {
 	.remount_fs    = ubifs_remount_fs,
 	.show_options  = ubifs_show_options,
 	.sync_fs       = ubifs_sync_fs,
+#ifdef CONFIG_QUOTA
+	.quota_read    = ubifs_quota_read,
+	.quota_write   = ubifs_quota_write,
+	.get_dquots    = ubifs_get_dquots,
+#endif
 };
 
 /**
-- 
1.8.4.2

Re: [PATCH v2 20/35] ubifs: implement IO functions for quota files

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We have to implement 3 functions to support quota in ubifs:
> ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas()
> 
> ubifs_quota_read():
> 	callback to read quota file.
> ubifs_quota_write():
> 	callback to write data to quota file.
> ubifs_get_quotas():
> 	callback to get quota instance for inode.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 219 insertions(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index bc57685..3609d7b 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -940,6 +940,220 @@ static int check_volume_empty(struct ubifs_info *c)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_QUOTA
> +static ssize_t ubifs_quota_read(struct super_block *sb, int type, char *data,
> +			       size_t len, loff_t off)
> +{
> +	struct inode *inode = sb_dqopt(sb)->files[type];
> +	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
> +	int offset = off & (sb->s_blocksize - 1);
> +	int tocopy = 0;
> +	size_t toread;
> +	char *block_buf;
> +	struct ubifs_data_node *dn;
> +	int ret, err = 0;
> +
> +	toread = len;

Minor nit, please group same types together and also group assignments
as other UBI/FS codes does.

> +	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
> +	if (!dn) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	block_buf = kmalloc(sb->s_blocksize, GFP_NOFS);
> +	if (!block_buf) {
> +		err = -ENOMEM;
> +		goto free_dn;
> +	}
> +
> +	if (offset) {
> +		/* Read the un-aligned data in first block */
> +		tocopy = sb->s_blocksize - offset;
> +		if (toread < tocopy)
> +			tocopy = toread;

min()?

> +		ret = ubifs_read_block(inode, block_buf, block, dn);
> +		if (ret) {
> +			if (ret != -ENOENT) {

if (ret && ret != -ENOENT)?

> +				err = ret;
> +				goto free_buf;
> +			}
> +		}
> +
> +		memcpy(data, block_buf + offset, tocopy);
> +
> +		block++;
> +		toread -= tocopy;
> +		data += tocopy;
> +		tocopy = 0;
> +	}
> +
> +	while (toread > 0) {
> +		tocopy = sb->s_blocksize < toread ?
> +				sb->s_blocksize : toread;

min()?

> +
> +		/* Break to read the last block */
> +		if (tocopy < sb->s_blocksize)
> +			break;

Hmm, can't you combine this check with the above condition?

> +		ret = ubifs_read_block(inode, data, block, dn);
> +		if (ret) {
> +			if (ret != -ENOENT) {

See above.

> +				err = ret;
> +				goto free_buf;
> +			}
> +		}
> +		block++;;

superfluous semicolon.

> +		toread -= tocopy;
> +		data += tocopy;
> +		tocopy = 0;
> +	}
> +
> +	if (tocopy) {
> +		/* Read the data in last block */
> +		ret = ubifs_read_block(inode, block_buf, block, dn);
> +		if (ret) {
> +			if (ret != -ENOENT) {

See above. Maybe the -ENOENT stuff can be integrated into ubifs_read_block().
All callers deal with it the same way...

> +				err = ret;
> +				goto free_buf;
> +			}
> +		}
> +
> +		memcpy(data, block_buf, tocopy);
> +	}
> +free_buf:
> +	kfree(block_buf);
> +free_dn:
> +	kfree(dn);
> +out:
> +	if (!err)
> +		return len;
> +	return err;
> +}
> +
> +static ssize_t ubifs_quota_write(struct super_block *sb, int type,
> +				const char *data, size_t len, loff_t off)
> +{
> +	struct inode *inode = sb_dqopt(sb)->files[type];
> +	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
> +	struct ubifs_info *c = inode->i_sb->s_fs_info;
> +	int offset = off & (sb->s_blocksize - 1);
> +	union ubifs_key key;
> +	int tocopy = 0;
> +	size_t towrite = len;
> +	int ret, err = 0;
> +	struct ubifs_budget_req req = {};
> +	int new_block = 0;
> +	struct ubifs_data_node *dn;
> +	char *block_buf;

See above.

> +	/* There are dirtied blocks and new blockes, but we call them all
> +	 * new blocks here, the budgeting is same for them.
> +	 */
> +	new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize);
> +	if (offset)
> +		new_block += 1;
> +
> +	req.new_block_num = new_block;
> +
> +	err = ubifs_budget_space(c, &req);
> +	if (err)
> +		goto out;
> +
> +	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
> +	if (!dn) {
> +		err = -ENOMEM;
> +		goto release_budget;
> +	}
> +
> +	block_buf = kmalloc(sb->s_blocksize, GFP_NOFS);
> +	if (!block_buf) {
> +		err = -ENOMEM;
> +		goto free_dn;
> +	}
> +
> +	if (offset) {
> +		/* Write the un-aligned data in first block */
> +		tocopy = sb->s_blocksize - offset;
> +		if (towrite < tocopy)
> +			tocopy = towrite;

See above.

> +		ret = ubifs_read_block(inode, block_buf, block, dn);
> +		if (ret) {
> +			if (ret != -ENOENT) {
> +				err = ret;
> +				goto free_buf;
> +			}
> +			memset(block_buf, 0, sb->s_blocksize);
> +		}
> +
> +		memcpy(block_buf + offset, data, tocopy);
> +
> +		data_key_init(c, &key, inode->i_ino, block);
> +		err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize);

Hmm, I'm confused. You write the block containing the quota information to an inode's data?
How can be determined whether data is real data and what is quota data?
This clearly shows that I know not much about the quota subsystem. 8^)

> +		if (err) {
> +			goto free_buf;
> +		}
> +		block++;
> +		towrite -= tocopy;
> +		data += tocopy;
> +		tocopy = 0;
> +	}
> +
> +	while (towrite > 0) {
> +		tocopy = sb->s_blocksize < towrite ?
> +				sb->s_blocksize : towrite;
> +
> +		/* Break to read the last block */
> +		if (tocopy < sb->s_blocksize)
> +			break;

See above. Maybe you can combine all this common code.

> +		data_key_init(c, &key, inode->i_ino, block);
> +		err = ubifs_jnl_write_data(c, inode, &key, data, sb->s_blocksize);
> +		if (err)
> +			goto free_buf;
> +		block++;;

See above.

I'll review your other patches these days.
For now I'm too sleepy.

Thanks,
//richard

Re: [PATCH v2 20/35] ubifs: implement IO functions for quota files

[Dongsheng Yang]

On 08/04/2015 05:46 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> We have to implement 3 functions to support quota in ubifs:
>> ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas()
>>
[...]
>
> I'll review your other patches these days.
> For now I'm too sleepy.

Thank you so much, Richard. :)

I will reply all the comments from you and Jan
when you finish the review.

Thanx
Yang
>
> Thanks,
> //richard
> .
>

Re: [PATCH v2 20/35] ubifs: implement IO functions for quota files

[Dongsheng Yang]

On 08/04/2015 05:46 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> We have to implement 3 functions to support quota in ubifs:
>> ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas()
>>
>> ubifs_quota_read():
>> 	callback to read quota file.
>> ubifs_quota_write():
>> 	callback to write data to quota file.
>> ubifs_get_quotas():
>> 	callback to get quota instance for inode.
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/super.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 219 insertions(+)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index bc57685..3609d7b 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -940,6 +940,220 @@ static int check_volume_empty(struct ubifs_info *c)
>>   	return 0;
>>   }
>>
>> +#ifdef CONFIG_QUOTA
>> +static ssize_t ubifs_quota_read(struct super_block *sb, int type, char *data,
>> +			       size_t len, loff_t off)
>> +{
>> +	struct inode *inode = sb_dqopt(sb)->files[type];
>> +	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
>> +	int offset = off & (sb->s_blocksize - 1);
>> +	int tocopy = 0;
>> +	size_t toread;
>> +	char *block_buf;
>> +	struct ubifs_data_node *dn;
>> +	int ret, err = 0;
>> +
>> +	toread = len;
>
> Minor nit, please group same types together and also group assignments
> as other UBI/FS codes does.

Hm, okey. Good suggestion.
>
>> +	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
>> +	if (!dn) {
>> +		err = -ENOMEM;
>> +		goto out;
>> +	}
>> +
>> +	block_buf = kmalloc(sb->s_blocksize, GFP_NOFS);
>> +	if (!block_buf) {
>> +		err = -ENOMEM;
>> +		goto free_dn;
>> +	}
>> +
>> +	if (offset) {
>> +		/* Read the un-aligned data in first block */
>> +		tocopy = sb->s_blocksize - offset;
>> +		if (toread < tocopy)
>> +			tocopy = toread;
>
> min()?

okey, thanx
>
>> +		ret = ubifs_read_block(inode, block_buf, block, dn);
>> +		if (ret) {
>> +			if (ret != -ENOENT) {
>
> if (ret && ret != -ENOENT)?

Yea, agree.
>
>> +				err = ret;
>> +				goto free_buf;
>> +			}
>> +		}
>> +
>> +		memcpy(data, block_buf + offset, tocopy);
>> +
>> +		block++;
>> +		toread -= tocopy;
>> +		data += tocopy;
>> +		tocopy = 0;
>> +	}
>> +
>> +	while (toread > 0) {
>> +		tocopy = sb->s_blocksize < toread ?
>> +				sb->s_blocksize : toread;
>
> min()?

Yes, correct.
>
>> +
>> +		/* Break to read the last block */
>> +		if (tocopy < sb->s_blocksize)
>> +			break;
>
> Hmm, can't you combine this check with the above condition?

Sure.
>
>> +		ret = ubifs_read_block(inode, data, block, dn);
>> +		if (ret) {
>> +			if (ret != -ENOENT) {
>
> See above.

Thanx
>
>> +				err = ret;
>> +				goto free_buf;
>> +			}
>> +		}
>> +		block++;;
>
> superfluous semicolon.

Oh, right you are.
>
>> +		toread -= tocopy;
>> +		data += tocopy;
>> +		tocopy = 0;
>> +	}
>> +
>> +	if (tocopy) {
>> +		/* Read the data in last block */
>> +		ret = ubifs_read_block(inode, block_buf, block, dn);
>> +		if (ret) {
>> +			if (ret != -ENOENT) {
>
> See above. Maybe the -ENOENT stuff can be integrated into ubifs_read_block().
> All callers deal with it the same way...

Good point. Will try it.
>
>> +				err = ret;
>> +				goto free_buf;
>> +			}
>> +		}
>> +
>> +		memcpy(data, block_buf, tocopy);
>> +	}
>> +free_buf:
>> +	kfree(block_buf);
>> +free_dn:
>> +	kfree(dn);
>> +out:
>> +	if (!err)
>> +		return len;
>> +	return err;
>> +}
>> +
>> +static ssize_t ubifs_quota_write(struct super_block *sb, int type,
>> +				const char *data, size_t len, loff_t off)
>> +{
>> +	struct inode *inode = sb_dqopt(sb)->files[type];
>> +	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
>> +	struct ubifs_info *c = inode->i_sb->s_fs_info;
>> +	int offset = off & (sb->s_blocksize - 1);
>> +	union ubifs_key key;
>> +	int tocopy = 0;
>> +	size_t towrite = len;
>> +	int ret, err = 0;
>> +	struct ubifs_budget_req req = {};
>> +	int new_block = 0;
>> +	struct ubifs_data_node *dn;
>> +	char *block_buf;
>
> See above.
>
>> +	/* There are dirtied blocks and new blockes, but we call them all
>> +	 * new blocks here, the budgeting is same for them.
>> +	 */
>> +	new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize);
>> +	if (offset)
>> +		new_block += 1;
>> +
>> +	req.new_block_num = new_block;
>> +
>> +	err = ubifs_budget_space(c, &req);
>> +	if (err)
>> +		goto out;
>> +
>> +	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
>> +	if (!dn) {
>> +		err = -ENOMEM;
>> +		goto release_budget;
>> +	}
>> +
>> +	block_buf = kmalloc(sb->s_blocksize, GFP_NOFS);
>> +	if (!block_buf) {
>> +		err = -ENOMEM;
>> +		goto free_dn;
>> +	}
>> +
>> +	if (offset) {
>> +		/* Write the un-aligned data in first block */
>> +		tocopy = sb->s_blocksize - offset;
>> +		if (towrite < tocopy)
>> +			tocopy = towrite;
>
> See above.
>
>> +		ret = ubifs_read_block(inode, block_buf, block, dn);
>> +		if (ret) {
>> +			if (ret != -ENOENT) {
>> +				err = ret;
>> +				goto free_buf;
>> +			}
>> +			memset(block_buf, 0, sb->s_blocksize);
>> +		}
>> +
>> +		memcpy(block_buf + offset, data, tocopy);
>> +
>> +		data_key_init(c, &key, inode->i_ino, block);
>> +		err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize);
>
> Hmm, I'm confused. You write the block containing the quota information to an inode's data?
> How can be determined whether data is real data and what is quota data?

Ha, so we need a special writing function for quota file, 
xxxfs_quota_write().
This function would bypass the quota recording, so quota
data would not be contained in by itself.
> This clearly shows that I know not much about the quota subsystem. 8^)
>
>> +		if (err) {
>> +			goto free_buf;
>> +		}
>> +		block++;
>> +		towrite -= tocopy;
>> +		data += tocopy;
>> +		tocopy = 0;
>> +	}
>> +
>> +	while (towrite > 0) {
>> +		tocopy = sb->s_blocksize < towrite ?
>> +				sb->s_blocksize : towrite;
>> +
>> +		/* Break to read the last block */
>> +		if (tocopy < sb->s_blocksize)
>> +			break;
>
> See above. Maybe you can combine all this common code.

Agree, thanx
>
>> +		data_key_init(c, &key, inode->i_ino, block);
>> +		err = ubifs_jnl_write_data(c, inode, &key, data, sb->s_blocksize);
>> +		if (err)
>> +			goto free_buf;
>> +		block++;;
>
> See above.
>
> I'll review your other patches these days.
> For now I'm too sleepy.

Thanx a lot Richard.

Your review is always helpful to me.

Yang
>
> Thanks,
> //richard
> .
>

[PATCH v2 21/35] ubifs: disable quota in ubifs_put_super

[Dongsheng Yang]

We need to disable quota in umounting ubifs. So we have to
disable quota in ubifs_put_super(). But quota disable will
write quota file, and we have to budget for it.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 3609d7b..7031c16 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2027,6 +2027,8 @@ static void ubifs_put_super(struct super_block *sb)
 
 	ubifs_msg(c, "un-mount UBI device %d", c->vi.ubi_num);
 
+	if (!c->ro_mount)
+		dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
 	/*
 	 * The following asserts are only valid if there has not been a failure
 	 * of the media. For example, there will be dirty inodes if we failed
-- 
1.8.4.2

Re: [PATCH v2 21/35] ubifs: disable quota in ubifs_put_super

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We need to disable quota in umounting ubifs. So we have to
> disable quota in ubifs_put_super(). But quota disable will
> write quota file, and we have to budget for it.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 3609d7b..7031c16 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -2027,6 +2027,8 @@ static void ubifs_put_super(struct super_block *sb)
>  
>  	ubifs_msg(c, "un-mount UBI device %d", c->vi.ubi_num);
>  
> +	if (!c->ro_mount)
> +		dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);

I fear this will explode in your face.
dquot_disable disables all inodes needed for quota support, once enabled
you have to shut down quota support.
IOW if someone remounts UBIFS read only or UBIFS does itself due to an error
you'll leak inodes.

Thanks,
//richard

Re: [PATCH v2 21/35] ubifs: disable quota in ubifs_put_super

[Dongsheng Yang]

On 08/09/2015 05:08 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> We need to disable quota in umounting ubifs. So we have to
>> disable quota in ubifs_put_super(). But quota disable will
>> write quota file, and we have to budget for it.
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/super.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index 3609d7b..7031c16 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -2027,6 +2027,8 @@ static void ubifs_put_super(struct super_block *sb)
>>
>>   	ubifs_msg(c, "un-mount UBI device %d", c->vi.ubi_num);
>>
>> +	if (!c->ro_mount)
>> +		dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
>
> I fear this will explode in your face.
> dquot_disable disables all inodes needed for quota support, once enabled
> you have to shut down quota support.
> IOW if someone remounts UBIFS read only or UBIFS does itself due to an error
> you'll leak inodes.

Yes, you are right. I filter the read-only for a bug in quota which
call sync_fs directly even in ro-mode. But I have addressed it in quota,
so I will update this patch in next version.

Thanx a lot, Richard.
Yang
>
> Thanks,
> //richard
>
> .
>

[PATCH v2 22/35] ubifs: write quota back in ubifs_sync

[Dongsheng Yang]

Call dquot_writeback_dquots to write quota back in ubifs_sync

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 7031c16..9fdfd69 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -490,6 +490,8 @@ static int ubifs_sync_fs(struct super_block *sb, int wait)
 	if (!wait)
 		return 0;
 
+	dquot_writeback_dquots(sb, -1);
+
 	/*
 	 * Synchronize write buffers, because 'ubifs_run_commit()' does not
 	 * do this if it waits for an already running commit.
-- 
1.8.4.2

Re: [PATCH v2 22/35] ubifs: write quota back in ubifs_sync

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> Call dquot_writeback_dquots to write quota back in ubifs_sync
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 7031c16..9fdfd69 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -490,6 +490,8 @@ static int ubifs_sync_fs(struct super_block *sb, int wait)
>  	if (!wait)
>  		return 0;
>  
> +	dquot_writeback_dquots(sb, -1);
> +

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

[PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Dongsheng Yang]

We need to set or clear MS_RDONLY in remounting.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 9fdfd69..6ed35e2 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1953,6 +1953,7 @@ static int ubifs_remount_rw(struct ubifs_info *c)
 		 */
 		err = dbg_check_space_info(c);
 	}
+	c->vfs_sb->s_flags &= ~MS_RDONLY;
 
 	mutex_unlock(&c->umount_mutex);
 	return err;
@@ -2019,6 +2020,7 @@ static void ubifs_remount_ro(struct ubifs_info *c)
 	err = dbg_check_space_info(c);
 	if (err)
 		ubifs_ro_mode(c, err);
+	c->vfs_sb->s_flags |= MS_RDONLY;
 	mutex_unlock(&c->umount_mutex);
 }
 
-- 
1.8.4.2

Re: [PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We need to set or clear MS_RDONLY in remounting.

Care to explain why?
Does the quota subsystem need that information?
If so, where?

Thanks,
//richard

Re: [PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Dongsheng Yang]

On 08/09/2015 05:17 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> We need to set or clear MS_RDONLY in remounting.
>
> Care to explain why?
> Does the quota subsystem need that information?
> If so, where?

Ha, yes, you are right.

When we remount to rw from ro, we need to call dquot_resume.
And dquot_resume will call vfs_load_quota_inode() which
will check the MS_RDONLY. If it's ro mode, will return
an -EROFS.

And I know that we are using ->ro_mount in ubifs
to store the mounted mode. So we don't care about
the MS_RDONLY in ubifs. But why not to tell vfs we are in
ro or rw? Does it cause any problem?

Thanx
YAng

> Thanks,
> //richard
> .
>

Re: [PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Dongsheng Yang]

On 08/10/2015 10:46 AM, Dongsheng Yang wrote:
> On 08/09/2015 05:17 AM, Richard Weinberger wrote:
>> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>>> We need to set or clear MS_RDONLY in remounting.
>>
>> Care to explain why?
>> Does the quota subsystem need that information?
>> If so, where?
>
> Ha, yes, you are right.
>
> When we remount to rw from ro, we need to call dquot_resume.
> And dquot_resume will call vfs_load_quota_inode() which
> will check the MS_RDONLY. If it's ro mode, will return
> an -EROFS.
>
> And I know that we are using ->ro_mount in ubifs
> to store the mounted mode. So we don't care about
> the MS_RDONLY in ubifs. But why not to tell vfs we are in
> ro or rw? Does it cause any problem?

Hi Artem,
	I found there is a commit 2ef13294d to introduce
ro_mount to ubifs. Yes, I see the reason to use ro_mount
rather than MS_RDONLY in ubifs. But why we do not set
MS_RDONLY to tell vfs it's in rdonly or not? Current
quota depends on it. Does this patch cause some problem?

Yang
>
> Thanx
> YAng
>
>> Thanks,
>> //richard
>> .
>>
>
> --
> 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 v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Artem Bityutskiy]

On Mon, 2015-08-24 at 09:29 +0800, Dongsheng Yang wrote:
> On 08/10/2015 10:46 AM, Dongsheng Yang wrote:
> > On 08/09/2015 05:17 AM, Richard Weinberger wrote:
> > > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> > > > We need to set or clear MS_RDONLY in remounting.
> > > 
> > > Care to explain why?
> > > Does the quota subsystem need that information?
> > > If so, where?
> > 
> > Ha, yes, you are right.
> > 
> > When we remount to rw from ro, we need to call dquot_resume.
> > And dquot_resume will call vfs_load_quota_inode() which
> > will check the MS_RDONLY. If it's ro mode, will return
> > an -EROFS.
> > 
> > And I know that we are using ->ro_mount in ubifs
> > to store the mounted mode. So we don't care about
> > the MS_RDONLY in ubifs. But why not to tell vfs we are in
> > ro or rw? Does it cause any problem?
> 
> Hi Artem,
> 	I found there is a commit 2ef13294d to introduce
> ro_mount to ubifs. Yes, I see the reason to use ro_mount
> rather than MS_RDONLY in ubifs. But why we do not set
> MS_RDONLY to tell vfs it's in rdonly or not? Current
> quota depends on it. Does this patch cause some problem?

MS_RDONLY is set at mount time, then it can be changed when we re
-mount, and then UBIFS can change it when there was a critical error,
to ask VFS to stop writing more data to the file-system. See
'ubifs_ro_mode()'.

The custom ro_mount is the same as MS_RDONLY, but it does not change in
'ubifs_ro_mode()'. It preserves the last mount state before the error
happened - was it R/O or R/W.

Why is this needed?

UBIFS allocate different amount of resources depending on whether we
are R/O or R/W. We use less memory in R/O mode, for example.

At unmount time, we need to know whether we allocated the "RW" amount
of resources or the "RO" amount of resources.

We cannot use the MS_RDONLY flag to detect that, because it is changed
in 'ubifs_ro_mode()' unconditionally, so we lose the information.
Therefore we use the custom ro_mount flag - it tells us how we were
mounted, so we know how much resources to free.

Artem.

Re: [PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Dongsheng Yang]

On 08/24/2015 03:02 PM, Artem Bityutskiy wrote:
> On Mon, 2015-08-24 at 09:29 +0800, Dongsheng Yang wrote:
>> On 08/10/2015 10:46 AM, Dongsheng Yang wrote:
>>> On 08/09/2015 05:17 AM, Richard Weinberger wrote:
>>>> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>>>>> We need to set or clear MS_RDONLY in remounting.
>>>>
>>>> Care to explain why?
>>>> Does the quota subsystem need that information?
>>>> If so, where?
>>>
>>> Ha, yes, you are right.
>>>
>>> When we remount to rw from ro, we need to call dquot_resume.
>>> And dquot_resume will call vfs_load_quota_inode() which
>>> will check the MS_RDONLY. If it's ro mode, will return
>>> an -EROFS.
>>>
>>> And I know that we are using ->ro_mount in ubifs
>>> to store the mounted mode. So we don't care about
>>> the MS_RDONLY in ubifs. But why not to tell vfs we are in
>>> ro or rw? Does it cause any problem?
>>
>> Hi Artem,
>> 	I found there is a commit 2ef13294d to introduce
>> ro_mount to ubifs. Yes, I see the reason to use ro_mount
>> rather than MS_RDONLY in ubifs. But why we do not set
>> MS_RDONLY to tell vfs it's in rdonly or not? Current
>> quota depends on it. Does this patch cause some problem?
>
> MS_RDONLY is set at mount time, then it can be changed when we re
> -mount, and then UBIFS can change it when there was a critical error,
> to ask VFS to stop writing more data to the file-system. See
> 'ubifs_ro_mode()'.
>
> The custom ro_mount is the same as MS_RDONLY, but it does not change in
> 'ubifs_ro_mode()'. It preserves the last mount state before the error
> happened - was it R/O or R/W.
>
> Why is this needed?
>
> UBIFS allocate different amount of resources depending on whether we
> are R/O or R/W. We use less memory in R/O mode, for example.
>
> At unmount time, we need to know whether we allocated the "RW" amount
> of resources or the "RO" amount of resources.
>
> We cannot use the MS_RDONLY flag to detect that, because it is changed
> in 'ubifs_ro_mode()' unconditionally, so we lose the information.
> Therefore we use the custom ro_mount flag - it tells us how we were
> mounted, so we know how much resources to free.

Great explanation!!! This makes it much more clear to me.

But about:
 > MS_RDONLY is set at mount time, then it can be changed when we re

Currently, ubifs did not set it at mount time and ubifs did not change
it in re-mounting. So vfs don't know the R/O or R/W mode of ubifs.

That's the reason I sent this patch to set the MS_RDONLY in ubifs
properly. I was concerned that will it cause some problem. But now, from
you explanation, this patch is a correct fix I think.

Thanx
Yang
>
> Artem.
> .
>

Re: [PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Artem Bityutskiy]

On Mon, 2015-08-24 at 15:12 +0800, Dongsheng Yang wrote:
> On 08/24/2015 03:02 PM, Artem Bityutskiy wrote:
> > On Mon, 2015-08-24 at 09:29 +0800, Dongsheng Yang wrote:
> > > On 08/10/2015 10:46 AM, Dongsheng Yang wrote:
> > > > On 08/09/2015 05:17 AM, Richard Weinberger wrote:
> > > > > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> > > > > > We need to set or clear MS_RDONLY in remounting.
> > > > > 
> > > > > Care to explain why?
> > > > > Does the quota subsystem need that information?
> > > > > If so, where?
> > > > 
> > > > Ha, yes, you are right.
> > > > 
> > > > When we remount to rw from ro, we need to call dquot_resume.
> > > > And dquot_resume will call vfs_load_quota_inode() which
> > > > will check the MS_RDONLY. If it's ro mode, will return
> > > > an -EROFS.
> > > > 
> > > > And I know that we are using ->ro_mount in ubifs
> > > > to store the mounted mode. So we don't care about
> > > > the MS_RDONLY in ubifs. But why not to tell vfs we are in
> > > > ro or rw? Does it cause any problem?
> > > 
> > > Hi Artem,
> > > 	I found there is a commit 2ef13294d to introduce
> > > ro_mount to ubifs. Yes, I see the reason to use ro_mount
> > > rather than MS_RDONLY in ubifs. But why we do not set
> > > MS_RDONLY to tell vfs it's in rdonly or not? Current
> > > quota depends on it. Does this patch cause some problem?
> > 
> > MS_RDONLY is set at mount time, then it can be changed when we re
> > -mount, and then UBIFS can change it when there was a critical
> > error,
> > to ask VFS to stop writing more data to the file-system. See
> > 'ubifs_ro_mode()'.
> > 
> > The custom ro_mount is the same as MS_RDONLY, but it does not
> > change in
> > 'ubifs_ro_mode()'. It preserves the last mount state before the
> > error
> > happened - was it R/O or R/W.
> > 
> > Why is this needed?
> > 
> > UBIFS allocate different amount of resources depending on whether
> > we
> > are R/O or R/W. We use less memory in R/O mode, for example.
> > 
> > At unmount time, we need to know whether we allocated the "RW"
> > amount
> > of resources or the "RO" amount of resources.
> > 
> > We cannot use the MS_RDONLY flag to detect that, because it is
> > changed
> > in 'ubifs_ro_mode()' unconditionally, so we lose the information.
> > Therefore we use the custom ro_mount flag - it tells us how we were
> > mounted, so we know how much resources to free.
> 
> Great explanation!!! This makes it much more clear to me.
> 
> But about:
>  > MS_RDONLY is set at mount time, then it can be changed when we re
> 
> Currently, ubifs did not set it at mount time and ubifs did not
> change
> it in re-mounting. So vfs don't know the R/O or R/W mode of ubifs.

Well, sounds like a bug. Either we missed that, or VFS used to set it,
and now does not. In either case, IIUC, the MS_RDONLY flag should be
set on remount and reflect the mount state. Please, verify /proc/mounts
after RO<->RW remounts - we must make sure mount options are correct
there. You can cook a patch and send it. Do not forget to add the
stable tag then.

Artem.

Re: [PATCH v2 23/35] ubifs: set/clear MS_RDONLY properly in ubifs_remount

[Dongsheng Yang]

On 08/24/2015 03:26 PM, Artem Bityutskiy wrote:
> On Mon, 2015-08-24 at 15:12 +0800, Dongsheng Yang wrote:
>> On 08/24/2015 03:02 PM, Artem Bityutskiy wrote:
[...]
>
> Well, sounds like a bug. Either we missed that, or VFS used to set it,
Hi Artem,

Yes, VFS is setting flags after sb->s_op->remount_fs(). But someone
would use it in remount_fs() such as quota. That's the reason for this
patch. What I want here is to make our ubifs working similarly with
other filesystems setting or clearing MS_RDONLY in remount_fs(). Because
someone in vfs needs this information.

Although that's another topic, we set MS_RDONLY twice, one in ubifs, two
in VFS. But they are different. in ubifs, we only set the MS_RDONLY in
remount_ro() specially for MS_RDONLY. But in VFS, we are
setting all flags for s_flags in remounting. So, I think it's okey in 
design.

> and now does not. In either case, IIUC, the MS_RDONLY flag should be
> set on remount and reflect the mount state. Please, verify /proc/mounts
> after RO<->RW remounts - we must make sure mount options are correct
> there. You can cook a patch and send it. Do not forget to add the
> stable tag then.

Before adding quota for ubifs, there is no user to use MS_RDONLY
before do_remount() returning. So there is no problem before
adding quota in ubifs and no need for stable then.

Thanx
Yang
>
> Artem.
> .
>

[PATCH v2 24/35] ubifs: suspend & resume quota properly in ubifs_remount

[Dongsheng Yang]

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 6ed35e2..06dd7af 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2128,11 +2128,13 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
 		err = ubifs_remount_rw(c);
 		if (err)
 			return err;
+		dquot_resume(sb, -1);
 	} else if (!c->ro_mount && (*flags & MS_RDONLY)) {
 		if (c->ro_error) {
 			ubifs_msg(c, "cannot re-mount R/O due to prior errors");
 			return -EROFS;
 		}
+		dquot_suspend(sb, -1);
 		ubifs_remount_ro(c);
 	}
 
-- 
1.8.4.2

Re: [PATCH v2 24/35] ubifs: suspend & resume quota properly in ubifs_remount

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 6ed35e2..06dd7af 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -2128,11 +2128,13 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
>  		err = ubifs_remount_rw(c);
>  		if (err)
>  			return err;
> +		dquot_resume(sb, -1);
>  	} else if (!c->ro_mount && (*flags & MS_RDONLY)) {
>  		if (c->ro_error) {
>  			ubifs_msg(c, "cannot re-mount R/O due to prior errors");
>  			return -EROFS;
>  		}
> +		dquot_suspend(sb, -1);

Is it really safe to ignore the return values of both function?

Beside of that,
Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

Re: [PATCH v2 24/35] ubifs: suspend & resume quota properly in ubifs_remount

[Dongsheng Yang]

On 08/09/2015 05:24 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/super.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index 6ed35e2..06dd7af 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -2128,11 +2128,13 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
>>   		err = ubifs_remount_rw(c);
>>   		if (err)
>>   			return err;
>> +		dquot_resume(sb, -1);
>>   	} else if (!c->ro_mount && (*flags & MS_RDONLY)) {
>>   		if (c->ro_error) {
>>   			ubifs_msg(c, "cannot re-mount R/O due to prior errors");
>>   			return -EROFS;
>>   		}
>> +		dquot_suspend(sb, -1);
>
> Is it really safe to ignore the return values of both function?

Good point. I will do it.

Thanx
Yang
>
> Beside of that,
> Reviewed-by: Richard Weinberger <richard@nod.at>
>
> Thanks,
> //richard
> .
>

[PATCH v2 25/35] ubifs: record quota information about inode in ubifs_new_inode

[Dongsheng Yang]

Initialize quota and alloc a inode quota information in
ubifs_new_inode(). Then quota would be aware of a new
inode is allocated.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/dir.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 8d93427..5bfce44 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -41,6 +41,7 @@
  */
 
 #include "ubifs.h"
+#include <linux/quotaops.h>
 
 /**
  * inherit_flags - inherit flags of the parent inode.
@@ -90,12 +91,13 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
 {
 	struct inode *inode;
 	struct ubifs_inode *ui;
+	int err = 0;
 
 	inode = new_inode(c->vfs_sb);
-	ui = ubifs_inode(inode);
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
+	ui = ubifs_inode(inode);
 	/*
 	 * Set 'S_NOCMTIME' to prevent VFS form updating [mc]time of inodes and
 	 * marking them dirty in file write path (see 'file_update_time()').
@@ -109,6 +111,11 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
 			 ubifs_current_time(inode);
 	inode->i_mapping->nrpages = 0;
 
+	dquot_initialize(inode);
+        err = dquot_alloc_inode(inode);
+        if (err)
+                goto fail_drop;
+
 	switch (mode & S_IFMT) {
 	case S_IFREG:
 		inode->i_mapping->a_ops = &ubifs_file_address_operations;
@@ -148,8 +155,8 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
 			spin_unlock(&c->cnt_lock);
 			ubifs_err(c, "out of inode numbers");
 			make_bad_inode(inode);
-			iput(inode);
-			return ERR_PTR(-EINVAL);
+			err = -EINVAL;
+			goto fail_free;
 		}
 		ubifs_warn(c, "running out of inode numbers (current %lu, max %u)",
 			   (unsigned long)c->highest_inum, INUM_WATERMARK);
@@ -166,6 +173,13 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
 	ui->creat_sqnum = ++c->max_sqnum;
 	spin_unlock(&c->cnt_lock);
 	return inode;
+
+fail_free:
+	dquot_free_inode(inode);
+fail_drop:
+	dquot_drop(inode);
+	iput(inode);
+	return ERR_PTR(err);
 }
 
 static int dbg_check_name(const struct ubifs_info *c,
-- 
1.8.4.2

Re: [PATCH v2 25/35] ubifs: record quota information about inode in ubifs_new_inode

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> Initialize quota and alloc a inode quota information in
> ubifs_new_inode(). Then quota would be aware of a new
> inode is allocated.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/dir.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 8d93427..5bfce44 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -41,6 +41,7 @@
>   */
>  
>  #include "ubifs.h"
> +#include <linux/quotaops.h>
>  
>  /**
>   * inherit_flags - inherit flags of the parent inode.
> @@ -90,12 +91,13 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
>  {
>  	struct inode *inode;
>  	struct ubifs_inode *ui;
> +	int err = 0;
>  
>  	inode = new_inode(c->vfs_sb);
> -	ui = ubifs_inode(inode);
>  	if (!inode)
>  		return ERR_PTR(-ENOMEM);
>  
> +	ui = ubifs_inode(inode);

Seems like an unrelated change.
All ubifs_inode() does is a container_of(), inode = NULL won't hurt.

Thanks,
//richard

Re: [PATCH v2 25/35] ubifs: record quota information about inode in ubifs_new_inode

[Dongsheng Yang]

On 08/09/2015 05:43 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> Initialize quota and alloc a inode quota information in
>> ubifs_new_inode(). Then quota would be aware of a new
>> inode is allocated.
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/dir.c | 20 +++++++++++++++++---
>>   1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
>> index 8d93427..5bfce44 100644
>> --- a/fs/ubifs/dir.c
>> +++ b/fs/ubifs/dir.c
>> @@ -41,6 +41,7 @@
>>    */
>>
>>   #include "ubifs.h"
>> +#include <linux/quotaops.h>
>>
>>   /**
>>    * inherit_flags - inherit flags of the parent inode.
>> @@ -90,12 +91,13 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
>>   {
>>   	struct inode *inode;
>>   	struct ubifs_inode *ui;
>> +	int err = 0;
>>
>>   	inode = new_inode(c->vfs_sb);
>> -	ui = ubifs_inode(inode);
>>   	if (!inode)
>>   		return ERR_PTR(-ENOMEM);
>>
>> +	ui = ubifs_inode(inode);
>
> Seems like an unrelated change.
> All ubifs_inode() does is a container_of(), inode = NULL won't hurt.

My bad, I planed to split this patch, but forgot it.

Yes, as you said, inode = NULL won't hurt, but I think checking inode
following new_inode() directly before using it seems more reasonable.
Although it's obvious that's okey to experts like you, I
believe this change could make the logic more "correct" to others. :)

It's a trivial fix from my opinion, do you think that's worthy?

Yang
>
> Thanks,
> //richard
> .
>

[PATCH v2 26/35] ubifs: free quota inode information in ubifs_evict_inode

[Dongsheng Yang]

In evict, we have to call dquot_free_inode() to tell quota
subsystem there is one inode to be free. Please update the
quota information.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 06dd7af..20500f0 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -361,6 +361,7 @@ static void ubifs_evict_inode(struct inode *inode)
 	if (is_bad_inode(inode))
 		goto out;
 
+	dquot_initialize(inode);
 	ui->ui_size = inode->i_size = 0;
 	err = ubifs_jnl_delete_inode(c, inode);
 	if (err)
@@ -370,7 +371,7 @@ static void ubifs_evict_inode(struct inode *inode)
 		 */
 		ubifs_err(c, "can't delete inode %lu, error %d",
 			  inode->i_ino, err);
-
+	dquot_free_inode(inode);
 out:
 	if (ui->dirty)
 		ubifs_release_dirty_inode_budget(c, ui);
@@ -380,6 +381,7 @@ out:
 		smp_wmb();
 	}
 done:
+	dquot_drop(inode);
 	clear_inode(inode);
 }
 
-- 
1.8.4.2

Re: [PATCH v2 26/35] ubifs: free quota inode information in ubifs_evict_inode

[Richard Weinberger]

Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> In evict, we have to call dquot_free_inode() to tell quota
> subsystem there is one inode to be free. Please update the
> quota information.
> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ubifs/super.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 06dd7af..20500f0 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -361,6 +361,7 @@ static void ubifs_evict_inode(struct inode *inode)
>  	if (is_bad_inode(inode))
>  		goto out;
>  
> +	dquot_initialize(inode);
>  	ui->ui_size = inode->i_size = 0;
>  	err = ubifs_jnl_delete_inode(c, inode);
>  	if (err)
> @@ -370,7 +371,7 @@ static void ubifs_evict_inode(struct inode *inode)
>  		 */
>  		ubifs_err(c, "can't delete inode %lu, error %d",
>  			  inode->i_ino, err);
> -
> +	dquot_free_inode(inode);
>  out:
>  	if (ui->dirty)
>  		ubifs_release_dirty_inode_budget(c, ui);
> @@ -380,6 +381,7 @@ out:
>  		smp_wmb();
>  	}
>  done:
> +	dquot_drop(inode);

Is it allowed to call this without a dquot_initialize()?
The if (inode->i_nlink) branch will hit that condition.

Thanks,
//richard

Re: [PATCH v2 26/35] ubifs: free quota inode information in ubifs_evict_inode

[Dongsheng Yang]

On 08/09/2015 05:51 AM, Richard Weinberger wrote:
> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
>> In evict, we have to call dquot_free_inode() to tell quota
>> subsystem there is one inode to be free. Please update the
>> quota information.
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ubifs/super.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
>> index 06dd7af..20500f0 100644
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -361,6 +361,7 @@ static void ubifs_evict_inode(struct inode *inode)
>>   	if (is_bad_inode(inode))
>>   		goto out;
>>
>> +	dquot_initialize(inode);
>>   	ui->ui_size = inode->i_size = 0;
>>   	err = ubifs_jnl_delete_inode(c, inode);
>>   	if (err)
>> @@ -370,7 +371,7 @@ static void ubifs_evict_inode(struct inode *inode)
>>   		 */
>>   		ubifs_err(c, "can't delete inode %lu, error %d",
>>   			  inode->i_ino, err);
>> -
>> +	dquot_free_inode(inode);
>>   out:
>>   	if (ui->dirty)
>>   		ubifs_release_dirty_inode_budget(c, ui);
>> @@ -380,6 +381,7 @@ out:
>>   		smp_wmb();
>>   	}
>>   done:
>> +	dquot_drop(inode);
>
> Is it allowed to call this without a dquot_initialize()?
> The if (inode->i_nlink) branch will hit that condition.

Yes, that's intentional. If (inode->i_nlink), we should
just call dquot_drop(). else, we have to dquot_initialize()->
dquot_free_inode()->dquot_drop();

Thanx
Yang
>
> Thanks,
> //richard
> .
>

[PATCH v2 27/35] ubifs: alloc quota space in ubifs_write_begin

[Dongsheng Yang]

Besides inode, quota also limit the space. When
use want to write something, we have to tell quota
subsystem there is something is going to be written
into ubifs, please check the quota limit. If it exceed
the limit, return EQUOT. Else, record it and go on.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/file.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index f307e86..b1e89ff 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -53,6 +53,7 @@
 #include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/slab.h>
+#include <linux/quotaops.h>
 
 int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
 		      struct ubifs_data_node *dn)
@@ -431,8 +432,10 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
 	int uninitialized_var(err), appending = !!(pos + len > inode->i_size);
+	int quota_size = 0;
 	int skipped_read = 0;
 	struct page *page;
+	int ret = 0;
 
 	ubifs_assert(ubifs_inode(inode)->ui_size == inode->i_size);
 	ubifs_assert(!c->ro_media && !c->ro_mount);
@@ -440,10 +443,22 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 	if (unlikely(c->ro_error))
 		return -EROFS;
 
+	quota_size = ((pos + len) - inode->i_size);
+	if (quota_size < 0)
+		quota_size = 0;
+	if (S_ISREG(inode->i_mode)) {
+		dquot_initialize(inode);
+		ret = dquot_alloc_space_nodirty(inode, quota_size);
+		if (unlikely(ret))
+			goto err;
+	}
+
 	/* Try out the fast-path part first */
 	page = grab_cache_page_write_begin(mapping, index, flags);
-	if (unlikely(!page))
-		return -ENOMEM;
+	if (unlikely(!page)) {
+		ret = -ENOMEM;
+		goto free_quot;
+	}
 
 	if (!PageUptodate(page)) {
 		/* The page is not loaded from the flash */
@@ -497,7 +512,9 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 		unlock_page(page);
 		page_cache_release(page);
 
-		return write_begin_slow(mapping, pos, len, pagep, flags);
+		ret = write_begin_slow(mapping, pos, len, pagep, flags);
+		if (unlikely(ret))
+			goto free_quot;
 	}
 
 	/*
@@ -507,7 +524,12 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 	 * otherwise. This is an optimization (slightly hacky though).
 	 */
 	*pagep = page;
-	return 0;
+
+	return ret;
+free_quot:
+	dquot_free_space_nodirty(inode, quota_size);
+err:
+	return ret;
 
 }
 
-- 
1.8.4.2

[PATCH v2 28/35] ubifs: free quota space in do_truncation

[Dongsheng Yang]

In truncation, we have to tell quota subsystem
to update the quota information of the space changing.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/dir.c  | 1 +
 fs/ubifs/file.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 5bfce44..d430bb6 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -607,6 +607,7 @@ static int ubifs_unlink(struct inode *dir, struct dentry *dentry)
 	err = ubifs_jnl_update(c, dir, &dentry->d_name, inode, 1, 0);
 	if (err)
 		goto out_cancel;
+
 	unlock_2_inodes(dir, inode);
 
 	if (budgeted)
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index b1e89ff..1e8ee41 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1130,6 +1130,7 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 	int err;
 	struct ubifs_budget_req req;
 	loff_t old_size = inode->i_size, new_size = attr->ia_size;
+	loff_t quota_size = (old_size - new_size);
 	int offset = new_size & (UBIFS_BLOCK_SIZE - 1), budgeted = 1;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 
@@ -1209,6 +1210,10 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 	do_attr_changes(inode, attr);
 	err = ubifs_jnl_truncate(c, inode, old_size, new_size);
 	mutex_unlock(&ui->ui_mutex);
+	if (quota_size < 0)
+		quota_size = 0;
+	if (S_ISREG(inode->i_mode))
+		dquot_free_space_nodirty(inode, quota_size);
 
 out_budg:
 	if (budgeted)
-- 
1.8.4.2

[PATCH v2 29/35] ubifs: free quota space when deleting a file

[Dongsheng Yang]

When we unlink an inode which has only one last reference,
we are going to free the space in this inode. So free
the quota space at the same time.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/journal.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 0b9da5b..a6e02f4 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -58,6 +58,8 @@
  * all the nodes.
  */
 
+#include <linux/quotaops.h>
+
 #include "ubifs.h"
 
 /**
@@ -614,6 +616,10 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
 			goto out_finish;
 		}
 		ui->del_cmtno = c->cmt_no;
+		if (S_ISREG(inode->i_mode)) {
+			dquot_initialize((struct inode *)inode);
+			dquot_free_space_nodirty((struct inode *)inode, inode->i_size);
+		}
 	}
 
 	err = write_head(c, BASEHD, dent, len, &lnum, &dent_offs, sync);
-- 
1.8.4.2

[PATCH v2 30/35] ubifs: adapt quota space informatin in do_setattr

[Dongsheng Yang]

In do_setattr, there is a possibility to change the size of
a file. In this case, we have to notify the changed size to
quota subsystem.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/file.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 1e8ee41..24af30c 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1250,6 +1250,17 @@ static int do_setattr(struct ubifs_info *c, struct inode *inode,
 
 	if (attr->ia_valid & ATTR_SIZE) {
 		dbg_gen("size %lld -> %lld", inode->i_size, new_size);
+		if (S_ISREG(inode->i_mode)) {
+			if (new_size > inode->i_size) {
+				err = dquot_alloc_space_nodirty(inode, new_size - inode->i_size);
+				if (err) {
+					ubifs_release_budget(c, &req);
+					return err;
+				}
+			} else {
+				dquot_free_space_nodirty(inode, inode->i_size - new_size);
+			}
+		}
 		truncate_setsize(inode, new_size);
 	}
 
-- 
1.8.4.2

[PATCH v2 31/35] ubifs: transfer quota information in changing owner or group

[Dongsheng Yang]

When we change the owner or group of a file, we need to transfer
the quota information at the same time.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/file.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 24af30c..c4651a8 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1308,6 +1308,15 @@ int ubifs_setattr(struct dentry *dentry, struct iattr *attr)
 	if (err)
 		return err;
 
+	if (is_quota_modification(inode, attr))
+		dquot_initialize(inode);
+	if ((attr->ia_valid & ATTR_UID && !uid_eq(attr->ia_uid, inode->i_uid)) ||
+	    (attr->ia_valid & ATTR_GID && !gid_eq(attr->ia_gid, inode->i_gid))) {
+		err = dquot_transfer(inode, attr);
+		if (err)
+			return err;
+	}
+
 	if ((attr->ia_valid & ATTR_SIZE) && attr->ia_size < inode->i_size)
 		/* Truncation to a smaller size */
 		err = do_truncation(c, inode, attr);
-- 
1.8.4.2

[PATCH v2 32/35] ubifs: write inode in ubifs_quota_write if we are appending

[Dongsheng Yang]

When we are appending writing quota file, we have to do a
budgeting for the inode and write inode in ubifs_quota_write.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 20500f0..acc5d2f 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1040,6 +1040,10 @@ static ssize_t ubifs_quota_write(struct super_block *sb, int type,
 				const char *data, size_t len, loff_t off)
 {
 	struct inode *inode = sb_dqopt(sb)->files[type];
+	struct ubifs_inode *ui = ubifs_inode(inode);
+	loff_t end_pos = off + len;
+	int appending = (end_pos > inode->i_size);
+	int ino_released = 0;
 	unsigned long block = off >> UBIFS_BLOCK_SHIFT;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 	int offset = off & (sb->s_blocksize - 1);
@@ -1048,6 +1052,8 @@ static ssize_t ubifs_quota_write(struct super_block *sb, int type,
 	size_t towrite = len;
 	int ret, err = 0;
 	struct ubifs_budget_req req = {};
+	struct ubifs_budget_req ino_req = { .dirtied_ino = 1,
+				.dirtied_ino_d = ALIGN(ui->data_len, 8) };
 	int new_block = 0;
 	struct ubifs_data_node *dn;
 	char *block_buf;
@@ -1058,13 +1064,17 @@ static ssize_t ubifs_quota_write(struct super_block *sb, int type,
 	new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize);
 	if (offset)
 		new_block += 1;
-
 	req.new_block_num = new_block;
-
 	err = ubifs_budget_space(c, &req);
 	if (err)
 		goto out;
 
+	if (appending) {
+		err = ubifs_budget_space(c, &ino_req);
+		if (err)
+			goto release_block;
+	}
+
 	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
 	if (!dn) {
 		err = -ENOMEM;
@@ -1140,11 +1150,23 @@ static ssize_t ubifs_quota_write(struct super_block *sb, int type,
 		if (err)
 			goto free_buf;
 	}
+	if (appending) {
+		mutex_lock(&ui->ui_mutex);
+		i_size_write(inode, end_pos);
+		ui->ui_size = end_pos;
+		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
+		mutex_unlock(&ui->ui_mutex);
+		err = inode->i_sb->s_op->write_inode(inode, NULL);
+		ino_released = 1;
+	}
 free_buf:
 	kfree(block_buf);
 free_dn:
 	kfree(dn);
 release_budget:
+	if (appending && !ino_released)
+		ubifs_release_budget(c, &ino_req);
+release_block:
 	ubifs_release_budget(c, &req);
 out:
 	if (!err)
-- 
1.8.4.2

[PATCH v2 33/35] fs: introduce a get_qsize() to file_operations

[Dongsheng Yang]

get_qsize() is used to allow inode to decide how much of the quota size
in itself. If file system implements the own get_qsize(), ioctl of
FIOQSIZe will call it to get quota size. If not implemented, get the
quota size in a generic way.

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ioctl.c         | 31 ++++++++++++++++++++++++-------
 include/linux/fs.h |  1 +
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 5d01d26..2c567db 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -545,6 +545,29 @@ static int ioctl_fsthaw(struct file *filp)
 	return thaw_super(sb);
 }
 
+static int ioctl_fioqsize(struct file *filp, int __user *argp)
+{
+	struct inode *inode = file_inode(filp);
+	loff_t res = 0;
+	int error = 0;
+
+	if (filp->f_op->get_qsize) {
+		res = filp->f_op->get_qsize(inode);
+		error = copy_to_user(argp, &res, sizeof(res)) ?
+				-EFAULT : 0;
+	} else {
+                if (S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) ||
+                    S_ISLNK(inode->i_mode)) {
+                        res = inode_get_bytes(inode);
+                        error = copy_to_user(argp, &res, sizeof(res)) ?
+                                        -EFAULT : 0;
+                } else {
+                        error = -ENOTTY;
+		}
+	}
+	return error;
+}
+
 /*
  * When you add any new common ioctls to the switches above and below
  * please update compat_sys_ioctl() too.
@@ -577,13 +600,7 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
 		break;
 
 	case FIOQSIZE:
-		if (S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) ||
-		    S_ISLNK(inode->i_mode)) {
-			loff_t res = inode_get_bytes(inode);
-			error = copy_to_user(argp, &res, sizeof(res)) ?
-					-EFAULT : 0;
-		} else
-			error = -ENOTTY;
+		error = ioctl_fioqsize(filp, argp);
 		break;
 
 	case FIFREEZE:
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 860b235..70695d3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1601,6 +1601,7 @@ struct file_operations {
 	long (*fallocate)(struct file *file, int mode, loff_t offset,
 			  loff_t len);
 	void (*show_fdinfo)(struct seq_file *m, struct file *f);
+	ssize_t (*get_qsize) (struct inode *);
 #ifndef CONFIG_MMU
 	unsigned (*mmap_capabilities)(struct file *);
 #endif
-- 
1.8.4.2

Re: [PATCH v2 33/35] fs: introduce a get_qsize() to file_operations

[Jan Kara]

On Thu 30-07-15 13:48:29, Dongsheng Yang wrote:
> get_qsize() is used to allow inode to decide how much of the quota size
> in itself. If file system implements the own get_qsize(), ioctl of
> FIOQSIZe will call it to get quota size. If not implemented, get the
> quota size in a generic way.

OK, can you ellaborate a bit why using i_blocks / i_bytes isn't good
enough for ubifs?

> 
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> ---
>  fs/ioctl.c         | 31 ++++++++++++++++++++++++-------
>  include/linux/fs.h |  1 +
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 5d01d26..2c567db 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -545,6 +545,29 @@ static int ioctl_fsthaw(struct file *filp)
>  	return thaw_super(sb);
>  }
>  
> +static int ioctl_fioqsize(struct file *filp, int __user *argp)
> +{
> +	struct inode *inode = file_inode(filp);
> +	loff_t res = 0;
> +	int error = 0;
> +
> +	if (filp->f_op->get_qsize) {

Using file->f_ops looks wrong to me. I think it would be more naturally an
inode operation.

								Honza

> +		res = filp->f_op->get_qsize(inode);
> +		error = copy_to_user(argp, &res, sizeof(res)) ?
> +				-EFAULT : 0;
> +	} else {
> +                if (S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) ||
> +                    S_ISLNK(inode->i_mode)) {
> +                        res = inode_get_bytes(inode);
> +                        error = copy_to_user(argp, &res, sizeof(res)) ?
> +                                        -EFAULT : 0;
> +                } else {
> +                        error = -ENOTTY;
> +		}
> +	}
> +	return error;
> +}
> +
>  /*
>   * When you add any new common ioctls to the switches above and below
>   * please update compat_sys_ioctl() too.
> @@ -577,13 +600,7 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
>  		break;
>  
>  	case FIOQSIZE:
> -		if (S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) ||
> -		    S_ISLNK(inode->i_mode)) {
> -			loff_t res = inode_get_bytes(inode);
> -			error = copy_to_user(argp, &res, sizeof(res)) ?
> -					-EFAULT : 0;
> -		} else
> -			error = -ENOTTY;
> +		error = ioctl_fioqsize(filp, argp);
>  		break;
>  
>  	case FIFREEZE:
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 860b235..70695d3 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1601,6 +1601,7 @@ struct file_operations {
>  	long (*fallocate)(struct file *file, int mode, loff_t offset,
>  			  loff_t len);
>  	void (*show_fdinfo)(struct seq_file *m, struct file *f);
> +	ssize_t (*get_qsize) (struct inode *);
>  #ifndef CONFIG_MMU
>  	unsigned (*mmap_capabilities)(struct file *);
>  #endif
> -- 
> 1.8.4.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2 33/35] fs: introduce a get_qsize() to file_operations

[Dongsheng Yang]

On 08/04/2015 04:15 AM, Jan Kara wrote:
> On Thu 30-07-15 13:48:29, Dongsheng Yang wrote:
>> get_qsize() is used to allow inode to decide how much of the quota size
>> in itself. If file system implements the own get_qsize(), ioctl of
>> FIOQSIZe will call it to get quota size. If not implemented, get the
>> quota size in a generic way.
>
> OK, can you ellaborate a bit why using i_blocks / i_bytes isn't good
> enough for ubifs?

Sure, below is the comment in ubifs_getattr(), I think it can make
it clear.
"because UBIFS does not have notion of "block". For example, it is
difficult to tell how many block a directory takes - it actually takes
less than 300 bytes, but we have to round it to block size, which
introduces large mistake. This makes utilities like 'du' to report
completely senseless numbers. This is the reason why UBIFS goes the
same way as JFFS2 - it reports zero blocks for everything but regular
files, which makes more sense than reporting completely wrong sizes."

So I followed the same reason here for quota_size. Because ubifs
is designed for raw flash, which have no "block". I hope I expressed
myself clearly.
>
>>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
>> ---
>>   fs/ioctl.c         | 31 ++++++++++++++++++++++++-------
>>   include/linux/fs.h |  1 +
>>   2 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/ioctl.c b/fs/ioctl.c
>> index 5d01d26..2c567db 100644
>> --- a/fs/ioctl.c
>> +++ b/fs/ioctl.c
>> @@ -545,6 +545,29 @@ static int ioctl_fsthaw(struct file *filp)
>>   	return thaw_super(sb);
>>   }
>>
>> +static int ioctl_fioqsize(struct file *filp, int __user *argp)
>> +{
>> +	struct inode *inode = file_inode(filp);
>> +	loff_t res = 0;
>> +	int error = 0;
>> +
>> +	if (filp->f_op->get_qsize) {
>
> Using file->f_ops looks wrong to me. I think it would be more naturally an
> inode operation.

Oh, sure. That should be inode operations. will update it in next version.

Thanx
Yang
>
> 								Honza
>
>> +		res = filp->f_op->get_qsize(inode);
>> +		error = copy_to_user(argp, &res, sizeof(res)) ?
>> +				-EFAULT : 0;
>> +	} else {
>> +                if (S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) ||
>> +                    S_ISLNK(inode->i_mode)) {
>> +                        res = inode_get_bytes(inode);
>> +                        error = copy_to_user(argp, &res, sizeof(res)) ?
>> +                                        -EFAULT : 0;
>> +                } else {
>> +                        error = -ENOTTY;
>> +		}
>> +	}
>> +	return error;
>> +}
>> +
>>   /*
>>    * When you add any new common ioctls to the switches above and below
>>    * please update compat_sys_ioctl() too.
>> @@ -577,13 +600,7 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
>>   		break;
>>
>>   	case FIOQSIZE:
>> -		if (S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode) ||
>> -		    S_ISLNK(inode->i_mode)) {
>> -			loff_t res = inode_get_bytes(inode);
>> -			error = copy_to_user(argp, &res, sizeof(res)) ?
>> -					-EFAULT : 0;
>> -		} else
>> -			error = -ENOTTY;
>> +		error = ioctl_fioqsize(filp, argp);
>>   		break;
>>
>>   	case FIFREEZE:
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 860b235..70695d3 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1601,6 +1601,7 @@ struct file_operations {
>>   	long (*fallocate)(struct file *file, int mode, loff_t offset,
>>   			  loff_t len);
>>   	void (*show_fdinfo)(struct seq_file *m, struct file *f);
>> +	ssize_t (*get_qsize) (struct inode *);
>>   #ifndef CONFIG_MMU
>>   	unsigned (*mmap_capabilities)(struct file *);
>>   #endif
>> --
>> 1.8.4.2
>>

[PATCH v2 34/35] ubifs: implement ubifs_get_qsize to get quota size in ubifs

[Dongsheng Yang]

We only care the size of regular file in ubifs for quota.
The reason is similar with the comment in ubifs_getattr().

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/dir.c   |  3 +++
 fs/ubifs/file.c  | 19 +++++++++++++++++++
 fs/ubifs/ubifs.h |  1 +
 3 files changed, 23 insertions(+)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index d430bb6..4230818 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1214,6 +1214,9 @@ const struct file_operations ubifs_dir_operations = {
 	.iterate        = ubifs_readdir,
 	.fsync          = ubifs_fsync,
 	.unlocked_ioctl = ubifs_ioctl,
+#ifdef CONFIG_QUOTA
+	.get_qsize	= ubifs_get_qsize,
+#endif
 #ifdef CONFIG_COMPAT
 	.compat_ioctl   = ubifs_compat_ioctl,
 #endif
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index c4651a8..e916306 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1640,6 +1640,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma)
 	return 0;
 }
 
+/*
+ * ubifs_get_qsize: get the quota size of a file
+ * @inode: inode which we are going to get the qsize
+ *
+ * We only care the size of regular file in ubifs
+ * for quota. The reason is similar with the comment
+ * in ubifs_getattr().
+ */
+ssize_t ubifs_get_qsize(struct inode *inode)
+{
+	if (S_ISREG(inode->i_mode))
+		return i_size_read(inode);
+	else
+		return 0;
+}
+
 const struct address_space_operations ubifs_file_address_operations = {
 	.readpage       = ubifs_readpage,
 	.writepage      = ubifs_writepage,
@@ -1685,6 +1701,9 @@ const struct file_operations ubifs_file_operations = {
 	.unlocked_ioctl = ubifs_ioctl,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
+#ifdef CONFIG_QUOTA
+	.get_qsize	= ubifs_get_qsize,
+#endif
 #ifdef CONFIG_COMPAT
 	.compat_ioctl   = ubifs_compat_ioctl,
 #endif
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 3b5a204..5e4ed2a 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1761,6 +1761,7 @@ int ubifs_read_block(struct inode *inode, void *addr, unsigned int block,
 int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync);
 int ubifs_setattr(struct dentry *dentry, struct iattr *attr);
 int ubifs_update_time(struct inode *inode, struct timespec *time, int flags);
+ssize_t ubifs_get_qsize(struct inode *inode);
 
 /* dir.c */
 struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
-- 
1.8.4.2

[PATCH v2 35/35] ubifs: make ubifs to support quota

[Dongsheng Yang]

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 fs/ubifs/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index acc5d2f..466f8e1 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2349,6 +2349,11 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op = &ubifs_super_operations;
 	sb->s_xattr = ubifs_xattr_handlers;
 
+#ifdef CONFIG_QUOTA
+	sb->dq_op = &dquot_operations;
+	sb->s_qcop = &dquot_quotactl_ops;
+	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
+#endif
 	mutex_lock(&c->umount_mutex);
 	err = mount_ubifs(c);
 	if (err) {
-- 
1.8.4.2