[PATCH 05/19] drm: move dev_mapping to the minor node

[Dave Airlie]

On Mon, Apr 30, 2012 at 3:48 PM, Ilija Hadzic
<ihadzic at research.bell-labs.com> wrote:
>
>
> On Fri, 20 Apr 2012, Dave Airlie wrote:
>
>> On Thu, Apr 12, 2012 at 7:19 PM, Ilija Hadzic
>> <ihadzic at research.bell-labs.com> wrote:
>>>
>>> Make dev_mapping per-minor instead of per device. This is
>>> a preparatory patch for introducing render nodes. This
>>> will allow per-node instead of per-device mapping range,
>>> once we introduce render nodes.
>>
>>
>> One problem is this introduces a ttm/drm dependency that we don't
>> really have so far.
>>
>
> Sorry for a belated follow-up (I was busy with something else). I understand
> the concern and I think that the patch can be reworked such that it does not
> introduce the dependency. What introduces the dependency is a call to
> drm_unmap_mapping in ttm_bo_unmap_virtual. If the patch is reworked such
> that the minor tracks i_mapping pointer instead of the whole drm_device
> structure, the need for drm_unmap_mapping may be eliminated, which in turn
> will take care of the dependency.
>
> That brings me to a question for you. The patch from which I derived this is
> your patch 7c5cc4f63556e351e9e5980ed22accad410e3fdc (on your original
> render-nodes branch). That's where you introduced drm_unmap_mapping
> function, which calls unmap_mapping_range for every minor known to the
> system.
>
> Why was that necessary at first place ? I mean we don't do the eqivalent of
> that currently when there are multiple physical GPUs in the system, so I
> don't quite get why introduction of multiple minors would require to go
> through the whole list of minors. If that's a "just in case"/"just to be
> safe" kind of thing, then I think we may be able to just call
> drm_unmap_mapping from the driver that needs to call it and only for the
> minor in question.

When we move a buffer from VRAM->RAM we have to invalidate all
userspace mappings for it.

There could be userspace mappings on any of the device nodes, so we
need to get them all.

I think some of this code could be redone, to be less complex, Al Viro
questions why we weren't just setting the
inode mapping ourselves, citing some references in an fs somewhere
(maybe cifs), I haven't had time to follow up and check.

This would also make the case where you have something else create a
device node inside a chroot.

Dave.