[PATCH v2] jbd2: Limit number of reserved credits

[PATCH v2] jbd2: Limit number of reserved credits

[Lukas Czerner]

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@redhat.com>
---
v2: added wait event Jan suggested

 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;

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

[Jan Kara]

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@redhat.com>
> ---
> v2: added wait event Jan suggested

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.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@suse.com>
SUSE Labs, CR

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

[Theodore Ts'o]

On Mon, Aug 03, 2015 at 12:54:34PM +0200, Jan Kara wrote:
> 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@redhat.com>
> > ---
> > v2: added wait event Jan suggested
> 
> The patch looks good. You can add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, applied.

						- Ted