On Mon, 2017-09-25 at 22:06 +0000, Bart Van Assche wrote:
> On Mon, 2017-09-25 at 15:14 +0900, Damien Le Moal wrote:
> > - return rq_entry_fifo(dd->fifo_list[data_dir].next);
> > + if (!dd->zones_wlock || data_dir == READ)
> > + return rq_entry_fifo(dd->fifo_list[data_dir].next);
> > +
> > + spin_lock_irqsave(&dd->zone_lock, flags);
> > +
> > + list_for_each_entry(rq, &dd->fifo_list[WRITE], queuelist) {
> > + if (deadline_can_dispatch_request(dd, rq))
> > + goto out;
> > + }
> > + rq = NULL;
> > +
> > +out:
> > + spin_unlock_irqrestore(&dd->zone_lock, flags);
>
> Is it documented somewhere what dd->zone_lock protects and when that lock
> should be
> acquired?
It was not well explained. I added comments in V6.
>
> > /*
> > * This may be a requeue of a request that has locked its
> > - * target zone. If this is the case, release the request zone
> > lock.
> > + * target zone. If this is the case, release the zone lock.
> > */
> > if (deadline_request_has_zone_wlock(rq))
> > deadline_wunlock_zone(dd, rq);
>
> Can this change be folded into the patch that introduced that comment?
Of course. Fixed in V6.
> > @@ -570,6 +621,9 @@ static void dd_insert_request(struct blk_mq_hw_ctx
> > *hctx, struct request *rq,
> >
> > blk_mq_sched_request_inserted(rq);
> >
> > + if (at_head && deadline_request_needs_zone_wlock(dd, rq))
> > + pr_info("######## Write at head !\n");
> > +
> > if (at_head || blk_rq_is_passthrough(rq)) {
> > if (at_head)
> > list_add(&rq->queuelist, &dd->dispatch);
>
> Will it be easy to users who analyze a kernel log to figure out why that
> message has been generated? Should that message perhaps include the block
> device name, zone number and request sector number?
This was just a debug message for me that I forgot to remove. I did in V6.
Thanks for catching this.
Best regards.
--
Damien Le Moal
Western Digital
