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
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) <[email protected]>
> ---
> fs/ext4/inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks good. You can add:
Reviewed-by: Jan Kara <[email protected]>
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 <[email protected]>
SUSE Labs, CR
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) <[email protected]>
---
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
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) <[email protected]>
Cc: [email protected]
---
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
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) <[email protected]>
> Cc: [email protected]
> ---
> fs/jbd2/transaction.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
The patch looks good. Thanks! You can add:
Reviewed-by: Jan Kara <[email protected]>
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 <[email protected]>
SUSE Labs, CR
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) <[email protected]>
> Cc: [email protected]
Thanks, applied.
- Ted
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) <[email protected]>
Thanks, applied.
- Ted