On Thu, 5 Jan 2017, Mike Snitzer wrote: > On Thu, Dec 29 2016 at 11:08pm -0500, > Eric Wheeler <dm-devel@xxxxxxxxxxxxxxxxxx> wrote: > > > On Sat, 17 Dec 2016, Mike Snitzer wrote: > > > On Fri, Dec 16 2016 at 5:29pm -0500, > > > Eric Wheeler <dm-devel@xxxxxxxxxxxxxxxxxx> wrote: > > > > On Wed, 14 Dec 2016, Eric Wheeler wrote: > > > > > Since dm-crypt queues writes (and sometimes reads) to a different kernel > > > > > thread (workqueue), the bios will dispatch from tasks with different > > > > > io_context->ioprio settings than the submitting task, thus giving > > > > > incorrect ioprio hints to the io scheduler. > > > > > > > > The motivation of this patch is for ioprio-driven writebackup/bypass > > > > hinting inside bcache when bcache is under dm-crypt which Jens is picking > > > > up for 4.10: > > > > https://lkml.org/lkml/2016/12/6/607 > > > > > > I now see your commits: > > > b71de4659fba4e42c7 bcache: introduce per-process ioprio-based bypass/writeback hints > > > 82e7192711c3855038 bcache: documentation for ioprio cache hinting > > > > > > You'd think this is the type of thing that you'd have proposed to a > > > wider audience. > > > > ( Its not really relevant to this bugfix, but please see this thread since > > you were curious about a wider audience discussion: > > https://www.redhat.com/archives/dm-devel/2016-July/msg00556.html ) > > > > The note above in the previous email was intended to explain how we > > discovered the dm-crypt problem, purely as an example use case. The > > stable commit note discusses the real issue: lost elevator hints. > > > > This commit fixes elevator ioprio hints passing through dm-crypt and is > > not intended to address dm-cache, nor enable a bcache feature. > > > > All impementations using ioprio hints beneath dm-crypt would > > benefit---most importantly, _CFQ_ ! > > > > As it is, all ioprio hints passing through dm-crypt are lost to the > > elevator; the elevator looses those useful bits because of queuing to > > another thread for crypto operations. > > How did you test the change? We discovered this by noticing that bcache never receives bios marked with ioprios that were submitted for writing by dm-crypt, but this seems to be an issue with dm-crypt specifically. Bcache gets ioprio information from upper-level drivers submitting from the original process context just fine. If the information is lost to bcache, it is certainly lost to the IO scheduler. Of course if ioprios are mapped between processes in some other way that I am missing, then please point that out for me. Interestingly, before bio_ioprio was split out from bi_rw in about 4.2-4.4, dm-crypt reads were marked fine because they do not usually get punted to another thread and complete from the submitting process. After splitting bi_ioprio out of bi_rw, reads broke because the simple cloning assignment of dst_bio->bi_rw = src_bio->bi_rw loses the priority bits. Of course this continues now that bi_rw has been pulled and changed into bi_opf. I suspect there are other places where bi_rw assignments lost bi_ioprio assignments long ago, and this probably needs some research and fixup---but that is a separate issue from the dm-crypt issue. > Or put differently: what is the easiest test to run against a dm-crypt > device to verify that ioprio is being passed through? You would probably need to modify dm-crypt just before it calls generic_make_request on the way out of writing bio's after encrypting either in kcryptd_crypt_write_io_submit or dmcrypt_write and calling something like WARN_ONCE if ioprio_valid(bio_prio(bio)). Then set your IO priority using ionice on some process writing with direct IO to the mapper device and see if you get any warnings: ionice -c3 dd bs=1 of=/dev/mapper/cryptotest if=/dev/zero oflag=direct count=1 My guess is that you will not since bios are cloned without copying ioprio information and submitted from processes different from those of the calling thread. I am curious what you find on your end, please let me know if you have any questions and thank you for your help! -- Eric Wheeler > -- > dm-devel mailing list > dm-devel@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/dm-devel > -- To unsubscribe from this list: send the line "unsubscribe linux-bcache" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
