| Message ID | 20211118231352.2051947-4-david@fromorbit.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfs: rework xlog_write() | expand |
On 19 Nov 2021 at 04:43, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > Remove another case where xlog_write() has to prepend an opheader to > a log transaction. The unmount record + ophdr is smaller than the > minimum amount of space guaranteed to be free in an iclog (2 * > sizeof(ophdr)) and so we don't have to care about an unmount record > being split across 2 iclogs. > All the corner cases w.r.t xlog_write() were handled correctly by the previous patch. Hence, Reviewed-by: Chandan Babu R <chandan.babu@oracle.com> > Signed-off-by: Dave Chinner <dchinner@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/xfs_log.c | 39 ++++++++++++++++++++++++++++----------- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index e2953ce470de..e5515d0c85c4 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -915,12 +915,22 @@ xlog_write_unmount_record( > struct xlog *log, > struct xlog_ticket *ticket) > { > - struct xfs_unmount_log_format ulf = { > - .magic = XLOG_UNMOUNT_TYPE, > + struct { > + struct xlog_op_header ophdr; > + struct xfs_unmount_log_format ulf; > + } unmount_rec = { > + .ophdr = { > + .oh_clientid = XFS_LOG, > + .oh_tid = cpu_to_be32(ticket->t_tid), > + .oh_flags = XLOG_UNMOUNT_TRANS, > + }, > + .ulf = { > + .magic = XLOG_UNMOUNT_TYPE, > + }, > }; > struct xfs_log_iovec reg = { > - .i_addr = &ulf, > - .i_len = sizeof(ulf), > + .i_addr = &unmount_rec, > + .i_len = sizeof(unmount_rec), > .i_type = XLOG_REG_TYPE_UNMOUNT, > }; > struct xfs_log_vec vec = { > @@ -928,8 +938,12 @@ xlog_write_unmount_record( > .lv_iovecp = ®, > }; > > + BUILD_BUG_ON((sizeof(struct xlog_op_header) + > + sizeof(struct xfs_unmount_log_format)) != > + sizeof(unmount_rec)); > + > /* account for space used by record data */ > - ticket->t_curr_res -= sizeof(ulf); > + ticket->t_curr_res -= sizeof(unmount_rec); > > return xlog_write(log, NULL, &vec, ticket, XLOG_UNMOUNT_TRANS); > } > @@ -2267,6 +2281,8 @@ xlog_write_calc_vec_length( > > /* Don't account for regions with embedded ophdrs */ > if (optype && headers > 0) { > + if (optype & XLOG_UNMOUNT_TRANS) > + headers--; > if (optype & XLOG_START_TRANS) { > ASSERT(headers >= 2); > headers -= 2; > @@ -2472,12 +2488,11 @@ xlog_write( > > /* > * If this is a commit or unmount transaction, we don't need a start > - * record to be written. We do, however, have to account for the > - * commit or unmount header that gets written. Hence we always have > - * to account for an extra xlog_op_header here for commit and unmount > - * records. > + * record to be written. We do, however, have to account for the commit > + * header that gets written. Hence we always have to account for an > + * extra xlog_op_header here for commit records. > */ > - if (optype & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS)) > + if (optype & XLOG_COMMIT_TRANS) > ticket->t_curr_res -= sizeof(struct xlog_op_header); > if (ticket->t_curr_res < 0) { > xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES, > @@ -2541,6 +2556,8 @@ xlog_write( > ophdr = reg->i_addr; > if (index) > optype &= ~XLOG_START_TRANS; > + } else if (optype & XLOG_UNMOUNT_TRANS) { > + ophdr = reg->i_addr; > } else { > ophdr = xlog_write_setup_ophdr(log, ptr, > ticket, optype); > @@ -2571,7 +2588,7 @@ xlog_write( > /* > * Copy region. > * > - * Commit and unmount records just log an opheader, so > + * Commit records just log an opheader, so > * we can have empty payloads with no data region to > * copy. Hence we only copy the payload if the vector > * says it has data to copy.
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index e2953ce470de..e5515d0c85c4 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -915,12 +915,22 @@ xlog_write_unmount_record( struct xlog *log, struct xlog_ticket *ticket) { - struct xfs_unmount_log_format ulf = { - .magic = XLOG_UNMOUNT_TYPE, + struct { + struct xlog_op_header ophdr; + struct xfs_unmount_log_format ulf; + } unmount_rec = { + .ophdr = { + .oh_clientid = XFS_LOG, + .oh_tid = cpu_to_be32(ticket->t_tid), + .oh_flags = XLOG_UNMOUNT_TRANS, + }, + .ulf = { + .magic = XLOG_UNMOUNT_TYPE, + }, }; struct xfs_log_iovec reg = { - .i_addr = &ulf, - .i_len = sizeof(ulf), + .i_addr = &unmount_rec, + .i_len = sizeof(unmount_rec), .i_type = XLOG_REG_TYPE_UNMOUNT, }; struct xfs_log_vec vec = { @@ -928,8 +938,12 @@ xlog_write_unmount_record( .lv_iovecp = ®, }; + BUILD_BUG_ON((sizeof(struct xlog_op_header) + + sizeof(struct xfs_unmount_log_format)) != + sizeof(unmount_rec)); + /* account for space used by record data */ - ticket->t_curr_res -= sizeof(ulf); + ticket->t_curr_res -= sizeof(unmount_rec); return xlog_write(log, NULL, &vec, ticket, XLOG_UNMOUNT_TRANS); } @@ -2267,6 +2281,8 @@ xlog_write_calc_vec_length( /* Don't account for regions with embedded ophdrs */ if (optype && headers > 0) { + if (optype & XLOG_UNMOUNT_TRANS) + headers--; if (optype & XLOG_START_TRANS) { ASSERT(headers >= 2); headers -= 2; @@ -2472,12 +2488,11 @@ xlog_write( /* * If this is a commit or unmount transaction, we don't need a start - * record to be written. We do, however, have to account for the - * commit or unmount header that gets written. Hence we always have - * to account for an extra xlog_op_header here for commit and unmount - * records. + * record to be written. We do, however, have to account for the commit + * header that gets written. Hence we always have to account for an + * extra xlog_op_header here for commit records. */ - if (optype & (XLOG_COMMIT_TRANS | XLOG_UNMOUNT_TRANS)) + if (optype & XLOG_COMMIT_TRANS) ticket->t_curr_res -= sizeof(struct xlog_op_header); if (ticket->t_curr_res < 0) { xfs_alert_tag(log->l_mp, XFS_PTAG_LOGRES, @@ -2541,6 +2556,8 @@ xlog_write( ophdr = reg->i_addr; if (index) optype &= ~XLOG_START_TRANS; + } else if (optype & XLOG_UNMOUNT_TRANS) { + ophdr = reg->i_addr; } else { ophdr = xlog_write_setup_ophdr(log, ptr, ticket, optype); @@ -2571,7 +2588,7 @@ xlog_write( /* * Copy region. * - * Commit and unmount records just log an opheader, so + * Commit records just log an opheader, so * we can have empty payloads with no data region to * copy. Hence we only copy the payload if the vector * says it has data to copy.