Skip to content

Commit

Permalink
dm crypt: constrain crypt device's max_segment_size to PAGE_SIZE
Browse files Browse the repository at this point in the history
Setting the dm-crypt device's max_segment_size to PAGE_SIZE is an
unfortunate constraint that is required to avoid the potential for
exceeding dm-crypt's underlying device's max_segments limits -- due to
crypt_alloc_buffer() possibly allocating pages for the encryption bio
that are not as physically contiguous as the original bio.

It is interesting to note that this problem was already fixed back in
2007 via commit 91e1062 ("dm crypt: use bio_add_page").  But Linux 4.0
commit cf2f1ab ("dm crypt: don't allocate pages for a partial
request") regressed dm-crypt back to _not_ using bio_add_page().  But
given dm-crypt's cpu parallelization changes all depend on commit
cf2f1ab's abandoning of the more complex io fragments processing that
dm-crypt previously had we cannot easily go back to using
bio_add_page().

So all said the cleanest way to resolve this issue is to fix dm-crypt to
properly constrain the original bios entering dm-crypt so the encryption
bios that dm-crypt generates from the original bios are always
compatible with the underlying device's max_segments queue limits.

It should be noted that technically Linux 4.3 does _not_ need this fix
because of the block core's new late bio-splitting capability.  But, it
is reasoned, there is little to be gained by having the block core split
the encrypted bio that is composed of PAGE_SIZE segments.  That said, in
the future we may revert this change.

Fixes: cf2f1ab ("dm crypt: don't allocate pages for a partial request")
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=104421
Suggested-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org # 4.0+
  • Loading branch information
Mike Snitzer committed Sep 14, 2015
1 parent 2160767 commit 586b286
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions drivers/md/dm-crypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,8 @@ static void crypt_free_buffer_pages(struct crypt_config *cc, struct bio *clone);

/*
* Generate a new unfragmented bio with the given size
* This should never violate the device limitations
* This should never violate the device limitations (but only because
* max_segment_size is being constrained to PAGE_SIZE).
*
* This function may be called concurrently. If we allocate from the mempool
* concurrently, there is a possibility of deadlock. For example, if we have
Expand Down Expand Up @@ -2045,9 +2046,20 @@ static int crypt_iterate_devices(struct dm_target *ti,
return fn(ti, cc->dev, cc->start, ti->len, data);
}

static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
{
/*
* Unfortunate constraint that is required to avoid the potential
* for exceeding underlying device's max_segments limits -- due to
* crypt_alloc_buffer() possibly allocating pages for the encryption
* bio that are not as physically contiguous as the original bio.
*/
limits->max_segment_size = PAGE_SIZE;
}

static struct target_type crypt_target = {
.name = "crypt",
.version = {1, 14, 0},
.version = {1, 14, 1},
.module = THIS_MODULE,
.ctr = crypt_ctr,
.dtr = crypt_dtr,
Expand All @@ -2058,6 +2070,7 @@ static struct target_type crypt_target = {
.resume = crypt_resume,
.message = crypt_message,
.iterate_devices = crypt_iterate_devices,
.io_hints = crypt_io_hints,
};

static int __init dm_crypt_init(void)
Expand Down

0 comments on commit 586b286

Please sign in to comment.