[dm-devel] [PATCH] dm-crypt: limit the number of allocated pages

[John Stoffel]

>>>>> "Mikulas" == Mikulas Patocka <mpatocka at redhat.com> writes:

Mikulas> On Thu, 17 Aug 2017, John Stoffel wrote:

>> >>>>> "Mikulas" == Mikulas Patocka <mpatocka at redhat.com> writes:
>> 
Mikulas> On Mon, 14 Aug 2017, John Stoffel wrote:
>> 
>> >> >>>>> "Mikulas" == Mikulas Patocka <mpatocka at redhat.com> writes:
>> >> 
Mikulas> dm-crypt consumes excessive amount memory when the user attempts to zero
Mikulas> a dm-crypt device with "blkdiscard -z". The command "blkdiscard -z" calls
Mikulas> the BLKZEROOUT ioctl, it goes to the function __blkdev_issue_zeroout,
Mikulas> __blkdev_issue_zeroout sends large amount of write bios that contain the
Mikulas> zero page as their payload.
>> >> 
Mikulas> For each incoming page, dm-crypt allocates another page that holds the
Mikulas> encrypted data, so when processing "blkdiscard -z", dm-crypt tries to
Mikulas> allocate the amount of memory that is equal to the size of the device.
Mikulas> This can trigger OOM killer or cause system crash.
>> >> 
Mikulas> This patch fixes the bug by limiting the amount of memory that dm-crypt
Mikulas> allocates to 2% of total system memory.
>> >> 
>> >> Is this limit per-device or system wide?  it's not clear to me if the
>> >> minimum is just one page, or more so it might be nice to clarify
>> >> that.
>> 
Mikulas> The limit is system-wide. The limit is divided by the number
Mikulas> of active dm-crypt devices and each device receives an equal
Mikulas> share.
>> 
>> Ok, thanks for the clarification.  Can it be expanded dynamically?  I
>> could see that for large systems, it might not scale well.  
>> 
Mikulas> There is a lower bound of BIO_MAX_PAGES * 16. The per-device
Mikulas> limit is never lower than this.
>> 
>> Nice.
>> 
>> >> Also, for large memory systems, that's still alot of pages.  Maybe it
>> >> should be more exponential in it's clamping as memory sizes go up?  2%
>> >> of 2T is 4G, which is pretty darn big...
>> 
Mikulas> The more we restrict it, the less parallelism is used in dm-crypt, so it 
Mikulas> makes sense to have a big value on machines with many cores.
>> 
>> True... but it's still a concern that it's hardcoded.
>> 
>> Thanks for your replies.

Mikulas> The limit can't be changed. I expect that no one will run a system with 
Mikulas> such a tight requirements that the memory is used up to the last 2%.

Mikulas> If the user is near hitting the memory limit, he can just add swap space 
Mikulas> to solve the problem instead of tuning dm-crypt.

My real concern is that we build up too many outstanding BIOs so that
they take along time to process.  I'm already seeing some reports
about timeouts after 120 seconds due to stacking of dm-crypt with
other stuff.

What I'm trying to say is that as memory grows, you need to slow down
the growth of the amount of memory that dm-crypt can allocate.  It's
more important to scale by the speed of the IO sub-system than the
size of memory.

But hey, you really addresed my concern with smaller memory systems
already.

John