On Mon, Jan 21, 2019 at 9:04 PM Bart Van Assche <bvanassche@xxxxxxx> 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
> Bart.
