On Fri, Jan 17, 2020 at 12:52:10AM -0800, Christoph Hellwig wrote: > Hi Satya, > > On Wed, Jan 08, 2020 at 10:43:05AM -0800, Satya Tangirala wrote: > > The fallback actually is in a separate file, and the software only fields > > are not allocated in the hardware case anymore, either - I should have > > made that clear(er) in the coverletter. > > I see this now, thanks. Either the changes weren't pushed to the > fscrypt report by the time I saw you mail, or I managed to look at a > stale local copy. > > > Alright, I'll look into this. I still think that the keyslot manager > > should maybe go in a separate file because it does a specific, fairly > > self contained task and isn't just block layer code - it's the interface > > between the device drivers and any upper layer. > > So are various other functions in the code like bio_crypt_clone or > bio_crypt_should_process. Also the keyslot_* naming is way to generic, > it really needs a blk_ or blk_crypto_ prefix. > > > > Also what I don't understand is why this managed key-slots on a per-bio > > > basis. Wou;dn't it make a whole lot more sense to manage them on a > > > struct request basis once most of the merging has been performed? > > I don't immediately see an issue with making it work on a struct request > > basis. I'll look into this more carefully. > > I think that should end up being simpler and more efficient. So I tried reading through more of blk-mq and the IO schedulers to figure out how to do this. As far as I can tell, requests may be merged with each other until they're taken off the scheduler. So ideally, we'd program a keyslot for a request when it's taken off the scheduler, but this happens from within an atomic context. Otoh, programming a keyslot might cause the thread to sleep (in the event that all keyslots are in use by other in-flight requests). Unless I'm missing something, or you had some other different idea in mind, I think it's a lot easier to stick to letting blk-crypto program keyslots and manage them per-bio...
