Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1267058 > unrolled thread
| Started by | Baolin Wang <baolin.wang@linaro.org> |
|---|---|
| First post | 2015-11-11 10:40 +0100 |
| Last post | 2015-11-12 10:10 +0100 |
| Articles | 9 on this page of 29 — 7 participants |
Back to article view | Back to linux.kernel
[PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-11 10:40 +0100
[PATCH 1/2] block: Introduce BIO_ENDIO_FREE for bio flags Baolin Wang <baolin.wang@linaro.org> - 2015-11-11 10:40 +0100
Re: [PATCH 1/2] block: Introduce BIO_ENDIO_FREE for bio flags Mike Snitzer <snitzer@redhat.com> - 2015-11-11 19:00 +0100
Re: [PATCH 1/2] block: Introduce BIO_ENDIO_FREE for bio flags Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 05:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Christoph Hellwig <hch@infradead.org> - 2015-11-11 10:50 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 03:20 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Jan Kara <jack@suse.cz> - 2015-11-12 10:20 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 10:50 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Jan Kara <jack@suse.cz> - 2015-11-12 12:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 12:50 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Jan Kara <jack@suse.cz> - 2015-11-12 13:30 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Arnd Bergmann <arnd@arndb.de> - 2015-11-12 14:00 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Mark Brown <broonie@kernel.org> - 2015-11-12 16:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-13 04:30 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-13 04:30 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 14:00 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Jan Kara <jack@suse.cz> - 2015-11-12 14:00 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-13 03:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Arnd Bergmann <arnd@arndb.de> - 2015-11-13 10:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-13 12:40 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Mike Snitzer <snitzer@redhat.com> - 2015-11-11 19:20 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 03:40 +0100
Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt Christoph Hellwig <hch@infradead.org> - 2015-11-12 10:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-12 09:30 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Mark Brown <broonie@kernel.org> - 2015-11-12 11:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Jens Axboe <axboe@kernel.dk> - 2015-11-12 16:30 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Baolin Wang <baolin.wang@linaro.org> - 2015-11-13 03:10 +0100
Re: [PATCH 0/2] Introduce the request handling for dm-crypt Mark Brown <broonie@kernel.org> - 2015-11-13 13:00 +0100
Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt Christoph Hellwig <hch@infradead.org> - 2015-11-12 10:10 +0100
Page 2 of 2 — ← Prev page 1 [2]
| From | Mike Snitzer <snitzer@redhat.com> |
|---|---|
| Date | 2015-11-11 19:20 +0100 |
| Message-ID | <qtI9X-6iG-11@gated-at.bofh.it> |
| In reply to | #1267058 |
On Wed, Nov 11 2015 at 4:31am -0500, Baolin Wang <baolin.wang@linaro.org> wrote: > Now the dm-crypt code only implemented the 'based-bio' method to encrypt/ > decrypt block data, which can only hanle one bio at one time. As we know, > one bio must use the sequential physical address and it also has a limitation > of length. Thus it may limit the big block encyrtion/decryption when some > hardware support the big block data encryption. > > This patch series introduc the 'based-request' method to handle the data > encryption/decryption. One request can contain multiple bios, so it can > handle big block data to improve the efficiency. The duality of bio-based vs request-based code paths in DM core frankly sucks. So the prospect of polluting dm-crypt with a similar duality is really _not_ interesting. Request-based DM requires more memory reserves per device than bio-based DM. Also, you cannot stack request-based DM ontop of bio-based devices (be them DM, MD, etc) so request-based DM's underlying storage stack gets a lot less interesting with this change. That said, it could be that the benefits of supporting both bio-based and request-based DM in dm-crypt outweigh any overhead/limitations. But you haven't given any performance data to justify this patchset. There needs to be a _really_ compelling benefit to do this. Also, FYI, having a big CONFIG knob to switch all of dm-crypt from bio-based to request-based is _not_ acceptable. Both modes would need to be supported in parallel. Could easily be that not all devices in a system will benefit from being request-based. Regardless, the risk of this change causing request-based DM to become more brittle than it already is concerns me. But I'm trying to keep an open mind... show me data that real hardware _really_ benefits and we'll go from there. Again, it needs to be "OMG, this is amazing!" level performance to warrant any further serious consideration. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Baolin Wang <baolin.wang@linaro.org> |
|---|---|
| Date | 2015-11-12 03:40 +0100 |
| Message-ID | <qtPXP-2RZ-3@gated-at.bofh.it> |
| In reply to | #1267364 |
On 12 November 2015 at 02:18, Mike Snitzer <snitzer@redhat.com> wrote: > On Wed, Nov 11 2015 at 4:31am -0500, > Baolin Wang <baolin.wang@linaro.org> wrote: > >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/ >> decrypt block data, which can only hanle one bio at one time. As we know, >> one bio must use the sequential physical address and it also has a limitation >> of length. Thus it may limit the big block encyrtion/decryption when some >> hardware support the big block data encryption. >> >> This patch series introduc the 'based-request' method to handle the data >> encryption/decryption. One request can contain multiple bios, so it can >> handle big block data to improve the efficiency. > > The duality of bio-based vs request-based code paths in DM core frankly > sucks. So the prospect of polluting dm-crypt with a similar duality is > really _not_ interesting. > That's right. But we'll not introduce the duality things, cause we will remove the bio based things in dm-crypt if the request based things are accepted. > Request-based DM requires more memory reserves per device than bio-based > DM. Also, you cannot stack request-based DM ontop of bio-based devices > (be them DM, MD, etc) so request-based DM's underlying storage stack > gets a lot less interesting with this change. > Yes, the request based requires more memory than bio based, but it is not too much. And the request based has a big performance improvement. > That said, it could be that the benefits of supporting both bio-based > and request-based DM in dm-crypt outweigh any overhead/limitations. But > you haven't given any performance data to justify this patchset. > Like I said above, we plan to remove the bio based things which are not good support for hardware engine encryption. And I'll show you the performance data to prove the request things have a good performance. > There needs to be a _really_ compelling benefit to do this. > > Also, FYI, having a big CONFIG knob to switch all of dm-crypt from > bio-based to request-based is _not_ acceptable. Both modes would need > to be supported in parallel. Could easily be that not all devices in a > system will benefit from being request-based. > OK. The CONFIG is not suitable here. I'll remove the CONFIG with just enable the request based things. > Regardless, the risk of this change causing request-based DM to become > more brittle than it already is concerns me. > > But I'm trying to keep an open mind... show me data that real hardware > _really_ benefits and we'll go from there. Again, it needs to be "OMG, > this is amazing!" level performance to warrant any further serious > consideration. OK. I'll show you the performance data. Thanks for your comments. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2015-11-12 10:10 +0100 |
| Subject | Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt |
| Message-ID | <qtW3g-6XW-23@gated-at.bofh.it> |
| In reply to | #1267595 |
On Thu, Nov 12, 2015 at 10:36:34AM +0800, Baolin Wang wrote: > That's right. But we'll not introduce the duality things, cause we > will remove the bio based things in dm-crypt if the request based > things are accepted. No, you will NOT remote the bio based path. That would break all kinds of perfectly valid setups. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Baolin Wang <baolin.wang@linaro.org> |
|---|---|
| Date | 2015-11-12 09:30 +0100 |
| Message-ID | <qtVqx-6ta-1@gated-at.bofh.it> |
| In reply to | #1267364 |
On 12 November 2015 at 02:18, Mike Snitzer <snitzer@redhat.com> wrote: > On Wed, Nov 11 2015 at 4:31am -0500, > Baolin Wang <baolin.wang@linaro.org> wrote: > >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/ >> decrypt block data, which can only hanle one bio at one time. As we know, >> one bio must use the sequential physical address and it also has a limitation >> of length. Thus it may limit the big block encyrtion/decryption when some >> hardware support the big block data encryption. >> >> This patch series introduc the 'based-request' method to handle the data >> encryption/decryption. One request can contain multiple bios, so it can >> handle big block data to improve the efficiency. > > The duality of bio-based vs request-based code paths in DM core frankly > sucks. So the prospect of polluting dm-crypt with a similar duality is > really _not_ interesting. > > Request-based DM requires more memory reserves per device than bio-based > DM. Also, you cannot stack request-based DM ontop of bio-based devices > (be them DM, MD, etc) so request-based DM's underlying storage stack > gets a lot less interesting with this change. > > That said, it could be that the benefits of supporting both bio-based > and request-based DM in dm-crypt outweigh any overhead/limitations. But > you haven't given any performance data to justify this patchset. > > There needs to be a _really_ compelling benefit to do this. > > Also, FYI, having a big CONFIG knob to switch all of dm-crypt from > bio-based to request-based is _not_ acceptable. Both modes would need > to be supported in parallel. Could easily be that not all devices in a > system will benefit from being request-based. > > Regardless, the risk of this change causing request-based DM to become > more brittle than it already is concerns me. > > But I'm trying to keep an open mind... show me data that real hardware > _really_ benefits and we'll go from there. Again, it needs to be "OMG, > this is amazing!" level performance to warrant any further serious > consideration. Thanks for your suggestion. But let me explain it again. Now for many vendors, they supply the encryption hardware (such as AES engine) to accelerate the encyrtion/decryption speed with handling a big block at one time. So if we want the hardware engine can play the best performance, the size of block handled at one time need to be expanded. But it can only handle one bio at one time for bio based dm-crypt, one bio has a size limitation and one bio's size can't make the hardware engine reach its best performance. So we want to introduce the request based dm-crypt. For request based things, some sequential bios can merged into one request to expand the IO size to be a big block handled by hardware engine at one time. With the hardware acceleration, it can improve the encryption/decryption speed. I think 3 questions need to be clarified. 1. Are there ways of enhancing the dm-crypt bio-based target to overcome this? The focus is the size limitation of one bio, its size can not meet the hardware requirement. But one request can have a big block size with merging multiple bios. So I think the request is the best choice. 2. Would any sort of bio aggregation mechanism help? The request can combined sequential bios by block layer automatically. But for bio aggregation, I think it will be similar to that, why do we need recomplement it again? 3. perforamence data It is just a simple dd test result, and will provide the formal report in future. But from the simple test, we can see the improvement. Hardware environment: Board: beaglebone black processor: AM335x 1GHz ARM Cortex-A8 RAM: 512M Cipher: cbc(aes) with AES hardware engine (1) bio based dm-crypt with hardware accelarate: read 64M command: dd if=/dev/dm-0 of=/dev/null bs=512k count=128 iflag=direct 67108864 bytes (67 MB) copied, 11.6592 s, 5.8 MB/s 67108864 bytes (67 MB) copied, 11.6391 s, 5.8 MB/s 67108864 bytes (67 MB) copied, 11.6296 s, 5.8 MB/s (2) request based dm-crypt with hardware accelarate read 64M command: dd if=/dev/dm-0 of=/dev/null bs=512k count=128 iflag=direct 67108864 bytes (67 MB) copied, 5.16586 s, 13.0 MB/s 67108864 bytes (67 MB) copied, 5.19338 s, 12.9 MB/s 67108864 bytes (67 MB) copied, 5.19169 s, 12.9 MB/s (3) bio based dm-crypt with hardware accelarate write 64M command: dd if=/dev/zero of=/dev/dm-0 bs=512k count=128 iflag=direct 67108864 bytes (67 MB) copied, 13.6852 s, 4.9 MB/s 67108864 bytes (67 MB) copied, 14.0873 s, 4.8 MB/s 67108864 bytes (67 MB) copied, 13.6649 s, 4.9 MB/s (4) request based dm-crypt with hardware accelarate write 64M command: dd if=/dev/zero of=/dev/dm-0 bs=512k count=128 iflag=direct 67108864 bytes (67 MB) copied, 7.27832 s, 9.2 MB/s 67108864 bytes (67 MB) copied, 7.29051 s, 9.2 MB/s 67108864 bytes (67 MB) copied, 7.28318 s, 9.2 MB/s From the simple result, we can see it at least has a double improvement of the encryption performance. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-11-12 11:10 +0100 |
| Message-ID | <qtWZk-7yp-21@gated-at.bofh.it> |
| In reply to | #1267709 |
[Multipart message — attachments visible in raw view] — view raw
On Thu, Nov 12, 2015 at 04:20:41PM +0800, Baolin Wang wrote: > 3. perforamence data > It is just a simple dd test result, and will provide the formal report > in future. But from the simple test, we can see the improvement. It's probably also worth pointing out that Qualcomm have been shipping an out of tree implementation of this as a separate module in their BSP (originally written by Danesh Garg who's on this thread): https://android.googlesource.com/kernel/msm/+/android-msm-dory-3.10-kitkat-wear/drivers/md/dm-req-crypt.c Android now wants to encrypt phones and tablets by default and have been seeing substantial performance hits as a result, we can try to get people to share performance data from productionish systems but it might be difficult.
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2015-11-12 16:30 +0100 |
| Message-ID | <qu1Z0-2aO-17@gated-at.bofh.it> |
| In reply to | #1267775 |
On 11/12/2015 03:04 AM, Mark Brown wrote: > On Thu, Nov 12, 2015 at 04:20:41PM +0800, Baolin Wang wrote: > >> 3. perforamence data >> It is just a simple dd test result, and will provide the formal report >> in future. But from the simple test, we can see the improvement. > > It's probably also worth pointing out that Qualcomm have been shipping > an out of tree implementation of this as a separate module in their BSP > (originally written by Danesh Garg who's on this thread): > > https://android.googlesource.com/kernel/msm/+/android-msm-dory-3.10-kitkat-wear/drivers/md/dm-req-crypt.c > > Android now wants to encrypt phones and tablets by default and have been > seeing substantial performance hits as a result, we can try to get > people to share performance data from productionish systems but it might > be difficult. Well, shame on them for developing out-of-tree, looks like they are reaping all the benefits of that. Guys, we need some numbers, enough with the hand waving. There's no point discussing this further until we know how much of a difference it makes to handle X MB chunks instead of Y MB chunks. As was previously stated, unless there's a _substantial_ performance benefit, this patchset isn't going anywhere. If there is a huge benefit, we can look into ways of making it actually work. That may not even be a request interface, it could just be proper utilization of plugging for in-dm bio merging. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Baolin Wang <baolin.wang@linaro.org> |
|---|---|
| Date | 2015-11-13 03:10 +0100 |
| Message-ID | <qubYn-gb-11@gated-at.bofh.it> |
| In reply to | #1268030 |
On 12 November 2015 at 23:26, Jens Axboe <axboe@kernel.dk> wrote: > On 11/12/2015 03:04 AM, Mark Brown wrote: >> >> On Thu, Nov 12, 2015 at 04:20:41PM +0800, Baolin Wang wrote: >> >>> 3. perforamence data >>> It is just a simple dd test result, and will provide the formal report >>> in future. But from the simple test, we can see the improvement. >> >> >> It's probably also worth pointing out that Qualcomm have been shipping >> an out of tree implementation of this as a separate module in their BSP >> (originally written by Danesh Garg who's on this thread): >> >> >> https://android.googlesource.com/kernel/msm/+/android-msm-dory-3.10-kitkat-wear/drivers/md/dm-req-crypt.c >> >> Android now wants to encrypt phones and tablets by default and have been >> seeing substantial performance hits as a result, we can try to get >> people to share performance data from productionish systems but it might >> be difficult. > > > Well, shame on them for developing out-of-tree, looks like they are reaping > all the benefits of that. > > Guys, we need some numbers, enough with the hand waving. There's no point > discussing this further until we know how much of a difference it makes to > handle X MB chunks instead of Y MB chunks. As was previously stated, unless > there's a _substantial_ performance benefit, this patchset isn't going > anywhere. That's fair enough and we will provide the performance data to measure the patchset. > > If there is a huge benefit, we can look into ways of making it actually > work. That may not even be a request interface, it could just be proper > utilization of plugging for in-dm bio merging. Make sense. Thanks. > > -- > Jens Axboe > -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2015-11-13 13:00 +0100 |
| Message-ID | <qulbk-5Uo-23@gated-at.bofh.it> |
| In reply to | #1268030 |
[Multipart message — attachments visible in raw view] — view raw
On Thu, Nov 12, 2015 at 08:26:26AM -0700, Jens Axboe wrote: > On 11/12/2015 03:04 AM, Mark Brown wrote: > >Android now wants to encrypt phones and tablets by default and have been > >seeing substantial performance hits as a result, we can try to get > >people to share performance data from productionish systems but it might > >be difficult. > Well, shame on them for developing out-of-tree, looks like they are reaping > all the benefits of that. > Guys, we need some numbers, enough with the hand waving. There's no point > discussing this further until we know how much of a difference it makes to > handle X MB chunks instead of Y MB chunks. As was previously stated, unless > there's a _substantial_ performance benefit, this patchset isn't going > anywhere. Yeah, what I'm saying here is that there will issues getting the numbers from relevant production systems - we are most likely to be looking at proxies which are hopefully reasonably representative but there's likely to be more divergence than you'd see just running benchmark workloads on similar systems to those used in production.
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2015-11-12 10:10 +0100 |
| Subject | Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt |
| Message-ID | <qtW3g-6XW-31@gated-at.bofh.it> |
| In reply to | #1267364 |
On Wed, Nov 11, 2015 at 01:18:13PM -0500, Mike Snitzer wrote: > But I'm trying to keep an open mind... show me data that real hardware > _really_ benefits and we'll go from there. Again, it needs to be "OMG, > this is amazing!" level performance to warrant any further serious > consideration. Also the numbers should be on 4.3+ with our arbitrarily sizeds bios, and a file system that isn't stupid and actually submits bios (xfs, btrfs for example). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web