On Sat, Nov 26, 2011 at 4:57 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Sat, 26 Nov 2011 10:44:17 +0600, Rakib Mullick <rakib.mullick@xxxxxxxxx> wrote: >> On Mon, Nov 21, 2011 at 11:16 PM, Keith Packard <keithp@xxxxxxxxxx> wrote: >> > On Mon, 21 Nov 2011 17:23:06 +0100, Daniel Vetter <daniel@xxxxxxxx> wrote: >> > >> >> Indeed, nice catch (albeit totally unlikely to be hit, because the error >> >> only happens when the gpu ceases to progress in the ring, so imo not >> >> stable material). Keith, please pick this up for fixes, thanks. >> > >> > It's already there and queued for airlied :-) >> > >> Thank you guys for reviewing and taking the patch. >> >> Now, while I was looking at the uses of i915_add_request(), I found >> the following code : >> >> ret = i915_gem_flush_ring(ring, 0, >> I915_GEM_GPU_DOMAINS); >> request = kzalloc(sizeof(*request), GFP_KERNEL); >> if (ret || request == NULL || >> i915_add_request(ring, NULL, request)) >> kfree(request); >> >> From above code, we might ended up by calling kfree(request) without >> allocating request. Though, it's unlikely cause if request is NULL >> then BUG_ON will be hit in i915_add_request(). So, to unify the callee >> uses of i915_add_request(), I'm proposing the following patch. Please >> let me know what do you guys think. If you guys agree, I can sent a >> formal patch. > > kfree(NULL) is permitted and taken advantage of here along with the > short-circuiting behaviour of '||'. Yes, no real problem with current code. I was just thinking from code cleanup's pov. Is BUG_ON really needed in i915_add_request() ? thanks, Rakib _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
