* [PATCH v3] xfs_logprint: handle log operation split of inode item correctly
@ 2017-01-19 13:32 Hou Tao
2017-01-20 12:09 ` Brian Foster
0 siblings, 1 reply; 2+ messages in thread
From: Hou Tao @ 2017-01-19 13:32 UTC (permalink / raw)
To: linux-xfs; +Cc: sandeen, bfoster
If an inode log item has 4 log operations, and the 4th operation
(attr fork op) is splitted to the next log record due to the size
limitation of log record, xfs_logprint doesn't check whether or not
the 4th operation is in the current log record and print invalid data.
xfs_logprint also needs to calculate the count of splitted log
operations correctly instead of just returning 1.
The following is the output before patch applied:
============================================================================
cycle: 120 version: 2 lsn: 120,11014 tail_lsn: 120,427
length of Log Record: 32256 prev offset: 10984 num ops: 243
......
h_size: 32768
----------------------------------------------------------------------------
Oper (0): tid: 2db4353b len: 0 clientid: TRANS flags: START
......
----------------------------------------------------------------------------
Oper (240): tid: 2db4353b len: 56 clientid: TRANS flags: none
INODE: #regs: 4 ino: 0x200a4bf flags: 0x45 dsize: 64
blkno: 10506832 len: 16 boff: 7936
Oper (241): tid: 2db4353b len: 96 clientid: TRANS flags: none
INODE CORE
......
Oper (242): tid: 2db4353b len: 64 clientid: TRANS flags: none
EXTENTS inode data
Oper (243): tid: 150000 len: 83886080 clientid: ERROR flags: none
LOCAL attr data
============================================================================
cycle: 120 version: 2 lsn: 120,11078 tail_lsn: 120,427
length of Log Record: 3584 prev offset: 11014 num ops: 44
......
h_size: 32768
----------------------------------------------------------------------------
Oper (0): tid: 2db4353b len: 52 clientid: TRANS flags: none
----------------------------------------------------------------------------
Oper (1): tid: 2db4353b len: 56 clientid: TRANS flags: none
INODE: #regs: 3 ino: 0x100047b flags: 0x5 dsize: 64
......
----------------------------------------------------------------------------
The following is the output after patch applied:
============================================================================
cycle: 120 version: 2 lsn: 120,11014 tail_lsn: 120,427
length of Log Record: 32256 prev offset: 10984 num ops: 243
......
h_size: 32768
----------------------------------------------------------------------------
Oper (0): tid: 2db4353b len: 0 clientid: TRANS flags: START
......
----------------------------------------------------------------------------
Oper (240): tid: 2db4353b len: 56 clientid: TRANS flags: none
INODE: #regs: 4 ino: 0x200a4bf flags: 0x45 dsize: 64
blkno: 10506832 len: 16 boff: 7936
Oper (241): tid: 2db4353b len: 96 clientid: TRANS flags: none
INODE CORE
......
Oper (242): tid: 2db4353b len: 64 clientid: TRANS flags: none
EXTENTS inode data
============================================================================
cycle: 120 version: 2 lsn: 120,11078 tail_lsn: 120,427
length of Log Record: 3584 prev offset: 11014 num ops: 44
......
h_size: 32768
----------------------------------------------------------------------------
Oper (0): tid: 2db4353b len: 52 clientid: TRANS flags: none
Left over region from split log item
----------------------------------------------------------------------------
Oper (1): tid: 2db4353b len: 56 clientid: TRANS flags: none
INODE: #regs: 3 ino: 0x100047b flags: 0x5 dsize: 64
......
----------------------------------------------------------------------------
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
logprint/log_misc.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
v3:
- update commit log description as suggested by Brian Foster
- add ASSERT(skip_count == 0) as suggested by Brian Foster
v2: https://www.spinics.net/lists/linux-xfs/msg03500.html
- rewrite the commit message to clarify the patch
- use "skip_count" suggested by Brian Foster
- fix the indentation
v1: http://www.spinics.net/lists/linux-xfs/msg03430.html
diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index a0f1766..0dfcfd1 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -524,6 +524,7 @@ xlog_print_trans_inode(
xfs_inode_log_format_t *f;
int mode;
int size;
+ int skip_count;
/*
* print inode type header region
@@ -555,15 +556,17 @@ xlog_print_trans_inode(
return f->ilf_size;
}
+ skip_count = f->ilf_size-1;
+
if (*i >= num_ops) /* end of LR */
- return f->ilf_size-1;
+ return skip_count;
/* core inode comes 2nd */
op_head = (xlog_op_header_t *)*ptr;
xlog_print_op_header(op_head, *i, ptr);
if (op_head->oh_flags & XLOG_CONTINUE_TRANS) {
- return f->ilf_size-1;
+ return skip_count;
}
memmove(&dino, *ptr, sizeof(dino));
@@ -571,13 +574,7 @@ xlog_print_trans_inode(
size = (int)dino.di_size;
xlog_print_trans_inode_core(&dino);
*ptr += xfs_log_dinode_size(dino.di_version);
-
- if (*i == num_ops-1 && f->ilf_size == 3) {
- return 1;
- }
-
- /* does anything come next */
- op_head = (xlog_op_header_t *)*ptr;
+ skip_count--;
switch (f->ilf_fields & (XFS_ILOG_DEV | XFS_ILOG_UUID)) {
case XFS_ILOG_DEV:
@@ -595,7 +592,12 @@ xlog_print_trans_inode(
ASSERT(f->ilf_size <= 4);
ASSERT((f->ilf_size == 3) || (f->ilf_fields & XFS_ILOG_AFORK));
+ /* does anything come next */
+ op_head = (xlog_op_header_t *)*ptr;
+
if (f->ilf_fields & XFS_ILOG_DFORK) {
+ if (*i == num_ops-1)
+ return skip_count;
(*i)++;
xlog_print_op_header(op_head, *i, ptr);
@@ -618,11 +620,14 @@ xlog_print_trans_inode(
*ptr += be32_to_cpu(op_head->oh_len);
if (op_head->oh_flags & XLOG_CONTINUE_TRANS)
- return 1;
+ return skip_count;
op_head = (xlog_op_header_t *)*ptr;
+ skip_count--;
}
if (f->ilf_fields & XFS_ILOG_AFORK) {
+ if (*i == num_ops-1)
+ return skip_count;
(*i)++;
xlog_print_op_header(op_head, *i, ptr);
@@ -644,9 +649,12 @@ xlog_print_trans_inode(
}
*ptr += be32_to_cpu(op_head->oh_len);
if (op_head->oh_flags & XLOG_CONTINUE_TRANS)
- return 1;
+ return skip_count;
+ skip_count--;
}
+ ASSERT(skip_count == 0);
+
return 0;
} /* xlog_print_trans_inode */
--
2.5.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] xfs_logprint: handle log operation split of inode item correctly
2017-01-19 13:32 [PATCH v3] xfs_logprint: handle log operation split of inode item correctly Hou Tao
@ 2017-01-20 12:09 ` Brian Foster
0 siblings, 0 replies; 2+ messages in thread
From: Brian Foster @ 2017-01-20 12:09 UTC (permalink / raw)
To: Hou Tao; +Cc: linux-xfs, sandeen
On Thu, Jan 19, 2017 at 09:32:47PM +0800, Hou Tao wrote:
> If an inode log item has 4 log operations, and the 4th operation
> (attr fork op) is splitted to the next log record due to the size
> limitation of log record, xfs_logprint doesn't check whether or not
> the 4th operation is in the current log record and print invalid data.
>
> xfs_logprint also needs to calculate the count of splitted log
> operations correctly instead of just returning 1.
>
> The following is the output before patch applied:
...
>
I wasn't necessarily asking to put the full output into the commit log,
but then again I suppose it doesn't hurt. Eric may want to trim this
down...
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
Otherwise this looks good to me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
Thanks for the fixups.
Brian
> logprint/log_misc.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> v3:
> - update commit log description as suggested by Brian Foster
> - add ASSERT(skip_count == 0) as suggested by Brian Foster
>
> v2: https://www.spinics.net/lists/linux-xfs/msg03500.html
> - rewrite the commit message to clarify the patch
> - use "skip_count" suggested by Brian Foster
> - fix the indentation
>
> v1: http://www.spinics.net/lists/linux-xfs/msg03430.html
>
> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> index a0f1766..0dfcfd1 100644
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -524,6 +524,7 @@ xlog_print_trans_inode(
> xfs_inode_log_format_t *f;
> int mode;
> int size;
> + int skip_count;
>
> /*
> * print inode type header region
> @@ -555,15 +556,17 @@ xlog_print_trans_inode(
> return f->ilf_size;
> }
>
> + skip_count = f->ilf_size-1;
> +
> if (*i >= num_ops) /* end of LR */
> - return f->ilf_size-1;
> + return skip_count;
>
> /* core inode comes 2nd */
> op_head = (xlog_op_header_t *)*ptr;
> xlog_print_op_header(op_head, *i, ptr);
>
> if (op_head->oh_flags & XLOG_CONTINUE_TRANS) {
> - return f->ilf_size-1;
> + return skip_count;
> }
>
> memmove(&dino, *ptr, sizeof(dino));
> @@ -571,13 +574,7 @@ xlog_print_trans_inode(
> size = (int)dino.di_size;
> xlog_print_trans_inode_core(&dino);
> *ptr += xfs_log_dinode_size(dino.di_version);
> -
> - if (*i == num_ops-1 && f->ilf_size == 3) {
> - return 1;
> - }
> -
> - /* does anything come next */
> - op_head = (xlog_op_header_t *)*ptr;
> + skip_count--;
>
> switch (f->ilf_fields & (XFS_ILOG_DEV | XFS_ILOG_UUID)) {
> case XFS_ILOG_DEV:
> @@ -595,7 +592,12 @@ xlog_print_trans_inode(
> ASSERT(f->ilf_size <= 4);
> ASSERT((f->ilf_size == 3) || (f->ilf_fields & XFS_ILOG_AFORK));
>
> + /* does anything come next */
> + op_head = (xlog_op_header_t *)*ptr;
> +
> if (f->ilf_fields & XFS_ILOG_DFORK) {
> + if (*i == num_ops-1)
> + return skip_count;
> (*i)++;
> xlog_print_op_header(op_head, *i, ptr);
>
> @@ -618,11 +620,14 @@ xlog_print_trans_inode(
>
> *ptr += be32_to_cpu(op_head->oh_len);
> if (op_head->oh_flags & XLOG_CONTINUE_TRANS)
> - return 1;
> + return skip_count;
> op_head = (xlog_op_header_t *)*ptr;
> + skip_count--;
> }
>
> if (f->ilf_fields & XFS_ILOG_AFORK) {
> + if (*i == num_ops-1)
> + return skip_count;
> (*i)++;
> xlog_print_op_header(op_head, *i, ptr);
>
> @@ -644,9 +649,12 @@ xlog_print_trans_inode(
> }
> *ptr += be32_to_cpu(op_head->oh_len);
> if (op_head->oh_flags & XLOG_CONTINUE_TRANS)
> - return 1;
> + return skip_count;
> + skip_count--;
> }
>
> + ASSERT(skip_count == 0);
> +
> return 0;
> } /* xlog_print_trans_inode */
>
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-01-20 12:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-19 13:32 [PATCH v3] xfs_logprint: handle log operation split of inode item correctly Hou Tao
2017-01-20 12:09 ` Brian Foster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).