On Thu, Jun 29, 2017 at 01:42:50AM -0700, Jakub Kicinski wrote:
> On Thu, 29 Jun 2017 10:14:29 +0200, Simon Horman wrote:
> > > > + /* Checking if buffer is empty. */
> > > > + if (ring->head == ring->tail) {
> > > > + *mask_id = freed_id;
> > > > + return -ENOENT;
> > > > + }
> > > > +
> > > > + memcpy(&temp_id, &ring->buf[ring->tail],
> > > > NFP_FLOWER_MASK_ELEMENT_RS);
> > > > + *mask_id = temp_id;
> > > > + memcpy(&ring->buf[ring->tail], &freed_id,
> > > > NFP_FLOWER_MASK_ELEMENT_RS);
> > > > + ring->tail = (ring->tail + NFP_FLOWER_MASK_ELEMENT_RS) %
> > > > + (NFP_FLOWER_MASK_ENTRY_RS *
> > > > NFP_FLOWER_MASK_ELEMENT_RS);
> > > > +
> > > > + getnstimeofday64(&now);
> > > > + delta = timespec64_sub(now, priv->mask_ids.last_used[*mask_id]);
> > > > +
> > > > + if (timespec64_to_ns(&delta) < NFP_FL_MASK_REUSE_TIME_NS) {
> > > > + nfp_release_mask_id(app, *mask_id);
> > >
> > > nfp_release_mask_id() will reset the time stamp and put the mask at the
> > > end of the queue. Is that OK?
> >
> > I discussed this with Pieter. He believes that it is ok as it would be too
> > early to use the entry and its better put it to the back of the list and
> > skip to the next one.
>
> But we shouldn't update the "last use" time if the grace period haven't
> elapsed otherwise we can live lock (I know, unlikely). Could we simply
> move the time check right after the:
>
> *mask_id = temp_id;
>
> line? I.e. move the check before we actually pick the mask off the
> queue?
Thanks, we now have a solution along those lines working.
