Re: [PATCH 2/2] md: dm-verity: allow parallel processing of bio blocks

[Mike Snitzer]

On Sun, Mar 25 2018 at  2:41pm -0400,
Yael Chemla <yael.chemla@xxxxxxxxxxxx> wrote:

>  Allow parallel processing of bio blocks by moving to async. completion
>  handling. This allows for better resource utilization of both HW and
>  software based hash tfm and therefore better performance in many cases,
>  depending on the specific tfm in use.
>  
>  Tested on ARM32 (zynq board) and ARM64 (Juno board).
>  Time of cat command was measured on a filesystem with various file sizes.
>  12% performance improvement when HW based hash was used (ccree driver).
>  SW based hash showed less than 1% improvement.
>  CPU utilization when HW based hash was used presented 10% less context
>  switch, 4% less cycles and 7% less instructions. No difference in
>  CPU utilization noticed with SW based hash.
>  
> Signed-off-by: Yael Chemla <yael.chemla@xxxxxxxxxxxx>

This one had various issues.  I've fixed most of what I saw and staged
in linux-next (purely for build test coverage purposes).  I may drop
this patch if others disagree with it (or my sg deallocation in the
error path question isn't answered).

I've staged the changes here (and in linux-next via 'for-next'):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-4.17

I switched all the new GFP_KERNEL uses to GFP_NOIO.  The fact that
you're doing allocations at all (per IO) is bad enough.  Using
GFP_KERNEL is a serious liability (risk of deadlock if dm-verity were to
be used for something like.. swap.. weird setup but possible).

But the gfp flags aside, the need for additional memory and the
expectation of scalable async parallel IO is potentially at odds with
changes like this (that I just staged, and had to rebase your 2 patches
ontop of):
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.17&id=a89f6a2cfec86fba7a115642ff082cb4e9450ea6

So I'm particulalry interested to hear from google folks to understand
if they are OK with your proposed verity async crypto API use.

Mike