On 1/4/2021 6:43 PM, Pavel Begunkov wrote:
On 18/12/2020 18:07, Bijan Mottahedeh wrote:
Apply fixed_rsrc functionality for fixed buffers support.
git generated a pretty messy diff...
I had tried to break this up a few ways but it didn't work well because
I think most of the code changes depend on the io_uring structure
changes. I can look again or if you some idea of how you want to split
it, I can do that.
Because it's do quiesce, fixed read/write access buffers from asynchronous
contexts without synchronisation. That won't work anymore, so
1. either we save it in advance, that would require extra req_async
allocation for linked fixed rw
2. or synchronise whenever async. But that would mean that a request
may get and do IO on two different buffers, that's rotten.
3. do mixed -- lazy, but if do IO then alloc.
3.5 also "synchronise" there would mean uring_lock, that's not welcome,
but we can probably do rcu.
Are you referring to a case where a fixed buffer request can be
submitted from async context while those buffers are being unregistered,
or something like that?
Let me think of a patch...
Ok, will wait for it.
@@ -8373,7 +8433,13 @@ static bool headpage_already_acct(struct io_ring_ctx *ctx, struct page **pages,
/* check previously registered pages */
for (i = 0; i < ctx->nr_user_bufs; i++) {
- struct io_mapped_ubuf *imu = &ctx->user_bufs[i];
+ struct fixed_rsrc_table *table;
+ struct io_mapped_ubuf *imu;
+ unsigned int index;
+
+ table = &ctx->buf_data->table[i >> IORING_BUF_TABLE_SHIFT];
+ index = i & IORING_BUF_TABLE_MASK;
+ imu = &table->bufs[index];
io_buf_from_index() may tak buf_data, so can be reused.
Ok.
+ for (i = 0; i < nr_tables; i++) {
+ struct fixed_rsrc_table *table = &buf_data->table[i];
+ unsigned int this_bufs;
+
+ this_bufs = min(nr_bufs, IORING_MAX_BUFS_TABLE);
+ table->bufs = kcalloc(this_bufs, sizeof(struct io_mapped_ubuf),
+ GFP_KERNEL);
+ if (!table->bufs)
+ break;
+ nr_bufs -= this_bufs;
+ }
+
+ if (i == nr_tables)
+ return 0;
+
+ io_free_buf_tables(buf_data, nr_tables);
Would work because kcalloc() zeroed buf_data->table, but
io_free_buf_tables(buf_data, __i__);
Ok.
ret = io_buffer_validate(&iov);
if (ret)
break;
+ table = &buf_data->table[i >> IORING_BUF_TABLE_SHIFT];
same, io_buf_from_index() can be reused
+ index = i & IORING_BUF_TABLE_MASK;
+ imu = &table->bufs[index];
Ok.
@@ -9854,6 +10023,7 @@ static bool io_register_op_must_quiesce(int op)
switch (op) {
case IORING_UNREGISTER_FILES:
case IORING_REGISTER_FILES_UPDATE:
+ case IORING_UNREGISTER_BUFFERS:
what about REGISTER_BUFFERS?
I followed the FILES case, which deals with UNREGISTER and UPDATE only,
should I add REGISTER for buffers only?
