[FOR INTERNAL REVIEW] [PATCH 1/4] staging: erofs: add the missing break in z_erofs_map_blocks_iter

[FOR INTERNAL REVIEW] [PATCH 1/4] staging: erofs: add the missing break in z_erofs_map_blocks_iter

[Gao Xiang]

This patch adds a missing break after adding the default case.

Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
---

- Tested with images generated by mkfs.erofs:
   1) mount and unmount operations
   2) md5sum `find . -type f`

 drivers/staging/erofs/unzip_vle.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index bd2d7a8..6d3ab31 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1596,10 +1596,10 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 	cluster_type = vle_cluster_type(di);
 
 	switch (cluster_type) {
-	case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
+	case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:	/* fallthrough */
 		if (ofs_rem >= logical_cluster_ofs)
 			map->m_flags ^= EROFS_MAP_ZIPPED;
-	case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
+	case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:	/* fallthrough */
 		if (ofs_rem == logical_cluster_ofs) {
 			pcn = le32_to_cpu(di->di_u.blkaddr);
 			goto exact_hitted;
@@ -1624,6 +1624,7 @@ int z_erofs_map_blocks_iter(struct inode *inode,
 		ofs = vle_get_logical_extent_head(inode, mpage_ret,
 			&kaddr, lcn, &pcn, &map->m_flags);
 		mpage = *mpage_ret;
+		break;
 	default:
 		errln("unknown cluster type %u at offset %llu of nid %llu",
 			cluster_type, ofs, EROFS_V(inode)->nid);
-- 
1.9.1

[FOR INTERNAL REVIEW] [PATCH 2/4] staging: erofs: fix superblock/inode flags (MS_RDONLY -> SB_RDONLY, S_NOATIME)

[Gao Xiang]

After commit 0a43a939c77e ("vfs: Suppress MS_* flag defs within
the kernel unless explicitly enabled"), there is no MS_RDONLY
and MS_NOATIME at all.

Reported-by: Stephen Rothwell <sfr at canb.auug.org.au>
Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
---

- Tested with images generated by mkfs.erofs:
   1) mount and unmount operations
   2) md5sum `find . -type f`
 
 drivers/staging/erofs/inode.c | 6 ++++++
 drivers/staging/erofs/super.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index fbf6ff2..809cbd0 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -242,6 +242,12 @@ struct inode *erofs_iget(struct super_block *sb,
 	if (inode->i_state & I_NEW) {
 		int err;
 		struct erofs_vnode *vi = EROFS_V(inode);
+
+		/*
+		 * no need to use 'inode_set_flags'
+		 * (see fuse_iget in fs/fuse/inode.c)
+		 */
+		inode->i_flags |= S_NOATIME;
 		vi->nid = nid;
 
 		err = fill_inode(inode, isdir);
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 1aec509..ae1eb20 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -340,7 +340,7 @@ static int erofs_read_super(struct super_block *sb,
 		goto err_sbread;
 
 	sb->s_magic = EROFS_SUPER_MAGIC;
-	sb->s_flags |= MS_RDONLY | MS_NOATIME;
+	sb->s_flags |= SB_RDONLY;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_time_gran = 1;
 
@@ -627,7 +627,7 @@ static int erofs_remount(struct super_block *sb, int *flags, char *data)
 {
 	BUG_ON(!sb_rdonly(sb));
 
-	*flags |= MS_RDONLY;
+	*flags |= SB_RDONLY;
 	return 0;
 }
 
-- 
1.9.1

[FOR INTERNAL REVIEW] [PATCH 3/4] staging: erofs: remove RADIX_TREE_EXCEPTIONAL_{ENTRY, SHIFT}

[Gao Xiang]

After commit 49520d317715 ("xarray: Replace exceptional entries"),
there are no RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT} at all, remove them
as a workaround but erofs actually needs an extra bit for each entry.

In the future, it should be converted to XArray of course.

Cc: Matthew Wilcox <willy at infradead.org>
Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
---

- Tested with images generated by mkfs.erofs:
   1) mount and unmount operations
   2) md5sum `find . -type f`

 drivers/staging/erofs/utils.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index 595cf90..25ff39f 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -47,9 +47,7 @@ struct erofs_workgroup *erofs_find_workgroup(
 	rcu_read_lock();
 	grp = radix_tree_lookup(&sbi->workstn_tree, index);
 	if (grp != NULL) {
-		*tag = radix_tree_exceptional_entry(grp);
-		grp = (void *)((unsigned long)grp &
-			~RADIX_TREE_EXCEPTIONAL_ENTRY);
+		*tag = 0;
 
 		if (erofs_workgroup_get(grp, &oldcount)) {
 			/* prefer to relax rcu read side */
@@ -68,7 +66,7 @@ struct erofs_workgroup *erofs_find_workgroup(
 
 int erofs_register_workgroup(struct super_block *sb,
 			     struct erofs_workgroup *grp,
-			     bool tag)
+			     bool __maybe_unused tag)
 {
 	struct erofs_sb_info *sbi;
 	int err;
@@ -76,6 +74,11 @@ int erofs_register_workgroup(struct super_block *sb,
 	/* grp->refcount should not < 1 */
 	BUG_ON(!atomic_read(&grp->refcount));
 
+	if (tag) {
+		DBG_BUGON(1);
+		return -EINVAL;
+	}
+
 	err = radix_tree_preload(GFP_NOFS);
 	if (err)
 		return err;
@@ -83,10 +86,6 @@ int erofs_register_workgroup(struct super_block *sb,
 	sbi = EROFS_SB(sb);
 	erofs_workstn_lock(sbi);
 
-	if (tag)
-		grp = (void *)((unsigned long)grp |
-			1UL << RADIX_TREE_EXCEPTIONAL_SHIFT);
-
 	err = radix_tree_insert(&sbi->workstn_tree,
 		grp->index, grp);
 
@@ -131,9 +130,7 @@ unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi,
 
 	for (i = 0; i < found; ++i) {
 		int cnt;
-		struct erofs_workgroup *grp = (void *)
-			((unsigned long)batch[i] &
-				~RADIX_TREE_EXCEPTIONAL_ENTRY);
+		struct erofs_workgroup *grp = batch[i];
 
 		first_index = grp->index + 1;
 
-- 
1.9.1

[FOR INTERNAL REVIEW] [PATCH 4/4] staging: erofs: update .mount and .remount_sb

[Gao Xiang]

This patch updates .mount and .remount_sb after
commit 286c6b145729 ("vfs: Require specification
of size of mount data for internal mounts").

Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
---

- Tested with images generated by mkfs.erofs:
   1) mount and unmount operations
   2) md5sum `find . -type f`

 drivers/staging/erofs/internal.h |  1 -
 drivers/staging/erofs/super.c    | 61 +++++++++++-----------------------------
 2 files changed, 16 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 367b39f..9074a56 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -111,7 +111,6 @@ struct erofs_sb_info {
 
 	u8 uuid[16];                    /* 128-bit uuid for volume */
 	u8 volume_name[16];             /* volume name */
-	char *dev_name;
 
 	unsigned int mount_opt;
 	unsigned int shrinker_run_no;
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index ae1eb20..190a538 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -313,14 +313,18 @@ static struct inode *erofs_init_managed_cache(struct super_block *sb)
 
 #endif
 
-static int erofs_read_super(struct super_block *sb,
-	const char *dev_name, void *data, int silent)
+static int erofs_read_super(struct super_block *sb, void *data,
+			    size_t data_size, int silent)
 {
 	struct inode *inode;
 	struct erofs_sb_info *sbi;
+	char b[BDEVNAME_SIZE];
 	int err = -EINVAL;
 
-	infoln("read_super, device -> %s", dev_name);
+	/* get the block device name */
+	bdevname(sb->s_bdev, b);
+
+	infoln("read_super, device -> %s", b);
 	infoln("options -> %s", (char *)data);
 
 	if (unlikely(!sb_set_blocksize(sb, EROFS_BLKSIZ))) {
@@ -392,16 +396,6 @@ static int erofs_read_super(struct super_block *sb,
 		goto err_makeroot;
 	}
 
-	/* save the device name to sbi */
-	sbi->dev_name = __getname();
-	if (sbi->dev_name == NULL) {
-		err = -ENOMEM;
-		goto err_devname;
-	}
-
-	snprintf(sbi->dev_name, PATH_MAX, "%s", dev_name);
-	sbi->dev_name[PATH_MAX - 1] = '\0';
-
 	erofs_register_super(sb);
 
 	/*
@@ -411,15 +405,13 @@ static int erofs_read_super(struct super_block *sb,
 	d_rehash(sb->s_root);
 
 	if (!silent)
-		infoln("mounted on %s with opts: %s.", dev_name,
-			(char *)data);
+		infoln("mounted on %s with opts: %s.", b, (char *)data);
 	return 0;
 	/*
 	 * please add a label for each exit point and use
 	 * the following name convention, thus new features
 	 * can be integrated easily without renaming labels.
 	 */
-err_devname:
 	dput(sb->s_root);
 err_makeroot:
 err_isdir:
@@ -445,6 +437,7 @@ static int erofs_read_super(struct super_block *sb,
 static void erofs_put_super(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	char b[BDEVNAME_SIZE];
 
 	/* for cases which are failed in "read_super" */
 	if (sbi == NULL)
@@ -452,8 +445,7 @@ static void erofs_put_super(struct super_block *sb)
 
 	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
 
-	infoln("unmounted for %s", sbi->dev_name);
-	__putname(sbi->dev_name);
+	infoln("unmounted for %s", bdevname(sb->s_bdev, b));
 
 #ifdef EROFS_FS_HAS_MANAGED_CACHE
 	iput(sbi->managed_cache);
@@ -472,33 +464,11 @@ static void erofs_put_super(struct super_block *sb)
 	sb->s_fs_info = NULL;
 }
 
-
-struct erofs_mount_private {
-	const char *dev_name;
-	char *options;
-};
-
-/* support mount_bdev() with options */
-static int erofs_fill_super(struct super_block *sb,
-	void *_priv, int silent)
-{
-	struct erofs_mount_private *priv = _priv;
-
-	return erofs_read_super(sb, priv->dev_name,
-		priv->options, silent);
-}
-
-static struct dentry *erofs_mount(
-	struct file_system_type *fs_type, int flags,
-	const char *dev_name, void *data)
+static struct dentry *erofs_mount(struct file_system_type *fs_type, int flags,
+	const char *dev_name, void *data, size_t data_size)
 {
-	struct erofs_mount_private priv = {
-		.dev_name = dev_name,
-		.options = data
-	};
-
-	return mount_bdev(fs_type, flags, dev_name,
-		&priv, erofs_fill_super);
+	return mount_bdev(fs_type, flags, dev_name, data, data_size,
+		erofs_read_super);
 }
 
 static void erofs_kill_sb(struct super_block *sb)
@@ -623,7 +593,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 	return 0;
 }
 
-static int erofs_remount(struct super_block *sb, int *flags, char *data)
+static int erofs_remount(struct super_block *sb, int *flags,
+	char *data, size_t data_size)
 {
 	BUG_ON(!sb_rdonly(sb));
 
-- 
1.9.1

[FOR INTERNAL REVIEW] [PATCH RESEND 4/4] staging: erofs: update .mount and .remount_sb

[Gao Xiang]

This patch updates .mount and .remount_sb after
commit 286c6b145729 ("vfs: Require specification
of size of mount data for internal mounts").

Reported-by: Stephen Rothwell <sfr at canb.auug.org.au>
Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
---

- Tested with images generated by mkfs.erofs:
   1) mount and unmount operations
   2) md5sum `find . -type f`

 drivers/staging/erofs/internal.h |  1 -
 drivers/staging/erofs/super.c    | 61 +++++++++++-----------------------------
 2 files changed, 16 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 367b39f..9074a56 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -111,7 +111,6 @@ struct erofs_sb_info {
 
 	u8 uuid[16];                    /* 128-bit uuid for volume */
 	u8 volume_name[16];             /* volume name */
-	char *dev_name;
 
 	unsigned int mount_opt;
 	unsigned int shrinker_run_no;
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index ae1eb20..190a538 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -313,14 +313,18 @@ static struct inode *erofs_init_managed_cache(struct super_block *sb)
 
 #endif
 
-static int erofs_read_super(struct super_block *sb,
-	const char *dev_name, void *data, int silent)
+static int erofs_read_super(struct super_block *sb, void *data,
+			    size_t data_size, int silent)
 {
 	struct inode *inode;
 	struct erofs_sb_info *sbi;
+	char b[BDEVNAME_SIZE];
 	int err = -EINVAL;
 
-	infoln("read_super, device -> %s", dev_name);
+	/* get the block device name */
+	bdevname(sb->s_bdev, b);
+
+	infoln("read_super, device -> %s", b);
 	infoln("options -> %s", (char *)data);
 
 	if (unlikely(!sb_set_blocksize(sb, EROFS_BLKSIZ))) {
@@ -392,16 +396,6 @@ static int erofs_read_super(struct super_block *sb,
 		goto err_makeroot;
 	}
 
-	/* save the device name to sbi */
-	sbi->dev_name = __getname();
-	if (sbi->dev_name == NULL) {
-		err = -ENOMEM;
-		goto err_devname;
-	}
-
-	snprintf(sbi->dev_name, PATH_MAX, "%s", dev_name);
-	sbi->dev_name[PATH_MAX - 1] = '\0';
-
 	erofs_register_super(sb);
 
 	/*
@@ -411,15 +405,13 @@ static int erofs_read_super(struct super_block *sb,
 	d_rehash(sb->s_root);
 
 	if (!silent)
-		infoln("mounted on %s with opts: %s.", dev_name,
-			(char *)data);
+		infoln("mounted on %s with opts: %s.", b, (char *)data);
 	return 0;
 	/*
 	 * please add a label for each exit point and use
 	 * the following name convention, thus new features
 	 * can be integrated easily without renaming labels.
 	 */
-err_devname:
 	dput(sb->s_root);
 err_makeroot:
 err_isdir:
@@ -445,6 +437,7 @@ static int erofs_read_super(struct super_block *sb,
 static void erofs_put_super(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	char b[BDEVNAME_SIZE];
 
 	/* for cases which are failed in "read_super" */
 	if (sbi == NULL)
@@ -452,8 +445,7 @@ static void erofs_put_super(struct super_block *sb)
 
 	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
 
-	infoln("unmounted for %s", sbi->dev_name);
-	__putname(sbi->dev_name);
+	infoln("unmounted for %s", bdevname(sb->s_bdev, b));
 
 #ifdef EROFS_FS_HAS_MANAGED_CACHE
 	iput(sbi->managed_cache);
@@ -472,33 +464,11 @@ static void erofs_put_super(struct super_block *sb)
 	sb->s_fs_info = NULL;
 }
 
-
-struct erofs_mount_private {
-	const char *dev_name;
-	char *options;
-};
-
-/* support mount_bdev() with options */
-static int erofs_fill_super(struct super_block *sb,
-	void *_priv, int silent)
-{
-	struct erofs_mount_private *priv = _priv;
-
-	return erofs_read_super(sb, priv->dev_name,
-		priv->options, silent);
-}
-
-static struct dentry *erofs_mount(
-	struct file_system_type *fs_type, int flags,
-	const char *dev_name, void *data)
+static struct dentry *erofs_mount(struct file_system_type *fs_type, int flags,
+	const char *dev_name, void *data, size_t data_size)
 {
-	struct erofs_mount_private priv = {
-		.dev_name = dev_name,
-		.options = data
-	};
-
-	return mount_bdev(fs_type, flags, dev_name,
-		&priv, erofs_fill_super);
+	return mount_bdev(fs_type, flags, dev_name, data, data_size,
+		erofs_read_super);
 }
 
 static void erofs_kill_sb(struct super_block *sb)
@@ -623,7 +593,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
 	return 0;
 }
 
-static int erofs_remount(struct super_block *sb, int *flags, char *data)
+static int erofs_remount(struct super_block *sb, int *flags,
+	char *data, size_t data_size)
 {
 	BUG_ON(!sb_rdonly(sb));
 
-- 
1.9.1

[FOR INTERNAL REVIEW] [PATCH 3/4] staging: erofs: remove RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT}

[Matthew Wilcox]

This review looks pretty external now ...

On Mon, Jul 30, 2018@05:18:30PM +0800, Gao Xiang wrote:
> After commit 49520d317715 ("xarray: Replace exceptional entries"),
> there are no RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT} at all, remove them
> as a workaround but erofs actually needs an extra bit for each entry.

This patch confuses me.  You say erofs needs an extra bit, but apparently
it works well with the bit removed.  What, then, is the point of the bit?

You can use bit 0 if you need to distinguish between two types of pointers.
Indeed, you can distinguish between three different types of pointers
-- x & 3 == { 0, 1, 3 }.

> In the future, it should be converted to XArray of course.
> 
> Cc: Matthew Wilcox <willy at infradead.org>
> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
> ---
> 
> - Tested with images generated by mkfs.erofs:
>    1) mount and unmount operations
>    2) md5sum `find . -type f`
> 
>  drivers/staging/erofs/utils.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
> index 595cf90..25ff39f 100644
> --- a/drivers/staging/erofs/utils.c
> +++ b/drivers/staging/erofs/utils.c
> @@ -47,9 +47,7 @@ struct erofs_workgroup *erofs_find_workgroup(
>  	rcu_read_lock();
>  	grp = radix_tree_lookup(&sbi->workstn_tree, index);
>  	if (grp != NULL) {
> -		*tag = radix_tree_exceptional_entry(grp);
> -		grp = (void *)((unsigned long)grp &
> -			~RADIX_TREE_EXCEPTIONAL_ENTRY);
> +		*tag = 0;
>  
>  		if (erofs_workgroup_get(grp, &oldcount)) {
>  			/* prefer to relax rcu read side */
> @@ -68,7 +66,7 @@ struct erofs_workgroup *erofs_find_workgroup(
>  
>  int erofs_register_workgroup(struct super_block *sb,
>  			     struct erofs_workgroup *grp,
> -			     bool tag)
> +			     bool __maybe_unused tag)
>  {
>  	struct erofs_sb_info *sbi;
>  	int err;
> @@ -76,6 +74,11 @@ int erofs_register_workgroup(struct super_block *sb,
>  	/* grp->refcount should not < 1 */
>  	BUG_ON(!atomic_read(&grp->refcount));
>  
> +	if (tag) {
> +		DBG_BUGON(1);
> +		return -EINVAL;
> +	}
> +
>  	err = radix_tree_preload(GFP_NOFS);
>  	if (err)
>  		return err;
> @@ -83,10 +86,6 @@ int erofs_register_workgroup(struct super_block *sb,
>  	sbi = EROFS_SB(sb);
>  	erofs_workstn_lock(sbi);
>  
> -	if (tag)
> -		grp = (void *)((unsigned long)grp |
> -			1UL << RADIX_TREE_EXCEPTIONAL_SHIFT);
> -
>  	err = radix_tree_insert(&sbi->workstn_tree,
>  		grp->index, grp);
>  
> @@ -131,9 +130,7 @@ unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi,
>  
>  	for (i = 0; i < found; ++i) {
>  		int cnt;
> -		struct erofs_workgroup *grp = (void *)
> -			((unsigned long)batch[i] &
> -				~RADIX_TREE_EXCEPTIONAL_ENTRY);
> +		struct erofs_workgroup *grp = batch[i];
>  
>  		first_index = grp->index + 1;
>  
> -- 
> 1.9.1
> 

[FOR INTERNAL REVIEW] [PATCH 3/4] staging: erofs: remove RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT}

[Gao Xiang]

Hi Matthew,

On 2018/7/30 21:19, Matthew Wilcox wrote:
> 
> This review looks pretty external now ...
> 

:-'( Internal means currently in the linux-erofs mailing list for preview.
I haven't sent to the linux-kernel mailing list yet..

> On Mon, Jul 30, 2018@05:18:30PM +0800, Gao Xiang wrote:
>> After commit 49520d317715 ("xarray: Replace exceptional entries"),
>> there are no RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT} at all, remove them
>> as a workaround but erofs actually needs an extra bit for each entry.
> 
> This patch confuses me.  You say erofs needs an extra bit, but apparently
> it works well with the bit removed.  What, then, is the point of the bit?
> 
The previous version of erofs used these bits for differentiating (un)cached entry...
Link: https://lists.ozlabs.org/pipermail/linux-erofs/2018-July/000093.html

It has no use currently, but I plan to reserve an extra bit for the future
VLE/non-VLE hybrid decompression...

> You can use bit 0 if you need to distinguish between two types of pointers.
> Indeed, you can distinguish between three different types of pointers
> -- x & 3 == { 0, 1, 3 }.

Thanks, I will look into the radix tree implementation and have a try.
But if XArray is merged into 4.19, I will also try to convert into the XArray...

Thanks,
Gao Xiang

> 
>> In the future, it should be converted to XArray of course.
>>
>> Cc: Matthew Wilcox <willy at infradead.org>
>> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
>> ---
>>
>> - Tested with images generated by mkfs.erofs:
>>    1) mount and unmount operations
>>    2) md5sum `find . -type f`
>>
>>  drivers/staging/erofs/utils.c | 19 ++++++++-----------
>>  1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
>> index 595cf90..25ff39f 100644
>> --- a/drivers/staging/erofs/utils.c
>> +++ b/drivers/staging/erofs/utils.c
>> @@ -47,9 +47,7 @@ struct erofs_workgroup *erofs_find_workgroup(
>>  	rcu_read_lock();
>>  	grp = radix_tree_lookup(&sbi->workstn_tree, index);
>>  	if (grp != NULL) {
>> -		*tag = radix_tree_exceptional_entry(grp);
>> -		grp = (void *)((unsigned long)grp &
>> -			~RADIX_TREE_EXCEPTIONAL_ENTRY);
>> +		*tag = 0;
>>  
>>  		if (erofs_workgroup_get(grp, &oldcount)) {
>>  			/* prefer to relax rcu read side */
>> @@ -68,7 +66,7 @@ struct erofs_workgroup *erofs_find_workgroup(
>>  
>>  int erofs_register_workgroup(struct super_block *sb,
>>  			     struct erofs_workgroup *grp,
>> -			     bool tag)
>> +			     bool __maybe_unused tag)
>>  {
>>  	struct erofs_sb_info *sbi;
>>  	int err;
>> @@ -76,6 +74,11 @@ int erofs_register_workgroup(struct super_block *sb,
>>  	/* grp->refcount should not < 1 */
>>  	BUG_ON(!atomic_read(&grp->refcount));
>>  
>> +	if (tag) {
>> +		DBG_BUGON(1);
>> +		return -EINVAL;
>> +	}
>> +
>>  	err = radix_tree_preload(GFP_NOFS);
>>  	if (err)
>>  		return err;
>> @@ -83,10 +86,6 @@ int erofs_register_workgroup(struct super_block *sb,
>>  	sbi = EROFS_SB(sb);
>>  	erofs_workstn_lock(sbi);
>>  
>> -	if (tag)
>> -		grp = (void *)((unsigned long)grp |
>> -			1UL << RADIX_TREE_EXCEPTIONAL_SHIFT);
>> -
>>  	err = radix_tree_insert(&sbi->workstn_tree,
>>  		grp->index, grp);
>>  
>> @@ -131,9 +130,7 @@ unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi,
>>  
>>  	for (i = 0; i < found; ++i) {
>>  		int cnt;
>> -		struct erofs_workgroup *grp = (void *)
>> -			((unsigned long)batch[i] &
>> -				~RADIX_TREE_EXCEPTIONAL_ENTRY);
>> +		struct erofs_workgroup *grp = batch[i];
>>  
>>  		first_index = grp->index + 1;
>>  
>> -- 
>> 1.9.1
>>

[FOR INTERNAL REVIEW] [PATCH 1/4] staging: erofs: add the missing break in z_erofs_map_blocks_iter

[Chao Yu]

On 2018/7/30 17:18, Gao Xiang wrote:
> This patch adds a missing break after adding the default case.
> 
> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

[FOR INTERNAL REVIEW] [PATCH 2/4] staging: erofs: fix superblock/inode flags (MS_RDONLY -> SB_RDONLY, S_NOATIME)

[Chao Yu]

Hi David,

staging/erofs was merged by Greg the other day, later, it went into
staging-next, yesterday, Stephen Rothwell reported us an compile failure in
linux-next tree, the reason is there were some common vfs stuff changes
including 0a43a939c77e ("vfs: Suppress MS_* flag defs within the kernel unless
explicitly enabled"), but erofs do not adjust those changes. We'd like to fix
the integration compiling error, Xiang Gao has made a patch for MS_* related
change, could you please help to review this patch?

On 2018/7/30 17:18, Gao Xiang wrote:
> After commit 0a43a939c77e ("vfs: Suppress MS_* flag defs within
> the kernel unless explicitly enabled"), there is no MS_RDONLY
> and MS_NOATIME at all.
> 
> Reported-by: Stephen Rothwell <sfr at canb.auug.org.au>
> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
> ---
> 
> - Tested with images generated by mkfs.erofs:
>    1) mount and unmount operations
>    2) md5sum `find . -type f`
>  
>  drivers/staging/erofs/inode.c | 6 ++++++
>  drivers/staging/erofs/super.c | 4 ++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index fbf6ff2..809cbd0 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -242,6 +242,12 @@ struct inode *erofs_iget(struct super_block *sb,
>  	if (inode->i_state & I_NEW) {
>  		int err;
>  		struct erofs_vnode *vi = EROFS_V(inode);
> +
> +		/*
> +		 * no need to use 'inode_set_flags'
> +		 * (see fuse_iget in fs/fuse/inode.c)
> +		 */
> +		inode->i_flags |= S_NOATIME;
>  		vi->nid = nid;
>  
>  		err = fill_inode(inode, isdir);
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 1aec509..ae1eb20 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -340,7 +340,7 @@ static int erofs_read_super(struct super_block *sb,
>  		goto err_sbread;
>  
>  	sb->s_magic = EROFS_SUPER_MAGIC;
> -	sb->s_flags |= MS_RDONLY | MS_NOATIME;
> +	sb->s_flags |= SB_RDONLY;
>  	sb->s_maxbytes = MAX_LFS_FILESIZE;
>  	sb->s_time_gran = 1;
>  
> @@ -627,7 +627,7 @@ static int erofs_remount(struct super_block *sb, int *flags, char *data)
>  {
>  	BUG_ON(!sb_rdonly(sb));
>  
> -	*flags |= MS_RDONLY;
> +	*flags |= SB_RDONLY;
>  	return 0;
>  }
>  
> 

[FOR INTERNAL REVIEW] [PATCH 2/4] staging: erofs: fix superblock/inode flags (MS_RDONLY -> SB_RDONLY, S_NOATIME)

[Chao Yu]

On 2018/7/30 17:18, Gao Xiang wrote:
> After commit 0a43a939c77e ("vfs: Suppress MS_* flag defs within
> the kernel unless explicitly enabled"), there is no MS_RDONLY
> and MS_NOATIME at all.
> 
> Reported-by: Stephen Rothwell <sfr at canb.auug.org.au>
> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

[FOR INTERNAL REVIEW] [PATCH 3/4] staging: erofs: remove RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT}

[Chao Yu]

On 2018/7/30 17:18, Gao Xiang wrote:
> After commit 49520d317715 ("xarray: Replace exceptional entries"),
> there are no RADIX_TREE_EXCEPTIONAL_{ENTRY,SHIFT} at all, remove them
> as a workaround but erofs actually needs an extra bit for each entry.
> 
> In the future, it should be converted to XArray of course.
> 
> Cc: Matthew Wilcox <willy at infradead.org>
> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>

Reviewed-by: Chao Yu <yuchao0 at huawei.com>

Thanks,

[FOR INTERNAL REVIEW] [PATCH 4/4] staging: erofs: update .mount and .remount_sb

[Chao Yu]

Hi David,

Could you please help to check this patch as well?

Thanks,

On 2018/7/30 17:18, Gao Xiang wrote:
> This patch updates .mount and .remount_sb after
> commit 286c6b145729 ("vfs: Require specification
> of size of mount data for internal mounts").
> 
> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
> ---
> 
> - Tested with images generated by mkfs.erofs:
>    1) mount and unmount operations
>    2) md5sum `find . -type f`
> 
>  drivers/staging/erofs/internal.h |  1 -
>  drivers/staging/erofs/super.c    | 61 +++++++++++-----------------------------
>  2 files changed, 16 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index 367b39f..9074a56 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -111,7 +111,6 @@ struct erofs_sb_info {
>  
>  	u8 uuid[16];                    /* 128-bit uuid for volume */
>  	u8 volume_name[16];             /* volume name */
> -	char *dev_name;
>  
>  	unsigned int mount_opt;
>  	unsigned int shrinker_run_no;
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index ae1eb20..190a538 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -313,14 +313,18 @@ static struct inode *erofs_init_managed_cache(struct super_block *sb)
>  
>  #endif
>  
> -static int erofs_read_super(struct super_block *sb,
> -	const char *dev_name, void *data, int silent)
> +static int erofs_read_super(struct super_block *sb, void *data,
> +			    size_t data_size, int silent)
>  {
>  	struct inode *inode;
>  	struct erofs_sb_info *sbi;
> +	char b[BDEVNAME_SIZE];
>  	int err = -EINVAL;
>  
> -	infoln("read_super, device -> %s", dev_name);
> +	/* get the block device name */
> +	bdevname(sb->s_bdev, b);
> +
> +	infoln("read_super, device -> %s", b);
>  	infoln("options -> %s", (char *)data);
>  
>  	if (unlikely(!sb_set_blocksize(sb, EROFS_BLKSIZ))) {
> @@ -392,16 +396,6 @@ static int erofs_read_super(struct super_block *sb,
>  		goto err_makeroot;
>  	}
>  
> -	/* save the device name to sbi */
> -	sbi->dev_name = __getname();
> -	if (sbi->dev_name == NULL) {
> -		err = -ENOMEM;
> -		goto err_devname;
> -	}
> -
> -	snprintf(sbi->dev_name, PATH_MAX, "%s", dev_name);
> -	sbi->dev_name[PATH_MAX - 1] = '\0';
> -
>  	erofs_register_super(sb);
>  
>  	/*
> @@ -411,15 +405,13 @@ static int erofs_read_super(struct super_block *sb,
>  	d_rehash(sb->s_root);
>  
>  	if (!silent)
> -		infoln("mounted on %s with opts: %s.", dev_name,
> -			(char *)data);
> +		infoln("mounted on %s with opts: %s.", b, (char *)data);
>  	return 0;
>  	/*
>  	 * please add a label for each exit point and use
>  	 * the following name convention, thus new features
>  	 * can be integrated easily without renaming labels.
>  	 */
> -err_devname:
>  	dput(sb->s_root);
>  err_makeroot:
>  err_isdir:
> @@ -445,6 +437,7 @@ static int erofs_read_super(struct super_block *sb,
>  static void erofs_put_super(struct super_block *sb)
>  {
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	char b[BDEVNAME_SIZE];
>  
>  	/* for cases which are failed in "read_super" */
>  	if (sbi == NULL)
> @@ -452,8 +445,7 @@ static void erofs_put_super(struct super_block *sb)
>  
>  	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
>  
> -	infoln("unmounted for %s", sbi->dev_name);
> -	__putname(sbi->dev_name);
> +	infoln("unmounted for %s", bdevname(sb->s_bdev, b));
>  
>  #ifdef EROFS_FS_HAS_MANAGED_CACHE
>  	iput(sbi->managed_cache);
> @@ -472,33 +464,11 @@ static void erofs_put_super(struct super_block *sb)
>  	sb->s_fs_info = NULL;
>  }
>  
> -
> -struct erofs_mount_private {
> -	const char *dev_name;
> -	char *options;
> -};
> -
> -/* support mount_bdev() with options */
> -static int erofs_fill_super(struct super_block *sb,
> -	void *_priv, int silent)
> -{
> -	struct erofs_mount_private *priv = _priv;
> -
> -	return erofs_read_super(sb, priv->dev_name,
> -		priv->options, silent);
> -}
> -
> -static struct dentry *erofs_mount(
> -	struct file_system_type *fs_type, int flags,
> -	const char *dev_name, void *data)
> +static struct dentry *erofs_mount(struct file_system_type *fs_type, int flags,
> +	const char *dev_name, void *data, size_t data_size)
>  {
> -	struct erofs_mount_private priv = {
> -		.dev_name = dev_name,
> -		.options = data
> -	};
> -
> -	return mount_bdev(fs_type, flags, dev_name,
> -		&priv, erofs_fill_super);
> +	return mount_bdev(fs_type, flags, dev_name, data, data_size,
> +		erofs_read_super);
>  }
>  
>  static void erofs_kill_sb(struct super_block *sb)
> @@ -623,7 +593,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>  	return 0;
>  }
>  
> -static int erofs_remount(struct super_block *sb, int *flags, char *data)
> +static int erofs_remount(struct super_block *sb, int *flags,
> +	char *data, size_t data_size)
>  {
>  	BUG_ON(!sb_rdonly(sb));
>  
> 

[FOR INTERNAL REVIEW] [PATCH 4/4] staging: erofs: update .mount and .remount_sb

[Gao Xiang]

Hi David,

On 2018/7/31 15:38, Chao Yu wrote:
> Hi David,
> 
> Could you please help to check this patch as well?
> 

If you don't mind seeing more code, the patched code of super/inode.c can also be accessed at

https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/tree/drivers/staging/erofs/super.c?h=erofs-dev
https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/tree/drivers/staging/erofs/inode.c?h=erofs-dev

It will be of great help...Thank you..

Thanks,
Gao Xiang

> Thanks,
> 
> On 2018/7/30 17:18, Gao Xiang wrote:
>> This patch updates .mount and .remount_sb after
>> commit 286c6b145729 ("vfs: Require specification
>> of size of mount data for internal mounts").
>>
>> Signed-off-by: Gao Xiang <gaoxiang25 at huawei.com>
>> ---
>>
>> - Tested with images generated by mkfs.erofs:
>>    1) mount and unmount operations
>>    2) md5sum `find . -type f`
>>
>>  drivers/staging/erofs/internal.h |  1 -
>>  drivers/staging/erofs/super.c    | 61 +++++++++++-----------------------------
>>  2 files changed, 16 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
>> index 367b39f..9074a56 100644
>> --- a/drivers/staging/erofs/internal.h
>> +++ b/drivers/staging/erofs/internal.h
>> @@ -111,7 +111,6 @@ struct erofs_sb_info {
>>  
>>  	u8 uuid[16];                    /* 128-bit uuid for volume */
>>  	u8 volume_name[16];             /* volume name */
>> -	char *dev_name;
>>  
>>  	unsigned int mount_opt;
>>  	unsigned int shrinker_run_no;
>> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
>> index ae1eb20..190a538 100644
>> --- a/drivers/staging/erofs/super.c
>> +++ b/drivers/staging/erofs/super.c
>> @@ -313,14 +313,18 @@ static struct inode *erofs_init_managed_cache(struct super_block *sb)
>>  
>>  #endif
>>  
>> -static int erofs_read_super(struct super_block *sb,
>> -	const char *dev_name, void *data, int silent)
>> +static int erofs_read_super(struct super_block *sb, void *data,
>> +			    size_t data_size, int silent)
>>  {
>>  	struct inode *inode;
>>  	struct erofs_sb_info *sbi;
>> +	char b[BDEVNAME_SIZE];
>>  	int err = -EINVAL;
>>  
>> -	infoln("read_super, device -> %s", dev_name);
>> +	/* get the block device name */
>> +	bdevname(sb->s_bdev, b);
>> +
>> +	infoln("read_super, device -> %s", b);
>>  	infoln("options -> %s", (char *)data);
>>  
>>  	if (unlikely(!sb_set_blocksize(sb, EROFS_BLKSIZ))) {
>> @@ -392,16 +396,6 @@ static int erofs_read_super(struct super_block *sb,
>>  		goto err_makeroot;
>>  	}
>>  
>> -	/* save the device name to sbi */
>> -	sbi->dev_name = __getname();
>> -	if (sbi->dev_name == NULL) {
>> -		err = -ENOMEM;
>> -		goto err_devname;
>> -	}
>> -
>> -	snprintf(sbi->dev_name, PATH_MAX, "%s", dev_name);
>> -	sbi->dev_name[PATH_MAX - 1] = '\0';
>> -
>>  	erofs_register_super(sb);
>>  
>>  	/*
>> @@ -411,15 +405,13 @@ static int erofs_read_super(struct super_block *sb,
>>  	d_rehash(sb->s_root);
>>  
>>  	if (!silent)
>> -		infoln("mounted on %s with opts: %s.", dev_name,
>> -			(char *)data);
>> +		infoln("mounted on %s with opts: %s.", b, (char *)data);
>>  	return 0;
>>  	/*
>>  	 * please add a label for each exit point and use
>>  	 * the following name convention, thus new features
>>  	 * can be integrated easily without renaming labels.
>>  	 */
>> -err_devname:
>>  	dput(sb->s_root);
>>  err_makeroot:
>>  err_isdir:
>> @@ -445,6 +437,7 @@ static int erofs_read_super(struct super_block *sb,
>>  static void erofs_put_super(struct super_block *sb)
>>  {
>>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>> +	char b[BDEVNAME_SIZE];
>>  
>>  	/* for cases which are failed in "read_super" */
>>  	if (sbi == NULL)
>> @@ -452,8 +445,7 @@ static void erofs_put_super(struct super_block *sb)
>>  
>>  	WARN_ON(sb->s_magic != EROFS_SUPER_MAGIC);
>>  
>> -	infoln("unmounted for %s", sbi->dev_name);
>> -	__putname(sbi->dev_name);
>> +	infoln("unmounted for %s", bdevname(sb->s_bdev, b));
>>  
>>  #ifdef EROFS_FS_HAS_MANAGED_CACHE
>>  	iput(sbi->managed_cache);
>> @@ -472,33 +464,11 @@ static void erofs_put_super(struct super_block *sb)
>>  	sb->s_fs_info = NULL;
>>  }
>>  
>> -
>> -struct erofs_mount_private {
>> -	const char *dev_name;
>> -	char *options;
>> -};
>> -
>> -/* support mount_bdev() with options */
>> -static int erofs_fill_super(struct super_block *sb,
>> -	void *_priv, int silent)
>> -{
>> -	struct erofs_mount_private *priv = _priv;
>> -
>> -	return erofs_read_super(sb, priv->dev_name,
>> -		priv->options, silent);
>> -}
>> -
>> -static struct dentry *erofs_mount(
>> -	struct file_system_type *fs_type, int flags,
>> -	const char *dev_name, void *data)
>> +static struct dentry *erofs_mount(struct file_system_type *fs_type, int flags,
>> +	const char *dev_name, void *data, size_t data_size)
>>  {
>> -	struct erofs_mount_private priv = {
>> -		.dev_name = dev_name,
>> -		.options = data
>> -	};
>> -
>> -	return mount_bdev(fs_type, flags, dev_name,
>> -		&priv, erofs_fill_super);
>> +	return mount_bdev(fs_type, flags, dev_name, data, data_size,
>> +		erofs_read_super);
>>  }
>>  
>>  static void erofs_kill_sb(struct super_block *sb)
>> @@ -623,7 +593,8 @@ static int erofs_show_options(struct seq_file *seq, struct dentry *root)
>>  	return 0;
>>  }
>>  
>> -static int erofs_remount(struct super_block *sb, int *flags, char *data)
>> +static int erofs_remount(struct super_block *sb, int *flags,
>> +	char *data, size_t data_size)
>>  {
>>  	BUG_ON(!sb_rdonly(sb));
>>  
>>
>