2022-07-20 08:42:36

by Zeng Jingxiang

Subject: [PATCH] fs/reiserfs/inode: remove dead code in _get_block_create_0()

From: Zeng Jingxiang <[email protected]>

Since commit 27b3a5c51b50 ("kill-the-bkl/reiserfs: drop the fs race
watchdog from _get_block_create_0()"), which removed a label that may
have the pointer 'p' touched in its control flow, related if statements
now eval to constant value now. Just remove them.

Assigning value NULL to p here
293 char *p = NULL;

In the following conditional expression, the value of p is always NULL,
As a result, the kunmap() cannot be executed.
308 if (p)
309 kunmap(bh_result->b_page);

355 if (p)
356 kunmap(bh_result->b_page);

366 if (p)
367 kunmap(bh_result->b_page);

Also, the kmap() cannot be executed.
399 if (!p)
400 p = (char *)kmap(bh_result->b_page);

Signed-off-by: Zeng Jingxiang <[email protected]>
Signed-off-by: Kairui Song <[email protected]>
---
fs/reiserfs/inode.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index 0cffe054b78e..fe26e1746af9 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -305,8 +305,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
result = search_for_position_by_key(inode->i_sb, &key, &path);
if (result != POSITION_FOUND) {
pathrelse(&path);
- if (p)
- kunmap(bh_result->b_page);
if (result == IO_ERROR)
return -EIO;
/*
@@ -352,8 +350,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
}

pathrelse(&path);
- if (p)
- kunmap(bh_result->b_page);
return ret;
}
/* requested data are in direct item(s) */
@@ -363,8 +359,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
* when it is stored in direct item(s)
*/
pathrelse(&path);
- if (p)
- kunmap(bh_result->b_page);
return -ENOENT;
}

@@ -396,9 +390,7 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
* sure we need to. But, this means the item might move if
* kmap schedules
*/
- if (!p)
- p = (char *)kmap(bh_result->b_page);
-
+ p = (char *)kmap(bh_result->b_page);
p += offset;
memset(p, 0, inode->i_sb->s_blocksize);
do {
--
2.27.0

2022-07-26 11:51:53

by Jan Kara

Subject: Re: [PATCH] fs/reiserfs/inode: remove dead code in _get_block_create_0()

On Wed 20-07-22 16:30:29, [email protected] wrote:
> From: Zeng Jingxiang <[email protected]>
>
> Since commit 27b3a5c51b50 ("kill-the-bkl/reiserfs: drop the fs race
> watchdog from _get_block_create_0()"), which removed a label that may
> have the pointer 'p' touched in its control flow, related if statements
> now eval to constant value now. Just remove them.
>
> Assigning value NULL to p here
> 293 char *p = NULL;
>
> In the following conditional expression, the value of p is always NULL,
> As a result, the kunmap() cannot be executed.
> 308 if (p)
> 309 kunmap(bh_result->b_page);
>
> 355 if (p)
> 356 kunmap(bh_result->b_page);
>
> 366 if (p)
> 367 kunmap(bh_result->b_page);
>
> Also, the kmap() cannot be executed.
> 399 if (!p)
> 400 p = (char *)kmap(bh_result->b_page);
>
> Signed-off-by: Zeng Jingxiang <[email protected]>
> Signed-off-by: Kairui Song <[email protected]>

Thanks! I've added the patch to my tree. I've also removed the unnecessary
initialization of 'p' to NULL. I'll push the patch to Linus in the coming
merge window.

Honza

> ---
> fs/reiserfs/inode.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
> index 0cffe054b78e..fe26e1746af9 100644
> --- a/fs/reiserfs/inode.c
> +++ b/fs/reiserfs/inode.c
> @@ -305,8 +305,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
> result = search_for_position_by_key(inode->i_sb, &key, &path);
> if (result != POSITION_FOUND) {
> pathrelse(&path);
> - if (p)
> - kunmap(bh_result->b_page);
> if (result == IO_ERROR)
> return -EIO;
> /*
> @@ -352,8 +350,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
> }
>
> pathrelse(&path);
> - if (p)
> - kunmap(bh_result->b_page);
> return ret;
> }
> /* requested data are in direct item(s) */
> @@ -363,8 +359,6 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
> * when it is stored in direct item(s)
> */
> pathrelse(&path);
> - if (p)
> - kunmap(bh_result->b_page);
> return -ENOENT;
> }
>
> @@ -396,9 +390,7 @@ static int _get_block_create_0(struct inode *inode, sector_t block,
> * sure we need to. But, this means the item might move if
> * kmap schedules
> */
> - if (!p)
> - p = (char *)kmap(bh_result->b_page);
> -
> + p = (char *)kmap(bh_result->b_page);
> p += offset;
> memset(p, 0, inode->i_sb->s_blocksize);
> do {
> --
> 2.27.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR