All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: "Kasireddy, Vivek" <vivek.kasireddy@intel.com>,
	David Airlie <airlied@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Gurchetan Singh <gurchetansingh@chromium.org>,
	Chia-I Wu <olvaffe@gmail.com>,
	Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Cc: "dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"virtualization@lists.linux.dev" <virtualization@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kernel@collabora.com" <kernel@collabora.com>
Subject: Re: [PATCH v1 2/2] drm/virtio: Fix missed dmabuf unpinning in error path of prepare_fb()
Date: Wed, 26 Mar 2025 14:54:00 +0300	[thread overview]
Message-ID: <16a30d03-9c98-47a4-959f-8671f7cb7fab@collabora.com> (raw)
In-Reply-To: <IA0PR11MB7185345D3DFA8C7900059144F8A62@IA0PR11MB7185.namprd11.prod.outlook.com>

On 3/26/25 08:14, Kasireddy, Vivek wrote:
...
>>  static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
>>  				       struct drm_plane_state *new_state)
>>  {
>> @@ -376,23 +386,16 @@ static int virtio_gpu_plane_prepare_fb(struct
>> drm_plane *plane,
>>  		vgplane_st->fence = virtio_gpu_fence_alloc(vgdev,
>>  						     vgdev->fence_drv.context,
>>  						     0);
>> -		if (!vgplane_st->fence)
>> +		if (!vgplane_st->fence) {
>> +			if (obj->import_attach)
>> +				virtio_gpu_cleanup_imported_obj(obj);
> I think checking for fence allocation failure before import would be much better.
> In other words, cleaning up the fence in case of any import errors would be
> much simpler IMO.
> 
> Regardless,
> Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> 

Another question, why do we need this fencing for imported dmabuf?
Fencing isn't done host/guest blobs in this code, while dmabuf is
essentially a guest blob. Could you please clarify why this fence is
needed? Maybe we shouldn't allocate fence in the first place for the dmabuf.

-- 
Best regards,
Dmitry

  reply	other threads:[~2025-03-26 11:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-26  1:49 [PATCH v1 1/2] drm/virtio: Don't attach GEM to a non-created context in gem_object_open() Dmitry Osipenko
2025-03-26  1:49 ` [PATCH v1 2/2] drm/virtio: Fix missed dmabuf unpinning in error path of prepare_fb() Dmitry Osipenko
2025-03-26  5:14   ` Kasireddy, Vivek
2025-03-26 11:54     ` Dmitry Osipenko [this message]
2025-03-27  7:14       ` Kasireddy, Vivek
2025-03-28  1:35         ` Dmitry Osipenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=16a30d03-9c98-47a4-959f-8671f7cb7fab@collabora.com \
    --to=dmitry.osipenko@collabora.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gurchetansingh@chromium.org \
    --cc=kernel@collabora.com \
    --cc=kraxel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olvaffe@gmail.com \
    --cc=pierre-eric.pelloux-prayer@amd.com \
    --cc=virtualization@lists.linux.dev \
    --cc=vivek.kasireddy@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.