Re: RFC bio flags and ops in v4.10+

[Bart Van Assche <bvanassche@acm.org>]

On Mon, 2019-01-21 at 21:12 -0800, Ashlie Martinez wrote:
> On Mon, Jan 21, 2019 at 9:04 PM Bart Van Assche <bvanassche@acm.org> wrote:
> > 
> > On 1/21/19 8:45 PM, Ashlie Martinez wrote:
> > > I was working on porting parts of a file system crash consistency
> > > checking tool called CrashMonkey [1] to linux kernels 4.9 and 4.14
> > > when I noticed an inconsistency in how the bio->bi_opf field is
> > > treated. According to the comments in /include/linux/blk_types.h, the
> > > REQ_OP should be the upper bits in bi_opf, while the request flags
> > > should be in the lower bits. In kernel 4.9, the accessor and setter
> > > methods for this field appear to be correct. However, kernels 4.10+ do
> > > not properly shift the REQ_OP argument up for either setting it or
> > > getting it from bi_opf. Is this the intended behavior or is it a
> > > mistake in how the code was written? Please note that I have not
> > > checked all the releases between 4.9 and 5.0-rc3, but I know 4.10,
> > > 4.14, 4.15, and 5.0-rc3 contain the same or similar code for setting
> > > bio->bi_opf.
> > 
> > Hi Ashlie,
> > 
> > Are you perhaps referring to commit ef295ecf090d ("block: better op and
> > flags encoding")? Have you noticed that that commit modified the
> > encoding such that the operation ends up in the lower 8 bits and the
> > flags in the upper 24 bits? That patch namely includes the following change:
> > 
> > +enum req_flag_bits {
> > +       __REQ_FAILFAST_DEV =    /* no driver retries of device errors */
> > +               REQ_OP_BITS,
> > 
> 
> Ah, I missed that assignment hiding on the next line (possibly because
> I've been staring at the code for various projects for too long :) ),
> thanks for pointing it out for me! One (minor) nit about the current
> code then: it appears that the comment on bio->bi_opf  still refers to
> the old arrangement where the REQ_OP occupied the upper bits of the
> field, making it hard to figure out what's happening at a glance

If you would like to submit a patch that updates the header file comments,
I think it would be welcome :-)

Bart.