All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: "Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
	"David Airlie" <airlied@linux.ie>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Gert Wollny" <gert.wollny@collabora.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.com>,
	"Daniel Stone" <daniel@fooishbar.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rob Clark" <robdclark@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Dmitry Osipenko <digetx@gmail.com>,
	linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org,
	amd-gfx@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	kernel@collabora.com, virtualization@lists.linux-foundation.org,
	spice-devel@lists.freedesktop.org, linux-rdma@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
Date: Fri, 15 Jul 2022 13:04:21 +0200	[thread overview]
Message-ID: <c6024ebb-4b3d-60c8-f0e7-006f994c9fae@amd.com> (raw)
In-Reply-To: <5c98385b-f154-0128-6f4b-5fac89529201@collabora.com>

Am 15.07.22 um 11:31 schrieb Dmitry Osipenko:
> On 7/15/22 10:19, Christian König wrote:
>>> -struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment
>>> *attach,
>>> -                    enum dma_data_direction direction)
>>> +struct sg_table *
>>> +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach,
>>> +                enum dma_data_direction direction)
>> The locking state of mapping and unmapping operations depend on if the
>> attachment is dynamic or not.
>>
>> So this here is not a good idea at all since it suggests that the
>> function is always called without holding the lock.
> I had the same thought while was working on this patch and initially was
> thinking about adding an "unlocked" alias to dma_buf_map_attachment().
> In the end I decided that it will create even more confusion and it's
> simpler just to rename this func here since there are only two drivers
> using the dynamic mapping.
>
> Do you have suggestions how to improve it?

Just keep it as it is. The ultimative goal is to switch all drivers over 
to the dynamic mapping or at least over to assume that the map/unmap 
callbacks are called with the lock held.

Regards,
Christian.

WARNING: multiple messages have this Message-ID (diff)
From: "Christian König via Virtualization" <virtualization@lists.linux-foundation.org>
To: "Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
	"David Airlie" <airlied@linux.ie>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Gert Wollny" <gert.wollny@collabora.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.com>,
	"Daniel Stone" <daniel@fooishbar.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rob Clark" <robdclark@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>
Cc: linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	amd-gfx@lists.freedesktop.org,
	virtualization@lists.linux-foundation.org,
	linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org,
	spice-devel@lists.freedesktop.org,
	Dmitry Osipenko <digetx@gmail.com>,
	kernel@collabora.com, linux-media@vger.kernel.org
Subject: Re: [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
Date: Fri, 15 Jul 2022 13:04:21 +0200	[thread overview]
Message-ID: <c6024ebb-4b3d-60c8-f0e7-006f994c9fae@amd.com> (raw)
In-Reply-To: <5c98385b-f154-0128-6f4b-5fac89529201@collabora.com>

Am 15.07.22 um 11:31 schrieb Dmitry Osipenko:
> On 7/15/22 10:19, Christian König wrote:
>>> -struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment
>>> *attach,
>>> -                    enum dma_data_direction direction)
>>> +struct sg_table *
>>> +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach,
>>> +                enum dma_data_direction direction)
>> The locking state of mapping and unmapping operations depend on if the
>> attachment is dynamic or not.
>>
>> So this here is not a good idea at all since it suggests that the
>> function is always called without holding the lock.
> I had the same thought while was working on this patch and initially was
> thinking about adding an "unlocked" alias to dma_buf_map_attachment().
> In the end I decided that it will create even more confusion and it's
> simpler just to rename this func here since there are only two drivers
> using the dynamic mapping.
>
> Do you have suggestions how to improve it?

Just keep it as it is. The ultimative goal is to switch all drivers over 
to the dynamic mapping or at least over to assume that the map/unmap 
callbacks are called with the lock held.

Regards,
Christian.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <christian.koenig@amd.com>
To: "Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
	"David Airlie" <airlied@linux.ie>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Gert Wollny" <gert.wollny@collabora.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.com>,
	"Daniel Stone" <daniel@fooishbar.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rob Clark" <robdclark@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>
Cc: linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	amd-gfx@lists.freedesktop.org,
	virtualization@lists.linux-foundation.org,
	linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org,
	spice-devel@lists.freedesktop.org,
	Dmitry Osipenko <digetx@gmail.com>,
	kernel@collabora.com, linux-media@vger.kernel.org
Subject: Re: [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
Date: Fri, 15 Jul 2022 13:04:21 +0200	[thread overview]
Message-ID: <c6024ebb-4b3d-60c8-f0e7-006f994c9fae@amd.com> (raw)
In-Reply-To: <5c98385b-f154-0128-6f4b-5fac89529201@collabora.com>

Am 15.07.22 um 11:31 schrieb Dmitry Osipenko:
> On 7/15/22 10:19, Christian König wrote:
>>> -struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment
>>> *attach,
>>> -                    enum dma_data_direction direction)
>>> +struct sg_table *
>>> +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach,
>>> +                enum dma_data_direction direction)
>> The locking state of mapping and unmapping operations depend on if the
>> attachment is dynamic or not.
>>
>> So this here is not a good idea at all since it suggests that the
>> function is always called without holding the lock.
> I had the same thought while was working on this patch and initially was
> thinking about adding an "unlocked" alias to dma_buf_map_attachment().
> In the end I decided that it will create even more confusion and it's
> simpler just to rename this func here since there are only two drivers
> using the dynamic mapping.
>
> Do you have suggestions how to improve it?

Just keep it as it is. The ultimative goal is to switch all drivers over 
to the dynamic mapping or at least over to assume that the map/unmap 
callbacks are called with the lock held.

Regards,
Christian.

WARNING: multiple messages have this Message-ID (diff)
From: "Christian König" <christian.koenig@amd.com>
To: "Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
	"David Airlie" <airlied@linux.ie>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Gurchetan Singh" <gurchetansingh@chromium.org>,
	"Chia-I Wu" <olvaffe@gmail.com>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Gert Wollny" <gert.wollny@collabora.com>,
	"Gustavo Padovan" <gustavo.padovan@collabora.com>,
	"Daniel Stone" <daniel@fooishbar.org>,
	"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Rob Clark" <robdclark@gmail.com>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Pan, Xinhui" <Xinhui.Pan@amd.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Tomasz Figa" <tfiga@chromium.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"Thomas Hellström" <thomas_os@shipmail.org>
Cc: linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	amd-gfx@lists.freedesktop.org,
	virtualization@lists.linux-foundation.org,
	linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org,
	spice-devel@lists.freedesktop.org,
	Dmitry Osipenko <digetx@gmail.com>,
	kernel@collabora.com, linux-media@vger.kernel.org
Subject: Re: [Intel-gfx] [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names
Date: Fri, 15 Jul 2022 13:04:21 +0200	[thread overview]
Message-ID: <c6024ebb-4b3d-60c8-f0e7-006f994c9fae@amd.com> (raw)
In-Reply-To: <5c98385b-f154-0128-6f4b-5fac89529201@collabora.com>

Am 15.07.22 um 11:31 schrieb Dmitry Osipenko:
> On 7/15/22 10:19, Christian König wrote:
>>> -struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment
>>> *attach,
>>> -                    enum dma_data_direction direction)
>>> +struct sg_table *
>>> +dma_buf_map_attachment_unlocked(struct dma_buf_attachment *attach,
>>> +                enum dma_data_direction direction)
>> The locking state of mapping and unmapping operations depend on if the
>> attachment is dynamic or not.
>>
>> So this here is not a good idea at all since it suggests that the
>> function is always called without holding the lock.
> I had the same thought while was working on this patch and initially was
> thinking about adding an "unlocked" alias to dma_buf_map_attachment().
> In the end I decided that it will create even more confusion and it's
> simpler just to rename this func here since there are only two drivers
> using the dynamic mapping.
>
> Do you have suggestions how to improve it?

Just keep it as it is. The ultimative goal is to switch all drivers over 
to the dynamic mapping or at least over to assume that the map/unmap 
callbacks are called with the lock held.

Regards,
Christian.

  reply	other threads:[~2022-07-15 11:04 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-15  0:52 [PATCH v1 0/6] Move all drivers to a common dma-buf locking convention Dmitry Osipenko
2022-07-15  0:52 ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52 ` Dmitry Osipenko
2022-07-15  0:52 ` [PATCH v1 1/6] dma-buf: Add _unlocked postfix to function names Dmitry Osipenko
2022-07-15  0:52   ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52   ` Dmitry Osipenko
2022-07-15  7:19   ` Christian König
2022-07-15  7:19     ` [Intel-gfx] " Christian König
2022-07-15  7:19     ` Christian König
2022-07-15  7:19     ` Christian König via Virtualization
2022-07-15  9:31     ` Dmitry Osipenko
2022-07-15  9:31       ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  9:31       ` Dmitry Osipenko
2022-07-15 11:04       ` Christian König [this message]
2022-07-15 11:04         ` [Intel-gfx] " Christian König
2022-07-15 11:04         ` Christian König
2022-07-15 11:04         ` Christian König via Virtualization
2022-07-15  0:52 ` [PATCH v1 2/6] drm/gem: Take reservation lock for vmap/vunmap operations Dmitry Osipenko
2022-07-15  0:52   ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52   ` Dmitry Osipenko
2022-07-15  7:31   ` Christian König
2022-07-15  7:31     ` Christian König via Virtualization
2022-07-15  7:31     ` [Intel-gfx] " Christian König
2022-07-15  7:31     ` Christian König
2022-07-15  0:52 ` [PATCH v1 3/6] dma-buf: Move all dma-bufs to dynamic locking specification Dmitry Osipenko
2022-07-15  0:52   ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52   ` Dmitry Osipenko
2022-07-15  0:52 ` [PATCH v1 4/6] dma-buf: Acquire wait-wound context on attachment Dmitry Osipenko
2022-07-15  0:52   ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52   ` Dmitry Osipenko
2022-07-15  3:38   ` kernel test robot
2022-07-15  3:38     ` kernel test robot
2022-07-15  6:50   ` Christian König
2022-07-15  6:50     ` [Intel-gfx] " Christian König
2022-07-15  6:50     ` Christian König
2022-07-15  6:50     ` Christian König via Virtualization
2022-07-15  6:59     ` Dmitry Osipenko
2022-07-15  6:59       ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  6:59       ` Dmitry Osipenko
2022-07-19 20:05       ` Dmitry Osipenko
2022-07-19 20:05         ` [Intel-gfx] " Dmitry Osipenko
2022-07-19 20:05         ` Dmitry Osipenko
2022-07-20  8:29         ` Christian König
2022-07-20  8:29           ` [Intel-gfx] " Christian König
2022-07-20  8:29           ` Christian König via Virtualization
2022-07-20  8:29           ` Christian König
2022-07-20 12:18           ` Dmitry Osipenko
2022-07-20 12:18             ` [Intel-gfx] " Dmitry Osipenko
2022-07-20 12:18             ` Dmitry Osipenko
2022-07-15  0:52 ` [PATCH v1 5/6] media: videobuf2: Stop using internal dma-buf lock Dmitry Osipenko
2022-07-15  0:52   ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52   ` Dmitry Osipenko
2022-07-15  0:52 ` [PATCH v1 6/6] dma-buf: Remove internal lock Dmitry Osipenko
2022-07-15  0:52   ` [Intel-gfx] " Dmitry Osipenko
2022-07-15  0:52   ` Dmitry Osipenko
2022-07-19  9:13 ` [PATCH v1 0/6] Move all drivers to a common dma-buf locking convention Tomasz Figa
2022-07-19  9:13   ` Tomasz Figa
2022-07-19  9:13   ` [Intel-gfx] " Tomasz Figa
2022-07-19  9:13   ` Tomasz Figa
2022-07-20 15:01 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork

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=c6024ebb-4b3d-60c8-f0e7-006f994c9fae@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Xinhui.Pan@amd.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=daniel.almeida@collabora.com \
    --cc=daniel@ffwll.ch \
    --cc=daniel@fooishbar.org \
    --cc=digetx@gmail.com \
    --cc=dmitry.osipenko@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gert.wollny@collabora.com \
    --cc=gurchetansingh@chromium.org \
    --cc=gustavo.padovan@collabora.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=kernel@collabora.com \
    --cc=kraxel@redhat.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mchehab@kernel.org \
    --cc=mripard@kernel.org \
    --cc=olvaffe@gmail.com \
    --cc=robdclark@gmail.com \
    --cc=rodrigo.vivi@intel.com \
    --cc=spice-devel@lists.freedesktop.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tfiga@chromium.org \
    --cc=thierry.reding@gmail.com \
    --cc=thomas_os@shipmail.org \
    --cc=tomeu.vizoso@collabora.com \
    --cc=tvrtko.ursulin@linux.intel.com \
    --cc=tzimmermann@suse.de \
    --cc=virtualization@lists.linux-foundation.org \
    /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.