All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Hellstrom <thellstrom@vmware.com>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: Martin Peres <martin.peres@labri.fr>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Alex Deucher <alexander.deucher@amd.com>,
	Ben Skeggs <bskeggs@redhat.com>, Dave Airlie <airlied@redhat.com>
Subject: Re: [PATCH v3 3/4] drm/ttm: convert to unified vma offset manager
Date: Fri, 19 Jul 2013 11:13:20 +0200	[thread overview]
Message-ID: <51E90330.5070702@vmware.com> (raw)
In-Reply-To: <CANq1E4SNzPqp-g7Ryo+n+GwpcJHZaLQVTc3AvHEXCvhuz8465w@mail.gmail.com>

On 07/18/2013 10:54 PM, David Herrmann wrote:
> Hi
>
> On Thu, Jul 18, 2013 at 1:24 PM, Thomas Hellstrom <thellstrom@vmware.com> wrote:

...

>>
>> I think that if there are good reasons to keep locking internal, I'm fine
>> with that, (And also, of course, with
>> Daniel's proposal). Currently the add / remove / lookup paths are mostly
>> used by TTM during object creation and
>> destruction.
>>
>> However, if the lookup path is ever used by pread / pwrite, that situation
>> might change and we would then like to
>> minimize the locking.
> I tried to keep the change as minimal as I could. Follow-up patches
> are welcome. I just thought pushing the lock into drm_vma_* would
> simplify things. If there are benchmarks that prove me wrong, I'll
> gladly spend some time optimizing that.

In the general case, one reason for designing the locking outside of a 
utilities like this, is that different callers may have
different requirements. For example, the call path is known not to be 
multithreaded at all, or
the caller may prefer a mutex over a spinlock for various reasons. It 
might also be that some callers will want to use
RCU locking in the future if the lookup path becomes busy, and that 
would require *all* users to adapt to RCU object destruction...

I haven't looked at the code closely enough to say that any of this 
applies in this particular case, though.


Thanks,
Thomas


> Thanks
> David

  parent reply	other threads:[~2013-07-19  9:13 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-17 18:14 [PATCH v3 0/4] Unified VMA Offset Manager v3 David Herrmann
2013-07-17 18:14 ` [PATCH v3 1/4] drm: add unified vma offset manager David Herrmann
2013-07-17 18:14 ` [PATCH v3 2/4] drm/gem: convert to new unified vma manager David Herrmann
2013-07-18 19:01   ` Patrik Jakobsson
2013-07-17 18:14 ` [PATCH v3 3/4] drm/ttm: convert to unified vma offset manager David Herrmann
2013-07-18  8:53   ` Thomas Hellstrom
2013-07-18 11:02     ` Daniel Vetter
2013-07-18 11:07     ` David Herrmann
2013-07-18 11:24       ` Thomas Hellstrom
2013-07-18 20:54         ` David Herrmann
2013-07-19  2:03           ` Jerome Glisse
2013-07-19  7:24           ` Maarten Lankhorst
2013-07-19  9:13           ` Thomas Hellstrom [this message]
     [not found]             ` <CANq1E4SSLtezptzo18TD3fLG_zoRwR17kMxEi96-e957F5wL1A@mail.gmail.com>
2013-07-22 10:55               ` David Herrmann
2013-07-22 11:45                 ` Thomas Hellstrom
2013-07-17 18:14 ` [PATCH v3 4/4] drm/vma: provide drm_vma_node_unmap() helper David Herrmann
2013-07-23 12:47 ` [PATCH v4 0/4] Unified VMA Offset Manager David Herrmann
2013-07-23 12:47   ` [PATCH v4 1/4] drm: add unified vma offset manager David Herrmann
2013-07-24 15:35     ` Daniel Vetter
2013-07-24 16:10       ` David Herrmann
2013-07-24 19:06     ` [PATCH v5 " David Herrmann
2013-07-23 12:47   ` [PATCH v4 2/4] drm/gem: convert to new unified vma manager David Herrmann
2013-07-24 15:52     ` Daniel Vetter
2013-07-24 16:27       ` David Herrmann
2013-07-24 19:07     ` [PATCH v5 " David Herrmann
2013-07-23 12:47   ` [PATCH v4 3/4] drm/ttm: convert to unified vma offset manager David Herrmann
2013-07-24 15:57     ` Daniel Vetter
2013-07-24 19:08     ` [PATCH v5 " David Herrmann
2013-07-23 12:47   ` [PATCH v4 4/4] drm/vma: provide drm_vma_node_unmap() helper David Herrmann
2013-07-24 15:58     ` Daniel Vetter
2013-07-24 19:10     ` [PATCH v5 " David Herrmann

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=51E90330.5070702@vmware.com \
    --to=thellstrom@vmware.com \
    --cc=airlied@redhat.com \
    --cc=alexander.deucher@amd.com \
    --cc=bskeggs@redhat.com \
    --cc=dh.herrmann@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=martin.peres@labri.fr \
    /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.