On Wed, 14 Dec 2022 15:20:44 -0500
Steven Sistare <steven.sistare@xxxxxxxxxx> wrote:
> On 12/14/2022 3:03 PM, Steven Sistare wrote:
> > On 12/14/2022 2:40 PM, Alex Williamson wrote:
> >> On Wed, 14 Dec 2022 11:22:47 -0800
> >> Steve Sistare <steven.sistare@xxxxxxxxxx> wrote:
> >>> @@ -3080,6 +3112,11 @@ static int vfio_iommu_type1_dma_rw_chunk(struct vfio_iommu *iommu,
> >>> size_t offset;
> >>> int ret;
> >>>
> >>> + if (iommu->vaddr_invalid_count) {
> >>> + WARN_ONCE(1, "mdev not allowed with VFIO_UPDATE_VADDR\n");
> >>> + return -EIO;
> >>> + }
> >>
> >> Same optimization above, but why are we letting the code iterate this
> >> multiple times in the _chunk function rather than testing once in the
> >> caller? Thanks,
> >
> > An oversight, I'll hoist it.
>
> It's actually a little nicer to leave the test here. The first call to
> here returns failure, and the caller exits the loop.
>
> Hoisting it requires jumping to an out label that releases the iommu lock.
>
> Which do you prefer?
The failure might exit on the first chunk, but non-failure incurs
overhead per chunk. I suspect we're not often iterating multiple
chunks, but I don't mind the goto in order to avoid that possibility.
Thanks,
Alex
