On Thu, Feb 25, 2021 at 07:43:38PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 24, 2021 at 05:34:56PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Vector out to an optimised version of xlog_write() if the entire
>
> s/Vector/Factor/ ?
>
> > + ptr = iclog->ic_datap + log_offset;
> > + while (lv && (!lv->lv_niovecs || index < lv->lv_niovecs)) {
> > +
> > +
> > + /* ordered log vectors have no regions to write */
>
> The two empty lines above look a little strange.
>
> > + if (lv->lv_buf_len == XFS_LOG_VEC_ORDERED) {
> > + ASSERT(lv->lv_niovecs == 0);
> > + goto next_lv;
> > + }
> > +
> > + reg = &lv->lv_iovecp[index];
> > + ASSERT(reg->i_len % sizeof(int32_t) == 0);
> > + ASSERT((unsigned long)ptr % sizeof(int32_t) == 0);
> > +
> > + ophdr = reg->i_addr;
> > + ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> > + ophdr->oh_len = cpu_to_be32(reg->i_len -
> > + sizeof(struct xlog_op_header));
> > +
> > + memcpy(ptr, reg->i_addr, reg->i_len);
> > + xlog_write_adv_cnt(&ptr, &len, &log_offset, reg->i_len);
> > + record_cnt++;
> > +
> > + /* move to the next iovec */
> > + if (++index < lv->lv_niovecs)
> > + continue;
> > +next_lv:
> > + /* move to the next logvec */
> > + lv = lv->lv_next;
> > + index = 0;
> > + }
>
> I always hated this (pre-existing) loop style.
Me too, but my brain was stretched just getting it factored into a
tighter loop so I wasn't looking too hard at the iteration methof
itself.
> What do you think of
> something like this (just whiteboard coding, might be completely broken),
> which also handles the ordered case with lv_niovecs == 0 as part of
> the natural loop:
>
> for (lv = log_vector; lv; lv->lv_next) {
> for (index = 0; index < lv->lv_niovecs; index++) {
> struct xfs_log_iovec *reg = &lv->lv_iovecp[index];
> struct xlog_op_header *ophdr = reg->i_addr;
>
> ASSERT(reg->i_len % sizeof(int32_t) == 0);
> ASSERT((unsigned long)ptr % sizeof(int32_t) == 0);
>
> ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> ophdr->oh_len = cpu_to_be32(reg->i_len -
> sizeof(struct xlog_op_header));
> memcpy(ptr, reg->i_addr, reg->i_len);
> xlog_write_adv_cnt(&ptr, &len, &log_offset, reg->i_len);
> record_cnt++;
> }
> }
Yup, that is definitely an improvement. It wasnt' an option with the
way the existing code nested, but this is much, much neater. Thanks!
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
