Re: [PATCH v3 06/13] epoll: introduce helpers for adding/removing events to uring

Roman Penyaev
2019-05-31T18:58:22Z

On 2019-05-31 18:51, Peter Zijlstra wrote:
> On Fri, May 31, 2019 at 04:21:30PM +0200, Roman Penyaev wrote:
>
>> The ep_add_event_to_uring() is lockless, thus I can't increase tail
>> after,
>> I need to reserve the index slot, where to write to. I can use shadow
>> tail,
>> which is not seen by userspace, but I have to guarantee that tail is
>> updated
>> with shadow tail *after* all callers of ep_add_event_to_uring() are
>> left.
>> That is possible, please see the code below, but it adds more
>> complexity:
>>
>> (code was tested on user side, thus has c11 atomics)
>>
>> static inline void add_event__kernel(struct ring *ring, unsigned bit)
>> {
>> unsigned i, cntr, commit_cntr, *item_idx, tail, old;
>>
>> i = __atomic_fetch_add(&ring->cntr, 1, __ATOMIC_ACQUIRE);
>> item_idx = &ring->user_itemsindex[i % ring->nr];
>>
>> /* Update data */
>> *item_idx = bit;
>>
>> commit_cntr = __atomic_add_fetch(&ring->commit_cntr, 1,
>> __ATOMIC_RELEASE);
>>
>> tail = ring->user_header->tail;
>> rmb();
>> do {
>> cntr = ring->cntr;
>> if (cntr != commit_cntr)
>> /* Someone else will advance tail */
>> break;
>>
>> old = tail;
>>
>> } while ((tail =
>> __sync_val_compare_and_swap(&ring->user_header->tail, old, cntr)) !=
>> old);
>> }
>
> Yes, I'm well aware of that particular problem (see
> kernel/events/ring_buffer.c:perf_output_put_handle for instance).

I'll take a look, thanks.

> But like you show, it can be done. It also makes the thing wait-free,
> as
> opposed to merely lockless.

You think it's better? I did not like this variant from the very
beginning because of the unnecessary complexity. But maybe you're
right. No busy loops on user side makes it wait-free. And also
I can avoid c11 in kernel using cmpxchg along with atomic_t.

--
Roman