[PATCH 1/1] xfs: remove lsn relevant fields from xfs_trans structure and its users

[PATCH 1/1] xfs: remove lsn relevant fields from xfs_trans structure and its users

[Shan Hai]

The t_lsn is not used anymore and the t_commit_lsn is used as a tmp
storage for the checkpoint sequence number only in the current code.

And the start/commit lsn are tracked as a transaction group tag in
the xfs_cil_ctx instead of a single transaction, so remove them from
the xfs_trans structure and their users to match with the design.

Signed-off-by: Shan Hai <shan.hai@oracle.com>
---
 fs/xfs/xfs_log_cil.c |  5 ++---
 fs/xfs/xfs_trace.h   | 19 -------------------
 fs/xfs/xfs_trans.h   |  4 ----
 3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 82f1cbc..373f5e8 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -990,9 +990,8 @@
 	if (tp->t_ticket->t_curr_res < 0)
 		xlog_print_tic_res(mp, tp->t_ticket);
 
-	tp->t_commit_lsn = cil->xc_ctx->sequence;
 	if (commit_lsn)
-		*commit_lsn = tp->t_commit_lsn;
+		*commit_lsn = cil->xc_ctx->sequence;
 
 	xfs_log_done(mp, tp->t_ticket, NULL, regrant);
 	xfs_trans_unreserve_and_mod_sb(tp);
@@ -1008,7 +1007,7 @@
 	 * the log items. This affects (at least) processing of stale buffers,
 	 * inodes and EFIs.
 	 */
-	xfs_trans_free_items(tp, tp->t_commit_lsn, false);
+	xfs_trans_free_items(tp, cil->xc_ctx->sequence, false);
 
 	xlog_cil_push_background(log);
 
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 7c5a165..0712eb7 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -1490,25 +1490,6 @@
 		  __entry->tlen)
 );
 
-TRACE_EVENT(xfs_trans_commit_lsn,
-	TP_PROTO(struct xfs_trans *trans),
-	TP_ARGS(trans),
-	TP_STRUCT__entry(
-		__field(dev_t, dev)
-		__field(struct xfs_trans *, tp)
-		__field(xfs_lsn_t, lsn)
-	),
-	TP_fast_assign(
-		__entry->dev = trans->t_mountp->m_super->s_dev;
-		__entry->tp = trans;
-		__entry->lsn = trans->t_commit_lsn;
-	),
-	TP_printk("dev %d:%d trans 0x%p commit_lsn 0x%llx",
-		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->tp,
-		  __entry->lsn)
-);
-
 TRACE_EVENT(xfs_agf,
 	TP_PROTO(struct xfs_mount *mp, struct xfs_agf *agf, int flags,
 		 unsigned long caller_ip),
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index a07acbf..80b3ed5 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -105,10 +105,6 @@ void	xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
 	unsigned int		t_rtx_res;	/* # of rt extents resvd */
 	unsigned int		t_rtx_res_used;	/* # of resvd rt extents used */
 	struct xlog_ticket	*t_ticket;	/* log mgr ticket */
-	xfs_lsn_t		t_lsn;		/* log seq num of start of
-						 * transaction. */
-	xfs_lsn_t		t_commit_lsn;	/* log seq num of end of
-						 * transaction. */
 	struct xfs_mount	*t_mountp;	/* ptr to fs mount struct */
 	struct xfs_dquot_acct   *t_dqinfo;	/* acctg info for dquots */
 	unsigned int		t_flags;	/* misc flags */
-- 
1.8.3.1

Re: [PATCH 1/1] xfs: remove lsn relevant fields from xfs_trans structure and its users

[Christoph Hellwig]

On Thu, May 11, 2017 at 02:40:09PM +0800, Shan Hai wrote:
> The t_lsn is not used anymore and the t_commit_lsn is used as a tmp
> storage for the checkpoint sequence number only in the current code.
> 
> And the start/commit lsn are tracked as a transaction group tag in
> the xfs_cil_ctx instead of a single transaction, so remove them from
> the xfs_trans structure and their users to match with the design.
> 
> Signed-off-by: Shan Hai <shan.hai@oracle.com>
> ---
>  fs/xfs/xfs_log_cil.c |  5 ++---
>  fs/xfs/xfs_trace.h   | 19 -------------------
>  fs/xfs/xfs_trans.h   |  4 ----
>  3 files changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 82f1cbc..373f5e8 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -990,9 +990,8 @@
>  	if (tp->t_ticket->t_curr_res < 0)
>  		xlog_print_tic_res(mp, tp->t_ticket);
>  
> -	tp->t_commit_lsn = cil->xc_ctx->sequence;
>  	if (commit_lsn)
> -		*commit_lsn = tp->t_commit_lsn;
> +		*commit_lsn = cil->xc_ctx->sequence;
>  
>  	xfs_log_done(mp, tp->t_ticket, NULL, regrant);
>  	xfs_trans_unreserve_and_mod_sb(tp);
> @@ -1008,7 +1007,7 @@
>  	 * the log items. This affects (at least) processing of stale buffers,
>  	 * inodes and EFIs.
>  	 */
> -	xfs_trans_free_items(tp, tp->t_commit_lsn, false);
> +	xfs_trans_free_items(tp, cil->xc_ctx->sequence, false);

Can we add a local variable to store cil->xc_ctx->sequence to make
this a little more clear?

Otherwise this looks fine to me.

Re: [PATCH 1/1] xfs: remove lsn relevant fields from xfs_trans structure and its users

[Shan Hai]

On 2017年05月11日 20:11, Christoph Hellwig wrote:
> On Thu, May 11, 2017 at 02:40:09PM +0800, Shan Hai wrote:
>> The t_lsn is not used anymore and the t_commit_lsn is used as a tmp
>> storage for the checkpoint sequence number only in the current code.
>>
>> And the start/commit lsn are tracked as a transaction group tag in
>> the xfs_cil_ctx instead of a single transaction, so remove them from
>> the xfs_trans structure and their users to match with the design.
>>
>> Signed-off-by: Shan Hai <shan.hai@oracle.com>
>> ---
>>   fs/xfs/xfs_log_cil.c |  5 ++---
>>   fs/xfs/xfs_trace.h   | 19 -------------------
>>   fs/xfs/xfs_trans.h   |  4 ----
>>   3 files changed, 2 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
>> index 82f1cbc..373f5e8 100644
>> --- a/fs/xfs/xfs_log_cil.c
>> +++ b/fs/xfs/xfs_log_cil.c
>> @@ -990,9 +990,8 @@
>>   	if (tp->t_ticket->t_curr_res < 0)
>>   		xlog_print_tic_res(mp, tp->t_ticket);
>>   
>> -	tp->t_commit_lsn = cil->xc_ctx->sequence;
>>   	if (commit_lsn)
>> -		*commit_lsn = tp->t_commit_lsn;
>> +		*commit_lsn = cil->xc_ctx->sequence;
>>   
>>   	xfs_log_done(mp, tp->t_ticket, NULL, regrant);
>>   	xfs_trans_unreserve_and_mod_sb(tp);
>> @@ -1008,7 +1007,7 @@
>>   	 * the log items. This affects (at least) processing of stale buffers,
>>   	 * inodes and EFIs.
>>   	 */
>> -	xfs_trans_free_items(tp, tp->t_commit_lsn, false);
>> +	xfs_trans_free_items(tp, cil->xc_ctx->sequence, false);
> Can we add a local variable to store cil->xc_ctx->sequence to make
> this a little more clear?

Ok, will be fixed in v2 patch.

Thanks
Shan Hai

> Otherwise this looks fine to me.
> --
> 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