Hi Dongli Thanks for your kindly response. On 3/25/19 3:12 PM, Dongli Zhang wrote: > Hi Jianchao, > > On 3/25/19 1:38 PM, Jianchao Wang wrote: >> tags->rqs[] will not been cleaned when free driver tag to avoid >> an extra store on a shared area in the per io path. But there >> is a window between get driver tag and write tags->rqs[], so we >> may see stale rq in tags->rqs[] which may have been freed, as >> following case, >> blk_mq_get_request blk_mq_queue_tag_busy_iter >> -> blk_mq_get_tag > > Sorry I forgot to mention below when I was studying the v1 patchset. > > If there is further review iteration, I would suggest clarify here that > blk_mq_get_tag() would set the bit in sbitmap (e.g., via __sbitmap_get_word()) > that bt_for_each() would iterate. Otherwise, please ignore my message. > > E.g., > > blk_mq_get_request > -> blk_mq_get_tag > -> taint sbitmap tag bit > without setting tags->rqs[tag] > > -> bt_for_each > -> iterate all dirty bit... > > This would be much more friendly for people not working on block layer and when > they are looking for the potential fix for a bug by simply searching over the > commit messages. People not working on this would be able to quickly understand > there is a window between setting bit and setting data. Yes, I will do that. > > >> -> bt_for_each >> -> bt_iter >> -> rq = taags->rqs[] > > %s/taags/tags/g Yes. Thanks Jianchao > >> -> rq->q >> -> blk_mq_rq_ctx_init >> -> data->hctx->tags->rqs[rq->tag] = rq; >> > > Thank you very much! > > Dongli Zhang >
