Hi, Adam, Adam Manzanares <adam.manzanares@xxxxxxxx> writes: > Patch adds an association between iocontext ioprio and the ioprio of > a request. This feature is only enabled if a queue flag is set to > indicate that requests should have ioprio associated with them. The > queue flag is exposed as the req_prio queue sysfs entry. > > Signed-off-by: Adam Mananzanares <adam.manzanares@xxxxxxxx> I like the idea of the patch, but I have a few comments. First, don't add a tunable, there's no need for it. (And in the future, if you do add tunables, document them.) That should make your patch much smaller. > @@ -1648,6 +1649,7 @@ out: > > void init_request_from_bio(struct request *req, struct bio *bio) > { > + struct io_context *ioc = rq_ioc(bio); That can return NULL, and you blindly dereference it later. > @@ -1656,7 +1658,11 @@ void init_request_from_bio(struct request *req, struct bio *bio) > > req->errors = 0; > req->__sector = bio->bi_iter.bi_sector; > - req->ioprio = bio_prio(bio); > + if (blk_queue_req_prio(req->q)) > + req->ioprio = ioprio_best(bio_prio(bio), ioc->ioprio); > + else > + req->ioprio = bio_prio(bio); > + If the bio actually has an ioprio (only happens for bcache at this point), you should use it. Something like this: req->ioprio = bio_prio(bio); if (!req->ioprio && ioc) req->ioprio = ioc->ioprio; Finally, please re-order your series as Hannes suggested. Thanks! Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html