On Tue, Aug 05 2008, David Woodhouse wrote: > According to the comment in blk_rq_io_prep(), about line 2030 of > blk-core.c: > > /* first two bits are identical in rq->cmd_flags and bio->bi_rw */ > rq->cmd_flags |= (bio->bi_rw & 3); > > Is this true? These are the bits in bi_rw: > > #define BIO_RW 0 > #define BIO_RW_AHEAD 1 > #define BIO_RW_BARRIER 2 > #define BIO_RW_FAILFAST 3 > #define BIO_RW_SYNC 4 > #define BIO_RW_META 5 > > And these are the (first) bits in rq->cmd_flags: > __REQ_RW, /* not set, read. set, write */ > __REQ_FAILFAST, /* no low level driver retries */ > __REQ_SORTED, /* elevator knows about this request */ > __REQ_SOFTBARRIER, /* may not be passed by ioscheduler */ > __REQ_HARDBARRIER, /* may not be passed by drive either */ > __REQ_FUA, /* forced unit access */ > > Are we intending that BIO_RW_AHEAD maps to REQ_FAILFAST? When I started > composing this mail I was just going to call it a lie -- but now I think > about it, it makes a certain amount of sense. The comment really ought > to be a little more explicit though, if that's the case. That's not > _identical_, even if it's intentional. > > In blk_alloc_request() we have an even more interesting claim -- this > time it's _three_ bits which are identical: > > /* > * first three bits are identical in rq->cmd_flags and bio->bi_rw, > * see bio.h and blkdev.h > */ > rq->cmd_flags = rw | REQ_ALLOCED; > > AFAICT the only thing passed in via the 'rw' variable is the R/W bit and > maybe REQ_RW_SYNC when it's called from get_request(), so the comment > doesn't matter and can just be removed, I think? That last comment is stale, the first two bits is the correct one (it used to be all three, but not anymore). RA does indeed just map to failfast at the request end, since we don't a lot of error handling on those. But yeah, the comment could do with an explicit note to that effect :-) -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
