[PATCH 2/4] fs/sysv: Change the signature of dir_get_page()

Fabio M. De Francesco posted 4 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH 2/4] fs/sysv: Change the signature of dir_get_page()
Posted by Fabio M. De Francesco 2 years, 8 months ago
Change the signature of dir_get_page() in order to prepare this function
to the conversion to the use of kmap_local_page(). Change also those call
sites which are required to conform to the new signature.

Cc: Ira Weiny <ira.weiny@intel.com>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
 fs/sysv/dir.c | 50 ++++++++++++++++++++++++--------------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/fs/sysv/dir.c b/fs/sysv/dir.c
index 685379bc9d64..0a8b5828c390 100644
--- a/fs/sysv/dir.c
+++ b/fs/sysv/dir.c
@@ -52,13 +52,15 @@ static int dir_commit_chunk(struct page *page, loff_t pos, unsigned len)
 	return err;
 }
 
-static struct page * dir_get_page(struct inode *dir, unsigned long n)
+static void *dir_get_page(struct inode *dir, unsigned long n, struct page **p)
 {
 	struct address_space *mapping = dir->i_mapping;
 	struct page *page = read_mapping_page(mapping, n, NULL);
-	if (!IS_ERR(page))
-		kmap(page);
-	return page;
+	if (IS_ERR(page))
+		return ERR_CAST(page);
+	kmap(page);
+	*p = page;
+	return page_address(page);
 }
 
 static int sysv_readdir(struct file *file, struct dir_context *ctx)
@@ -80,11 +82,11 @@ static int sysv_readdir(struct file *file, struct dir_context *ctx)
 	for ( ; n < npages; n++, offset = 0) {
 		char *kaddr, *limit;
 		struct sysv_dir_entry *de;
-		struct page *page = dir_get_page(inode, n);
+		struct page *page;
 
-		if (IS_ERR(page))
+		kaddr = dir_get_page(inode, n, &page);
+		if (IS_ERR(kaddr))
 			continue;
-		kaddr = (char *)page_address(page);
 		de = (struct sysv_dir_entry *)(kaddr+offset);
 		limit = kaddr + PAGE_SIZE - SYSV_DIRSIZE;
 		for ( ;(char*)de <= limit; de++, ctx->pos += sizeof(*de)) {
@@ -142,11 +144,10 @@ struct sysv_dir_entry *sysv_find_entry(struct dentry *dentry, struct page **res_
 	n = start;
 
 	do {
-		char *kaddr;
-		page = dir_get_page(dir, n);
-		if (!IS_ERR(page)) {
-			kaddr = (char*)page_address(page);
-			de = (struct sysv_dir_entry *) kaddr;
+		char *kaddr = dir_get_page(dir, n, &page);
+
+		if (!IS_ERR(kaddr)) {
+			de = (struct sysv_dir_entry *)kaddr;
 			kaddr += PAGE_SIZE - SYSV_DIRSIZE;
 			for ( ; (char *) de <= kaddr ; de++) {
 				if (!de->inode)
@@ -185,11 +186,10 @@ int sysv_add_link(struct dentry *dentry, struct inode *inode)
 
 	/* We take care of directory expansion in the same loop */
 	for (n = 0; n <= npages; n++) {
-		page = dir_get_page(dir, n);
+		kaddr = dir_get_page(dir, n, &page);
 		err = PTR_ERR(page);
-		if (IS_ERR(page))
-			goto out;
-		kaddr = (char*)page_address(page);
+		if (IS_ERR(kaddr))
+			return PTR_ERR(kaddr);
 		de = (struct sysv_dir_entry *)kaddr;
 		kaddr += PAGE_SIZE - SYSV_DIRSIZE;
 		while ((char *)de <= kaddr) {
@@ -219,7 +219,6 @@ int sysv_add_link(struct dentry *dentry, struct inode *inode)
 	mark_inode_dirty(dir);
 out_page:
 	dir_put_page(page);
-out:
 	return err;
 out_unlock:
 	unlock_page(page);
@@ -288,12 +287,11 @@ int sysv_empty_dir(struct inode * inode)
 	for (i = 0; i < npages; i++) {
 		char *kaddr;
 		struct sysv_dir_entry * de;
-		page = dir_get_page(inode, i);
 
-		if (IS_ERR(page))
+		kaddr = dir_get_page(inode, i, &page);
+		if (IS_ERR(kaddr))
 			continue;
 
-		kaddr = (char *)page_address(page);
 		de = (struct sysv_dir_entry *)kaddr;
 		kaddr += PAGE_SIZE-SYSV_DIRSIZE;
 
@@ -339,16 +337,16 @@ void sysv_set_link(struct sysv_dir_entry *de, struct page *page,
 	mark_inode_dirty(dir);
 }
 
-struct sysv_dir_entry * sysv_dotdot (struct inode *dir, struct page **p)
+struct sysv_dir_entry *sysv_dotdot(struct inode *dir, struct page **p)
 {
-	struct page *page = dir_get_page(dir, 0);
-	struct sysv_dir_entry *de = NULL;
+	struct page *page = NULL;
+	struct sysv_dir_entry *de = dir_get_page(dir, 0, &page);
 
-	if (!IS_ERR(page)) {
-		de = (struct sysv_dir_entry*) page_address(page) + 1;
+	if (!IS_ERR(de)) {
 		*p = page;
+		return (struct sysv_dir_entry *)page_address(page) + 1;
 	}
-	return de;
+	return NULL;
 }
 
 ino_t sysv_inode_by_name(struct dentry *dentry)
-- 
2.39.0
Re: [PATCH 2/4] fs/sysv: Change the signature of dir_get_page()
Posted by Dan Carpenter 2 years, 8 months ago
Hi Fabio,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/fs-sysv-Use-the-offset_in_page-helper/20221231-155850
base:   git://git.infradead.org/users/hch/configfs.git for-next
patch link:    https://lore.kernel.org/r/20221231075717.10258-3-fmdefrancesco%40gmail.com
patch subject: [PATCH 2/4] fs/sysv: Change the signature of dir_get_page()
config: xtensa-randconfig-m031-20230101
compiler: xtensa-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

smatch warnings:
fs/sysv/dir.c:190 sysv_add_link() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +190 fs/sysv/dir.c

^1da177e4c3f41 Linus Torvalds        2005-04-16  174  int sysv_add_link(struct dentry *dentry, struct inode *inode)
^1da177e4c3f41 Linus Torvalds        2005-04-16  175  {
2b0143b5c986be David Howells         2015-03-17  176  	struct inode *dir = d_inode(dentry->d_parent);
^1da177e4c3f41 Linus Torvalds        2005-04-16  177  	const char * name = dentry->d_name.name;
^1da177e4c3f41 Linus Torvalds        2005-04-16  178  	int namelen = dentry->d_name.len;
^1da177e4c3f41 Linus Torvalds        2005-04-16  179  	struct page *page = NULL;
^1da177e4c3f41 Linus Torvalds        2005-04-16  180  	struct sysv_dir_entry * de;
^1da177e4c3f41 Linus Torvalds        2005-04-16  181  	unsigned long npages = dir_pages(dir);
^1da177e4c3f41 Linus Torvalds        2005-04-16  182  	unsigned long n;
^1da177e4c3f41 Linus Torvalds        2005-04-16  183  	char *kaddr;
26a6441aadde86 Nicholas Piggin       2007-10-16  184  	loff_t pos;
^1da177e4c3f41 Linus Torvalds        2005-04-16  185  	int err;
^1da177e4c3f41 Linus Torvalds        2005-04-16  186  
^1da177e4c3f41 Linus Torvalds        2005-04-16  187  	/* We take care of directory expansion in the same loop */
^1da177e4c3f41 Linus Torvalds        2005-04-16  188  	for (n = 0; n <= npages; n++) {
4b8a9c0afda16b Fabio M. De Francesco 2022-12-31  189  		kaddr = dir_get_page(dir, n, &page);
^1da177e4c3f41 Linus Torvalds        2005-04-16 @190  		err = PTR_ERR(page);

This "err" assignment is a dead store (pointless/never used).

4b8a9c0afda16b Fabio M. De Francesco 2022-12-31  191  		if (IS_ERR(kaddr))
4b8a9c0afda16b Fabio M. De Francesco 2022-12-31  192  			return PTR_ERR(kaddr);
^1da177e4c3f41 Linus Torvalds        2005-04-16  193  		de = (struct sysv_dir_entry *)kaddr;
09cbfeaf1a5a67 Kirill A. Shutemov    2016-04-01  194  		kaddr += PAGE_SIZE - SYSV_DIRSIZE;
^1da177e4c3f41 Linus Torvalds        2005-04-16  195  		while ((char *)de <= kaddr) {
^1da177e4c3f41 Linus Torvalds        2005-04-16  196  			if (!de->inode)
^1da177e4c3f41 Linus Torvalds        2005-04-16  197  				goto got_it;
^1da177e4c3f41 Linus Torvalds        2005-04-16  198  			err = -EEXIST;
^1da177e4c3f41 Linus Torvalds        2005-04-16  199  			if (namecompare(namelen, SYSV_NAMELEN, name, de->name)) 
^1da177e4c3f41 Linus Torvalds        2005-04-16  200  				goto out_page;
^1da177e4c3f41 Linus Torvalds        2005-04-16  201  			de++;
^1da177e4c3f41 Linus Torvalds        2005-04-16  202  		}
^1da177e4c3f41 Linus Torvalds        2005-04-16  203  		dir_put_page(page);
^1da177e4c3f41 Linus Torvalds        2005-04-16  204  	}
^1da177e4c3f41 Linus Torvalds        2005-04-16  205  	BUG();
^1da177e4c3f41 Linus Torvalds        2005-04-16  206  	return -EINVAL;
^1da177e4c3f41 Linus Torvalds        2005-04-16  207  
^1da177e4c3f41 Linus Torvalds        2005-04-16  208  got_it:
1023904333f9cb Fabio M. De Francesco 2022-12-31  209  	pos = page_offset(page) + offset_in_page(de);
^1da177e4c3f41 Linus Torvalds        2005-04-16  210  	lock_page(page);
f4e420dc423148 Christoph Hellwig     2010-06-04  211  	err = sysv_prepare_chunk(page, pos, SYSV_DIRSIZE);
^1da177e4c3f41 Linus Torvalds        2005-04-16  212  	if (err)
^1da177e4c3f41 Linus Torvalds        2005-04-16  213  		goto out_unlock;
^1da177e4c3f41 Linus Torvalds        2005-04-16  214  	memcpy (de->name, name, namelen);
^1da177e4c3f41 Linus Torvalds        2005-04-16  215  	memset (de->name + namelen, 0, SYSV_DIRSIZE - namelen - 2);
^1da177e4c3f41 Linus Torvalds        2005-04-16  216  	de->inode = cpu_to_fs16(SYSV_SB(inode->i_sb), inode->i_ino);
26a6441aadde86 Nicholas Piggin       2007-10-16  217  	err = dir_commit_chunk(page, pos, SYSV_DIRSIZE);
02027d42c3f747 Deepa Dinamani        2016-09-14  218  	dir->i_mtime = dir->i_ctime = current_time(dir);
^1da177e4c3f41 Linus Torvalds        2005-04-16  219  	mark_inode_dirty(dir);
^1da177e4c3f41 Linus Torvalds        2005-04-16  220  out_page:
^1da177e4c3f41 Linus Torvalds        2005-04-16  221  	dir_put_page(page);
^1da177e4c3f41 Linus Torvalds        2005-04-16  222  	return err;
^1da177e4c3f41 Linus Torvalds        2005-04-16  223  out_unlock:
^1da177e4c3f41 Linus Torvalds        2005-04-16  224  	unlock_page(page);
^1da177e4c3f41 Linus Torvalds        2005-04-16  225  	goto out_page;
^1da177e4c3f41 Linus Torvalds        2005-04-16  226  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp
Re: [PATCH 2/4] fs/sysv: Change the signature of dir_get_page()
Posted by Fabio M. De Francesco 2 years, 8 months ago
On mercoledì 4 gennaio 2023 12:59:24 CET Dan Carpenter wrote:
> Hi Fabio,
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:   
> https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/fs-sysv-U
> se-the-offset_in_page-helper/20221231-155850 base:  
> git://git.infradead.org/users/hch/configfs.git for-next
> patch link:   
> https://lore.kernel.org/r/20221231075717.10258-3-fmdefrancesco%40gmail.com
> patch subject: [PATCH 2/4] fs/sysv: Change the signature of dir_get_page()
> config: xtensa-randconfig-m031-20230101
> compiler: xtensa-linux-gcc (GCC) 12.1.0
> 
> If you fix the issue, kindly add following tag where applicable
> 
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> 
> smatch warnings:
> fs/sysv/dir.c:190 sysv_add_link() warn: passing zero to 'PTR_ERR'
> 
> vim +/PTR_ERR +190 fs/sysv/dir.c
> 
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  174  int 
sysv_add_link(struct
> dentry *dentry, struct inode *inode) ^1da177e4c3f41 Linus Torvalds       
> 2005-04-16  175  {
> 2b0143b5c986be David Howells         2015-03-17  176  	struct inode *dir 
=
> d_inode(dentry->d_parent); ^1da177e4c3f41 Linus Torvalds        2005-04-16 
> 177  	const char * name = dentry->d_name.name; ^1da177e4c3f41 Linus 
Torvalds 
>       2005-04-16  178  	int namelen = dentry->d_name.len; 
^1da177e4c3f41
> Linus Torvalds        2005-04-16  179  	struct page *page = NULL;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  180  	struct 
sysv_dir_entry
> * de; ^1da177e4c3f41 Linus Torvalds        2005-04-16  181  	
unsigned long
> npages = dir_pages(dir); ^1da177e4c3f41 Linus Torvalds        2005-04-16  
182
>  	unsigned long n; ^1da177e4c3f41 Linus Torvalds        2005-04-16  
183 
> 	char *kaddr;
> 26a6441aadde86 Nicholas Piggin       2007-10-16  184  	loff_t pos;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  185  	int err;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  186
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  187  	/* We take care 
of
> directory expansion in the same loop */ ^1da177e4c3f41 Linus Torvalds       
> 2005-04-16  188  	for (n = 0; n <= npages; n++) { 4b8a9c0afda16b Fabio M. De
> Francesco 2022-12-31  189  		kaddr = dir_get_page(dir, n, &page);
> ^1da177e4c3f41 Linus Torvalds        2005-04-16 @190  		err = 
PTR_ERR(page);
> 
> This "err" assignment is a dead store (pointless/never used).

Hi Dan,

Thanks for catching it.
I'll wait for comments on this series one or two more days and then delete 
that assignment.

Again thanks,

Fabio

> 4b8a9c0afda16b Fabio M. De Francesco 2022-12-31  191  		if 
(IS_ERR(kaddr))
> 4b8a9c0afda16b Fabio M. De Francesco 2022-12-31  192  			
return
> PTR_ERR(kaddr); ^1da177e4c3f41 Linus Torvalds        2005-04-16  193  	
	de =
> (struct sysv_dir_entry *)kaddr; 09cbfeaf1a5a67 Kirill A. Shutemov   
> 2016-04-01  194  		kaddr += PAGE_SIZE - SYSV_DIRSIZE; 
^1da177e4c3f41 Linus
> Torvalds        2005-04-16  195  		while ((char *)de <= kaddr) 
{
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  196  			
if (!de->inode)
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  197  			
	goto got_it;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  198  			
err = -EEXIST;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  199  			
if
> (namecompare(namelen, SYSV_NAMELEN, name, de->name)) ^1da177e4c3f41 Linus
> Torvalds        2005-04-16  200  				goto 
out_page; ^1da177e4c3f41 Linus
> Torvalds        2005-04-16  201  			de++;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  202  		}
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  203  		
dir_put_page(page);
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  204  	}
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  205  	BUG();
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  206  	return -EINVAL;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  207
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  208  got_it:
> 1023904333f9cb Fabio M. De Francesco 2022-12-31  209  	pos = 
page_offset(page)
> + offset_in_page(de); ^1da177e4c3f41 Linus Torvalds        2005-04-16  210 
> 	lock_page(page); f4e420dc423148 Christoph Hellwig     2010-06-04  
211  	err
> = sysv_prepare_chunk(page, pos, SYSV_DIRSIZE); ^1da177e4c3f41 Linus Torvalds 
>       2005-04-16  212  	if (err)
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  213  		goto 
out_unlock;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  214  	memcpy (de->name, 
name,
> namelen); ^1da177e4c3f41 Linus Torvalds        2005-04-16  215  	memset
> (de->name + namelen, 0, SYSV_DIRSIZE - namelen - 2); ^1da177e4c3f41 Linus
> Torvalds        2005-04-16  216  	de->inode =
> cpu_to_fs16(SYSV_SB(inode->i_sb), inode->i_ino); 26a6441aadde86 Nicholas
> Piggin       2007-10-16  217  	err = dir_commit_chunk(page, pos,
> SYSV_DIRSIZE); 02027d42c3f747 Deepa Dinamani        2016-09-14  218 
> 	dir->i_mtime = dir->i_ctime = current_time(dir); ^1da177e4c3f41 
Linus
> Torvalds        2005-04-16  219  	mark_inode_dirty(dir); ^1da177e4c3f41 
Linus
> Torvalds        2005-04-16  220  out_page:
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  221  	
dir_put_page(page);
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  222  	return err;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  223  out_unlock:
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  224  	
unlock_page(page);
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  225  	goto out_page;
> ^1da177e4c3f41 Linus Torvalds        2005-04-16  226  }
> 
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp