Re: [PATCH v3 04/31] elx: libefc_sli: queue create/destroy/parse routines

Daniel Wagner
2020-04-24T07:29:33Z

On Tue, Apr 21, 2020 at 10:05:26PM -0700, James Smart wrote:
> On 4/15/2020 3:04 AM, Daniel Wagner wrote:
> ...
> > > +static void
> > > +__sli_queue_destroy(struct sli4 *sli4, struct sli4_queue *q)
> > > +{
> > > + if (!q->dma.size)
> > > + return;
> > > +
> > > + dma_free_coherent(&sli4->pcidev->dev, q->dma.size,
> > > + q->dma.virt, q->dma.phys);
> > > + memset(&q->dma, 0, sizeof(struct efc_dma));
> >
> > Is this necessary to clear q->dma? Just asking if it's possible to
> > avoid the additional work.
>
> unfortunately, yes - at least q->dma.size must be cleared. It's used to
> detect validity (must be non-0).

I see.

> > > +sli_mq_write(struct sli4 *sli4, struct sli4_queue *q,
> > > + u8 *entry)
> > > +{
> > > + u8 *qe = q->dma.virt;
> > > + u32 qindex;
> > > + u32 val = 0;
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&q->lock, flags);
> > > + qindex = q->index;
> > > + qe += q->index * q->size;
> > > +
> > > + memcpy(qe, entry, q->size);
> > > + q->n_posted = 1;
> >
> > Shouldn't this be q->n_posted++ ? Or is it garanteed n_posted is 0?
>
> yes - we post 1 at a time.

Sorry to ask again, but I worried that sli_mq_write() is called while there is
work pending. Maybe it could at least documented with something like
WARN_ON_ONCE(n_posted != 0)