From: Chandan Babu R <chandanrlinux@gmail.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 6/8] xfs: remove need_start_rec parameter from xlog_write()
Date: Wed, 24 Feb 2021 12:47:53 +0530 [thread overview]
Message-ID: <87tuq2orxa.fsf@garuda> (raw)
In-Reply-To: <20210223033442.3267258-7-david@fromorbit.com>
On 23 Feb 2021 at 09:04, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> The CIL push is the only call to xlog_write that sets this variable
> to true. The other callers don't need a start rec, and they tell
> xlog_write what to do by passing the type of ophdr they need written
> in the flags field. The need_start_rec parameter essentially tells
> xlog_write to to write an extra ophdr with a XLOG_START_TRANS type,
> so get rid of the variable to do this and pass XLOG_START_TRANS as
> the flag value into xlog_write() from the CIL push.
>
> $ size fs/xfs/xfs_log.o*
> text data bss dec hex filename
> 27595 560 8 28163 6e03 fs/xfs/xfs_log.o.orig
> 27454 560 8 28022 6d76 fs/xfs/xfs_log.o.patched
>
Looks good to me.
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_log.c | 44 +++++++++++++++++++++----------------------
> fs/xfs/xfs_log_cil.c | 3 ++-
> fs/xfs/xfs_log_priv.h | 3 +--
> 3 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 493454c98c6f..6c3fb6dcb505 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -871,9 +871,7 @@ xlog_wait_on_iclog_lsn(
> static int
> xlog_write_unmount_record(
> struct xlog *log,
> - struct xlog_ticket *ticket,
> - xfs_lsn_t *lsn,
> - uint flags)
> + struct xlog_ticket *ticket)
> {
> struct xfs_unmount_log_format ulf = {
> .magic = XLOG_UNMOUNT_TYPE,
> @@ -890,7 +888,7 @@ xlog_write_unmount_record(
>
> /* account for space used by record data */
> ticket->t_curr_res -= sizeof(ulf);
> - return xlog_write(log, &vec, ticket, lsn, NULL, flags, false);
> + return xlog_write(log, &vec, ticket, NULL, NULL, XLOG_UNMOUNT_TRANS);
> }
>
> /*
> @@ -904,15 +902,13 @@ xlog_unmount_write(
> struct xfs_mount *mp = log->l_mp;
> struct xlog_in_core *iclog;
> struct xlog_ticket *tic = NULL;
> - xfs_lsn_t lsn;
> - uint flags = XLOG_UNMOUNT_TRANS;
> int error;
>
> error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, 0);
> if (error)
> goto out_err;
>
> - error = xlog_write_unmount_record(log, tic, &lsn, flags);
> + error = xlog_write_unmount_record(log, tic);
> /*
> * At this point, we're umounting anyway, so there's no point in
> * transitioning log state to IOERROR. Just continue...
> @@ -1604,8 +1600,7 @@ xlog_commit_record(
> if (XLOG_FORCED_SHUTDOWN(log))
> return -EIO;
>
> - error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS,
> - false);
> + error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
> if (error)
> xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> return error;
> @@ -2202,13 +2197,16 @@ static int
> xlog_write_calc_vec_length(
> struct xlog_ticket *ticket,
> struct xfs_log_vec *log_vector,
> - bool need_start_rec)
> + uint optype)
> {
> struct xfs_log_vec *lv;
> - int headers = need_start_rec ? 1 : 0;
> + int headers = 0;
> int len = 0;
> int i;
>
> + if (optype & XLOG_START_TRANS)
> + headers++;
> +
> for (lv = log_vector; lv; lv = lv->lv_next) {
> /* we don't write ordered log vectors */
> if (lv->lv_buf_len == XFS_LOG_VEC_ORDERED)
> @@ -2428,8 +2426,7 @@ xlog_write(
> struct xlog_ticket *ticket,
> xfs_lsn_t *start_lsn,
> struct xlog_in_core **commit_iclog,
> - uint flags,
> - bool need_start_rec)
> + uint optype)
> {
> struct xlog_in_core *iclog = NULL;
> struct xfs_log_vec *lv = log_vector;
> @@ -2457,8 +2454,9 @@ xlog_write(
> xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> }
>
> - len = xlog_write_calc_vec_length(ticket, log_vector, need_start_rec);
> - *start_lsn = 0;
> + len = xlog_write_calc_vec_length(ticket, log_vector, optype);
> + if (start_lsn)
> + *start_lsn = 0;
> while (lv && (!lv->lv_niovecs || index < lv->lv_niovecs)) {
> void *ptr;
> int log_offset;
> @@ -2472,7 +2470,7 @@ xlog_write(
> ptr = iclog->ic_datap + log_offset;
>
> /* start_lsn is the first lsn written to. That's all we need. */
> - if (!*start_lsn)
> + if (start_lsn && !*start_lsn)
> *start_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
>
> /*
> @@ -2485,6 +2483,7 @@ xlog_write(
> int copy_len;
> int copy_off;
> bool ordered = false;
> + bool wrote_start_rec = false;
>
> /* ordered log vectors have no regions to write */
> if (lv->lv_buf_len == XFS_LOG_VEC_ORDERED) {
> @@ -2502,13 +2501,15 @@ xlog_write(
> * write a start record. Only do this for the first
> * iclog we write to.
> */
> - if (need_start_rec) {
> + if (optype & XLOG_START_TRANS) {
> xlog_write_start_rec(ptr, ticket);
> xlog_write_adv_cnt(&ptr, &len, &log_offset,
> sizeof(struct xlog_op_header));
> + optype &= ~XLOG_START_TRANS;
> + wrote_start_rec = true;
> }
>
> - ophdr = xlog_write_setup_ophdr(log, ptr, ticket, flags);
> + ophdr = xlog_write_setup_ophdr(log, ptr, ticket, optype);
> if (!ophdr)
> return -EIO;
>
> @@ -2539,14 +2540,13 @@ xlog_write(
> }
> copy_len += sizeof(struct xlog_op_header);
> record_cnt++;
> - if (need_start_rec) {
> + if (wrote_start_rec) {
> copy_len += sizeof(struct xlog_op_header);
> record_cnt++;
> - need_start_rec = false;
> }
> data_cnt += contwr ? copy_len : 0;
>
> - error = xlog_write_copy_finish(log, iclog, flags,
> + error = xlog_write_copy_finish(log, iclog, optype,
> &record_cnt, &data_cnt,
> &partial_copy,
> &partial_copy_len,
> @@ -2590,7 +2590,7 @@ xlog_write(
> spin_lock(&log->l_icloglock);
> xlog_state_finish_copy(log, iclog, record_cnt, data_cnt);
> if (commit_iclog) {
> - ASSERT(flags & XLOG_COMMIT_TRANS);
> + ASSERT(optype & XLOG_COMMIT_TRANS);
> *commit_iclog = iclog;
> } else {
> error = xlog_state_release_iclog(log, iclog);
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 8bcacd463f06..4093d2d0db7c 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -827,7 +827,8 @@ xlog_cil_push_work(
> */
> wait_for_completion(&bdev_flush);
>
> - error = xlog_write(log, &lvhdr, tic, &ctx->start_lsn, NULL, 0, true);
> + error = xlog_write(log, &lvhdr, tic, &ctx->start_lsn, NULL,
> + XLOG_START_TRANS);
> if (error)
> goto out_abort_free_ticket;
>
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index a7ac85aaff4e..10a41b1dd895 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -480,8 +480,7 @@ void xlog_print_tic_res(struct xfs_mount *mp, struct xlog_ticket *ticket);
> void xlog_print_trans(struct xfs_trans *);
> int xlog_write(struct xlog *log, struct xfs_log_vec *log_vector,
> struct xlog_ticket *tic, xfs_lsn_t *start_lsn,
> - struct xlog_in_core **commit_iclog, uint flags,
> - bool need_start_rec);
> + struct xlog_in_core **commit_iclog, uint optype);
> int xlog_commit_record(struct xlog *log, struct xlog_ticket *ticket,
> struct xlog_in_core **iclog, xfs_lsn_t *lsn);
> void xfs_log_ticket_ungrant(struct xlog *log, struct xlog_ticket *ticket);
--
chandan
next prev parent reply other threads:[~2021-02-24 7:19 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-23 3:34 [PATCH v2] xfs: various log stuff Dave Chinner
2021-02-23 3:34 ` [PATCH 1/8] xfs: log stripe roundoff is a property of the log Dave Chinner
2021-02-23 10:29 ` Chandan Babu R
2021-02-24 20:14 ` Darrick J. Wong
2021-02-25 8:32 ` Christoph Hellwig
2021-03-01 15:13 ` Brian Foster
2021-02-23 3:34 ` [PATCH 2/8] xfs: separate CIL commit record IO Dave Chinner
2021-02-23 12:12 ` Chandan Babu R
2021-02-24 20:34 ` Darrick J. Wong
2021-02-24 21:44 ` Dave Chinner
2021-02-24 23:06 ` Darrick J. Wong
2021-02-25 8:34 ` Christoph Hellwig
2021-02-25 20:47 ` Dave Chinner
2021-03-01 9:09 ` Christoph Hellwig
2021-03-03 0:11 ` Dave Chinner
2021-02-26 2:48 ` Darrick J. Wong
2021-02-28 16:36 ` Brian Foster
2021-02-28 23:46 ` Dave Chinner
2021-03-01 15:33 ` Brian Foster
2021-03-01 15:19 ` Brian Foster
2021-03-03 0:41 ` Dave Chinner
2021-03-03 15:22 ` Brian Foster
2021-03-04 22:57 ` Dave Chinner
2021-03-05 0:44 ` Dave Chinner
2021-02-23 3:34 ` [PATCH 3/8] xfs: move and rename xfs_blkdev_issue_flush Dave Chinner
2021-02-23 12:57 ` Chandan Babu R
2021-02-24 20:45 ` Darrick J. Wong
2021-02-24 22:01 ` Dave Chinner
2021-02-25 8:36 ` Christoph Hellwig
2021-02-23 3:34 ` [PATCH 4/8] xfs: async blkdev cache flush Dave Chinner
2021-02-23 5:29 ` Chaitanya Kulkarni
2021-02-23 14:02 ` Chandan Babu R
2021-02-24 20:51 ` Darrick J. Wong
2021-02-23 3:34 ` [PATCH 5/8] xfs: CIL checkpoint flushes caches unconditionally Dave Chinner
2021-02-24 7:16 ` Chandan Babu R
2021-02-24 20:57 ` Darrick J. Wong
2021-02-25 8:42 ` Christoph Hellwig
2021-02-25 21:07 ` Dave Chinner
2021-02-23 3:34 ` [PATCH 6/8] xfs: remove need_start_rec parameter from xlog_write() Dave Chinner
2021-02-24 7:17 ` Chandan Babu R [this message]
2021-02-24 20:59 ` Darrick J. Wong
2021-02-25 8:49 ` Christoph Hellwig
2021-02-25 20:55 ` Dave Chinner
2021-02-23 3:34 ` [PATCH 7/8] xfs: journal IO cache flush reductions Dave Chinner
2021-02-23 8:05 ` [PATCH 7/8 v2] " Dave Chinner
2021-02-24 12:27 ` Chandan Babu R
2021-02-24 20:32 ` Dave Chinner
2021-02-24 21:13 ` Darrick J. Wong
2021-02-24 22:03 ` Dave Chinner
2021-02-25 4:09 ` Chandan Babu R
2021-02-25 7:13 ` Chandan Babu R
2021-03-01 5:44 ` Dave Chinner
2021-03-01 5:56 ` Dave Chinner
2021-02-25 8:58 ` Christoph Hellwig
2021-02-25 21:06 ` Dave Chinner
2021-03-01 19:29 ` Brian Foster
2021-02-23 3:34 ` [PATCH 8/8] xfs: Fix CIL throttle hang when CIL space used going backwards Dave Chinner
2021-02-24 21:18 ` Darrick J. Wong
2021-02-24 22:05 ` Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tuq2orxa.fsf@garuda \
--to=chandanrlinux@gmail.com \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.