Hi Christian,
> Subject: Re: [PATCH 0/4] cover-letter: Allow MMIO regions to be exported
> through dmabuf
>
>
>> I will resend the patch series. I was experiencing issues with my email
>> client, which inadvertently split the series into two separate emails.
>
>
> Alternatively I can also copy the code from the list archive and explain why
> that doesn't work:
>
> +void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool
> +revoked) {
> + struct vfio_pci_dma_buf *priv;
> + struct vfio_pci_dma_buf *tmp;
> +
> + lockdep_assert_held_write(&vdev->memory_lock);
>
> This makes sure that the caller is holding vdev->memory_lock.
>
> +
> + list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) {
> + if (!dma_buf_try_get(priv->dmabuf))
>
> This here only works because vfio_pci_dma_buf_release() also grabs vdev-
> >memory_lock and so we are protected against concurrent releases.
>
> + continue;
> + if (priv->revoked != revoked) {
> + dma_resv_lock(priv->dmabuf->resv, NULL);
> + priv->revoked = revoked;
> + dma_buf_move_notify(priv->dmabuf);
> + dma_resv_unlock(priv->dmabuf->resv);
> + }
> + dma_buf_put(priv->dmabuf);
>
> The problem is now that this here might drop the last reference which in turn
> calls vfio_pci_dma_buf_release() which also tries to grab vdev-
> >memory_lock and so results in a deadlock.
AFAICS, vfio_pci_dma_buf_release() would not be called synchronously after the
last reference is dropped by dma_buf_put(). This is because fput(), which is called
by dma_buf_put() triggers f_op->release() asynchronously; therefore, a deadlock
is unlikely to occur in this scenario, unless I am overlooking something.
Thanks,
Vivek
>
> + }
> +}
>
> This pattern was suggested multiple times and I had to rejected it every time
> because the whole idea is just fundamentally broken.
>
> It's really astonishing how people always come up with the same broken
> pattern.
>
> Regards,
> Christian.
>
>
>
>
>
>
>
> Apart from that I have to reject the adding of
> dma_buf_try_get(), that is clearly not something we should do.
>
>
>
> Understood. It appears that Vivek has confirmed that his v2 has
> resolved the issue. I will follow up with him to determine if he plans to
> resume his patch, and if so, I will apply my last patch on top of his updated
> patch series
>
> Thanks,
> Wei Lin
>
>
> Thanks,
> Christian.
>
>
>
>
