On Thu, Feb 25, 2021 at 07:27:20PM +0100, Christoph Hellwig wrote:
> > + if (optype && index) {
> > + optype &= ~XLOG_START_TRANS;
> > + } else if (partial_copy) {
> > ophdr = xlog_write_setup_ophdr(ptr, ticket);
>
> This line uses whitespaces for indentation, we should probably fix that
> up somewhere in the series.
It goes away entirely so, yes, it is fixed up :)
> > static inline void *
> > xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> > uint type)
> > {
> > - struct xfs_log_iovec *vec = *vecp;
> > + struct xfs_log_iovec *vec = *vecp;
> > + struct xlog_op_header *oph;
> > + uint32_t len;
> > + void *buf;
> >
> > if (vec) {
> > ASSERT(vec - lv->lv_iovecp < lv->lv_niovecs);
> > @@ -44,21 +54,36 @@ xlog_prepare_iovec(struct xfs_log_vec *lv, struct xfs_log_iovec **vecp,
> > vec = &lv->lv_iovecp[0];
> > }
> >
> > - if (!IS_ALIGNED(lv->lv_buf_len, sizeof(uint64_t)))
> > - lv->lv_buf_len = round_up(lv->lv_buf_len, sizeof(uint64_t));
> > + len = lv->lv_buf_len + sizeof(struct xlog_op_header);
> > + if (!IS_ALIGNED(len, sizeof(uint64_t))) {
> > + lv->lv_buf_len = round_up(len, sizeof(uint64_t)) -
> > + sizeof(struct xlog_op_header);
> > + }
> >
> > vec->i_type = type;
> > vec->i_addr = lv->lv_buf + lv->lv_buf_len;
> >
> > - ASSERT(IS_ALIGNED((unsigned long)vec->i_addr, sizeof(uint64_t)));
> > + oph = vec->i_addr;
> > + oph->oh_clientid = XFS_TRANSACTION;
> > + oph->oh_res2 = 0;
> > + oph->oh_flags = 0;
> > +
> > + buf = vec->i_addr + sizeof(struct xlog_op_header);
> > + ASSERT(IS_ALIGNED((unsigned long)buf, sizeof(uint64_t)));
> >
> > *vecp = vec;
> > - return vec->i_addr;
> > + return buf;
> > }
>
> I think this function is growing a little too larger to stay inlined.
Possibly. let me have a look at code size and if it does make a
difference I'll move it out of line in another patch.
>
> > - nbytes += niovecs * sizeof(uint64_t);
> > + nbytes += niovecs * (sizeof(uint64_t) +
> > + sizeof(struct xlog_op_header));;
>
> Is it just me, or would
>
> nbytes += niovecs *
> (sizeof(uint64_t) + sizeof(struct xlog_op_header));
>
> be a little easier to read?
Yes, that's better.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
