Date: Mon, 27 Apr 2015 16:21:35 +0200 From: Svatopluk Kraus <onwahe@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Jason Harmening <jason.harmening@gmail.com>, FreeBSD Arch <freebsd-arch@freebsd.org> Subject: Re: bus_dmamap_sync() for bounced client buffers from user address space Message-ID: <CAFHCsPX05ZFw4iZUyjwdL6_G-r7OQEZ0fsk16i9Qn97NNMmzKw@mail.gmail.com> In-Reply-To: <20150427081453.GZ2390@kib.kiev.ua> References: <20150425094152.GE2390@kib.kiev.ua> <553B9E64.8030907@gmail.com> <20150425163444.GL2390@kib.kiev.ua> <553BC9D1.1070502@gmail.com> <20150425172833.GM2390@kib.kiev.ua> <553BD501.4010109@gmail.com> <20150425181846.GN2390@kib.kiev.ua> <553BE12B.4000105@gmail.com> <20150425201410.GP2390@kib.kiev.ua> <553D2890.4020107@gmail.com> <20150427081453.GZ2390@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Apr 27, 2015 at 10:14 AM, Konstantin Belousov
<kostikbel@gmail.com> wrote:
> On Sun, Apr 26, 2015 at 01:04:00PM -0500, Jason Harmening wrote:
>>
>> On 04/25/15 15:14, Konstantin Belousov wrote:
>> > On Sat, Apr 25, 2015 at 01:47:07PM -0500, Jason Harmening wrote:
>> >> On 04/25/15 13:18, Konstantin Belousov wrote:
>> >>> On Sat, Apr 25, 2015 at 12:55:13PM -0500, Jason Harmening wrote:
>> >>>> Ah, that looks much better. A few things though:
>> >>>> 1) _bus_dmamap_load_ma (note the underscore) is still part of the MI/MD
>> >>>> interface, which we tell drivers not to use. It looks like it's
>> >>>> implemented for every arch though. Should there be a public and
>> >>>> documented bus_dmamap_load_ma ?
>> >>> Might be yes. But at least one consumer of the KPI must appear before
>> >>> the facility is introduced.
>> >> Could some of the GART/GTT code consume that?
>> > Do you mean, by GEM/GTT code ? Indeed, this is interesting and probably
>> > workable suggestion. I thought that I would need to provide a special
>> > interface from DMAR for the GEM, but your proposal seems to fit. Still,
>> > an issue is that the Linux code is structured significantly different,
>> > and this code, although isolated, is significant divergent from the
>> > upstream.
>>
>> Yes, GEM/GTT. I know it would be useful for i915, maybe other drm2
>> drivers too.
>>
>> >
>> >>>> 3) Using bus_dmamap_load_ma would mean always using physcopy for bounce
>> >>>> buffers...seems like the sfbufs would slow things down ?
>> >>> For amd64, sfbufs are nop, due to the direct map. But, I doubt that
>> >>> we can combine bounce buffers and performance in the single sentence.
>> >> In fact the amd64 implementation of uiomove_fromphys doesn't use sfbufs
>> >> at all thanks to the direct map. sparc64 seems to avoid sfbufs as much
>> >> as possible too. I don't know what arm64/aarch64 will be able to use.
>> >> Those seem like the platforms where bounce buffering would be the most
>> >> likely, along with i386 + PAE. They might still be used on 32-bit
>> >> platforms for alignment or devices with < 32-bit address width, but then
>> >> those are likely to be old and slow anyway.
>> >>
>> >> I'm still a bit worried about the slowness of waiting for an sfbuf if
>> >> one is needed, but in practice that might not be a big issue.
>> >>
>> I noticed the following in vm_map_delete, which is called by sys_munmap:
>>
>>
>> 2956 * Wait for wiring or unwiring of an entry to complete.
>> 2957 * Also wait for any system wirings to disappear on
>> 2958 * user maps.
>> 2959 */
>> 2960 if ((entry->eflags & MAP_ENTRY_IN_TRANSITION) != 0 ||
>> 2961 (vm_map_pmap(map) != kernel_pmap &&
>> 2962 vm_map_entry_system_wired_count(entry) != 0)) {
>> ...
>> 2970 (void) vm_map_unlock_and_wait(map, 0);
>>
>> It looks like munmap does wait on wired pages (well, system-wired pages, not mlock'ed pages).
>> The system-wire count on the PTE will be non-zero if vslock/vm_map_wire(...VM_MAP_WIRE_SYSTEM...) was called on it.
>> Does that mean UIO_USERSPACE dmamaps are actually safe from getting the UVA taken out from under them?
>> Obviously it doesn't make bcopy safe to do in the wrong process context, but that seems easily fixable.
>
> vslock() indeed would prevent the unmap, but it also causes very serious
> user address space fragmentation. vslock() carves map entry covering the
> specified region, which, for the typical application use of malloced
> memory for buffers, could easily fragment the bss into per-page map
> entries. It is not very important for the current vslock() use by sysctl
> code, since apps usually do bounded number of sysctls at the startup,
> but definitely it would be an issue if vslock() appears on the i/o path.
In the scope of this thread, there are two things which must be
fulfilled during DMA operations:
(1) Affected physical pages must be kept in system at any cost. It
means no swapping and no freeing.
(2) DMA sync must be doable. It means that physical pages must be
mapped somewhere if needed, even temporarily.
The point (1) must be fulfilled by DMA client by the way which is
suitable for it. It should not be a part of any DMA load or unload
method.
The subject of this thread was meant to be about point (2). I have no
problem that it was extented to point (1) too. In fact, I welcome
that. But there are still two proposed solutions how to fix bouncing
for user space buffers here.
The first solution is very simple and user space buffers could be
looked at like unbuffered ones. If a mapping is needed, some temporaty
KVA is used.
The second solution is simple too. If a mapping is needed and context
is correct, UVA is used. Otherwise, some temporaty KVA is used. I
prefer this solution as on cache not coherent DMA case, cache
maintainace operations must be taken and buffer must always have valid
mapping for them in DMA sync.
I think that support for DMA from/to user space buffers is important
for graphic adapters, fast data grabbers, whatever what needs fast
user process interaction with a device. IMHO, There is no way to
cancel support for it.
Thus some fix for bouncing must be done in all archs.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFHCsPX05ZFw4iZUyjwdL6_G-r7OQEZ0fsk16i9Qn97NNMmzKw>