[PATCH 0/2] ext4: fix two compile problems

[PATCH 0/2] ext4: fix two compile problems

[zhangyi (F)]

Hi,

Found two compile problems when using JBUFFER_TRACE and BUFFER_TRACE.

The first patch fix two compile warnings when enabling JBUFFER_TRACE,
and the second patch fix a compile error when enabling BUFFER_TRACE.
Please review.

Thanks,
Yi.

zhangyi (F) (2):
  jbd2: fix compile warning when using JBUFFER_TRACE
  ext4: fix compile error when using BUFFER_TRACE

 fs/ext4/inode.c       |  2 +-
 fs/jbd2/transaction.c | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCH 1/2] jbd2: fix compile warning when using JBUFFER_TRACE

[zhangyi (F)]

The jh pointer may be used uninitialized in the two cases below and the
compiler complain about it when enabling JBUFFER_TRACE macro, fix them.

In file included from fs/jbd2/transaction.c:19:0:
fs/jbd2/transaction.c: In function ‘jbd2_journal_get_undo_access’:
./include/linux/jbd2.h:1637:38: warning: ‘jh’ is used uninitialized in this function [-Wuninitialized]
 #define JBUFFER_TRACE(jh, info) do { printk("%s: %d\n", __func__, jh->b_jcount);} while (0)
                                      ^
fs/jbd2/transaction.c:1219:23: note: ‘jh’ was declared here
  struct journal_head *jh;
                       ^
In file included from fs/jbd2/transaction.c:19:0:
fs/jbd2/transaction.c: In function ‘jbd2_journal_dirty_metadata’:
./include/linux/jbd2.h:1637:38: warning: ‘jh’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 #define JBUFFER_TRACE(jh, info) do { printk("%s: %d\n", __func__, jh->b_jcount);} while (0)
                                      ^
fs/jbd2/transaction.c:1332:23: note: ‘jh’ was declared here
  struct journal_head *jh;
                       ^

Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Cc: stable@vger.kernel.org
---
 fs/jbd2/transaction.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index c0b66a7..4b51177 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1219,11 +1219,12 @@ int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh)
 	struct journal_head *jh;
 	char *committed_data = NULL;
 
-	JBUFFER_TRACE(jh, "entry");
 	if (jbd2_write_access_granted(handle, bh, true))
 		return 0;
 
 	jh = jbd2_journal_add_journal_head(bh);
+	JBUFFER_TRACE(jh, "entry");
+
 	/*
 	 * Do this first --- it can drop the journal lock, so we want to
 	 * make sure that obtaining the committed_data is done
@@ -1334,15 +1335,17 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
 
 	if (is_handle_aborted(handle))
 		return -EROFS;
-	if (!buffer_jbd(bh)) {
-		ret = -EUCLEAN;
-		goto out;
-	}
+	if (!buffer_jbd(bh))
+		return -EUCLEAN;
+
 	/*
 	 * We don't grab jh reference here since the buffer must be part
 	 * of the running transaction.
 	 */
 	jh = bh2jh(bh);
+	jbd_debug(5, "journal_head %p\n", jh);
+	JBUFFER_TRACE(jh, "entry");
+
 	/*
 	 * This and the following assertions are unreliable since we may see jh
 	 * in inconsistent state unless we grab bh_state lock. But this is
@@ -1376,9 +1379,6 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
 	}
 
 	journal = transaction->t_journal;
-	jbd_debug(5, "journal_head %p\n", jh);
-	JBUFFER_TRACE(jh, "entry");
-
 	jbd_lock_bh_state(bh);
 
 	if (jh->b_modified == 0) {
-- 
2.7.4

Re: [PATCH 1/2] jbd2: fix compile warning when using JBUFFER_TRACE

[Jan Kara]

On Thu 10-01-19 14:06:38, zhangyi (F) wrote:
> The jh pointer may be used uninitialized in the two cases below and the
> compiler complain about it when enabling JBUFFER_TRACE macro, fix them.
> 
> In file included from fs/jbd2/transaction.c:19:0:
> fs/jbd2/transaction.c: In function ‘jbd2_journal_get_undo_access’:
> ./include/linux/jbd2.h:1637:38: warning: ‘jh’ is used uninitialized in this function [-Wuninitialized]
>  #define JBUFFER_TRACE(jh, info) do { printk("%s: %d\n", __func__, jh->b_jcount);} while (0)
>                                       ^
> fs/jbd2/transaction.c:1219:23: note: ‘jh’ was declared here
>   struct journal_head *jh;
>                        ^
> In file included from fs/jbd2/transaction.c:19:0:
> fs/jbd2/transaction.c: In function ‘jbd2_journal_dirty_metadata’:
> ./include/linux/jbd2.h:1637:38: warning: ‘jh’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>  #define JBUFFER_TRACE(jh, info) do { printk("%s: %d\n", __func__, jh->b_jcount);} while (0)
>                                       ^
> fs/jbd2/transaction.c:1332:23: note: ‘jh’ was declared here
>   struct journal_head *jh;
>                        ^
> 
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/jbd2/transaction.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

The patch looks good. Thanks! You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza


> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index c0b66a7..4b51177 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1219,11 +1219,12 @@ int jbd2_journal_get_undo_access(handle_t *handle, struct buffer_head *bh)
>  	struct journal_head *jh;
>  	char *committed_data = NULL;
>  
> -	JBUFFER_TRACE(jh, "entry");
>  	if (jbd2_write_access_granted(handle, bh, true))
>  		return 0;
>  
>  	jh = jbd2_journal_add_journal_head(bh);
> +	JBUFFER_TRACE(jh, "entry");
> +
>  	/*
>  	 * Do this first --- it can drop the journal lock, so we want to
>  	 * make sure that obtaining the committed_data is done
> @@ -1334,15 +1335,17 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
>  
>  	if (is_handle_aborted(handle))
>  		return -EROFS;
> -	if (!buffer_jbd(bh)) {
> -		ret = -EUCLEAN;
> -		goto out;
> -	}
> +	if (!buffer_jbd(bh))
> +		return -EUCLEAN;
> +
>  	/*
>  	 * We don't grab jh reference here since the buffer must be part
>  	 * of the running transaction.
>  	 */
>  	jh = bh2jh(bh);
> +	jbd_debug(5, "journal_head %p\n", jh);
> +	JBUFFER_TRACE(jh, "entry");
> +
>  	/*
>  	 * This and the following assertions are unreliable since we may see jh
>  	 * in inconsistent state unless we grab bh_state lock. But this is
> @@ -1376,9 +1379,6 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
>  	}
>  
>  	journal = transaction->t_journal;
> -	jbd_debug(5, "journal_head %p\n", jh);
> -	JBUFFER_TRACE(jh, "entry");
> -
>  	jbd_lock_bh_state(bh);
>  
>  	if (jh->b_modified == 0) {
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/2] jbd2: fix compile warning when using JBUFFER_TRACE

[Theodore Y. Ts'o]

On Thu, Jan 10, 2019 at 02:06:38PM +0800, zhangyi (F) wrote:
> The jh pointer may be used uninitialized in the two cases below and the
> compiler complain about it when enabling JBUFFER_TRACE macro, fix them.
> 
> In file included from fs/jbd2/transaction.c:19:0:
> fs/jbd2/transaction.c: In function ‘jbd2_journal_get_undo_access’:
> ./include/linux/jbd2.h:1637:38: warning: ‘jh’ is used uninitialized in this function [-Wuninitialized]
>  #define JBUFFER_TRACE(jh, info) do { printk("%s: %d\n", __func__, jh->b_jcount);} while (0)
>                                       ^
> fs/jbd2/transaction.c:1219:23: note: ‘jh’ was declared here
>   struct journal_head *jh;
>                        ^
> In file included from fs/jbd2/transaction.c:19:0:
> fs/jbd2/transaction.c: In function ‘jbd2_journal_dirty_metadata’:
> ./include/linux/jbd2.h:1637:38: warning: ‘jh’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>  #define JBUFFER_TRACE(jh, info) do { printk("%s: %d\n", __func__, jh->b_jcount);} while (0)
>                                       ^
> fs/jbd2/transaction.c:1332:23: note: ‘jh’ was declared here
>   struct journal_head *jh;
>                        ^
> 
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> Cc: stable@vger.kernel.org

Thanks, applied.

					- Ted

[PATCH 2/2] ext4: fix compile error when using BUFFER_TRACE

[zhangyi (F)]

Fix compile error below when using BUFFER_TRACE.

fs/ext4/inode.c: In function ‘ext4_expand_extra_isize’:
fs/ext4/inode.c:5979:19: error: request for member ‘bh’ in something not a structure or union
  BUFFER_TRACE(iloc.bh, "get_write_access");

Fixes: c03b45b853f58 ("ext4, project: expand inode extra size if possible")
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
 fs/ext4/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 22a9d81..b7384d5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5976,7 +5976,7 @@ int ext4_expand_extra_isize(struct inode *inode,
 
 	ext4_write_lock_xattr(inode, &no_expand);
 
-	BUFFER_TRACE(iloc.bh, "get_write_access");
+	BUFFER_TRACE(iloc->bh, "get_write_access");
 	error = ext4_journal_get_write_access(handle, iloc->bh);
 	if (error) {
 		brelse(iloc->bh);
-- 
2.7.4

Re: [PATCH 2/2] ext4: fix compile error when using BUFFER_TRACE

[Jan Kara]

On Thu 10-01-19 14:06:39, zhangyi (F) wrote:
> Fix compile error below when using BUFFER_TRACE.
> 
> fs/ext4/inode.c: In function ‘ext4_expand_extra_isize’:
> fs/ext4/inode.c:5979:19: error: request for member ‘bh’ in something not a structure or union
>   BUFFER_TRACE(iloc.bh, "get_write_access");
> 
> Fixes: c03b45b853f58 ("ext4, project: expand inode extra size if possible")
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
>  fs/ext4/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 22a9d81..b7384d5 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5976,7 +5976,7 @@ int ext4_expand_extra_isize(struct inode *inode,
>  
>  	ext4_write_lock_xattr(inode, &no_expand);
>  
> -	BUFFER_TRACE(iloc.bh, "get_write_access");
> +	BUFFER_TRACE(iloc->bh, "get_write_access");
>  	error = ext4_journal_get_write_access(handle, iloc->bh);
>  	if (error) {
>  		brelse(iloc->bh);
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/2] ext4: fix compile error when using BUFFER_TRACE

[Theodore Y. Ts'o]

On Thu, Jan 10, 2019 at 02:06:39PM +0800, zhangyi (F) wrote:
> Fix compile error below when using BUFFER_TRACE.
> 
> fs/ext4/inode.c: In function ‘ext4_expand_extra_isize’:
> fs/ext4/inode.c:5979:19: error: request for member ‘bh’ in something not a structure or union
>   BUFFER_TRACE(iloc.bh, "get_write_access");
> 
> Fixes: c03b45b853f58 ("ext4, project: expand inode extra size if possible")
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>

Thanks, applied.

						- Ted