From: Christoph Hellwig <hch@infradead.org> To: Eric Biggers <ebiggers@kernel.org> Cc: Christoph Hellwig <hch@infradead.org>, Satya Tangirala <satyat@google.com>, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Kim Boojin <boojin.kim@samsung.com> Subject: Re: [PATCH v6 0/9] Inline Encryption Support Date: Wed, 5 Feb 2020 10:05:41 -0800 [thread overview] Message-ID: <20200205180541.GA32041@infradead.org> (raw) In-Reply-To: <20200205073601.GA191054@sol.localdomain> On Tue, Feb 04, 2020 at 11:36:01PM -0800, Eric Biggers wrote: > The vendor-specific SMC calls do seem to work in atomic context, at least on > SDA845. However, in ufshcd_program_key(), the calls to pm_runtime_get_sync() > and ufshcd_hold() can also sleep. > > I think we can move the pm_runtime_get_sync() to ufshcd_crypto_keyslot_evict(), > since the block layer already ensures the device is not runtime-suspended while > requests are being processed (see blk_queue_enter()). I.e., keyslots can be > evicted independently of any bio, but that's not the case for programming them. Yes. > That still leaves ufshcd_hold(), which is still needed to ungate the UFS clocks. > It does accept an 'async' argument, which is used by ufshcd_queuecommand() to > schedule work to ungate the clocks and return SCSI_MLQUEUE_HOST_BUSY. > > So in blk_mq_dispatch_rq_list(), we could potentially try to acquire the > keyslot, and if it can't be done because either none are available or because > something else needs to be waited for, we can put the request back on the > dispatch list -- similar to how failure to get a driver tag is handled. Yes, that is what I had in mind. > However, if I understand correctly, that would mean that all requests to the > same hardware queue would be blocked whenever someone is waiting for a keyslot > -- even unencrypted requests and requests for unrelated keyslots. At least for an initial dumb implementation, yes. But if we care enough we can improve the code to check for the encrypted flag and only put back encrypted flags in that case. > It's possible that would still be fine for the Android use case, as vendors tend > to add enough keyslots to work with Android, provided that they choose the > fscrypt format that uses one key per encryption policy rather than one key per > file. I.e., it might be the case that no one waits for keyslots in practice > anyway. But, it seems it would be undesirable for a general Linux kernel > framework, which could potentially be used with per-file keys or with hardware > that only has a *very* small number of keyslots. > > Another option would be to allocate the keyslot in blk_mq_get_request(), where > sleeping is still allowed, but some merging was already done. That is another good idea. In blk_mq_get_request we acquire other resources like the tag, so this would be a very logical places to acquire the key slots. We can should also be able to still merge into the request while it is waiting.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org> To: Eric Biggers <ebiggers@kernel.org> Cc: linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, Kim Boojin <boojin.kim@samsung.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Satya Tangirala <satyat@google.com>, Christoph Hellwig <hch@infradead.org>, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH v6 0/9] Inline Encryption Support Date: Wed, 5 Feb 2020 10:05:41 -0800 [thread overview] Message-ID: <20200205180541.GA32041@infradead.org> (raw) In-Reply-To: <20200205073601.GA191054@sol.localdomain> On Tue, Feb 04, 2020 at 11:36:01PM -0800, Eric Biggers wrote: > The vendor-specific SMC calls do seem to work in atomic context, at least on > SDA845. However, in ufshcd_program_key(), the calls to pm_runtime_get_sync() > and ufshcd_hold() can also sleep. > > I think we can move the pm_runtime_get_sync() to ufshcd_crypto_keyslot_evict(), > since the block layer already ensures the device is not runtime-suspended while > requests are being processed (see blk_queue_enter()). I.e., keyslots can be > evicted independently of any bio, but that's not the case for programming them. Yes. > That still leaves ufshcd_hold(), which is still needed to ungate the UFS clocks. > It does accept an 'async' argument, which is used by ufshcd_queuecommand() to > schedule work to ungate the clocks and return SCSI_MLQUEUE_HOST_BUSY. > > So in blk_mq_dispatch_rq_list(), we could potentially try to acquire the > keyslot, and if it can't be done because either none are available or because > something else needs to be waited for, we can put the request back on the > dispatch list -- similar to how failure to get a driver tag is handled. Yes, that is what I had in mind. > However, if I understand correctly, that would mean that all requests to the > same hardware queue would be blocked whenever someone is waiting for a keyslot > -- even unencrypted requests and requests for unrelated keyslots. At least for an initial dumb implementation, yes. But if we care enough we can improve the code to check for the encrypted flag and only put back encrypted flags in that case. > It's possible that would still be fine for the Android use case, as vendors tend > to add enough keyslots to work with Android, provided that they choose the > fscrypt format that uses one key per encryption policy rather than one key per > file. I.e., it might be the case that no one waits for keyslots in practice > anyway. But, it seems it would be undesirable for a general Linux kernel > framework, which could potentially be used with per-file keys or with hardware > that only has a *very* small number of keyslots. > > Another option would be to allocate the keyslot in blk_mq_get_request(), where > sleeping is still allowed, but some merging was already done. That is another good idea. In blk_mq_get_request we acquire other resources like the tag, so this would be a very logical places to acquire the key slots. We can should also be able to still merge into the request while it is waiting. _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2020-02-05 18:05 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-18 14:51 [PATCH v6 0/9] Inline Encryption Support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 14:51 ` [PATCH v6 1/9] block: Keyslot Manager for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 20:13 ` Eric Biggers 2019-12-18 20:13 ` [f2fs-dev] " Eric Biggers 2020-01-17 9:10 ` Christoph Hellwig 2020-01-17 9:10 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 2/9] block: Add encryption context to struct bio Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-18 21:10 ` Eric Biggers 2019-12-18 21:10 ` [f2fs-dev] " Eric Biggers 2019-12-18 21:21 ` Darrick J. Wong 2019-12-18 21:21 ` [f2fs-dev] " Darrick J. Wong 2019-12-18 21:25 ` Martin K. Petersen 2019-12-18 21:25 ` [f2fs-dev] " Martin K. Petersen 2019-12-18 22:27 ` Eric Biggers 2019-12-18 22:27 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:47 ` Martin K. Petersen 2019-12-19 0:47 ` [f2fs-dev] " Martin K. Petersen 2019-12-20 3:52 ` Eric Biggers 2019-12-20 3:52 ` [f2fs-dev] " Eric Biggers 2020-01-07 4:35 ` Martin K. Petersen 2020-01-07 4:35 ` [f2fs-dev] " Martin K. Petersen 2020-01-08 14:07 ` Christoph Hellwig 2020-01-08 14:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 17:26 ` Eric Biggers 2020-01-08 17:26 ` [f2fs-dev] " Eric Biggers 2020-01-17 8:32 ` Christoph Hellwig 2020-01-17 8:32 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:11 ` Eric Biggers 2020-01-18 5:11 ` [f2fs-dev] " Eric Biggers 2020-01-21 22:05 ` Satya Tangirala 2020-01-21 22:05 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-09 3:40 ` Martin K. Petersen 2020-01-09 3:40 ` [f2fs-dev] " Martin K. Petersen 2020-01-14 21:24 ` Eric Biggers 2020-01-14 21:24 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 3/9] block: blk-crypto for Inline Encryption Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 3:14 ` Eric Biggers 2019-12-20 3:14 ` [f2fs-dev] " Eric Biggers 2019-12-20 5:10 ` Eric Biggers 2019-12-20 5:10 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:22 ` Eric Biggers 2020-01-14 21:22 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 4/9] scsi: ufs: UFS driver v2.1 spec crypto additions Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 12:31 ` Christoph Hellwig 2020-01-17 12:31 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 5/9] scsi: ufs: UFS crypto API Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:48 ` Eric Biggers 2019-12-20 4:48 ` [f2fs-dev] " Eric Biggers 2020-01-14 21:16 ` Eric Biggers 2020-01-14 21:16 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:51 ` Christoph Hellwig 2020-01-17 13:51 ` [f2fs-dev] " Christoph Hellwig 2019-12-18 14:51 ` [PATCH v6 6/9] scsi: ufs: Add inline encryption support to UFS Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 5:44 ` Eric Biggers 2019-12-20 5:44 ` [f2fs-dev] " Eric Biggers 2020-01-17 13:58 ` Christoph Hellwig 2020-01-17 13:58 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 5:27 ` Eric Biggers 2020-01-18 5:27 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:07 ` Christoph Hellwig 2020-02-05 18:07 ` [f2fs-dev] " Christoph Hellwig 2020-01-18 3:58 ` Eric Biggers 2020-01-18 3:58 ` [f2fs-dev] " Eric Biggers 2020-02-05 20:47 ` Eric Biggers 2020-02-05 20:47 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 7/9] fscrypt: add inline encryption support Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-14 21:12 ` Eric Biggers 2020-01-14 21:12 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 8/9] f2fs: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-20 4:23 ` Eric Biggers 2019-12-20 4:23 ` [f2fs-dev] " Eric Biggers 2019-12-18 14:51 ` [PATCH v6 9/9] ext4: " Satya Tangirala 2019-12-18 14:51 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-19 0:12 ` Eric Biggers 2019-12-19 0:12 ` [f2fs-dev] " Eric Biggers 2019-12-19 0:31 ` Satya Tangirala 2019-12-19 0:31 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-12-22 0:16 ` Eric Biggers 2019-12-22 0:16 ` [f2fs-dev] " Eric Biggers 2020-01-08 14:05 ` [PATCH v6 0/9] Inline Encryption Support Christoph Hellwig 2020-01-08 14:05 ` [f2fs-dev] " Christoph Hellwig 2020-01-08 18:43 ` Satya Tangirala 2020-01-08 18:43 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-01-17 8:52 ` Christoph Hellwig 2020-01-17 8:52 ` [f2fs-dev] " Christoph Hellwig 2020-02-01 0:53 ` Satya Tangirala 2020-02-01 0:53 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-03 9:15 ` Christoph Hellwig 2020-02-03 9:15 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 3:39 ` Satya Tangirala 2020-02-04 3:39 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-04 14:58 ` Christoph Hellwig 2020-02-04 14:58 ` [f2fs-dev] " Christoph Hellwig 2020-02-04 21:21 ` Eric Biggers 2020-02-04 21:21 ` [f2fs-dev] " Eric Biggers 2020-02-05 7:36 ` Eric Biggers 2020-02-05 7:36 ` [f2fs-dev] " Eric Biggers 2020-02-05 18:05 ` Christoph Hellwig [this message] 2020-02-05 18:05 ` Christoph Hellwig 2020-02-21 12:30 ` Satya Tangirala 2020-02-21 12:30 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2020-02-21 14:20 ` Christoph Hellwig 2020-02-21 14:20 ` [f2fs-dev] " Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200205180541.GA32041@infradead.org \ --to=hch@infradead.org \ --cc=bmuthuku@qti.qualcomm.com \ --cc=boojin.kim@samsung.com \ --cc=ebiggers@kernel.org \ --cc=kuohong.wang@mediatek.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=satyat@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.