linux-ext4 - Re: [PATCH v2] jbd2: Limit number of reserved credits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150803105434.GA9657@quack.suse.cz>
Date:	Mon, 3 Aug 2015 12:54:34 +0200
From:	Jan Kara <jack@...e.cz>
To:	Lukas Czerner <lczerner@...hat.com>
Cc:	linux-ext4@...r.kernel.org, jack@...e.cz
Subject: Re: [PATCH v2] jbd2: Limit number of reserved credits

On Fri 31-07-15 17:11:34, Lukas Czerner wrote:
> Currently there is no limitation on number of reserved credits we can
> ask for. If we ask for more reserved credits than 1/2 of maximum
> transaction size, or if total number of credits exceeds the maximum
> transaction size per operation (which is currently only possible with
> the former) we will spin forever in start_this_handle().
> 
> Fix this by adding this limitation at the start of start_this_handle().
> 
> This patch also removes the credit limitation 1/2 of maximum transaction
> size, since we really only want to limit the number of reserved credits.
> There is not much point to limit the credits if there is still space in
> the journal.
> 
> This accidentally also fixes the online resize, where due to the
> limitation of the journal credits we're unable to grow file systems with
> 1k block size and size between 16M and 32M. It has been partially fixed
> by 2c869b262a10ca99cb866d04087d75311587a30c, but not entirely.
> 
> Thanks Jan Kara for helping me getting the correct fix.
> 
> Signed-off-by: Lukas Czerner <lczerner@...hat.com>
> ---
> v2: added wait event Jan suggested

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> 
>  fs/jbd2/transaction.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index f3d0617..f8183d4 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -204,6 +204,20 @@ static int add_transaction_credits(journal_t *journal, int blocks,
>  		 * attach this handle to a new transaction.
>  		 */
>  		atomic_sub(total, &t->t_outstanding_credits);
> +
> +		/*
> +		 * Is the number of reserved credits in the current transaction too
> +		 * big to fit this handle? Wait until reserved credits are freed.
> +		 */
> +		if (atomic_read(&journal->j_reserved_credits) + total >
> +		    journal->j_max_transaction_buffers) {
> +			read_unlock(&journal->j_state_lock);
> +			wait_event(journal->j_wait_reserved,
> +				   atomic_read(&journal->j_reserved_credits) + total <=
> +				   journal->j_max_transaction_buffers);
> +			return 1;
> +		}
> +
>  		wait_transaction_locked(journal);
>  		return 1;
>  	}
> @@ -262,20 +276,24 @@ static int start_this_handle(journal_t *journal, handle_t *handle,
>  	int		rsv_blocks = 0;
>  	unsigned long ts = jiffies;
>  
> +	if (handle->h_rsv_handle)
> +		rsv_blocks = handle->h_rsv_handle->h_buffer_credits;
> +
>  	/*
> -	 * 1/2 of transaction can be reserved so we can practically handle
> -	 * only 1/2 of maximum transaction size per operation
> +	 * Limit the number of reserved credits to 1/2 of maximum transaction
> +	 * size and limit the number of total credits to not exceed maximum
> +	 * transaction size per operation.
>  	 */
> -	if (WARN_ON(blocks > journal->j_max_transaction_buffers / 2)) {
> -		printk(KERN_ERR "JBD2: %s wants too many credits (%d > %d)\n",
> -		       current->comm, blocks,
> -		       journal->j_max_transaction_buffers / 2);
> +	if ((rsv_blocks > journal->j_max_transaction_buffers / 2) ||
> +	    (rsv_blocks + blocks > journal->j_max_transaction_buffers)) {
> +		printk(KERN_ERR "JBD2: %s wants too many credits "
> +		       "credits:%d rsv_credits:%d max:%d\n",
> +		       current->comm, blocks, rsv_blocks,
> +		       journal->j_max_transaction_buffers);
> +		WARN_ON(1);
>  		return -ENOSPC;
>  	}
>  
> -	if (handle->h_rsv_handle)
> -		rsv_blocks = handle->h_rsv_handle->h_buffer_credits;
> -
>  alloc_transaction:
>  	if (!journal->j_running_transaction) {
>  		/*
> -- 
> 1.8.3.1
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html