Tejun Heo <tj@xxxxxxxxxx> writes: >> -#define REQ_CLONE_MASK REQ_COMMON_MASK >> +/* >> + * Cloned requests are inserted into the elevator via blk_insert_cloned_request. >> + * Because the flush flags exported by the request-based dm target may in >> + * theory be different from the flush flags of the underlying request_queue, >> + * we need to pass along information regarding whether a particular request >> + * is part of a flush sequence. This is primarily used to complete I/Os early >> + * that would otherwise not be necessary (such as an empty flush for a request >> + * queue that does not support flush). In such a case, the end_io path for >> + * the request would try to account the I/O instead of ignoring it, resulting >> + * in a null pointer dereference. >> + */ >> +#define REQ_CLONE_MASK (REQ_COMMON_MASK | REQ_FLUSH_SEQ) > > I'm probably missing something, but why do we still need to copy > REQ_FLUSH_SEQ? Why doesn't the following work? > > * dm driver always advertises REQ_FLUSH|FUA like other stacking > drivers. > > * blk-flush for the dm, decomposes flushes to FLUSH + FUA write and > send it down. > > * dm driver clones the requests and send them down to each member > queue. > > * blk-flush on member queue, handles FLUSH as FLUSH and decomposes FUA > write as necessary. > > What am I missing? Why does end_io path still matter when it goes > through blk-flush on the member device too? You're missing the I/O completion of an empty flush trying to do I/O accounting, and oopsing, as shown in the stack trace I provided before. We could avoid passing REQ_FLUSH_SEQ, and then set it when completing an empty flush, but I thought that was even worse. Or, maybe we could clear REQ_IO_STAT when completing such requests. -Jeff -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
