Re: dsp mmap change

On Mon, May 09, 2011 at 01:38:24PM -0400, John Baldwin wrote:
> On Monday, May 09, 2011 11:35:07 am Alan Cox wrote:
> > On Mon, May 9, 2011 at 8:45 AM, John Baldwin <jhb_at_freebsd.org> wrote:
> > 
> > > On Saturday, May 07, 2011 3:16:25 pm Kostik Belousov wrote:
> > > > On Fri, May 06, 2011 at 04:16:40PM -0400, John Baldwin wrote:
> > > > > On Friday, May 06, 2011 10:04:28 am Kostik Belousov wrote:
> > > > > > On Fri, May 06, 2011 at 04:38:00PM +0300, Andriy Gapon wrote:
> > > > > > > on 06/05/2011 16:32 Kostik Belousov said the following:
> > > > > > > > On Fri, May 06, 2011 at 11:55:00AM +0300, Andriy Gapon wrote:
> > > > > > > >>
> > > > > > > >> I would like to ask for a review and/or testing of the following
> > > patch:
> > > > > > > >> http://people.freebsd.org/~avg/dev_dsp_mmap.diff
> > > > > > > >>
> > > > > > > >> It's supposed to fix an issue described here:
> > > > > > > >> http://lists.freebsd.org/pipermail/freebsd-multimedia/2011-
> > > > > February/011691.html
> > > > > > > >>
> > > > > > > >> In short, the following pseudo-code should do the right thing:
> > > > > > > >> fd = open(/dev/dsp, O_RDWR);
> > > > > > > >> mmap(PROT_READ, fd);
> > > > > > > >> mmap(PROT_WRITE, fd);
> > > > > > > >>
> > > > > > > >> Thank you!
> > > > > > > >
> > > > > > > > I think that you have to call PCM_GIANT_LEAVE() when returning
> > > > > > > > EINVAL on the vm_pager_alloc() failure.
> > > > > > >
> > > > > > > Yes, thank you.
> > > > > > >
> > > > > > > > Your patch hardcodes an assumption that sndbufs are always
> > > > > > > > contiguous. I was unable to convince myself that this is true.
> > > > > > >
> > > > > > > I think that this should be true for the case when DMA is used?
> > > > > > In the current driver, yes, but there is nothing that theoretically
> > > > > > prevents scatter-gather from be used.
> > > > >
> > > > > You could "fix" this by creating an sglist (via sglist_build()) and an
> > > > > OBJT_SG VM object that the d_mmap_single callback returned.  I wish
> > > there
> > > > > was a cleaner way to just create a VM object and populate it with pages
> > > > > though, and then use vm_map_insert() to map it into the kernel rather
> > > > > than the more roundabout method of OBJT_SG.
> > > >
> > > > You cannot have one page inserted into two vm objects. Contigmalloc()
> > > > inserts the allocated pages into kernel_object.
> > >
> > > Yes, I would want to allocate N unmapped pages and stuff them into a VM
> > > object that can then be mapped into the kernel and/or into user mappings.
> > > This would be a much cleaner approach for the nvidia driver for example.
> > >
> > >
> > There is a relatively new function, vm_object_populate(), that allocates a
> > collection of pages, inserts them into a vm object, and validates them for
> > use.  This function exits in FreeBSD 7.x, 8.x, and HEAD.
> 
> Hmm, is there a way to specify restrictions on the pages allocated similar to
> what contigmalloc() supports (e.g. being able to allocate pages in the lower
> 4GB is something the Nvidia driver needs)?
> 
> IOW, a method about like contigmalloc() that returned a VM object holding
> pages meeting the desired restrictions but didn't necessarily map the
> pages.  kmem_alloc_attr() is close to this except it always inserts the
> pages into the kernel_object and always maps the pages into the address space.
> 
For GEM, that needs something very similar, I just allocate the swap object,
and then do vm_page_grab() over all range.

In fact, I never need the full kernel mapping of the object into the
KVA using physical addresses, it is enough to do only per-page temporal
mappings with sfbuf. On the other hand, both usermode and kernelmode
need to access the pages through the aperture remap. Usermode might need
to establish several mappings for one GTT page, and this causes troubles
because pmap_remove_all() does not work on the fictitious pages.
[You was on Cc:].

Other then that, and the fact that vm_page_grab() cannot satisfy the
restrictions on the page placement in the physical space,
normal swap object is enough for much more complicated device
then Azalia.