LinuxLists.cc - [PATCH 2/4] fs/sysv: Change the signature of dir_get

2022-12-31 07:59:17

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

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 <[email protected]>
Suggested-by: Al Viro <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
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

2023-01-04 12:04:43

by Dan Carpenter

[permalink] [raw]

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

Hi Fabio,

/docs/git-format-patch#_base_tree_information]">https://git-scm.com/docs/git-format-patch#_base_tree_information]
hub.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/fs-sysv-Use-the-offset_in_page-helper/20221231-155850">https://github.com/intel-lab-lkp/linux/commits/Fabio-M-De-Francesco/fs-sysv-Use-the-offset_in_page-helper/20221231-155850
rg/users/hch/configfs.git for-next
s://lore.kernel.org/r/20221231075717.10258-3-fmdefrancesco%40gmail.com">https://lore.kernel.org/r/20221231075717.10258-3-fmdefrancesco%40gmail.com
2/4] fs/sysv: Change the signature of dir_get_page()
/> (GCC) 12.1.0
kindly add following tag where applicable
test robot <[email protected]>
<[email protected]>
warn: passing zero to 'PTR_ERR'
2005-04-16 174 int sysv_add_link(struct dentry *dentry, struct inode *inode)
2005-04-16 175 {
2015-03-17 176 struct inode *dir = d_inode(dentry->d_parent);
2005-04-16 177 const char * name = dentry->d_name.name;
2005-04-16 178 int namelen = dentry->d_name.len;
2005-04-16 179 struct page *page = NULL;
2005-04-16 180 struct sysv_dir_entry * de;
2005-04-16 181 unsigned long npages = dir_pages(dir);
2005-04-16 182 unsigned long n;
2005-04-16 183 char *kaddr;
2007-10-16 184 loff_t pos;
2005-04-16 185 int err;
2005-04-16 186
2005-04-16 187 /* We take care of directory expansion in the same loop */
2005-04-16 188 for (n = 0; n <= npages; n++) {
M. De Francesco 2022-12-31 189 kaddr = dir_get_page(dir, n, &page);
2005-04-16 @190 err = PTR_ERR(page);
assignment is a dead store (pointless/never used).
M. De Francesco 2022-12-31 191 if (IS_ERR(kaddr))
M. De Francesco 2022-12-31 192 return PTR_ERR(kaddr);
2005-04-16 193 de = (struct sysv_dir_entry *)kaddr;
2016-04-01 194 kaddr += PAGE_SIZE - SYSV_DIRSIZE;
2005-04-16 195 while ((char *)de <= kaddr) {
2005-04-16 196 if (!de->inode)
2005-04-16 197 goto got_it;
2005-04-16 198 err = -EEXIST;
2005-04-16 199 if (namecompare(namelen, SYSV_NAMELEN, name, de->name))
2005-04-16 200 goto out_page;
2005-04-16 201 de++;
2005-04-16 202 }
2005-04-16 203 dir_put_page(page);
2005-04-16 204 }
2005-04-16 205 BUG();
2005-04-16 206 return -EINVAL;
2005-04-16 207
2005-04-16 208 got_it:
M. De Francesco 2022-12-31 209 pos = page_offset(page) + offset_in_page(de);
2005-04-16 210 lock_page(page);
2010-06-04 211 err = sysv_prepare_chunk(page, pos, SYSV_DIRSIZE);
2005-04-16 212 if (err)
2005-04-16 213 goto out_unlock;
2005-04-16 214 memcpy (de->name, name, namelen);
2005-04-16 215 memset (de->name + namelen, 0, SYSV_DIRSIZE - namelen - 2);
2005-04-16 216 de->inode = cpu_to_fs16(SYSV_SB(inode->i_sb), inode->i_ino);
2007-10-16 217 err = dir_commit_chunk(page, pos, SYSV_DIRSIZE);
2016-09-14 218 dir->i_mtime = dir->i_ctime = current_time(dir);
2005-04-16 219 mark_inode_dirty(dir);
2005-04-16 220 out_page:
2005-04-16 221 dir_put_page(page);
2005-04-16 222 return err;
2005-04-16 223 out_unlock:
2005-04-16 224 unlock_page(page);
2005-04-16 225 goto out_page;
2005-04-16 226 }
>https://01.org/lkp

2023-01-04 13:19:36

by Fabio M. De Francesco

[permalink] [raw]

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

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 <[email protected]>
> | Reported-by: Dan Carpenter <[email protected]>
>
> 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