Re: [PATCH 05/17] Add io_uring IO interface

[Jens Axboe <axboe@kernel.dk>]

On 1/21/19 2:13 AM, Roman Penyaev wrote:
> On 2019-01-18 17:12, Jens Axboe wrote:
> 
> [...]
> 
>> +
>> +static int io_uring_create(unsigned entries, struct io_uring_params 
>> *p,
>> +			   bool compat)
>> +{
>> +	struct user_struct *user = NULL;
>> +	struct io_ring_ctx *ctx;
>> +	int ret;
>> +
>> +	if (entries > IORING_MAX_ENTRIES)
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * Use twice as many entries for the CQ ring. It's possible for the
>> +	 * application to drive a higher depth than the size of the SQ ring,
>> +	 * since the sqes are only used at submission time. This allows for
>> +	 * some flexibility in overcommitting a bit.
>> +	 */
>> +	p->sq_entries = roundup_pow_of_two(entries);
>> +	p->cq_entries = 2 * p->sq_entries;
>> +
>> +	if (!capable(CAP_IPC_LOCK)) {
>> +		user = get_uid(current_user());
>> +		ret = __io_account_mem(user, ring_pages(p->sq_entries,
>> +							p->cq_entries));
>> +		if (ret) {
>> +			free_uid(user);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	ctx = io_ring_ctx_alloc(p);
>> +	if (!ctx)
>> +		return -ENOMEM;
> 
> Hi Jens,
> 
> It seems pages should be "unaccounted" back here and uid freed if path
> with "if (!capable(CAP_IPC_LOCK))" above was taken.

Thanks, yes that is leaky. I'll fix that up.

> But really, could please someone explain me what is wrong with 
> allocating
> all urings in mmap() without touching RLIMIT_MEMLOCK at all?  Thus all
> memory will be accounted to the caller app and if app is greedy it will
> be killed by oom.  What I'm missing?

I don't really what that'd change, if we do it off the ->mmap() or when
we setup the io_uring instance with io_uring_setup(2). We need this memory
to be pinned, we can't fault on it.

-- 
Jens Axboe