[PATCH] kernel/power: cast PAGE_SIZE to int when comparing with error code

[Chengguang Xu]

If PAGE_SIZE is unsigned type then negative error code will be
larger than PAGE_SIZE.

Signed-off-by: Chengguang Xu 
---
 kernel/power/swap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index c2bcf97d24c8..d7f6c1a288d3 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -923,7 +923,7 @@ int swsusp_write(unsigned int flags)
}
memset(&snapshot, 0, sizeof(struct snapshot_handle));
error = snapshot_read_next(&snapshot);
-   if (error < PAGE_SIZE) {
+   if (error < (int)PAGE_SIZE) {
if (error >= 0)
error = -EFAULT;
 
@@ -1483,7 +1483,7 @@ int swsusp_read(unsigned int *flags_p)
 
memset(&snapshot, 0, sizeof(struct snapshot_handle));
error = snapshot_write_next(&snapshot);
-   if (error < PAGE_SIZE)
+   if (error < (int)PAGE_SIZE)
return error < 0 ? error : -EFAULT;
header = (struct swsusp_info *)data_of(snapshot);
error = get_swap_reader(&handle, flags_p);
-- 
2.17.1

[PATCH] mm: adjust max read count in generic_file_buffered_read()

[Chengguang Xu]

When we try to truncate read count in generic_file_buffered_read(),
should deliver (sb->s_maxbytes - offset) as maximum count not
sb->s_maxbytes itself.

Signed-off-by: Chengguang Xu 
---
 mm/filemap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 52517f28e6f4..5c2d481d21cf 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2064,7 +2064,7 @@ static ssize_t generic_file_buffered_read(struct kiocb 
*iocb,
 
if (unlikely(*ppos >= inode->i_sb->s_maxbytes))
return 0;
-   iov_iter_truncate(iter, inode->i_sb->s_maxbytes);
+   iov_iter_truncate(iter, inode->i_sb->s_maxbytes - *ppos);
 
index = *ppos >> PAGE_SHIFT;
prev_index = ra->prev_pos >> PAGE_SHIFT;
-- 
2.17.1

Re: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file status

[Chengguang Xu]

Hi Yuanliang,

Can you explain how to reproduce it? and what filesystem do you use as 
underlying fs?


> Sent: Tuesday, March 20, 2018 at 2:03 PM
> From: 王元良 
> To: mik...@szeredi.hu
> Cc: linux-unio...@vger.kernel.org, linux-kernel@vger.kernel.org, 王元良 
> 
> Subject: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file 
> status
>
> When the dentry cache is not destroyed, statfs will
> take unknown status '?'
> Undestroyed cache dentry also caused new file creating fails
> 
> Drop it using d_delete may be a choice
> 
> [root@host /]
> Loaded plugins: bestyumcache, branch, fastestmirror, langpacks
> Repodata is over 2 weeks old. Install yum-cron? Or run: yum makecache
> fast
> Cleaning repos: xxx.base.x86_64 ops.x.noarch ops.x.x86_64
> Cleaning up everything
> Cleaning up list of best yumcache
> Cleaning up list of fastest mirrors
> 
> [root@host /]
> Loaded plugins: bestyumcache, branch, fastestmirror, langpacks
> Cleaning repos: xxx.base.x86_64 ops.x.noarch ops.x.x86_64
> Cleaning up everything
> Cleaning up list of best yumcache
> Traceback (most recent call last):
> File "/usr/bin/yum", line 29, in 
> yummain.user_main(sys.argv[1:], exit_code=True)
> File "/usr/share/yum-cli/yummain.py", line 365, in user_main
> errcode = main(args)
> File "/usr/share/yum-cli/yummain.py", line 174, in main
> result, resultmsgs = base.doCommands()
> File "/usr/share/yum-cli/cli.py", line 573, in doCommands
> return self.yum_cli_commands[self.basecmd].doCommand(self,
>   self.basecmd, self.extcmds)
>  File "/usr/share/yum-cli/yumcommands.py", line
> 1475, in doCommand
> return base.cleanCli(extcmds)
> File "/usr/share/yum-cli/cli.py", line 1666, in
> cleanCli
> self.plugins.run('clean')
> File
> "/usr/lib/python2.7/site-packages/yum/plugins.py", line 188, in run
> func(conduitcls(self, self.base, conf, **kwargs))
> File "/usr/lib/yum-plugins/branch.py", line 65, in clean_hook
> os.remove(os.path.join(root,filespath))
> OSError: [Errno 2] No such file or directory:
> '/var/cache/yum/x86_64/x/.noarch.stable/
>b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2'
> 
> [root@host /]
> ls: cannot access
> /var/cache/yum/x86_64/x/.noarch.stable/
> b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2:
> No such file or directory
> ls: cannot access
> /var/cache/yum/x86_64/x/.noarch.stable/cachecookie:
> No such file or directory
> ls: cannot access
> /var/cache/yum/x86_64/x/.noarch.stable/repomd.xml:
> No such file or directory
> total 0
> ?? ? ?? ??
> b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2
> ?? ? ?? ??
> cachecookie
> drwxr-xr-x 1 root root 30 Mar 15 16:12 gen
> drwxr-xr-x 2 root root  6 Jan 31 13:13
> packages
> ?? ? ?? ??
> repomd.xml
> 
> Signed-off-by: Yuanliang Wang 
> ---
>  fs/overlayfs/super.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index e7c8ac4..f9343d3 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -380,9 +380,15 @@ static int ovl_dentry_weak_revalidate(struct dentry 
> *dentry, unsigned int flags)
>   return ret;
>  }
>  
> +static int ovl_dentry_delete(const struct dentry *dentry)
> +{
> + return 1;
> +}
> +
>  static const struct dentry_operations ovl_dentry_operations = {
>   .d_release = ovl_dentry_release,
>   .d_real = ovl_d_real,
> + .d_delete = ovl_dentry_delete,
>  };
>  
>  static const struct dentry_operations ovl_reval_dentry_operations = {
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file status

[Chengguang Xu]

Hi Yuanliang,

Can you try ext4 or xfs(with ftype=1) as underlying fs? 
It seems the phenomenon that I met on xfs(with ftype=0) as underlying fs.
Thanks,
Chengguang.


> Sent: Tuesday, March 20, 2018 at 2:44 PM
> From: "Chengguang Xu" 
> To: 王元良 
> Cc: mik...@szeredi.hu, linux-unio...@vger.kernel.org, 
> linux-kernel@vger.kernel.org, 王元良 
> Subject: Re: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file 
> status
>
> Hi Yuanliang,
> 
> Can you explain how to reproduce it? and what filesystem do you use as 
> underlying fs?
> 
> 
> > Sent: Tuesday, March 20, 2018 at 2:03 PM
> > From: 王元良 
> > To: mik...@szeredi.hu
> > Cc: linux-unio...@vger.kernel.org, linux-kernel@vger.kernel.org, 王元良 
> > 
> > Subject: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file 
> > status
> >
> > When the dentry cache is not destroyed, statfs will
> > take unknown status '?'
> > Undestroyed cache dentry also caused new file creating fails
> > 
> > Drop it using d_delete may be a choice
> > 
> > [root@host /]
> > Loaded plugins: bestyumcache, branch, fastestmirror, langpacks
> > Repodata is over 2 weeks old. Install yum-cron? Or run: yum makecache
> > fast
> > Cleaning repos: xxx.base.x86_64 ops.x.noarch ops.x.x86_64
> > Cleaning up everything
> > Cleaning up list of best yumcache
> > Cleaning up list of fastest mirrors
> > 
> > [root@host /]
> > Loaded plugins: bestyumcache, branch, fastestmirror, langpacks
> > Cleaning repos: xxx.base.x86_64 ops.x.noarch ops.x.x86_64
> > Cleaning up everything
> > Cleaning up list of best yumcache
> > Traceback (most recent call last):
> > File "/usr/bin/yum", line 29, in 
> > yummain.user_main(sys.argv[1:], exit_code=True)
> > File "/usr/share/yum-cli/yummain.py", line 365, in user_main
> > errcode = main(args)
> > File "/usr/share/yum-cli/yummain.py", line 174, in main
> > result, resultmsgs = base.doCommands()
> > File "/usr/share/yum-cli/cli.py", line 573, in doCommands
> > return self.yum_cli_commands[self.basecmd].doCommand(self,
> > self.basecmd, self.extcmds)
> >File "/usr/share/yum-cli/yumcommands.py", line
> > 1475, in doCommand
> > return base.cleanCli(extcmds)
> > File "/usr/share/yum-cli/cli.py", line 1666, in
> > cleanCli
> > self.plugins.run('clean')
> > File
> > "/usr/lib/python2.7/site-packages/yum/plugins.py", line 188, in run
> > func(conduitcls(self, self.base, conf, **kwargs))
> > File "/usr/lib/yum-plugins/branch.py", line 65, in clean_hook
> > os.remove(os.path.join(root,filespath))
> > OSError: [Errno 2] No such file or directory:
> >   '/var/cache/yum/x86_64/x/.noarch.stable/
> >  b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2'
> > 
> > [root@host /]
> > ls: cannot access
> > /var/cache/yum/x86_64/x/.noarch.stable/
> > b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2:
> > No such file or directory
> > ls: cannot access
> > /var/cache/yum/x86_64/x/.noarch.stable/cachecookie:
> > No such file or directory
> > ls: cannot access
> > /var/cache/yum/x86_64/x/.noarch.stable/repomd.xml:
> > No such file or directory
> > total 0
> > ?? ? ?? ??
> > b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2
> > ?? ? ?? ??
> > cachecookie
> > drwxr-xr-x 1 root root 30 Mar 15 16:12 gen
> > drwxr-xr-x 2 root root  6 Jan 31 13:13
> > packages
> > ?? ? ?? ??
> > repomd.xml
> > 
> > Signed-off-by: Yuanliang Wang 
> > ---
> >  fs/overlayfs/super.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index e7c8ac4..f9343d3 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -380,9 +380,15 @@ static int ovl_dentry_weak_revalidate(struct dentry 
> > *dentry, unsigned int flags)
> > return ret;
> >  }
> >  
> > +static int ovl_dentry_delete(const struct dentry *dentry)
> > +{
> > +   return 1;
> > +}
> > +
> >  static const struct dentry_operations ovl_dentry_operations = {
> > .d_release = ovl_dentry_release,
> > .d_real = ovl_d_real,
> > +   .d_delete = ovl_dentry_delete,
> >  };
> >  
> >  static const struct dentry_operations ovl_reval_dentry_operations = {
> > -- 
> > 1.8.3.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

(Resend) Re: 回复：[PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file status

[Chengguang Xu]

Sorry, resend email because of sending failure.
If you have already received, please ignore it.

Hi Yuanliang,
 
If you use ext4, you can try below command to check if filetype
feature has already enabled.
 
tune2fs -l /dev/xxx | grep feature
 
If filetype feature is not enabled, please enable it first and
check if the problem still persist.
 
The phenomenon I met is similar to yours in the previous email.
 
Thanks,
Chengguang.
 
-

Sent: Tuesday, March 20, 2018 at 3:22 PM
From: 王元良 
To: "Chengguang Xu" , "Chengguang Xu" 
Cc: miklos , linux-unionfs , 
linux-kernel 
Subject: 回复：[PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file 
status

Hi, Chengguang
 
is the ftype for xfs only?
 
Could you describe in detail the problems that you have encountered on your 
side and give more on-site information may help?
 
----------
发件人：Chengguang Xu 
发送时间：2018年3月20日(星期二) 15:07
收件人：Chengguang Xu 
抄　送：王元良(尉犁) ; miklos ; 
linux-unionfs ; linux-kernel 

主　题：Re: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file status
 Hi Yuanliang,

Can you try ext4 or xfs(with ftype=1) as underlying fs? 
It seems the phenomenon that I met on xfs(with ftype=0) as underlying fs.
Thanks,
Chengguang.


> Sent: Tuesday, March 20, 2018 at 2:44 PM
> From: "Chengguang Xu" 
> To: 王元良 
> Cc: mik...@szeredi.hu, linux-unio...@vger.kernel.org, 
>linux-kernel@vger.kernel.org, 王元良 
> Subject: Re: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file 
>status
>
> Hi Yuanliang,
> 
> Can you explain how to reproduce it? and what filesystem do you use as 
>underlying fs?
> 
> 
> > Sent: Tuesday, March 20, 2018 at 2:03 PM
> > From: 王元良 
> > To: mik...@szeredi.hu
> > Cc: linux-unio...@vger.kernel.org, linux-kernel@vger.kernel.org, 王元良 
>
> > Subject: [PATCH] fs/overlayfs: Drop dentry cache to prevent unknown file 
>status
> >
> > When the dentry cache is not destroyed, statfs will
> > take unknown status '?'
> > Undestroyed cache dentry also caused new file creating fails
> > 
> > Drop it using d_delete may be a choice
> > 
> > [root@host /]
> > Loaded plugins: bestyumcache, branch, fastestmirror, langpacks
> > Repodata is over 2 weeks old. Install yum-cron? Or run: yum makecache
> > fast
> > Cleaning repos: xxx.base.x86_64 ops.x.noarch ops.x.x86_64
> > Cleaning up everything
> > Cleaning up list of best yumcache
> > Cleaning up list of fastest mirrors
> > 
> > [root@host /]
> > Loaded plugins: bestyumcache, branch, fastestmirror, langpacks
> > Cleaning repos: xxx.base.x86_64 ops.x.noarch ops.x.x86_64
> > Cleaning up everything
> > Cleaning up list of best yumcache
> > Traceback (most recent call last):
> > File "/usr/bin/yum", line 29, in 
> > yummain.user_main(sys.argv[1:], exit_code=True)
> > File "/usr/share/yum-cli/yummain.py", line 365, in user_main
> > errcode = main(args)
> > File "/usr/share/yum-cli/yummain.py", line 174, in main
> > result, resultmsgs = base.doCommands()
> > File "/usr/share/yum-cli/cli.py", line 573, in doCommands
> > return self.yum_cli_commands[self.basecmd].doCommand(self,
> >  self.basecmd, self.extcmds)
> > File "/usr/share/yum-cli/yumcommands.py", line
> > 1475, in doCommand
> > return base.cleanCli(extcmds)
> > File "/usr/share/yum-cli/cli.py", line 1666, in
> > cleanCli
> > self.plugins.run('clean')
> > File
> > "/usr/lib/python2.7/site-packages/yum/plugins.py", line 188, in run
> > func(conduitcls(self, self.base, conf, **kwargs))
> > File "/usr/lib/yum-plugins/branch.py", line 65, in clean_hook
> > os.remove(os.path.join(root,filespath))
> > OSError: [Errno 2] No such file or directory:
> >'/var/cache/yum/x86_64/x/.noarch.stable/
> >   b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2'
> > 
> > [root@host /]
> > ls: cannot access
> > /var/cache/yum/x86_64/x/.noarch.stable/
> > b49e49b87d17818d799363091c7a01dde83b421d-primary.sqlite.bz2:
> > No such file or directory
> > ls: cannot access
> > /var/cache/yum/x86_64/x/.noarch.stable/cachecookie:
> > No such file or directory
> > ls: cannot access
> > /var/cache/yum/x86_64/x/.noarch.stable/repomd.xml:
> > No such file or directory
> > total 0
> > ?? ? ?? ??
> > b49e49b87d17818d799363091c7a01dde83b421d-primary.

[PATCH] 9p: check memory allocation result for cachetag

[Chengguang Xu]

Check memory allocation result for cachetag in mount option
parsing and fix potential memory leak in the error case.

Signed-off-by: Chengguang Xu 
---
 fs/9p/v9fs.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 8fb89dd..e622f0f 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -292,6 +292,10 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
 #ifdef CONFIG_9P_FSCACHE
kfree(v9ses->cachetag);
v9ses->cachetag = match_strdup(&args[0]);
+   if (!v9ses->cachetag) {
+   ret = -ENOMEM;
+   goto free_and_return;
+   }
 #endif
break;
case Opt_cache:
@@ -471,6 +475,9 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info 
*v9ses,
return fid;
 
 err_clnt:
+#ifdef CONFIG_9P_FSCACHE
+   kfree(v9ses->cachetag);
+#endif
p9_client_destroy(v9ses->clnt);
 err_names:
kfree(v9ses->uname);
-- 
1.8.3.1

[RESEND PATCH] x86: remove redundant check for kmem_cache_create()

[Chengguang Xu]

The flag 'SLAB_PANIC' implies panic when encouter failure,
so there is no need to check NULL pointer and return error
code.

Signed-off-by: Chengguang Xu 
---
 arch/x86/mm/pgtable.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 47b5951..4392414 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -312,14 +312,14 @@ static void pgd_prepopulate_pmd(struct mm_struct *mm, 
pgd_t *pgd, pmd_t *pmds[])
 
 static struct kmem_cache *pgd_cache;
 
-static int __init pgd_cache_init(void)
+static void __init pgd_cache_init(void)
 {
/*
 * When PAE kernel is running as a Xen domain, it does not use
 * shared kernel pmd. And this requires a whole page for pgd.
 */
if (!SHARED_KERNEL_PMD)
-   return 0;
+   return;
 
/*
 * when PAE kernel is not running as a Xen domain, it uses
@@ -329,10 +329,6 @@ static int __init pgd_cache_init(void)
 */
pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_ALIGN,
  SLAB_PANIC, NULL);
-   if (!pgd_cache)
-   return -ENOMEM;
-
-   return 0;
 }
 core_initcall(pgd_cache_init);
 
-- 
1.8.3.1

[PATCH v2] x86: remove redundant check for kmem_cache_create()

[Chengguang Xu]

The flag 'SLAB_PANIC' implies panic when encouter failure,
So there is no need to check NULL pointer for cache creation.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Keep return type as int instead of changing to void.

 arch/x86/mm/pgtable.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 47b5951..406873d 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -329,9 +329,6 @@ static int __init pgd_cache_init(void)
 */
pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_ALIGN,
  SLAB_PANIC, NULL);
-   if (!pgd_cache)
-   return -ENOMEM;
-
return 0;
 }
 core_initcall(pgd_cache_init);
-- 
1.8.3.1

[PATCH 2/2] fs/exofs: only use true/false for asignment of bool type variable

[Chengguang Xu]

Signed-off-by: Chengguang Xu 
---
 fs/exofs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index f3e17a9..75b5db0 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -118,7 +118,7 @@ static int parse_options(char *options, struct 
exofs_mountopt *opts)
  EXOFS_MIN_PID);
return -EINVAL;
}
-   s_pid = 1;
+   s_pid = true;
break;
case Opt_to:
if (match_int(&args[0], &option))
-- 
1.8.3.1

[PATCH 1/2] fs/exofs: fix potential memory leak in mount option parsing

[Chengguang Xu]

There are some cases can cause memory leak when parsing
option 'osdname'.

Signed-off-by: Chengguang Xu 
---
 fs/exofs/super.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 719a315..f3e17a9 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -101,6 +101,7 @@ static int parse_options(char *options, struct 
exofs_mountopt *opts)
token = match_token(p, tokens, args);
switch (token) {
case Opt_name:
+   kfree(opts->dev_name);
opts->dev_name = match_strdup(&args[0]);
if (unlikely(!opts->dev_name)) {
EXOFS_ERR("Error allocating dev_name");
@@ -867,8 +868,10 @@ static struct dentry *exofs_mount(struct file_system_type 
*type,
int ret;
 
ret = parse_options(data, &opts);
-   if (ret)
+   if (ret) {
+   kfree(opts.dev_name);
return ERR_PTR(ret);
+   }
 
if (!opts.dev_name)
opts.dev_name = dev_name;
-- 
1.8.3.1

[RESEND PATCH] chardev: set variable ret to -EBUSY before checking minor range overlap

[Chengguang Xu]

When allocating dynamic major, the minor range overlap check
in __register_chrdev_region() will not fail, so actually
there is no real case to passing non negative error code to
caller. However, set variable ret to -EBUSY before chekcking
minor range overlap will avoid false-positive warning from
code analyzing tool(like Smatch) and also make the code more
easy to understand.

Suggested-by: Dan Carpenter 
Signed-off-by: Chengguang Xu 
---
 fs/char_dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index d18cad28c1c3..00dfe17871ac 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -98,7 +98,7 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
   int minorct, const char *name)
 {
struct char_device_struct *cd, *curr, *prev = NULL;
-   int ret = -EBUSY;
+   int ret;
int i;

if (major >= CHRDEV_MAJOR_MAX) {
@@ -129,6 +129,7 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
major = ret;
}

+   ret = -EBUSY;
i = major_to_index(major);
for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
if (curr->major < major)
--
2.20.1

Re: [V9fs-developer] [RFC PATCH] 9p: create writeback fid on shared mmap

[Chengguang Xu]

  在 星期一, 2020-12-07 19:24:10 Dominique Martinet  撰写 

 > Chengguang Xu wrote on Mon, Dec 07, 2020:
 > >  > , VM_MAYWRITE is set anytime we have a shared map where file has
 > >  > been opened read-write, which seems to be what you want with regards to
 > >  > protecting from mprotect calls.
 > >  > 
 > >  > How about simply changing check from WRITE to MAYWRITE?
 > > 
 > > It would be fine and based on the code in do_mmap(), it  seems we even 
 > > don't
 > > need extra check here.  The condition (vma->vm_flags & VM_SHARED) will be 
 > > enough.
 > > Am I missing something?
 > 
 > VM_MAYWRITE is unset if the file hasn't been open for writing (in which
 > case the mapping can't be mprotect()ed to writable map), so checking it
 > is a bit more efficient.
 > 
 > Anyway I'd like to obsolete the writeback fid uses now that fids have a
 > refcount (this usecase can be a simple refcount increase), in which case
 > efficiency is less of a problem, but we're not there yet...
 > 
 > Please resend with MAYWRITE if you want authorship and I'll try to take
 > some time to test incl. the mprotect usecase.
 > 

Thanks for the review, I'll send revised version later.

Thanks,
Chengguang

[V9fs-developer][PATCH v2] 9p: create writeback fid on shared writable mmap

[Chengguang Xu]

If vma is shared and the file was opened for writing,
we should also create writeback fid because vma may be
mprotected writable even if now readonly.

Signed-off-by: Chengguang Xu 
---
 fs/9p/vfs_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index b177fd3b1eb3..e4256dbcb754 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -517,7 +517,7 @@ v9fs_mmap_file_mmap(struct file *filp, struct 
vm_area_struct *vma)
mutex_lock(&v9inode->v_mutex);
if (!v9inode->writeback_fid &&
(vma->vm_flags & VM_SHARED) &&
-   (vma->vm_flags & VM_WRITE)) {
+   (vma->vm_flags & VM_MAYWRITE)) {
/*
 * clone a fid and add it to writeback_fid
 * we do it during mmap instead of
-- 
2.26.2

[RFC PATCH] 9p: create writeback fid on shared mmap

[Chengguang Xu]

If vma is shared and the file was opened for writing,
we should also create writeback fid because vma may be
mprotected writable even if now readonly.

Signed-off-by: Chengguang Xu 
---
Caveat: Only compile tested.

 fs/9p/vfs_file.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index b177fd3b1eb3..791839c2dd5c 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -516,8 +516,7 @@ v9fs_mmap_file_mmap(struct file *filp, struct 
vm_area_struct *vma)
v9inode = V9FS_I(inode);
mutex_lock(&v9inode->v_mutex);
if (!v9inode->writeback_fid &&
-   (vma->vm_flags & VM_SHARED) &&
-   (vma->vm_flags & VM_WRITE)) {
+   mapping_writably_mapped(filp->f_mapping)) {
/*
 * clone a fid and add it to writeback_fid
 * we do it during mmap instead of
-- 
2.26.2

Re: [V9fs-developer] [RFC PATCH] 9p: create writeback fid on shared mmap

[Chengguang Xu]

  在 星期一, 2020-12-07 04:53:18 Dominique Martinet  撰写 

 > Dominique Martinet wrote on Sun, Dec 06, 2020:
 > > Chengguang Xu wrote on Sat, Dec 05, 2020:
 > > > If vma is shared and the file was opened for writing,
 > > > we should also create writeback fid because vma may be
 > > > mprotected writable even if now readonly.
 > > 
 > > Hm, I guess it makes sense.
 > 
 > I had a second look, and generic_file_readonly_mmap uses vma's
 > `vma->vm_flags & VM_MAYWRITE` instead (together with VM_SHARED),
 > while mapping_writably_mapped ultimately basically only seems to
 > validate that the mapping is shared from a look at mapping_map_writable
 > callers? It's not very clear to me.
 > 
 > , VM_MAYWRITE is set anytime we have a shared map where file has
 > been opened read-write, which seems to be what you want with regards to
 > protecting from mprotect calls.
 > 
 > How about simply changing check from WRITE to MAYWRITE?

It would be fine and based on the code in do_mmap(), it  seems we even don't
need extra check here.  The condition (vma->vm_flags & VM_SHARED) will be 
enough.
Am I missing something?

Thanks,
Chengguang

 > 
 >  v9inode = V9FS_I(inode);
 >  mutex_lock(&v9inode->v_mutex);
 >  if (!v9inode->writeback_fid &&
 >  (vma->vm_flags & VM_SHARED) &&
 > -(vma->vm_flags & VM_WRITE)) {
 > +(vma->vm_flags & VM_MAYWRITE)) {
 >  /*
 >   * clone a fid and add it to writeback_fid
 >   * we do it during mmap instead of

[PATCH] mei: expand minor range when registering chrdev region

[Chengguang Xu]

Actually, total amount of available minor number
for a single major is MINORMARK + 1. So expand
minor range when registering chrdev region.

Signed-off-by: Chengguang Xu 
---
 drivers/misc/mei/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index 87281b3695e6..3df54f1e1a8b 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -869,7 +869,7 @@ static const struct file_operations mei_fops = {
 
 static struct class *mei_class;
 static dev_t mei_devt;
-#define MEI_MAX_DEVS  MINORMASK
+#define MEI_MAX_DEVS  (MINORMASK + 1)
 static DEFINE_MUTEX(mei_minor_lock);
 static DEFINE_IDR(mei_idr);
 
-- 
2.20.1

[PATCH 1/2] chardev: fix an overlap misjudgement case in __register_chrdev_region()

[Chengguang Xu]

Current overlap check of minor range cannot correctly
handle a case which is baseminor < existing baseminor &&
baseminor + minorct > existing baseminor + minorct.
Fix it and meanwhile do some code cleanups.

Fixes: 01d553d0fe9f90 ("Chardev checking of overlapping ranges")
Signed-off-by: Chengguang Xu 
Cc: sta...@vger.kernel.org
---
 fs/char_dev.c | 94 ---
 1 file changed, 44 insertions(+), 50 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index a279c58fe360..b25b1da097d5 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -88,86 +88,80 @@ static int find_dynamic_major(void)
 /*
  * Register a single major with a specified minor range.
  *
- * If major == 0 this functions will dynamically allocate a major and return
- * its number.
- *
- * If major > 0 this function will attempt to reserve the passed range of
- * minors and will return zero on success.
+ * If major == 0 this function will dynamically allocate an unused major,
+ * otherwise attempt to reserve the range of minors with given major.
  *
- * Returns a -ve errno on failure.
  */
 static struct char_device_struct *
 __register_chrdev_region(unsigned int major, unsigned int baseminor,
   int minorct, const char *name)
 {
-   struct char_device_struct *cd, **cp;
-   int ret = 0;
+   struct char_device_struct *new, *curr, *prev = NULL;
+   int ret = -EBUSY;
int i;
 
-   cd = kzalloc(sizeof(struct char_device_struct), GFP_KERNEL);
-   if (cd == NULL)
+   if (major >= CHRDEV_MAJOR_MAX) {
+   pr_err("CHRDEV \"%s\" major requested (%u) is greater than the 
maximum (%u)\n",
+  name, major, CHRDEV_MAJOR_MAX-1);
+   return  ERR_PTR(-EINVAL);
+   }
+
+   if (minorct > MINORMASK + 1 - baseminor) {
+   pr_err("CHRDEV \"%s\" minor range requested (%u-%u) is out of 
range of maximum range (%u-%u) for a single major\n",
+  name, baseminor, baseminor + minorct - 1, 0, MINORMASK);
+   return ERR_PTR(-EINVAL);
+   }
+
+   new = kzalloc(sizeof(struct char_device_struct), GFP_KERNEL);
+   if (new == NULL)
return ERR_PTR(-ENOMEM);
 
mutex_lock(&chrdevs_lock);
 
if (major == 0) {
-   ret = find_dynamic_major();
-   if (ret < 0) {
+   major = find_dynamic_major();
+   if (major < 0) {
pr_err("CHRDEV \"%s\" dynamic allocation region is 
full\n",
   name);
goto out;
}
-   major = ret;
}
 
-   if (major >= CHRDEV_MAJOR_MAX) {
-   pr_err("CHRDEV \"%s\" major requested (%u) is greater than the 
maximum (%u)\n",
-  name, major, CHRDEV_MAJOR_MAX-1);
-   ret = -EINVAL;
-   goto out;
-   }
-
-   cd->major = major;
-   cd->baseminor = baseminor;
-   cd->minorct = minorct;
-   strlcpy(cd->name, name, sizeof(cd->name));
-
i = major_to_index(major);
+   for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
+   if (curr->major < major)
+   continue;
 
-   for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
-   if ((*cp)->major > major ||
-   ((*cp)->major == major &&
-(((*cp)->baseminor >= baseminor) ||
- ((*cp)->baseminor + (*cp)->minorct > baseminor
+   if (curr->major > major)
break;
 
-   /* Check for overlapping minor ranges.  */
-   if (*cp && (*cp)->major == major) {
-   int old_min = (*cp)->baseminor;
-   int old_max = (*cp)->baseminor + (*cp)->minorct - 1;
-   int new_min = baseminor;
-   int new_max = baseminor + minorct - 1;
+   if (curr->baseminor + curr->minorct <= baseminor)
+   continue;
 
-   /* New driver overlaps from the left.  */
-   if (new_max >= old_min && new_max <= old_max) {
-   ret = -EBUSY;
-   goto out;
-   }
+   if (curr->baseminor >= baseminor + minorct)
+   break;
 
-   /* New driver overlaps from the right.  */
-   if (new_min <= old_max && new_min >= old_min) {
-   ret = -EBUSY;
-   goto out;
-   }
+   goto out;
+   }
+
+   new->major = major;
+   new->baseminor = baseminor;
+   new->minorct = minorct;
+   strlcpy(new->name, name, si

[PATCH 2/2] chardev: showing minor range for chardev in the output of /proc/devices

[Chengguang Xu]

Currently chardev allows to share major, showing
major with minor range for chardev will be more
helpful.

Signed-off-by: Chengguang Xu 
---
 fs/char_dev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index b25b1da097d5..6f00acdeb308 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -55,7 +55,9 @@ void chrdev_show(struct seq_file *f, off_t offset)
mutex_lock(&chrdevs_lock);
for (cd = chrdevs[major_to_index(offset)]; cd; cd = cd->next) {
if (cd->major == offset)
-   seq_printf(f, "%3d %s\n", cd->major, cd->name);
+   seq_printf(f, "%3d %s (%u-%u)\n", cd->major, cd->name,
+  cd->baseminor,
+  cd->baseminor + cd->minorct - 1);
}
mutex_unlock(&chrdevs_lock);
 }
-- 
2.20.1

[PATCH] quota: code cleanup for __dquot_alloc_space()

[Chengguang Xu]

Replace (flags & DQUOT_SPACE_RESERVE) with
variable reserve.

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index fc20e06c56ba..00de508f9d2e 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1663,7 +1663,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t 
number, int flags)
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
if (!dquots[cnt])
continue;
-   if (flags & DQUOT_SPACE_RESERVE) {
+   if (reserve) {
ret = dquot_add_space(dquots[cnt], 0, number, flags,
  &warn[cnt]);
} else {
@@ -1676,7 +1676,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t 
number, int flags)
if (!dquots[cnt])
continue;
spin_lock(&dquots[cnt]->dq_dqb_lock);
-   if (flags & DQUOT_SPACE_RESERVE) {
+   if (reserve) {
dquots[cnt]->dq_dqb.dqb_rsvspace -=
number;
} else {
--
2.20.1

[PATCH] quota: check quota type in early stage

[Chengguang Xu]

Check quota type in early stage so we can avoid many
unncessary operations when the type is wrong.

Signed-off-by: Chengguang Xu 
---
 fs/quota/quota.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index cb13fb76dbee..5444d3c4d93f 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -60,8 +60,6 @@ static int quota_sync_all(int type)
 {
int ret;
 
-   if (type >= MAXQUOTAS)
-   return -EINVAL;
ret = security_quotactl(Q_SYNC, type, 0, NULL);
if (!ret)
iterate_supers(quota_sync_one, &type);
@@ -686,8 +684,6 @@ static int do_quotactl(struct super_block *sb, int type, 
int cmd, qid_t id,
 {
int ret;
 
-   if (type >= MAXQUOTAS)
-   return -EINVAL;
type = array_index_nospec(type, MAXQUOTAS);
/*
 * Quota not supported on this fs? Check this before s_quota_types
@@ -831,6 +827,9 @@ int kernel_quotactl(unsigned int cmd, const char __user 
*special,
cmds = cmd >> SUBCMDSHIFT;
type = cmd & SUBCMDMASK;
 
+   if (type >= MAXQUOTAS)
+   return -EINVAL;
+
/*
 * As a special case Q_SYNC can be called without a specific device.
 * It will iterate all superblocks that have quota enabled and call
-- 
2.21.0

(RESEND) [PATCH] ocfs2: Fix error handling in ocfs2_setattr()

[Chengguang Xu]

Should set transfer_to[USRQUOTA/GRPQUOTA] to NULL
on error case before jump to do dqput().

Signed-off-by: Chengguang Xu 
---
 fs/ocfs2/file.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 2e982db3e1ae..53939bf9d7d2 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1230,6 +1230,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr 
*attr)
transfer_to[USRQUOTA] = dqget(sb, 
make_kqid_uid(attr->ia_uid));
if (IS_ERR(transfer_to[USRQUOTA])) {
status = PTR_ERR(transfer_to[USRQUOTA]);
+   transfer_to[USRQUOTA] = NULL;
goto bail_unlock;
}
}
@@ -1239,6 +1240,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr 
*attr)
transfer_to[GRPQUOTA] = dqget(sb, 
make_kqid_gid(attr->ia_gid));
if (IS_ERR(transfer_to[GRPQUOTA])) {
status = PTR_ERR(transfer_to[GRPQUOTA]);
+   transfer_to[GRPQUOTA] = NULL;
goto bail_unlock;
}
}
-- 
2.20.1

[PATCH] quota: avoid increasing DQST_LOOKUPS when iterating over dirty/inuse list

[Chengguang Xu]

It is meaningless to increase DQST_LOOKUPS number while iterating
over dirty/inuse list, so just avoid it.

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 6e826b454082..00a3c6df2ea3 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -595,7 +595,6 @@ int dquot_scan_active(struct super_block *sb,
/* Now we have active dquot so we can just increase use count */
atomic_inc(&dquot->dq_count);
spin_unlock(&dq_list_lock);
-   dqstats_inc(DQST_LOOKUPS);
dqput(old_dquot);
old_dquot = dquot;
/*
@@ -649,7 +648,6 @@ int dquot_writeback_dquots(struct super_block *sb, int type)
 * use count */
dqgrab(dquot);
spin_unlock(&dq_list_lock);
-   dqstats_inc(DQST_LOOKUPS);
err = sb->dq_op->write_dquot(dquot);
if (err) {
/*
-- 
2.20.1

[PATCH] quota: code cleanup for hash bits calculation

[Chengguang Xu]

Code cleanup for hash bits calculation by
calling rounddown_pow_of_two() and ilog2()

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 6e826b454082..679dd3b5db70 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2983,13 +2983,9 @@ static int __init dquot_init(void)
 
/* Find power-of-two hlist_heads which can fit into allocation */
nr_hash = (1UL << order) * PAGE_SIZE / sizeof(struct hlist_head);
-   dq_hash_bits = 0;
-   do {
-   dq_hash_bits++;
-   } while (nr_hash >> dq_hash_bits);
-   dq_hash_bits--;
+   nr_hash = rounddown_pow_of_two(nr_hash);
+   dq_hash_bits = ilog2(nr_hash);
 
-   nr_hash = 1UL << dq_hash_bits;
dq_hash_mask = nr_hash - 1;
for (i = 0; i < nr_hash; i++)
INIT_HLIST_HEAD(dquot_hash + i);
-- 
2.21.0

[PATCH v2] quota: code cleanup for hash bits calculation

[Chengguang Xu]

Code cleanup for hash bits calculation by
calling ilog2().

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Calculate hash bits by directly calling ilog2().

 fs/quota/dquot.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 6e826b454082..fde1b94ea587 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2983,11 +2983,7 @@ static int __init dquot_init(void)
 
/* Find power-of-two hlist_heads which can fit into allocation */
nr_hash = (1UL << order) * PAGE_SIZE / sizeof(struct hlist_head);
-   dq_hash_bits = 0;
-   do {
-   dq_hash_bits++;
-   } while (nr_hash >> dq_hash_bits);
-   dq_hash_bits--;
+   dq_hash_bits = ilog2(nr_hash);
 
nr_hash = 1UL << dq_hash_bits;
dq_hash_mask = nr_hash - 1;
-- 
2.21.0

[PATCH] 9p: avoid attaching writeback_fid on mmap with type PRIVATE

[Chengguang Xu]

Currently on mmap cache policy, we always attach writeback_fid
whether mmap type is SHARED or PRIVATE. However, in the use case
of kata-container which combines 9p(Guest OS) with overlayfs(Host OS),
this behavior will trigger overlayfs' copy-up when excute command
inside container.

Signed-off-by: Chengguang Xu 
---
 fs/9p/vfs_file.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index 4cc966a31cb3..fe7f0bd2048e 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -513,6 +513,7 @@ v9fs_mmap_file_mmap(struct file *filp, struct 
vm_area_struct *vma)
v9inode = V9FS_I(inode);
mutex_lock(&v9inode->v_mutex);
if (!v9inode->writeback_fid &&
+   (vma->vm_flags & VM_SHARED) &&
(vma->vm_flags & VM_WRITE)) {
/*
 * clone a fid and add it to writeback_fid
@@ -614,6 +615,8 @@ static void v9fs_mmap_vm_close(struct vm_area_struct *vma)
(vma->vm_end - vma->vm_start - 1),
};
 
+   if (!(vma->vm_flags & VM_SHARED))
+   return;
 
p9_debug(P9_DEBUG_VFS, "9p VMA close, %p, flushing", vma);
 
-- 
2.20.1

[PATCH] quota: minor code cleanup for v1_format_ops

[Chengguang Xu]

It's not a functinal change, it's just for keeping
consistent coding style.

Signed-off-by: Chengguang Xu 
---
 fs/quota/quota_v1.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/quota/quota_v1.c b/fs/quota/quota_v1.c
index c740e5572eb8..cd92e5fa0062 100644
--- a/fs/quota/quota_v1.c
+++ b/fs/quota/quota_v1.c
@@ -217,7 +217,6 @@ static const struct quota_format_ops v1_format_ops = {
.check_quota_file   = v1_check_quota_file,
.read_file_info = v1_read_file_info,
.write_file_info= v1_write_file_info,
-   .free_file_info = NULL,
.read_dqblk = v1_read_dqblk,
.commit_dqblk   = v1_commit_dqblk,
 };
-- 
2.21.0

[PATCH] hugetlbfs: fix error handling in init_hugetlbfs_fs()

[Chengguang Xu]

In order to avoid using incorrect mnt, we should set
mnt to NULL when we get error from mount_one_hugetlbfs().

Signed-off-by: Chengguang Xu 
---
 fs/hugetlbfs/inode.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index a478df035651..427d845e7706 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1470,9 +1470,12 @@ static int __init init_hugetlbfs_fs(void)
i = 0;
for_each_hstate(h) {
mnt = mount_one_hugetlbfs(h);
-   if (IS_ERR(mnt) && i == 0) {
-   error = PTR_ERR(mnt);
-   goto out;
+   if (IS_ERR(mnt)) {
+   if (i == 0) {
+   error = PTR_ERR(mnt);
+   goto out;
+   }
+   mnt = NULL;
}
hugetlbfs_vfsmount[i] = mnt;
i++;
-- 
2.20.1

[PATCH v2 1/4] chardev: add additional check for minor range overlap

[Chengguang Xu]

Current overlap checking cannot correctly handle
a case which is baseminor < existing baseminor &&
baseminor + minorct > existing baseminor + minorct.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index a279c58fe360..8a63cfa29005 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -159,6 +159,12 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
ret = -EBUSY;
goto out;
}
+
+   if (new_min < old_min && new_max > old_max) {
+   ret = -EBUSY;
+   goto out;
+   }
+
}
 
cd->next = *cp;
-- 
2.20.1

[PATCH v2 4/4] chardev: update comment based on the code

[Chengguang Xu]

The function comment of __register_chrdev_region()
is out of date, so update it based on the code.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index bd2bf1fd847e..d18cad28c1c3 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -88,13 +88,10 @@ static int find_dynamic_major(void)
 /*
  * Register a single major with a specified minor range.
  *
- * If major == 0 this functions will dynamically allocate a major and return
- * its number.
- *
- * If major > 0 this function will attempt to reserve the passed range of
- * minors and will return zero on success.
+ * If major == 0 this function will dynamically allocate an unused major.
+ * If major > 0 this function will attempt to reserve the range of minors
+ * with given major.
  *
- * Returns a -ve errno on failure.
  */
 static struct char_device_struct *
 __register_chrdev_region(unsigned int major, unsigned int baseminor,
-- 
2.20.1

[PATCH v2 2/4] chardev: add a check for given minor range

[Chengguang Xu]

register_chrdev_region() carefully checks minor range
before calling __register_chrdev_region() but there is
another path from alloc_chrdev_region() which does not
check the range properly. So add a check for given minor
range in __register_chrdev_region().

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 8a63cfa29005..6803e98414f1 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -104,6 +104,12 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
int ret = 0;
int i;
 
+   if (minorct > MINORMASK + 1 - baseminor) {
+   pr_err("CHRDEV \"%s\" minor range requested (%u-%u) is out of 
range of maximum range (%u-%u) for a single major\n",
+   name, baseminor, baseminor + minorct - 1, 0, MINORMASK);
+   return ERR_PTR(-EINVAL);
+   }
+
cd = kzalloc(sizeof(struct char_device_struct), GFP_KERNEL);
if (cd == NULL)
return ERR_PTR(-ENOMEM);
-- 
2.20.1

[PATCH v2 3/4] chardev: code cleanup for __register_chrdev_region()

[Chengguang Xu]

It's just code cleanup, not functional change.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 69 +--
 1 file changed, 28 insertions(+), 41 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 6803e98414f1..bd2bf1fd847e 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -100,10 +100,16 @@ static struct char_device_struct *
 __register_chrdev_region(unsigned int major, unsigned int baseminor,
   int minorct, const char *name)
 {
-   struct char_device_struct *cd, **cp;
-   int ret = 0;
+   struct char_device_struct *cd, *curr, *prev = NULL;
+   int ret = -EBUSY;
int i;
 
+   if (major >= CHRDEV_MAJOR_MAX) {
+   pr_err("CHRDEV \"%s\" major requested (%u) is greater than the 
maximum (%u)\n",
+  name, major, CHRDEV_MAJOR_MAX-1);
+   return ERR_PTR(-EINVAL);
+   }
+
if (minorct > MINORMASK + 1 - baseminor) {
pr_err("CHRDEV \"%s\" minor range requested (%u-%u) is out of 
range of maximum range (%u-%u) for a single major\n",
name, baseminor, baseminor + minorct - 1, 0, MINORMASK);
@@ -126,55 +132,36 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
major = ret;
}
 
-   if (major >= CHRDEV_MAJOR_MAX) {
-   pr_err("CHRDEV \"%s\" major requested (%u) is greater than the 
maximum (%u)\n",
-  name, major, CHRDEV_MAJOR_MAX-1);
-   ret = -EINVAL;
-   goto out;
-   }
-
-   cd->major = major;
-   cd->baseminor = baseminor;
-   cd->minorct = minorct;
-   strlcpy(cd->name, name, sizeof(cd->name));
-
i = major_to_index(major);
+   for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
+   if (curr->major < major)
+   continue;
 
-   for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
-   if ((*cp)->major > major ||
-   ((*cp)->major == major &&
-(((*cp)->baseminor >= baseminor) ||
- ((*cp)->baseminor + (*cp)->minorct > baseminor
+   if (curr->major > major)
break;
 
-   /* Check for overlapping minor ranges.  */
-   if (*cp && (*cp)->major == major) {
-   int old_min = (*cp)->baseminor;
-   int old_max = (*cp)->baseminor + (*cp)->minorct - 1;
-   int new_min = baseminor;
-   int new_max = baseminor + minorct - 1;
+   if (curr->baseminor + curr->minorct <= baseminor)
+   continue;
 
-   /* New driver overlaps from the left.  */
-   if (new_max >= old_min && new_max <= old_max) {
-   ret = -EBUSY;
-   goto out;
-   }
+   if (curr->baseminor >= baseminor + minorct)
+   break;
 
-   /* New driver overlaps from the right.  */
-   if (new_min <= old_max && new_min >= old_min) {
-   ret = -EBUSY;
-   goto out;
-   }
+   goto out;
+   }
 
-   if (new_min < old_min && new_max > old_max) {
-   ret = -EBUSY;
-   goto out;
-   }
+   cd->major = major;
+   cd->baseminor = baseminor;
+   cd->minorct = minorct;
+   strlcpy(cd->name, name, sizeof(cd->name));
 
+   if (!prev) {
+   cd->next = curr;
+   chrdevs[i] = cd;
+   } else {
+   cd->next = prev->next;
+   prev->next = cd;
}
 
-   cd->next = *cp;
-   *cp = cd;
mutex_unlock(&chrdevs_lock);
return cd;
 out:
-- 
2.20.1

[PATCH] uio: fix potential memory leak in error case

[Chengguang Xu]

Should jump to lable err_infoopen when idev->info is NULL
in uio_open().

Signed-off-by: Chengguang Xu 
---
 drivers/uio/uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 131342280b46..46e5c1c31ebe 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -491,7 +491,7 @@ static int uio_open(struct inode *inode, struct file *filep)
if (!idev->info) {
mutex_unlock(&idev->info_lock);
ret = -EINVAL;
-   goto err_alloc_listener;
+   goto err_infoopen;
}
 
if (idev->info && idev->info->open)
-- 
2.17.2

[PATCH] uio: remove redundant check

[Chengguang Xu]

It is not necessary to check idev->info several times under
mutex lock, so just remove redundant check.

Signed-off-by: Chengguang Xu 
---
 drivers/uio/uio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 46e5c1c31ebe..effe72834c2f 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -494,7 +494,7 @@ static int uio_open(struct inode *inode, struct file *filep)
goto err_infoopen;
}
 
-   if (idev->info && idev->info->open)
+   if (idev->info->open)
ret = idev->info->open(idev->info, inode);
mutex_unlock(&idev->info_lock);
if (ret)
@@ -635,7 +635,7 @@ static ssize_t uio_write(struct file *filep, const char 
__user *buf,
goto out;
}
 
-   if (!idev->info || !idev->info->irq) {
+   if (!idev->info->irq) {
retval = -EIO;
goto out;
}
-- 
2.17.2

[PATCH 2/2] proc: code cleanup for proc_setup_thread_self()

[Chengguang Xu]

Remove unnecessary ERR_PTR()/PTR_ERR() cast in
proc_setup_thread_self().

Signed-off-by: Chengguang Xu 
---
 fs/proc/thread_self.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index b905010ca9eb..f61ae53533f5 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -38,6 +38,7 @@ int proc_setup_thread_self(struct super_block *s)
struct inode *root_inode = d_inode(s->s_root);
struct pid_namespace *ns = proc_pid_ns(root_inode);
struct dentry *thread_self;
+   int ret = -ENOMEM;
 
inode_lock(root_inode);
thread_self = d_alloc_name(s->s_root, "thread-self");
@@ -51,20 +52,19 @@ int proc_setup_thread_self(struct super_block *s)
inode->i_gid = GLOBAL_ROOT_GID;
inode->i_op = &proc_thread_self_inode_operations;
d_add(thread_self, inode);
+   ret = 0;
} else {
dput(thread_self);
-   thread_self = ERR_PTR(-ENOMEM);
}
-   } else {
-   thread_self = ERR_PTR(-ENOMEM);
}
inode_unlock(root_inode);
-   if (IS_ERR(thread_self)) {
+
+   if (ret)
pr_err("proc_fill_super: can't allocate /proc/thread_self\n");
-   return PTR_ERR(thread_self);
-   }
-   ns->proc_thread_self = thread_self;
-   return 0;
+   else
+   ns->proc_thread_self = thread_self;
+
+   return ret;
 }
 
 void __init proc_thread_self_init(void)
-- 
2.20.1

[PATCH 1/2] proc: code cleanup for proc_setup_self()

[Chengguang Xu]

Remove unnecessary ERR_PTR()/PTR_ERR() cast in proc_setup_self().

Signed-off-by: Chengguang Xu 
---
 fs/proc/self.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/proc/self.c b/fs/proc/self.c
index 127265e5c55f..57c0a1047250 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -38,6 +38,7 @@ int proc_setup_self(struct super_block *s)
struct inode *root_inode = d_inode(s->s_root);
struct pid_namespace *ns = proc_pid_ns(root_inode);
struct dentry *self;
+   int ret = -ENOMEM;

inode_lock(root_inode);
self = d_alloc_name(s->s_root, "self");
@@ -51,20 +52,19 @@ int proc_setup_self(struct super_block *s)
inode->i_gid = GLOBAL_ROOT_GID;
inode->i_op = &proc_self_inode_operations;
d_add(self, inode);
+   ret = 0;
} else {
dput(self);
-   self = ERR_PTR(-ENOMEM);
}
-   } else {
-   self = ERR_PTR(-ENOMEM);
}
inode_unlock(root_inode);
-   if (IS_ERR(self)) {
+
+   if (ret)
pr_err("proc_fill_super: can't allocate /proc/self\n");
-   return PTR_ERR(self);
-   }
-   ns->proc_self = self;
-   return 0;
+   else
+   ns->proc_self = self;
+
+   return ret;
 }
 
 void __init proc_self_init(void)
-- 
2.20.1

[PATCH v2] jbd2: fix potential double free

[Chengguang Xu]

When fail from creating cache jbd2_inode_cache, we will
destroy previously created cache jbd2_handle_cache twice.
This patch fixes it by sperating each cache
initialization/destruction to individual function.

Signed-off-by: Chengguang Xu 
---
v2:
- Seperate cache initialization/destruction to individual function.

 fs/jbd2/journal.c | 51 +++
 fs/jbd2/revoke.c  | 32 +--
 fs/jbd2/transaction.c |  8 ---
 include/linux/jbd2.h  |  8 ---
 4 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 37e16d969925..0f1ac43d0560 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2299,7 +2299,7 @@ static void jbd2_journal_destroy_slabs(void)
}
 }
 
-static int jbd2_journal_create_slab(size_t size)
+static int __init jbd2_journal_create_slab(size_t size)
 {
static DEFINE_MUTEX(jbd2_slab_create_mutex);
int i = order_base_2(size) - 10;
@@ -2375,22 +2375,19 @@ static struct kmem_cache *jbd2_journal_head_cache;
 static atomic_t nr_journal_heads = ATOMIC_INIT(0);
 #endif
 
-static int jbd2_journal_init_journal_head_cache(void)
+static int __init jbd2_journal_init_journal_head_cache(void)
 {
-   int retval;
-
-   J_ASSERT(jbd2_journal_head_cache == NULL);
+   J_ASSERT(!jbd2_journal_head_cache);
jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
sizeof(struct journal_head),
0,  /* offset */
SLAB_TEMPORARY | SLAB_TYPESAFE_BY_RCU,
NULL);  /* ctor */
-   retval = 0;
if (!jbd2_journal_head_cache) {
-   retval = -ENOMEM;
printk(KERN_EMERG "JBD2: no memory for journal_head cache\n");
+   return -ENOMEM;
}
-   return retval;
+   return 0;
 }
 
 static void jbd2_journal_destroy_journal_head_cache(void)
@@ -2636,28 +2633,38 @@ static void __exit 
jbd2_remove_jbd_stats_proc_entry(void)
 
 struct kmem_cache *jbd2_handle_cache, *jbd2_inode_cache;
 
+static int __init jbd2_journal_init_inode_cache(void)
+{
+   J_ASSERT(!jbd2_inode_cache);
+   jbd2_inode_cache = KMEM_CACHE(jbd2_inode, 0);
+   if (!jbd2_inode_cache) {
+   pr_emerg("JBD2: failed to create inode cache\n");
+   return -ENOMEM;
+   }
+   return 0;
+}
+
 static int __init jbd2_journal_init_handle_cache(void)
 {
+   J_ASSERT(!jbd2_handle_cache);
jbd2_handle_cache = KMEM_CACHE(jbd2_journal_handle, SLAB_TEMPORARY);
-   if (jbd2_handle_cache == NULL) {
+   if (!jbd2_handle_cache) {
printk(KERN_EMERG "JBD2: failed to create handle cache\n");
return -ENOMEM;
}
-   jbd2_inode_cache = KMEM_CACHE(jbd2_inode, 0);
-   if (jbd2_inode_cache == NULL) {
-   printk(KERN_EMERG "JBD2: failed to create inode cache\n");
-   kmem_cache_destroy(jbd2_handle_cache);
-   return -ENOMEM;
-   }
return 0;
 }
 
+static void jbd2_journal_destroy_inode_cache(void)
+{
+   kmem_cache_destroy(jbd2_inode_cache);
+   jbd2_inode_cache = NULL;
+}
+
 static void jbd2_journal_destroy_handle_cache(void)
 {
kmem_cache_destroy(jbd2_handle_cache);
jbd2_handle_cache = NULL;
-   kmem_cache_destroy(jbd2_inode_cache);
-   jbd2_inode_cache = NULL;
 }
 
 /*
@@ -2668,11 +2675,15 @@ static int __init journal_init_caches(void)
 {
int ret;
 
-   ret = jbd2_journal_init_revoke_caches();
+   ret = jbd2_journal_init_revoke_record_cache();
+   if (ret == 0)
+   ret = jbd2_journal_init_revoke_table_cache();
if (ret == 0)
ret = jbd2_journal_init_journal_head_cache();
if (ret == 0)
ret = jbd2_journal_init_handle_cache();
+   if (ret == 0)
+   ret = jbd2_journal_init_inode_cache();
if (ret == 0)
ret = jbd2_journal_init_transaction_cache();
return ret;
@@ -2680,9 +2691,11 @@ static int __init journal_init_caches(void)
 
 static void jbd2_journal_destroy_caches(void)
 {
-   jbd2_journal_destroy_revoke_caches();
+   jbd2_journal_destroy_revoke_record_cache();
+   jbd2_journal_destroy_revoke_table_cache();
jbd2_journal_destroy_journal_head_cache();
jbd2_journal_destroy_handle_cache();
+   jbd2_journal_destroy_inode_cache();
jbd2_journal_destroy_transaction_cache();
jbd2_journal_destroy_slabs();
 }
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index a1143e57a718..69b9bc329964 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -178,33 +178,41 @@ static struct jbd2_revoke_record_s 
*find_revoke_record(journal_t *journal,
return NULL;
 }
 
-void jbd2_journal_destroy_revoke_caches(void)
+

[PATCH] qnx6: convert mount option checking code to test_opt()

[Chengguang Xu]

Convert mount option checking code to test_opt() in
qnx6_show_options().

Signed-off-by: Chengguang Xu 
---
 fs/qnx6/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
index 59cf45f6be49..c3a80700e7a6 100644
--- a/fs/qnx6/inode.c
+++ b/fs/qnx6/inode.c
@@ -46,9 +46,8 @@ static const struct super_operations qnx6_sops = {
 static int qnx6_show_options(struct seq_file *seq, struct dentry *root)
 {
struct super_block *sb = root->d_sb;
-   struct qnx6_sb_info *sbi = QNX6_SB(sb);
 
-   if (sbi->s_mount_opt & QNX6_MOUNT_MMI_FS)
+   if (test_opt(sb, MMI_FS))
seq_puts(seq, ",mmi_fs");
return 0;
 }
-- 
2.17.2

[PATCH v3 1/4] chardev: add additional check for minor range overlap

[Chengguang Xu]

Current overlap checking cannot correctly handle
a case which is baseminor < existing baseminor &&
baseminor + minorct > existing baseminor + minorct.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index a279c58fe360..8a63cfa29005 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -159,6 +159,12 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
ret = -EBUSY;
goto out;
}
+
+   if (new_min < old_min && new_max > old_max) {
+   ret = -EBUSY;
+   goto out;
+   }
+
}

cd->next = *cp;
--
2.20.1

[PATCH v3 2/4] chardev: add a check for given minor range

[Chengguang Xu]

register_chrdev_region() carefully checks minor range
before calling __register_chrdev_region() but there is
another path from alloc_chrdev_region() which does not
check the range properly. So add a check for given minor
range in __register_chrdev_region().

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 8a63cfa29005..6803e98414f1 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -104,6 +104,12 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
int ret = 0;
int i;

+   if (minorct > MINORMASK + 1 - baseminor) {
+   pr_err("CHRDEV \"%s\" minor range requested (%u-%u) is out of 
range of maximum range (%u-%u) for a single major\n",
+   name, baseminor, baseminor + minorct - 1, 0, MINORMASK);
+   return ERR_PTR(-EINVAL);
+   }
+
cd = kzalloc(sizeof(struct char_device_struct), GFP_KERNEL);
if (cd == NULL)
return ERR_PTR(-ENOMEM);
--
2.20.1

[PATCH v3 4/4] chardev: update comment based on the code

[Chengguang Xu]

The function comment of __register_chrdev_region()
is out of date, so update it based on the code.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

 fs/char_dev.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 47fd0561b03d..00dfe17871ac 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -88,13 +88,10 @@ static int find_dynamic_major(void)
 /*
  * Register a single major with a specified minor range.
  *
- * If major == 0 this functions will dynamically allocate a major and return
- * its number.
- *
- * If major > 0 this function will attempt to reserve the passed range of
- * minors and will return zero on success.
+ * If major == 0 this function will dynamically allocate an unused major.
+ * If major > 0 this function will attempt to reserve the range of minors
+ * with given major.
  *
- * Returns a -ve errno on failure.
  */
 static struct char_device_struct *
 __register_chrdev_region(unsigned int major, unsigned int baseminor,
--
2.20.1

[PATCH v3 3/4] chardev: code cleanup for __register_chrdev_region()

[Chengguang Xu]

It's just code cleanup, not functional change.

Signed-off-by: Chengguang Xu 
---
v1->v2:
- Split fix and cleanup patches.
- Remove printing minor range in chrdev_show().

v2->v3:
- Set variable ret to '-EBUSY' before checking minor range overlap.

 fs/char_dev.c | 70 +--
 1 file changed, 29 insertions(+), 41 deletions(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index 6803e98414f1..47fd0561b03d 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -100,10 +100,16 @@ static struct char_device_struct *
 __register_chrdev_region(unsigned int major, unsigned int baseminor,
   int minorct, const char *name)
 {
-   struct char_device_struct *cd, **cp;
-   int ret = 0;
+   struct char_device_struct *cd, *curr, *prev = NULL;
+   int ret;
int i;

+   if (major >= CHRDEV_MAJOR_MAX) {
+   pr_err("CHRDEV \"%s\" major requested (%u) is greater than the 
maximum (%u)\n",
+  name, major, CHRDEV_MAJOR_MAX-1);
+   return ERR_PTR(-EINVAL);
+   }
+
if (minorct > MINORMASK + 1 - baseminor) {
pr_err("CHRDEV \"%s\" minor range requested (%u-%u) is out of 
range of maximum range (%u-%u) for a single major\n",
name, baseminor, baseminor + minorct - 1, 0, MINORMASK);
@@ -126,55 +132,37 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
major = ret;
}

-   if (major >= CHRDEV_MAJOR_MAX) {
-   pr_err("CHRDEV \"%s\" major requested (%u) is greater than the 
maximum (%u)\n",
-  name, major, CHRDEV_MAJOR_MAX-1);
-   ret = -EINVAL;
-   goto out;
-   }
-
-   cd->major = major;
-   cd->baseminor = baseminor;
-   cd->minorct = minorct;
-   strlcpy(cd->name, name, sizeof(cd->name));
-
+   ret = -EBUSY;
i = major_to_index(major);
+   for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
+   if (curr->major < major)
+   continue;

-   for (cp = &chrdevs[i]; *cp; cp = &(*cp)->next)
-   if ((*cp)->major > major ||
-   ((*cp)->major == major &&
-(((*cp)->baseminor >= baseminor) ||
- ((*cp)->baseminor + (*cp)->minorct > baseminor
+   if (curr->major > major)
break;

-   /* Check for overlapping minor ranges.  */
-   if (*cp && (*cp)->major == major) {
-   int old_min = (*cp)->baseminor;
-   int old_max = (*cp)->baseminor + (*cp)->minorct - 1;
-   int new_min = baseminor;
-   int new_max = baseminor + minorct - 1;
+   if (curr->baseminor + curr->minorct <= baseminor)
+   continue;

-   /* New driver overlaps from the left.  */
-   if (new_max >= old_min && new_max <= old_max) {
-   ret = -EBUSY;
-   goto out;
-   }
+   if (curr->baseminor >= baseminor + minorct)
+   break;

-   /* New driver overlaps from the right.  */
-   if (new_min <= old_max && new_min >= old_min) {
-   ret = -EBUSY;
-   goto out;
-   }
+   goto out;
+   }

-   if (new_min < old_min && new_max > old_max) {
-   ret = -EBUSY;
-   goto out;
-   }
+   cd->major = major;
+   cd->baseminor = baseminor;
+   cd->minorct = minorct;
+   strlcpy(cd->name, name, sizeof(cd->name));

+   if (!prev) {
+   cd->next = curr;
+   chrdevs[i] = cd;
+   } else {
+   cd->next = prev->next;
+   prev->next = cd;
}

-   cd->next = *cp;
-   *cp = cd;
mutex_unlock(&chrdevs_lock);
return cd;
 out:
--
2.20.1

Re: [PATCH v3 3/4] chardev: code cleanup for __register_chrdev_region()

[Chengguang Xu]

> Sent: Friday, April 05, 2019 at 8:32 PM
> From: "Greg KH" 
> To: "Chengguang Xu" 
> Cc: dan.carpen...@oracle.com, linux-fsde...@vger.kernel.org, 
> v...@zeniv.linux.org.uk, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 3/4] chardev: code cleanup for 
> __register_chrdev_region()
>
> On Fri, Apr 05, 2019 at 07:27:10PM +0800, Chengguang Xu wrote:
> > It's just code cleanup, not functional change.
> >
> > Signed-off-by: Chengguang Xu 
> > ---
> > v1->v2:
> > - Split fix and cleanup patches.
> > - Remove printing minor range in chrdev_show().
> >
> > v2->v3:
> > - Set variable ret to '-EBUSY' before checking minor range overlap.
>
> Can you just send the follow-on patch that does the v3 change as I have
> already taken your series.

OK, I'll send another patch for it.

Thanks.
Chengguang.

[PATCH] chardev: set variable ret to -EBUSY before checking minor range overlap

[Chengguang Xu]

When allocating dynamic major, the minor range overlap check
in __register_chrdev_region() will not fail, so actually
there is no real case to passing non negative error code to
caller. However, set variable ret to -EBUSY before chekcking
minor range overlap will avoid false-positive warning from
code analyzing tool(like Smatch) and also make the code more
easy to understand.

Suggested-by: Dan Carpenter 
Signed-off-by: Chengguang Xu 
---
 fs/char_dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index df6a54045ac4..00dfe17871ac 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -98,7 +98,7 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
   int minorct, const char *name)
 {
struct char_device_struct *cd, *curr, *prev = NULL;
-   int ret -EBUSY;
+   int ret;
int i;

if (major >= CHRDEV_MAJOR_MAX) {
@@ -129,6 +129,7 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
major = ret;
}

+   ret = -EBUSY;
i = major_to_index(major);
for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
if (curr->major < major)
--
2.17.2

[PATCH] quota: fix wrong indentation

[Chengguang Xu]

We need to check return code only when calling ->read_dqblk(),
so fix it properly.

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index fc20e06c56ba..934b8f3f9e57 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -424,10 +424,11 @@ int dquot_acquire(struct dquot *dquot)
struct quota_info *dqopt = sb_dqopt(dquot->dq_sb);

mutex_lock(&dquot->dq_lock);
-   if (!test_bit(DQ_READ_B, &dquot->dq_flags))
+   if (!test_bit(DQ_READ_B, &dquot->dq_flags)) {
ret = dqopt->ops[dquot->dq_id.type]->read_dqblk(dquot);
-   if (ret < 0)
-   goto out_iolock;
+   if (ret < 0)
+   goto out_iolock;
+   }
/* Make sure flags update is visible after dquot has been filled */
smp_mb__before_atomic();
set_bit(DQ_READ_B, &dquot->dq_flags);
--
2.20.1

[PATCH 2/3] jbd2: code cleanup for jbd2_journal_init_revoke_caches()

[Chengguang Xu]

jbd2_journal_destroy_caches() can handle destruction work
for all caches, so we don't have to destroy previously
created cache in error path, it also makes the code simpler.

Signed-off-by: Chengguang Xu 
---
 fs/jbd2/revoke.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index a1143e57a718..3b46b45d085d 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -194,17 +194,14 @@ int __init jbd2_journal_init_revoke_caches(void)
jbd2_revoke_record_cache = KMEM_CACHE(jbd2_revoke_record_s,
SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY);
if (!jbd2_revoke_record_cache)
-   goto record_cache_failure;
+   return -ENOMEM;
 
jbd2_revoke_table_cache = KMEM_CACHE(jbd2_revoke_table_s,
 SLAB_TEMPORARY);
if (!jbd2_revoke_table_cache)
-   goto table_cache_failure;
-   return 0;
-table_cache_failure:
-   jbd2_journal_destroy_revoke_caches();
-record_cache_failure:
return -ENOMEM;
+
+   return 0;
 }
 
 static struct jbd2_revoke_table_s *jbd2_journal_init_revoke_table(int 
hash_size)
-- 
2.20.1

[PATCH 3/3] jbd2: add __init annotation for jbd2_journal_init_journal_head_cache()

[Chengguang Xu]

jbd2_journal_init_journal_head_cache() only be called once,
so add __init annotation for it and do some code cleanup.

Signed-off-by: Chengguang Xu 
---
 fs/jbd2/journal.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 49797854ccb8..43399ad423d0 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2371,22 +2371,19 @@ static struct kmem_cache *jbd2_journal_head_cache;
 static atomic_t nr_journal_heads = ATOMIC_INIT(0);
 #endif
 
-static int jbd2_journal_init_journal_head_cache(void)
+static int __init jbd2_journal_init_journal_head_cache(void)
 {
-   int retval;
-
J_ASSERT(jbd2_journal_head_cache == NULL);
jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
sizeof(struct journal_head),
0,  /* offset */
SLAB_TEMPORARY | SLAB_TYPESAFE_BY_RCU,
NULL);  /* ctor */
-   retval = 0;
if (!jbd2_journal_head_cache) {
-   retval = -ENOMEM;
printk(KERN_EMERG "JBD2: no memory for journal_head cache\n");
+   return -ENOMEM;
}
-   return retval;
+   return 0;
 }
 
 static void jbd2_journal_destroy_journal_head_cache(void)
-- 
2.20.1

[PATCH 1/3] jbd2: fix potential double free

[Chengguang Xu]

When fail from creating cache jbd2_inode_cache, we will
destroy previously created cache jbd2_handle_cache twice.
This patch fixes it by removing first destroy in error path.

Signed-off-by: Chengguang Xu 
---
 fs/jbd2/journal.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 382c030cc78b..49797854ccb8 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2642,7 +2642,6 @@ static int __init jbd2_journal_init_handle_cache(void)
jbd2_inode_cache = KMEM_CACHE(jbd2_inode, 0);
if (jbd2_inode_cache == NULL) {
printk(KERN_EMERG "JBD2: failed to create inode cache\n");
-   kmem_cache_destroy(jbd2_handle_cache);
return -ENOMEM;
}
return 0;
-- 
2.20.1

[PATCH] ns: modify function comment for projid

[Chengguang Xu]

Some function comments about projid are described
as uid, so fix it.

Signed-off-by: Chengguang Xu 
---
 kernel/user_namespace.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 923414a246e9..6e917fc072d0 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -516,8 +516,8 @@ EXPORT_SYMBOL(from_kgid_munged);
  * @ns:  User namespace that the projid is in
  * @projid: Project identifier
  *
- * Maps a user-namespace uid pair into a kernel internal kuid,
- * and returns that kuid.
+ * Maps a user-namespace projid pair into a kernel internal kprojid,
+ * and returns that kprojid.
  *
  * When there is no mapping defined for the user-namespace projid
  * pair INVALID_PROJID is returned.  Callers are expected to test
@@ -526,7 +526,7 @@ EXPORT_SYMBOL(from_kgid_munged);
  */
 kprojid_t make_kprojid(struct user_namespace *ns, projid_t projid)
 {
-   /* Map the uid to a global kernel uid */
+   /* Map the projid to a global kernel projid */
return KPROJIDT_INIT(map_id_down(&ns->projid_map, projid));
 }
 EXPORT_SYMBOL(make_kprojid);
@@ -545,7 +545,7 @@ EXPORT_SYMBOL(make_kprojid);
  */
 projid_t from_kprojid(struct user_namespace *targ, kprojid_t kprojid)
 {
-   /* Map the uid from a global kernel uid */
+   /* Map the projid from a global kernel projid */
return map_id_up(&targ->projid_map, __kprojid_val(kprojid));
 }
 EXPORT_SYMBOL(from_kprojid);
-- 
2.17.2

[PATCH] quota: add dqi_dirty_list description to comment of Dquot List Management

[Chengguang Xu]

Actually there are four lists for dquot management, so add
the description of dqui_dirty_list to comment.

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index fc20e06c56ba..6a236bdaef89 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -223,9 +223,9 @@ static void put_quota_format(struct quota_format_type *fmt)
 
 /*
  * Dquot List Management:
- * The quota code uses three lists for dquot management: the inuse_list,
- * free_dquots, and dquot_hash[] array. A single dquot structure may be
- * on all three lists, depending on its current state.
+ * The quota code uses four lists for dquot management: the inuse_list,
+ * free_dquots, dqi_dirty_list, and dquot_hash[] array. A single dquot
+ * structure may be on some of those lists, depending on its current state.
  *
  * All dquots are placed to the end of inuse_list when first created, and this
  * list is used for invalidate operation, which must look at every dquot.
@@ -236,6 +236,10 @@ static void put_quota_format(struct quota_format_type *fmt)
  * dqstats.free_dquots gives the number of dquots on the list. When
  * dquot is invalidated it's completely released from memory.
  *
+ * Dirty dquots are added to the dqi_dirty_list of quota_info when mark
+ * dirtied, and this list is searched when writeback diry dquots to
+ * quota file.
+ *
  * Dquots with a specific identity (device, type and id) are placed on
  * one of the dquot_hash[] hash chains. The provides an efficient search
  * mechanism to locate a specific dquot.
-- 
2.17.2

[PATCH] quota: set init_needed flag only when successfully getting dquot

[Chengguang Xu]

Set init_needed flag only when successfully getting dquot,
so that we can skip unnecessary subsequent operation.

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index fc20e06c56ba..8d4ce2a2b5c8 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1449,8 +1449,6 @@ static int __dquot_initialize(struct inode *inode, int 
type)
if (!sb_has_quota_active(sb, cnt))
continue;

-   init_needed = 1;
-
switch (cnt) {
case USRQUOTA:
qid = make_kqid_uid(inode->i_uid);
@@ -1475,6 +1473,9 @@ static int __dquot_initialize(struct inode *inode, int 
type)
dquot = NULL;
}
got[cnt] = dquot;
+
+   if (got[cnt])
+   init_needed = 1;
}

/* All required i_dquot has been initialized */
--
2.20.1

[PATCH] quota: check time limit when back out space/inode change

[Chengguang Xu]

When we fail from allocating inode/space, we back out
the change we already did. In a special case which has
exceeded soft limit by the change, we should also check
time limit and reset it properly.

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 9d7dfc47c854..58f15a083dd1 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1681,13 +1681,11 @@ int __dquot_alloc_space(struct inode *inode, qsize_t 
number, int flags)
if (!dquots[cnt])
continue;
spin_lock(&dquots[cnt]->dq_dqb_lock);
-   if (reserve) {
-   dquots[cnt]->dq_dqb.dqb_rsvspace -=
-   number;
-   } else {
-   dquots[cnt]->dq_dqb.dqb_curspace -=
-   number;
-   }
+   if (reserve)
+   dquot_free_reserved_space(dquots[cnt],
+ number);
+   else
+   dquot_decr_space(dquots[cnt], number);
spin_unlock(&dquots[cnt]->dq_dqb_lock);
}
spin_unlock(&inode->i_lock);
@@ -1738,7 +1736,7 @@ int dquot_alloc_inode(struct inode *inode)
continue;
/* Back out changes we already did */
spin_lock(&dquots[cnt]->dq_dqb_lock);
-   dquots[cnt]->dq_dqb.dqb_curinodes--;
+   dquot_decr_inodes(dquots[cnt], 1);
spin_unlock(&dquots[cnt]->dq_dqb_lock);
}
goto warn_put_all;
--
2.20.1

[PATCH v2] chardev: set variable ret to -EBUSY before checking minor range overlap

[Chengguang Xu]

When allocating dynamic major, the minor range overlap check
in __register_chrdev_region() will not fail, so actually
there is no real case to passing non negative error code to
caller. However, set variable ret to -EBUSY before checking
minor range overlap will avoid false-positive warning from
code analyzing tool(like Smatch) and also make the code more
easy to understand.

Suggested-by: Dan Carpenter 
Signed-off-by: Chengguang Xu 
---
v1->v2:
- Rebase against the latest char-misc-next.
- Modify signed-off mail address.

 fs/char_dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/char_dev.c b/fs/char_dev.c
index d18cad28c1c3..00dfe17871ac 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -98,7 +98,7 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
   int minorct, const char *name)
 {
struct char_device_struct *cd, *curr, *prev = NULL;
-   int ret = -EBUSY;
+   int ret;
int i;
 
if (major >= CHRDEV_MAJOR_MAX) {
@@ -129,6 +129,7 @@ __register_chrdev_region(unsigned int major, unsigned int 
baseminor,
major = ret;
}
 
+   ret = -EBUSY;
i = major_to_index(major);
for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
if (curr->major < major)
-- 
2.17.2

[PATCH] quota: fix condition for resetting time limit in do_set_dqblk()

[Chengguang Xu]

We reset time limit when current usage is smaller
or equal to soft limit in other place, so follow
this rule in do_set_dqblk().

Signed-off-by: Chengguang Xu 
---
 fs/quota/dquot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index be9c471cdbc8..6e826b454082 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2731,7 +2731,7 @@ static int do_set_dqblk(struct dquot *dquot, struct 
qc_dqblk *di)
 
if (check_blim) {
if (!dm->dqb_bsoftlimit ||
-   dm->dqb_curspace + dm->dqb_rsvspace < dm->dqb_bsoftlimit) {
+   dm->dqb_curspace + dm->dqb_rsvspace <= dm->dqb_bsoftlimit) {
dm->dqb_btime = 0;
clear_bit(DQ_BLKS_B, &dquot->dq_flags);
} else if (!(di->d_fieldmask & QC_SPC_TIMER))
@@ -2740,7 +2740,7 @@ static int do_set_dqblk(struct dquot *dquot, struct 
qc_dqblk *di)
}
if (check_ilim) {
if (!dm->dqb_isoftlimit ||
-   dm->dqb_curinodes < dm->dqb_isoftlimit) {
+   dm->dqb_curinodes <= dm->dqb_isoftlimit) {
dm->dqb_itime = 0;
clear_bit(DQ_INODES_B, &dquot->dq_flags);
} else if (!(di->d_fieldmask & QC_INO_TIMER))
-- 
2.20.1

[PATCH] fs/affs: fix potential memory leak in option parsing

[Chengguang Xu]

When specifying option 'prefix' multiple times,
current option parsing will cause memory leak.
Hence, call kfree for previous one in this case.

Signed-off-by: Chengguang Xu 
---
 fs/affs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/affs/super.c b/fs/affs/super.c
index e602619..d1ad11a 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -241,6 +241,7 @@ enum {
affs_set_opt(*mount_opts, SF_NO_TRUNCATE);
break;
case Opt_prefix:
+   kfree(*prefix);
*prefix = match_strdup(&args[0]);
if (!*prefix)
return 0;
-- 
1.8.3.1

[V9fs-developer][PATCH v3 1/2] net/9p: detecting invalid options as much as possible

[Chengguang Xu]

Currently when detecting invalid options in option parsing,
some options(e.g. msize) just set errno and allow to continuously
validate other options so that it can detect invalid options
as much as possible and give proper error messages together.

This patch applies same rule to option 'trans' and 'version'
when detecting -EINVAL.

Signed-off-by: Chengguang Xu 
---
Changes since v2:
- Introduce a new temporary variable to avoid overriding error code.

Changes since v1:
- Do not change behavior when detecting ENOMEM or unrecognized options.

 net/9p/client.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 21e6df1..18c5271 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -198,8 +198,6 @@ static int parse_opts(char *opts, struct p9_client *clnt)
pr_info("Could not find request transport: 
%s\n",
s);
ret = -EINVAL;
-   kfree(s);
-   goto free_and_return;
}
kfree(s);
break;
@@ -214,13 +212,12 @@ static int parse_opts(char *opts, struct p9_client *clnt)
 "problem allocating copy of version 
arg\n");
goto free_and_return;
}
-   ret = get_protocol_version(s);
-   if (ret == -EINVAL) {
-   kfree(s);
-   goto free_and_return;
-   }
+   r = get_protocol_version(s);
+   if (r < 0)
+   ret = r;
+   else
+   clnt->proto_version = r;
kfree(s);
-   clnt->proto_version = ret;
break;
default:
continue;
-- 
1.8.3.1

[V9fs-developer][PATCH v3 2/2] fs/9p: detecting invalid options as much as possible

[Chengguang Xu]

Currently when detecting invalid options in option parsing,
some options(e.g. msize) just set errno and allow to continuously
validate other options so that it can detect invalid options
as much as possible and give proper error messages together.

This patch applies same rule to option 'cache' and 'access'
when detecting -EINVAL.

Signed-off-by: Chengguang Xu 
---
Changes since v2:
- Introduce a new temporary variable to avoid overriding error code.
- Delete redundant continue in error case.

Changes since v1:
- Do not change behavior when detecting ENOMEM or unrecognized options. 

 fs/9p/v9fs.c | 29 -
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index e622f0f..0429c8e 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -210,12 +210,12 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
p9_debug(P9_DEBUG_ERROR,
 "integer field, but no integer?\n");
ret = r;
-   continue;
-   }
-   v9ses->debug = option;
+   } else {
+   v9ses->debug = option;
 #ifdef CONFIG_NET_9P_DEBUG
-   p9_debug_level = option;
+   p9_debug_level = option;
 #endif
+   }
break;
 
case Opt_dfltuid:
@@ -231,7 +231,6 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
p9_debug(P9_DEBUG_ERROR,
 "uid field, but not a uid?\n");
ret = -EINVAL;
-   continue;
}
break;
case Opt_dfltgid:
@@ -247,7 +246,6 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
p9_debug(P9_DEBUG_ERROR,
 "gid field, but not a gid?\n");
ret = -EINVAL;
-   continue;
}
break;
case Opt_afid:
@@ -256,9 +254,9 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
p9_debug(P9_DEBUG_ERROR,
 "integer field, but no integer?\n");
ret = r;
-   continue;
+   } else {
+   v9ses->afid = option;
}
-   v9ses->afid = option;
break;
case Opt_uname:
kfree(v9ses->uname);
@@ -306,13 +304,12 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
 "problem allocating copy of cache 
arg\n");
goto free_and_return;
}
-   ret = get_cache_mode(s);
-   if (ret == -EINVAL) {
-   kfree(s);
-   goto free_and_return;
-   }
+   r = get_cache_mode(s);
+   if (r < 0)
+   ret = r;
+   else
+   v9ses->cache = r;
 
-   v9ses->cache = ret;
kfree(s);
break;
 
@@ -341,14 +338,12 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
pr_info("Unknown access argument %s\n",
s);
kfree(s);
-   goto free_and_return;
+   continue;
}
v9ses->uid = make_kuid(current_user_ns(), uid);
if (!uid_valid(v9ses->uid)) {
ret = -EINVAL;
pr_info("Uknown uid %s\n", s);
-   kfree(s);
-   goto free_and_return;
}
}
 
-- 
1.8.3.1

[PATCH] isofs: fix potential memory leak in mount option parsing

[Chengguang Xu]

When specifying string type mount option (e.g., iocharset)
several times in a mount, current option parsing may
cause memory leak. Hence, call kfree for previous one
in this case. Meanwhile, check memory allocation result
for it.

Signed-off-by: Chengguang Xu 
---
 fs/isofs/inode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index bc258a4..ec3fba7 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -394,7 +394,10 @@ static int parse_options(char *options, struct 
iso9660_options *popt)
break;
 #ifdef CONFIG_JOLIET
case Opt_iocharset:
+   kfree(popt->iocharset);
popt->iocharset = match_strdup(&args[0]);
+   if (!popt->iocharset)
+   return 0;
break;
 #endif
case Opt_map_a:
-- 
1.8.3.1

[PATCH] exofs: fix potential memory leak in mount option parsing

[Chengguang Xu]

When specifying string type mount option several times
in a mount, current option parsing may cause memory leak.
Hence, call kfree for previous one in this case.

Signed-off-by: Chengguang Xu 
---
 fs/exofs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 179cd5c..106b818 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -101,6 +101,7 @@ static int parse_options(char *options, struct 
exofs_mountopt *opts)
token = match_token(p, tokens, args);
switch (token) {
case Opt_name:
+   kfree(opts->dev_name);
opts->dev_name = match_strdup(&args[0]);
if (unlikely(!opts->dev_name)) {
EXOFS_ERR("Error allocating dev_name");
-- 
1.8.3.1

[PATCH 2/2] fs/9p: detecting invalid options as much as possible

[Chengguang Xu]

Currently when detecting invalid options in option parsing,
some options(e.g. debug) just set errno and allow to continusly
validate other options so that it can detect invalid options
as much as possible and give proper error messages together.

This patch apply this policy to all options and the case of memory
allocation error in option parsing.

Signed-off-by: Chengguang Xu 
---
 fs/9p/v9fs.c | 49 -
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index e622f0f..29ba937 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -192,10 +192,9 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
return 0;
 
tmp_options = kstrdup(opts, GFP_KERNEL);
-   if (!tmp_options) {
-   ret = -ENOMEM;
-   goto fail_option_alloc;
-   }
+   if (!tmp_options)
+   return -ENOMEM;
+
options = tmp_options;
 
while ((p = strsep(&options, ",")) != NULL) {
@@ -263,18 +262,16 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
case Opt_uname:
kfree(v9ses->uname);
v9ses->uname = match_strdup(&args[0]);
-   if (!v9ses->uname) {
-   ret = -ENOMEM;
-   goto free_and_return;
-   }
+   if (!v9ses->uname)
+   if (!ret)
+   ret = -ENOMEM;
break;
case Opt_remotename:
kfree(v9ses->aname);
v9ses->aname = match_strdup(&args[0]);
-   if (!v9ses->aname) {
-   ret = -ENOMEM;
-   goto free_and_return;
-   }
+   if (!v9ses->aname)
+   if (!ret)
+   ret = -ENOMEM;
break;
case Opt_nodevmap:
v9ses->nodev = 1;
@@ -292,24 +289,24 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
 #ifdef CONFIG_9P_FSCACHE
kfree(v9ses->cachetag);
v9ses->cachetag = match_strdup(&args[0]);
-   if (!v9ses->cachetag) {
-   ret = -ENOMEM;
-   goto free_and_return;
-   }
+   if (!v9ses->cachetag)
+   if (!ret)
+   ret = -ENOMEM;
 #endif
break;
case Opt_cache:
s = match_strdup(&args[0]);
if (!s) {
-   ret = -ENOMEM;
+   if (!ret)
+   ret = -ENOMEM;
p9_debug(P9_DEBUG_ERROR,
 "problem allocating copy of cache 
arg\n");
-   goto free_and_return;
+   continue;
}
ret = get_cache_mode(s);
if (ret == -EINVAL) {
kfree(s);
-   goto free_and_return;
+   continue;
}
 
v9ses->cache = ret;
@@ -319,10 +316,11 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
case Opt_access:
s = match_strdup(&args[0]);
if (!s) {
-   ret = -ENOMEM;
+   if (!ret)
+   ret = -ENOMEM;
p9_debug(P9_DEBUG_ERROR,
 "problem allocating copy of access 
arg\n");
-   goto free_and_return;
+   continue;
}
 
v9ses->flags &= ~V9FS_ACCESS_MASK;
@@ -341,14 +339,14 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
pr_info("Unknown access argument %s\n",
s);
kfree(s);
-   goto free_and_return;
+   continue;
}
v9ses->uid = make_kuid(current_user_ns(), uid);
if (!uid_valid(v9ses->uid)) {

[PATCH 1/2] net/9p: detecting invalid options as much as possible

[Chengguang Xu]

Currently when detecting invalid options in option parsing,
some options(e.g. msize) just set errno and allow to continusly
validate other options so that it can detect invalid options
as much as possible and give proper error messages together.

This patch apply this policy to all options and the case of memory
allocation error in option parsing.

Signed-off-by: Chengguang Xu 
---
 net/9p/client.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 21e6df1..066f136 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -186,10 +186,11 @@ static int parse_opts(char *opts, struct p9_client *clnt)
case Opt_trans:
s = match_strdup(&args[0]);
if (!s) {
-   ret = -ENOMEM;
+   if (!ret)
+   ret = -ENOMEM;
p9_debug(P9_DEBUG_ERROR,
 "problem allocating copy of trans 
arg\n");
-   goto free_and_return;
+   continue;
}
 
v9fs_put_trans(clnt->trans_mod);
@@ -199,7 +200,7 @@ static int parse_opts(char *opts, struct p9_client *clnt)
s);
ret = -EINVAL;
kfree(s);
-   goto free_and_return;
+   continue;
}
kfree(s);
break;
@@ -209,25 +210,28 @@ static int parse_opts(char *opts, struct p9_client *clnt)
case Opt_version:
s = match_strdup(&args[0]);
if (!s) {
-   ret = -ENOMEM;
+   if (!ret)
+   ret = -ENOMEM;
p9_debug(P9_DEBUG_ERROR,
 "problem allocating copy of version 
arg\n");
-   goto free_and_return;
+   continue;
}
ret = get_protocol_version(s);
if (ret == -EINVAL) {
kfree(s);
-   goto free_and_return;
+   continue;
}
kfree(s);
clnt->proto_version = ret;
break;
default:
+   p9_debug(P9_DEBUG_ERROR,
+"unrecognized option \"%s\" or missing 
value\n",
+   p);
continue;
}
}
 
-free_and_return:
v9fs_put_trans(clnt->trans_mod);
kfree(tmp_options);
return ret;
-- 
1.8.3.1

[V9fs-developer][PATCH v2 2/2] fs/9p: detecting invalid options as much as possible

[Chengguang Xu]

Currently when detecting invalid options in option parsing,
some options(e.g. msize) just set errno and allow to continuously
validate other options so that it can detect invalid options
as much as possible and give proper error messages together.

This patch applies same rule to option 'cache' and 'access'
when detecting EINVAL.

Signed-off-by: Chengguang Xu 
---
Changes since v1:
- Do not change behavior when detecting ENOMEM or unrecognized options. 

 fs/9p/v9fs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index e622f0f..2c7a0ef 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -309,7 +309,7 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
ret = get_cache_mode(s);
if (ret == -EINVAL) {
kfree(s);
-   goto free_and_return;
+   continue;
}
 
v9ses->cache = ret;
@@ -341,14 +341,14 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
pr_info("Unknown access argument %s\n",
s);
kfree(s);
-   goto free_and_return;
+   continue;
}
v9ses->uid = make_kuid(current_user_ns(), uid);
if (!uid_valid(v9ses->uid)) {
ret = -EINVAL;
pr_info("Uknown uid %s\n", s);
kfree(s);
-   goto free_and_return;
+   continue;
}
}
 
-- 
1.8.3.1

[V9fs-developer][PATCH v2 1/2] net/9p: detecting invalid options as much as possible

[Chengguang Xu]

Currently when detecting invalid options in option parsing,
some options(e.g. msize) just set errno and allow to continuously
validate other options so that it can detect invalid options
as much as possible and give proper error messages together.

This patch applies same rule to option 'trans' and 'version'
when detecting EINVAL.

Signed-off-by: Chengguang Xu 
---
Changes since v1:
- Do not change behavior when detecting ENOMEM or unrecognized options.

 net/9p/client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 21e6df1..38b02fb 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -199,7 +199,7 @@ static int parse_opts(char *opts, struct p9_client *clnt)
s);
ret = -EINVAL;
kfree(s);
-   goto free_and_return;
+   continue;
}
kfree(s);
break;
@@ -217,7 +217,7 @@ static int parse_opts(char *opts, struct p9_client *clnt)
ret = get_protocol_version(s);
if (ret == -EINVAL) {
kfree(s);
-   goto free_and_return;
+   continue;
}
kfree(s);
clnt->proto_version = ret;
-- 
1.8.3.1

[PATCH 1/2] hfs: fix potential refcnt problem of nls module

[Chengguang Xu]

When specifying iocharset/codepage multiple times in a mount,
current option parsing will cause inaccurate refcount of nls
module. Hence, call unload_nls for previous one in this case.

Signed-off-by: Chengguang Xu 
---
 fs/hfs/super.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 1738767..6a5f6fd 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -329,8 +329,10 @@ static int parse_options(char *options, struct hfs_sb_info 
*hsb)
return 0;
}
p = match_strdup(&args[0]);
-   if (p)
+   if (p) {
+   unload_nls(hsb->nls_disk);
hsb->nls_disk = load_nls(p);
+   }
if (!hsb->nls_disk) {
pr_err("unable to load codepage \"%s\"\n", p);
kfree(p);
@@ -344,8 +346,10 @@ static int parse_options(char *options, struct hfs_sb_info 
*hsb)
return 0;
}
p = match_strdup(&args[0]);
-   if (p)
+   if (p) {
+   unload_nls(hsb->nls_io);
hsb->nls_io = load_nls(p);
+   }
if (!hsb->nls_io) {
pr_err("unable to load iocharset \"%s\"\n", p);
kfree(p);
-- 
1.8.3.1

[PATCH 2/2] hfsplus: fix potential refcnt problem of nls module

[Chengguang Xu]

When specifying nls option multiple times in a mount,
current option parsing will cause inaccurate refcount of nls
module. Hence, call unload_nls for previous one in this case.

Signed-off-by: Chengguang Xu 
---
 fs/hfsplus/options.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c
index 047e05c..068d4e2 100644
--- a/fs/hfsplus/options.c
+++ b/fs/hfsplus/options.c
@@ -171,8 +171,10 @@ int hfsplus_parse_options(char *input, struct 
hfsplus_sb_info *sbi)
return 0;
}
p = match_strdup(&args[0]);
-   if (p)
+   if (p) {
+   unload_nls(sbi->nls);
sbi->nls = load_nls(p);
+   }
if (!sbi->nls) {
pr_err("unable to load nls mapping \"%s\"\n",
   p);
-- 
1.8.3.1

[PATCH] quota: remvoe redundant variable assignment

[Chengguang Xu]

The assignment of variable ret is redundant because the
value of ret is 0 after calling v2_read_header() in normal
case.

Signed-off-by: Chengguang Xu 
---
 fs/quota/quota_v2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/quota/quota_v2.c b/fs/quota/quota_v2.c
index a73e5b34db41..25d36607be6e 100644
--- a/fs/quota/quota_v2.c
+++ b/fs/quota/quota_v2.c
@@ -158,7 +158,6 @@ static int v2_read_file_info(struct super_block *sb, int 
type)
qinfo->dqi_entry_size = sizeof(struct v2r1_disk_dqblk);
qinfo->dqi_ops = &v2r1_qtree_ops;
}
-   ret = 0;
 out:
up_read(&dqopt->dqio_sem);
return ret;
--
2.17.2

[PATCH] quota: minor optimization for __dquot_initialize()

[Chengguang Xu]

Count the number of quotas which are needed to initialize,
instead of just setting to 1 so that we may avoid unnecessary
process in special case.

Signed-off-by: Chengguang Xu 
---
 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 7b4bac91146b..f93bd60c3998 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1459,7 +1459,7 @@ static int __dquot_initialize(struct inode *inode, int 
type)
if (!sb_has_quota_active(sb, cnt))
continue;
 
-   init_needed = 1;
+   init_needed++;
 
switch (cnt) {
case USRQUOTA:
@@ -1483,6 +1483,7 @@ static int __dquot_initialize(struct inode *inode, int 
type)
goto out_put;
}
dquot = NULL;
+   init_needed--;
}
got[cnt] = dquot;
}
-- 
2.17.2

[PATCH] udf: fix potential refcnt problem of nls module

[Chengguang Xu]

When specifiying iocharset multiple times in a mount
or once/multiple in a remount, current option parsing
may cause inaccurate refcount of nls module.
Also, in the failure cleanup of option parsing,
the condition of calling unload_nls is not sufficient.

Signed-off-by: Chengguang Xu 
---

Hi Jan,

I found this issue by checking code so now post the fix for rewiew,
but sorry I did't get enough resource to fully test it, I can only
make sure the fix can successfuly comiple with no error.

 fs/udf/super.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 7b27b06..7e73a98 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -523,8 +523,12 @@ static int udf_parse_options(char *options, struct 
udf_options *uopt,
break;
 #ifdef CONFIG_UDF_NLS
case Opt_iocharset:
-   uopt->nls_map = load_nls(args[0].from);
-   uopt->flags |= (1 << UDF_FLAG_NLS_MAP);
+   if (!remount) {
+   if (uopt->nls_map)
+   unload_nls(uopt->nls_map);
+   uopt->nls_map = load_nls(args[0].from);
+   uopt->flags |= (1 << UDF_FLAG_NLS_MAP);
+   }
break;
 #endif
case Opt_uignore:
@@ -2082,8 +2086,8 @@ error_out:
for (i = 0; i < sbi->s_partitions; i++)
udf_free_partition(&sbi->s_partmaps[i]);
 #ifdef CONFIG_UDF_NLS
-   if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP))
-   unload_nls(sbi->s_nls_map);
+   if (uopt.nls_map)
+   unload_nls(uopt.nls_map);
 #endif
if (!(sb->s_flags & MS_RDONLY))
udf_close_lvid(sb);
-- 
1.8.3.1

[tip:x86/mm] x86/mm: Remove redundant check for kmem_cache_create()

[tip-bot for Chengguang Xu]

Commit-ID:  765d28f136291f9639e3c031a1070fb76d6625c7
Gitweb: https://git.kernel.org/tip/765d28f136291f9639e3c031a1070fb76d6625c7
Author: Chengguang Xu 
AuthorDate: Tue, 12 Jun 2018 19:48:52 +0800
Committer:  Thomas Gleixner 
CommitDate: Thu, 2 Aug 2018 14:27:35 +0200

x86/mm: Remove redundant check for kmem_cache_create()

The flag 'SLAB_PANIC' implies panic on failure, So there is no need to
check the returned pointer for NULL.

Signed-off-by: Chengguang Xu 
Signed-off-by: Thomas Gleixner 
Cc: h...@zytor.com
Link: 
https://lkml.kernel.org/r/1528804132-154948-1-git-send-email-cgxu...@gmx.com

---
 arch/x86/mm/pgtable.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index e3deefb891da..0f1683fcb196 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -329,9 +329,6 @@ static int __init pgd_cache_init(void)
 */
pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_ALIGN,
  SLAB_PANIC, NULL);
-   if (!pgd_cache)
-   return -ENOMEM;
-
return 0;
 }
 core_initcall(pgd_cache_init);