On Wed, 2017-08-02 at 11:01 +0800, Ming Lei wrote: > On Tue, Aug 01, 2017 at 04:14:07PM +0000, Bart Van Assche wrote: > > On Tue, 2017-08-01 at 18:44 +0800, Ming Lei wrote: > > > On Mon, Jul 31, 2017 at 11:42:21PM +0000, Bart Van Assche wrote: > > > > Since setting, clearing and testing of BLK_MQ_S_BUSY can happen concurrently > > > > and since clearing and testing happens without any locks held I'm afraid this > > > > patch introduces the following race conditions: > > > > [ ... ] > > > > * Checking BLK_MQ_S_BUSY after requests have been removed from the dispatch list > > > > but before that bit is cleared, resulting in test_bit(BLK_MQ_S_BUSY, &hctx->state) > > > > reporting that the BLK_MQ_S_BUSY > > > > has been set although there are no requests > > > > on the dispatch list. > > > > > > That won't be a problem, because dispatch will be started in the > > > context in which dispatch list is flushed, since the BUSY bit > > > is cleared after blk_mq_dispatch_rq_list() returns. So no I/O > > > hang. > > > > Hello Ming, > > > > Please consider changing the name of the BLK_MQ_S_BUSY constant. That bit > > is used to serialize dispatching requests from the hctx dispatch list but > > that's not clear from the name of that constant. > > Actually what we want to do is to stop taking request from sw/scheduler > queue when ->dispatch aren't flushed completely, I think BUSY isn't > a bad name for this case, or how about DISPATCH_BUSY? or > FLUSHING_DISPATCH? Hello Ming, FLUSHING_DISPATCH sounds fine to me. In case you would prefer a shorter name, how about BLK_MQ_S_DISPATCHING (refers to dispatching requests to the driver)? Bart.
