On Thu, 2017-03-30 at 09:27 -0600, Jens Axboe wrote:
> On 03/29/2017 03:32 PM, Bart Van Assche wrote:
> > Make it possible to check whether or not a block layer queue has
> > been stopped. Make it possible to start and to run a blk-mq queue
> > from user space.
>
> Looks good, minor nitpicking below.
>
> > +static int blk_queue_flags_show(struct seq_file *m, void *v)
> > +{
> > + struct request_queue *q = m->private;
> > + bool sep = false;
> > + int i;
> > +
> > + for (i = 0; i < sizeof(q->queue_flags) * BITS_PER_BYTE; i++) {
> > + if (!(q->queue_flags & BIT(i)))
> > + continue;
> > + if (sep)
> > + seq_puts(m, " ");
> > + sep = true;
>
> Put that sep = true in the else branch?
I can do that, but that would result in slightly less efficient assembler
code (additional jump) while stores can be pipelined easily.
> > + if (strcmp(op, "run") == 0) {
> > + blk_mq_run_hw_queues(q, true);
> > + } else if (strcmp(op, "start") == 0) {
> > + blk_mq_start_stopped_hw_queues(q, true);
> > + } else {
> > + pr_err("%s: unsupported operation %s\n", __func__, op);
> > + return -EINVAL;
> > + }
>
> You are inconsistent with your braces. I'd prefer no braces for single
> lines.
Sorry but if braces are used for one side of an if-then-else statement then
checkpatch wants braces to be used for the other side too, even if that other
side is only a single line. From the checkpatch source code:
# check for single line unbalanced braces
if ($sline =~ /^.\s*\}\s*else\s*$/ ||
$sline =~ /^.\s*else\s*\{\s*$/) {
CHK("BRACES", "Unbalanced braces around else
statement\n" . $herecurr);
}
> Should the error case include valid commands? It'd be nice to have this
> available, and not have to resort to looking at the source to figure out
> what commands are available.
OK, I will update the error message.
Thanks for the review.
Bart.
