> + if (dev->queue_mode == NULL_Q_BIO &&
> + bio_op(cmd->bio) != REQ_OP_FLUSH) {
> + is_flush = false;
> + sector = cmd->bio->bi_iter.bi_sector;
> + size = bio_sectors(cmd->bio);
> + }
> + if (dev->queue_mode != NULL_Q_BIO &&
> + req_op(cmd->rq) != REQ_OP_FLUSH) {
> + is_flush = false;
> + sector = blk_rq_pos(cmd->rq);
> + size = blk_rq_sectors(cmd->rq);
> + }
> + if (is_flush)
> + goto out;
This isn't really new in your patch, but looks very odd. Why not:
if (dev->queue_mode == NULL_Q_BIO) {
if (bio_op(cmd->bio) == REQ_OP_FLUSH)
return BLK_STS_OK;
sector = cmd->bio->bi_iter.bi_sector;
size = bio_sectors(cmd->bio);
} else {
if (req_op(cmd->rq) == REQ_OP_FLUSH)
return BLK_STS_OK;
sector = blk_rq_pos(cmd->rq);
size = blk_rq_sectors(cmd->rq);
}
> + if (badblocks_check(bb, sector, size, &first_bad, &bad_sectors)) {
> + cmd->error = BLK_STS_IOERR;
> + sts = BLK_STS_IOERR;
> + }
> +out:
> + return sts;
Also I find the idea of a goto label that just does a return rather odd.
Please just return directly to make it obvious what is going on.
But in general for the whole series: I'd much prefer moving the
bio vs request handling out of null_handle_cmd and into the callers
rather than hiding them one layer deeper in helpers. Patch 1 is
a good help for that, and anything else factoring out common code,
but code for request vs bio should much rather move to the callers.
