lkml.org 
[lkml]   [2008]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [git pull] drm patches for 2.6.27-rc1
From
Date
On Fri, 2008-10-17 at 15:43 -0700, Linus Torvalds wrote:
>
> On Fri, 17 Oct 2008, Dave Airlie wrote:
> >
> > Please pull the 'drm-next' branch from
> > ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git drm-next
>
> Grr.
>
> This whole merge series has been full of people sending me UNTESTED CRAP.
>
> So what's the excuse _this_ time for adding all these stupid warnings to
> my build log? Did nobody test this?
>
> drivers/gpu/drm/drm_proc.c: In function ‘drm_gem_one_name_info’:
> drivers/gpu/drm/drm_proc.c:525: warning: format ‘%d’ expects type ‘int’, but argument 3 has type ‘size_t’
> drivers/gpu/drm/drm_proc.c:533: warning: format ‘%9d’ expects type ‘int’, but argument 4 has type ‘size_t’
> drivers/gpu/drm/i915/i915_gem.c: In function ‘i915_gem_gtt_pwrite’:
> drivers/gpu/drm/i915/i915_gem.c:184: warning: unused variable ‘vaddr_atomic’
>
> and I wonder how many other warnings got added that I never even noticed
> because I don't build them?
>
> And yes, it's not just warnings. One of thse is horribly bad code:
>
> nid->len += sprintf(&nid->buf[nid->len],
> "%6d%9d%8d%9d\n",
> obj->name, obj->size,
> atomic_read(&obj->handlecount.refcount),
> atomic_read(&obj->refcount.refcount));
>
> where it's just wrong to use the field width as a separator. Because if
> the counts are big enough, the separator suddenly goes away!
>
> So that format string should be
>
> "%6d %8zd %7d %8d\n"
>
> instead. Which gives the same output when you don't overflow, and doesn't
> have the overflow bug when you do.
>
> As to that "vaddr_atomic" thing, the warning would have been avoided if
> you had just cleanly split up the optimistic fast case.
>
> IOW, write cleaner code, and the warning just goes away on its own.
> Something like the appended. UNTESTED!

Yeah, none of us are on x86-64, so we missed those warnings in testing.
The change looks pretty good except for s/return 1/return 0/. We wanted
to pull the i_wish_it_was_ioremap_atomic() hack out so that we could use
it for relocation updates as well (which aren't copy_from_user), and
I've started writing that patch. Expect something pushed at you soon.

--
Eric Anholt
eric@anholt.net eric.anholt@intel.com


[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-10-18 04:15    [from the cache]
©2003-2011 Jasper Spaans