Re: [PATCH v4 07/11] io_uring/rw: add support to send meta along with read/write

[Anuj Gupta <anuj20.g@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

> What is the meta_type for?  To distintinguish PI from non-PI metadata?

meta_type field is kept so that meta_types beyond integrity can also
be supported in future. Pavel suggested this to Kanchan when this was
discussed in LSF/MM.

> Why doesn't this support non-PI metadata?

It supports that. We have tested that (pi_type = 0 case).

> Also PI or TO_PI might be
> a better name than the rather generic integrity.  (but I'll defer to
> Martin if he has any good arguments for naming here).

Open to a different/better name.

> 
> >  static bool need_complete_io(struct io_kiocb *req)
> >  {
> > +	struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
> > +
> > +	/* Exclude meta IO as we don't support partial completion for that */
> >  	return req->flags & REQ_F_ISREG ||
> > -		S_ISBLK(file_inode(req->file)->i_mode);
> > +		S_ISBLK(file_inode(req->file)->i_mode) ||
> > +		!(rw->kiocb.ki_flags & IOCB_HAS_METADATA);
> >  }
> 
> What partial ocmpletions aren't supported?  Note that this would
> trigger easily as right now metadata is only added for block devices
> anyway.

It seems that this scenario is less likely to happen. The plumbing
seemed a bit non trivial. I have the plan to look at it, once the
initial version of this series goes in.

> 
> > +	if (unlikely(kiocb->ki_flags & IOCB_HAS_METADATA)) {
> 
> For a workload using metadata this is everything but unlikely.  Is
> there a specific reason you're trying to override the existing
> branch predictor here (although on at least x86_64 gcc these kinds
> of unlikely calls tend to be no-ops anyway).

The branch predictions were added to make it a bit friendly for
non-metadata read/write case. 


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]