Re: [PATCH v9 5/6] iommu/dma: Allow a single FQ in addition to per-CPU FQs

[Robin Murphy]

On 2023-05-23 13:02, Niklas Schnelle wrote:
[...]
> > > +static void fq_flush_single(struct iommu_dma_cookie *cookie)
> > > +{
> > > +	struct iova_fq *fq = cookie->single_fq;
> > > +	unsigned long flags;
> > > +
> > > +	spin_lock_irqsave(&fq->lock, flags);
> > > +	fq_ring_free(cookie, fq);
> > > +	spin_unlock_irqrestore(&fq->lock, flags)
> >
> > Nit: this should clearly just be a self-locked version of fq_ring_free()
> > that takes fq as an argument, then both the new case and the existing
> > loop body become trivial one-line calls.
>
> Sure will do. Just one question about names. As an example
> pci_reset_function_locked() means that the relevant lock is already
> taken with pci_reset_function() adding the lock/unlock. In your wording
> the implied function names sound the other way around. I can't find
> anything similar in drivers/iommu so would you mind going the PCI way
> and having:
>
> fq_ring_free_locked(): Called in queue_iova() with the lock held
> fr_ring_free(): Called in fq_flush_timeout() takes the lock itself
>
> Or maybe I'm just biased because I've used the PCI ..locked() functions
> before and there is a better convention.

Yes, that's the form that's most familiar to me too - sorry I failed to 
express it clearly :)

Thanks,
Robin.