Re: [RFC] [PATCH] DRM TTM Memory Manager patch

[Greg KH <greg@kroah.com>]

On Thu, Apr 26, 2007 at 04:55:14PM +1000, Dave Airlie wrote:
>  Hi,
> 
>  The patch is too big to fit on the list and I've no idea how we could
>  break it down further, it just happens to be a lot of new code..
> 
>  http://people.freedesktop.org/~airlied/ttm/0001-drm-Implement-TTM-Memory-manager-core-functionality.txt
> 
>  The patch header and diffstat are below,

A few easy and simple comments based on looking at this for 5 minutes:
  - drop the typedefs.  Yeah, they might be a drm thing, but we don't
    need them here.
  - the comment style for the functions is "odd" and not in kerneldoc
    form, but something else.  Either use kerneldoc or nothing, don't
    invent something new please.
  - what's with the /proc interface?  Don't add new proc code for
    non-process related things.  This should all go into sysfs
    somewhere.  And yes, I know /proc/dri/ is there today, but don't add
    new stuff please.
  - struct drm_bo_arg can't use an int or unsigned, as it crosses the
    userspace/kernelspace boundry, use the proper types for all values
    in those types of structures (__u32, etc.)
  - there doesn't seem to be any validity checking for the arguments
    passed into this new ioctl.  Possibly that's just the way the rest
    of the dri interface is, which is scary, but with the memory stuff,
    you really should check things properly...

that's enough to start with :)

thanks,

greg k-h