On Tue, Feb 2, 2010 at 10:50 PM, Dave Airlie <airl...@gmail.com> wrote: > On Tue, Feb 2, 2010 at 3:11 AM, Pauli Nieminen <suok...@gmail.com> wrote: >> Allocating multiple pages of memory for data that is coming >> from user space may fail. To fix memory allocation failures >> the buffer object should be split to multiple independ pages. > > Just some quick comments > > radeon_cs.c already has something similiar to this but non-generic. It only > uses two pages on the kernel side and does the copy from user as needed. >
I knew that and I tough maybe it could be done same way here. But then it would require error checking for any access to buffer. Conversion wasn't small operation and checking that errors are handled correctly everywhere would have added quite a lot work. So I tough it would be easier to use just allocating and doing all the copy in begin of parsing. Of course handling return values with minimal code changes could be done using macros. But I hate macro implementations doing some magic returns. > Its less generic as we need to also copy the data to UC memory. > > Is this generic enough to be in the kernel proper? like flex_array? > flex_buffer? > I think this interface would require some extending. It now has only interface that is required for radeon parser to function. Even for that it should still get one more function because OUT_RING_DRM_BUFFER in 2nd patch is reading the private data directly from the buffer object. drm_buffer_copy_from_user is also implemented using DRM_COPY_FROM_USER macro. My idea was to use drm specific wrapper so this code could be ported to others kernels (BSDs) with minimal work. Also the interface that drm_buffer_copy_from_user offers might be a bit limiting. Problem would be if user data is coming in multiple parts too then buffer doesn't offer any way to append some data into the end of previous data. Good solution to this could be changing internally buffer implementation to use ring buffer which would allow appending data to the end after processing the first chunk. But this would increase complexity without anything to gain for drm. > I'll review it further later. > > Dave. > >> >> drm buffer provides generic interface to copy and process >> large data arrays from user space. >> >> Interface includes allocation and free functions to allocate >> the buffer object and data storage pages. >> >> All access operations are performed relative to a internal >> pointer which is advanced with drm_buffer_advance function. >> >> The buffer can be accessed using drm_buffer_pointer_to_XXX >> functions if it is known that requested object doesn't split >> over a page boundary. These functions don't do any error >> checking to maximize performance. >> >> If there is large object which could be split there is special >> drm_buffer_read_object function. drm_buffer_read_object takes >> a pointer as argument which is used as temporary store for >> data if it is split over boundary in the buffer. >> >> Signed-off-by: Pauli Nieminen <suok...@gmail.com> > ------------------------------------------------------------------------------ The Planet: dedicated and managed hosting, cloud storage, colocation Stay online with enterprise data centers and the best network in the business Choose flexible plans and management services without long-term contracts Personal 24x7 support from experience hosting pros just a phone call away. http://p.sf.net/sfu/theplanet-com -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
