On Fri, May 31, 2019 at 01:15:21PM +0200, Roman Penyaev wrote:
> On 2019-05-31 11:56, Peter Zijlstra wrote:
> > On Thu, May 16, 2019 at 10:58:03AM +0200, Roman Penyaev wrote:
> > > +static inline bool ep_add_event_to_uring(struct epitem *epi,
> > > __poll_t pollflags)
> > > +{
> > > + struct eventpoll *ep = epi->ep;
> > > + struct epoll_uitem *uitem;
> > > + bool added = false;
> > > +
> > > + if (WARN_ON(!pollflags))
> > > + return false;
> > > +
> > > + uitem = &ep->user_header->items[epi->bit];
> > > + /*
> > > + * Can be represented as:
> > > + *
> > > + * was_ready = uitem->ready_events;
> > > + * uitem->ready_events &= ~EPOLLREMOVED;
> > > + * uitem->ready_events |= pollflags;
> > > + * if (!was_ready) {
> > > + * // create index entry
> > > + * }
> > > + *
> > > + * See the big comment inside ep_remove_user_item(), why it is
> > > + * important to mask EPOLLREMOVED.
> > > + */
> > > + if (!atomic_or_with_mask(&uitem->ready_events,
> > > + pollflags, EPOLLREMOVED)) {
> > > + unsigned int i, *item_idx, index_mask;
> > > +
> > > + /*
> > > + * Item was not ready before, thus we have to insert
> > > + * new index to the ring.
> > > + */
> > > +
> > > + index_mask = ep_max_index_nr(ep) - 1;
> > > + i = __atomic_fetch_add(&ep->user_header->tail, 1,
> > > + __ATOMIC_ACQUIRE);
> > > + item_idx = &ep->user_index[i & index_mask];
> > > +
> > > + /* Signal with a bit, which is > 0 */
> > > + *item_idx = epi->bit + 1;
> >
> > Did you just increment the user visible tail pointer before you filled
> > the data? That is, can the concurrent userspace observe the increment
> > before you put credible data in its place?
>
> No, the "data" is the "ready_events" mask, which was updated before,
> using cmpxchg, atomic_or_with_mask() call. All I need is to put an
> index of just updated item to the uring.
>
> Userspace, in its turn, gets the index from the ring and then checks
> the mask.
But where do you write the index into the shared memory? That index
should be written before you publish the new tail.
