All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: io-uring@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH 1/2] io_uring: set ctx sq/cq entry count earlier
Date: Thu, 6 Aug 2020 07:20:29 -0600	[thread overview]
Message-ID: <8277cc52-b591-e577-6a99-dd9c8a8146ab@kernel.dk> (raw)
In-Reply-To: <20200806073933.khoasyujngaxbcq4@steredhat>

On 8/6/20 1:39 AM, Stefano Garzarella wrote:
> On Wed, Aug 05, 2020 at 01:02:23PM -0600, Jens Axboe wrote:
>> If we hit an earlier error path in io_uring_create(), then we will have
>> accounted memory, but not set ctx->{sq,cq}_entries yet. Then when the
>> ring is torn down in error, we use those values to unaccount the memory.
>>
>> Ensure we set the ctx entries before we're able to hit a potential error
>> path.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  fs/io_uring.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 8f96566603f3..0d857f7ca507 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -8193,6 +8193,10 @@ static int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>>  	struct io_rings *rings;
>>  	size_t size, sq_array_offset;
>>  
>> +	/* make sure these are sane, as we already accounted them */
>> +	ctx->sq_entries = p->sq_entries;
>> +	ctx->cq_entries = p->cq_entries;
>> +
>>  	size = rings_size(p->sq_entries, p->cq_entries, &sq_array_offset);
>>  	if (size == SIZE_MAX)
>>  		return -EOVERFLOW;
>> @@ -8209,8 +8213,6 @@ static int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>>  	rings->cq_ring_entries = p->cq_entries;
>>  	ctx->sq_mask = rings->sq_ring_mask;
>>  	ctx->cq_mask = rings->cq_ring_mask;
>> -	ctx->sq_entries = rings->sq_ring_entries;
>> -	ctx->cq_entries = rings->cq_ring_entries;
>>  
>>  	size = array_size(sizeof(struct io_uring_sqe), p->sq_entries);
>>  	if (size == SIZE_MAX) {
>> -- 
>> 2.28.0
>>
> 
> While reviewing I was asking if we should move io_account_mem() before
> io_allocate_scq_urings(), then I saw the second patch :-)

Indeed, just split it in two to avoid any extra issues around backporting.

> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks, added.

-- 
Jens Axboe


  reply	other threads:[~2020-08-06 17:23 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05 19:02 [PATCH 0/2] io_uring memory accounting fixes Jens Axboe
2020-08-05 19:02 ` [PATCH 1/2] io_uring: set ctx sq/cq entry count earlier Jens Axboe
2020-08-06  7:39   ` Stefano Garzarella
2020-08-06 13:20     ` Jens Axboe [this message]
2020-08-05 19:02 ` [PATCH 2/2] io_uring: account locked memory before potential error case Jens Axboe
2020-08-06  7:42   ` Stefano Garzarella
2020-08-06 13:21     ` Jens Axboe
2020-08-06 13:38       ` Stefano Garzarella
2020-08-06 13:46         ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8277cc52-b591-e577-6a99-dd9c8a8146ab@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=sgarzare@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.