Hi Akihiko, > Subject: Re: [PATCH v1 2/7] virtio-gpu: Don't rely on res->blob to identify > blob > resources > > > > >> Subject: Re: [PATCH v1 2/7] virtio-gpu: Don't rely on res->blob to identify > blob > >> resources > >> > >> On 2025/10/04 8:35, Vivek Kasireddy wrote: > >>> The res->blob pointer may not be valid (non-NULL) for some blobs > >>> where the backing storage is not memfd based. Therefore, we cannot > >>> use it to determine if a resource is a blob or not. Instead, we > >>> could use res->blob_size to make this determination as it is > >>> non-zero for blob resources regardless of where their backing > >>> storage is located. > >> > >> I think this patch is no longer necessary since now you add code to > >> mmap() VFIO storage with "[PATCH v1 7/7] virtio-gpu-udmabuf: Create > >> dmabuf for blobs associated with VFIO devices". > > Right, but given that mmap() can still fail for various reasons and this > > use-case can work as long as dmabuf creation succeeds, I think it makes > > sense to not rely on res->blob to determine if a resource is blob or not. > > I think the code will be simpler by making resource creation fail when > mmap() fails, and I am concerned that the guest may mulfunction with > such an incomplete resource. AFAICT, mmap() is a slow, optional path except for the cursor (which needs further improvement). So, failing resource creation when mmap() fails does not seem like a good idea to me given the fact that supporting mmap() is considered optional for dmabuf providers. And, even with vfio, mmap() can be blocked for various reasons by the kernel driver IIUC.
> > To motivate the proposed patch, there should be a use-case that requires > to have a resource without mmap(), not one that "can work" a resource > without mmap(). It is extraneous complexity otherwise. > > Such a use case should be explained in the patch message and perhaps > also with a comment in the code. The current patch message needs an > update as it sounds like it is unnecessary when theere is code to mmap() > VFIO-based backing storage, which this series has already gained. Although VFIO supports mmap(), it is not guaranteed to work in all cases and with a different dmabuf provider (in the future), it may not be possible at all. Thanks, Vivek > > Regards, > Akihiko Odaki
