Re: [PATCH 2/2] dm integrity: use init_completion instead of COMPLETION_INITIALIZER_ONSTACK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This patch is OK.

Acked-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>

On Tue, 15 Aug 2017, Arnd Bergmann wrote:

> The new lockdep support for completions causeed the stack usage
> in dm-integrity to explode, in case of write_journal from 504 bytes
> to 1120 (using arm gcc-7.1.1):
> 
> drivers/md/dm-integrity.c: In function 'write_journal':
> drivers/md/dm-integrity.c:827:1: error: the frame size of 1120 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> 
> The problem is that not only the size of 'struct completion' grows
> significantly, but we end up having multiple copies of it on the stack
> when we assign it from a local variable after the initial declaration.
> 
> COMPLETION_INITIALIZER_ONSTACK() is the right thing to use when we
> want to declare and initialize a completion on the stack. However,
> this driver doesn't do that and instead initializes the completion
> just before it is used.
> 
> In this case, init_completion() does the same thing more efficiently,
> and drops the stack usage for the function above down to 496 bytes.
> While the other functions in this file are not bad enough to cause
> a warning, they benefit equally from the change, so I do the change
> across the entire file. In the one place where we reuse a completion,
> I picked the cheaper reinit_completion() over init_completion().
> 
> Fixes: cd8084f91c02 ("locking/lockdep: Apply crossrelease to completions")
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> The patch causing this is currently part of linux-next, scheduled for
> 4.14, so it would be good to have this in the same release.
> ---
>  drivers/md/dm-integrity.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 293a19652d55..b16010bcbd17 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -773,13 +773,13 @@ static void write_journal(struct dm_integrity_c *ic, unsigned commit_start, unsi
>  	unsigned i;
>  
>  	io_comp.ic = ic;
> -	io_comp.comp = COMPLETION_INITIALIZER_ONSTACK(io_comp.comp);
> +	init_completion(&io_comp.comp);
>  
>  	if (commit_start + commit_sections <= ic->journal_sections) {
>  		io_comp.in_flight = (atomic_t)ATOMIC_INIT(1);
>  		if (ic->journal_io) {
>  			crypt_comp_1.ic = ic;
> -			crypt_comp_1.comp = COMPLETION_INITIALIZER_ONSTACK(crypt_comp_1.comp);
> +			init_completion(&crypt_comp_1.comp);
>  			crypt_comp_1.in_flight = (atomic_t)ATOMIC_INIT(0);
>  			encrypt_journal(ic, true, commit_start, commit_sections, &crypt_comp_1);
>  			wait_for_completion_io(&crypt_comp_1.comp);
> @@ -795,18 +795,18 @@ static void write_journal(struct dm_integrity_c *ic, unsigned commit_start, unsi
>  		to_end = ic->journal_sections - commit_start;
>  		if (ic->journal_io) {
>  			crypt_comp_1.ic = ic;
> -			crypt_comp_1.comp = COMPLETION_INITIALIZER_ONSTACK(crypt_comp_1.comp);
> +			init_completion(&crypt_comp_1.comp);
>  			crypt_comp_1.in_flight = (atomic_t)ATOMIC_INIT(0);
>  			encrypt_journal(ic, true, commit_start, to_end, &crypt_comp_1);
>  			if (try_wait_for_completion(&crypt_comp_1.comp)) {
>  				rw_journal(ic, REQ_OP_WRITE, REQ_FUA, commit_start, to_end, &io_comp);
> -				crypt_comp_1.comp = COMPLETION_INITIALIZER_ONSTACK(crypt_comp_1.comp);
> +				reinit_completion(&crypt_comp_1.comp);
>  				crypt_comp_1.in_flight = (atomic_t)ATOMIC_INIT(0);
>  				encrypt_journal(ic, true, 0, commit_sections - to_end, &crypt_comp_1);
>  				wait_for_completion_io(&crypt_comp_1.comp);
>  			} else {
>  				crypt_comp_2.ic = ic;
> -				crypt_comp_2.comp = COMPLETION_INITIALIZER_ONSTACK(crypt_comp_2.comp);
> +				init_completion(&crypt_comp_2.comp);
>  				crypt_comp_2.in_flight = (atomic_t)ATOMIC_INIT(0);
>  				encrypt_journal(ic, true, 0, commit_sections - to_end, &crypt_comp_2);
>  				wait_for_completion_io(&crypt_comp_1.comp);
> @@ -1679,7 +1679,7 @@ static void dm_integrity_map_continue(struct dm_integrity_io *dio, bool from_map
>  	dio->in_flight = (atomic_t)ATOMIC_INIT(2);
>  
>  	if (need_sync_io) {
> -		read_comp = COMPLETION_INITIALIZER_ONSTACK(read_comp);
> +		init_completion(&read_comp);
>  		dio->completion = &read_comp;
>  	} else
>  		dio->completion = NULL;
> @@ -1836,7 +1836,7 @@ static void do_journal_write(struct dm_integrity_c *ic, unsigned write_start,
>  
>  	comp.ic = ic;
>  	comp.in_flight = (atomic_t)ATOMIC_INIT(1);
> -	comp.comp = COMPLETION_INITIALIZER_ONSTACK(comp.comp);
> +	init_completion(&comp.comp);
>  
>  	i = write_start;
>  	for (n = 0; n < write_sections; n++, i++, wraparound_section(ic, &i)) {
> @@ -2063,7 +2063,7 @@ static void replay_journal(struct dm_integrity_c *ic)
>  		if (ic->journal_io) {
>  			struct journal_completion crypt_comp;
>  			crypt_comp.ic = ic;
> -			crypt_comp.comp = COMPLETION_INITIALIZER_ONSTACK(crypt_comp.comp);
> +			init_completion(&crypt_comp.comp);
>  			crypt_comp.in_flight = (atomic_t)ATOMIC_INIT(0);
>  			encrypt_journal(ic, false, 0, ic->journal_sections, &crypt_comp);
>  			wait_for_completion(&crypt_comp.comp);
> @@ -2636,7 +2636,7 @@ static int create_journal(struct dm_integrity_c *ic, char **error)
>  			memset(iv, 0x00, ivsize);
>  
>  			skcipher_request_set_crypt(req, sg, sg, PAGE_SIZE * ic->journal_pages + sizeof ic->commit_ids, iv);
> -			comp.comp = COMPLETION_INITIALIZER_ONSTACK(comp.comp);
> +			init_completion(&comp.comp);
>  			comp.in_flight = (atomic_t)ATOMIC_INIT(1);
>  			if (do_crypt(true, req, &comp))
>  				wait_for_completion(&comp.comp);
> @@ -2693,7 +2693,7 @@ static int create_journal(struct dm_integrity_c *ic, char **error)
>  
>  				sg_init_one(&sg, crypt_data, crypt_len);
>  				skcipher_request_set_crypt(req, &sg, &sg, crypt_len, iv);
> -				comp.comp = COMPLETION_INITIALIZER_ONSTACK(comp.comp);
> +				init_completion(&comp.comp);
>  				comp.in_flight = (atomic_t)ATOMIC_INIT(1);
>  				if (do_crypt(true, req, &comp))
>  					wait_for_completion(&comp.comp);
> -- 
> 2.9.0
> 
> --
> dm-devel mailing list
> dm-devel@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel



[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux