dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
To: "Thomas Hellström" <thellstrom@vmware.com>
Cc: jakob@vmware.com, dri-devel@lists.freedesktop.org,
	linaro-mm-sig@lists.linaro.org, sumit.semwal@linaro.org,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] reservation: cross-device reservation support
Date: Fri, 28 Sep 2012 18:01:46 +0200	[thread overview]
Message-ID: <5065C9EA.1090206@canonical.com> (raw)
In-Reply-To: <5065C269.30406@vmware.com>

Op 28-09-12 17:29, Thomas Hellström schreef:
> On 9/28/12 2:43 PM, Maarten Lankhorst wrote:
>> This adds support for a generic reservations framework that can be
>> hooked up to ttm and dma-buf and allows easy sharing of reservations
>> across devices.
>>
>> The idea is that a dma-buf and ttm object both will get a pointer
>> to a struct reservation_object, which has to be reserved before
>> anything is done with the buffer.
> "Anything is done with the buffer" should probably be rephrased, as different members of the buffer struct
> may be protected by different locks. It may not be practical or even possible to
> protect all buffer members with reservation.
Agreed.
>> Some followup patches are needed in ttm so the lru_lock is no longer
>> taken during the reservation step. This makes the lockdep annotation
>> patch a lot more useful, and the assumption that the lru lock protects
>> atomic removal off the lru list will fail soon, anyway.
> As previously discussed, I'm unfortunately not prepared to accept removal of the reserve-lru atomicity
>  into the TTM code at this point.
> The current code is based on this assumption and removing it will end up with
> efficiencies, breaking the delayed delete code and probably a locking nightmare when trying to write
> new TTM code.
The lru lock removal patch fixed the delayed delete code, it really is not different from the current
situation. In fact it is more clear without the guarantee what various parts are trying to protect.

Nothing prevents you from holding the lru_lock while trylocking,
leaving that guarantee intact for that part. Can you really just review
the patch and tell me where it breaks and/or makes the code unreadable?

See my preemptive reply to patch 1/5 for details. I would prefer you
followup there. :-)

~Maarten

  reply	other threads:[~2012-09-28 16:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-28 12:41 [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER Maarten Lankhorst
2012-09-28 12:42 ` [PATCH 2/5] fence: dma-buf cross-device synchronization (v9) Maarten Lankhorst
2012-10-07 16:31   ` Maarten Lankhorst
2012-09-28 12:42 ` [PATCH 3/5] seqno-fence: Hardware dma-buf implementation of fencing (v3) Maarten Lankhorst
2012-09-28 12:43 ` [PATCH 4/5] reservation: cross-device reservation support Maarten Lankhorst
2012-09-28 15:29   ` Thomas Hellström
2012-09-28 16:01     ` Maarten Lankhorst [this message]
2012-10-03 12:33   ` Thomas Hellstrom
2012-09-28 12:43 ` [PATCH 5/5] reservation: Add lockdep annotation and selftests Maarten Lankhorst
2012-09-28 13:20 ` [PATCH 1/5] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER Daniel Vetter
2012-09-28 14:14 ` Maarten Lankhorst
2012-09-28 19:42   ` Thomas Hellstrom
2012-09-28 20:10     ` Thomas Hellstrom
2012-09-29 15:16       ` Maarten Lankhorst
2012-10-01  8:49         ` Thomas Hellstrom
2012-10-01  9:47     ` Maarten Lankhorst
2012-10-02  6:46       ` Thomas Hellstrom
2012-10-02  8:03         ` Daniel Vetter
2012-10-03  7:45           ` Thomas Hellstrom
2012-10-03  7:54             ` Daniel Vetter
2012-10-03  8:37               ` Thomas Hellstrom
2012-10-03  8:53                 ` Daniel Vetter
2012-10-03 10:53                   ` Thomas Hellstrom
2012-10-03 12:46                     ` Maarten Lankhorst
2012-10-03 12:56                       ` Thomas Hellstrom
2012-10-03  7:57             ` Maarten Lankhorst
2012-10-03  8:35               ` Thomas Hellstrom

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=5065C9EA.1090206@canonical.com \
    --to=maarten.lankhorst@canonical.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jakob@vmware.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=thellstrom@vmware.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).