sglist(9)

[Jeff Roberson]

On Tue, 19 May 2009, John Baldwin wrote:

> So one of the things I worked on while hacking away at unmapped disk I/O
> requests was a little API to manage scatter/gather lists of phyiscal
> addresses.  The basic premise is that a sglist describes a logical object
> that is backed by one or more physical address ranges.  To minimize locking,
> the sglist objects themselves are immutable once they are shared.  The
> unmapped disk I/O project is still very much a WIP (and I'm not even working
> on any of the really hard bits myself).  However, I actually found this
> object to be useful for something else I have been working on: the mmap()
> extensions for the Nvidia amd64 driver.  For the Nvidia patches I have
> created a new type of VM object that is very similar to OBJT_DEVICE objects
> except that it uses a sglist to determine the physical pages backing the
> object instead of calling the d_mmap() method for each page.  Anyway, adding
> this little API is just the first in a series of patches needed for the
> Nvidia driver work.  I plan to MFC them to 7.x relatively soon in the hopes
> that we can soon have a supported Nvidia driver on amd64 on 7.x.
>
> The current patches for all the Nvidia stuff is at
> http://www.FreeBSD.org/~jhb/pat/
>
> This particular patch to just add the sglist(9) API is at
> http://www.FreeBSD.org/~jhb/patches/sglist.patch and is slightly more
> polished in that it includes a manpage. :)

I have a couple of minor comments:

1) SGLIST_APPEND() contains a return() within a macro.  Shouldn't this be 
an inline that returns an error code that is always checked?  These kinds 
of macros get people into trouble.  It also could be written in such a way 
that you don't have to handle nseg == 0 at each callsite and then it's big 
enough that it probably shouldn't be a macro or an inline.

2) I worry that if all users do sglist_count() followed by a dynamic 
allocation and then an _append() they will be very expensive. 
pmap_kextract() is much more expensive than it may first seem to be.  Do 
you have a user of count already?

3) Rather than having sg_segs be an actual pointer, did you consider 
making it an unsized array?  This removes the overhead of one pointer from 
the structure while enforcing that it's always contiguously allocated.

4) SGLIST_INIT might be better off as an inline, and may not even belong 
in the header file.

In general I think this is a good idea.  It'd be nice to work on replacing 
the buf layer's implementation with something like this that could be used 
directly by drivers.  Have you considered a busdma operation to load from 
a sglist?

Thanks,
Jeff

>
> -- 
> John Baldwin
> _______________________________________________
> freebsd-arch at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe at freebsd.org"
>