From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Romanick Subject: Re: [PATCH 01/13] drm/mgag200: Remove unnecessary NULL check in bo_unref Date: Mon, 07 Apr 2014 13:08:43 -0700 Message-ID: <534305CB.3040500@freedesktop.org> References: <1396691102-22904-1-git-send-email-daniel.vetter@ffwll.ch> <1396691102-22904-3-git-send-email-daniel.vetter@ffwll.ch> <5342C211.1010506@freedesktop.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from q7.q7.com (q7.q7.com [69.168.48.248]) by gabe.freedesktop.org (Postfix) with ESMTP id 251696E795 for ; Mon, 7 Apr 2014 13:08:48 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Dave Jones , DRI Development List-Id: dri-devel@lists.freedesktop.org On 04/07/2014 12:56 PM, Daniel Vetter wrote: > On Mon, Apr 7, 2014 at 5:19 PM, Ian Romanick wrote: >> On 04/05/2014 02:44 AM, Daniel Vetter wrote: >>> ttm_bo_unref unconditionally calls kref_put on it's argument, so the >>> thing can't be NULL without already causing Oopses. >> >> Doesn't this mean the NULL check is in the wrong place (rather than the >> NULL check should be removed)? > > Afaics chasing callchains it's a bug to call this with NULL pointer > and no one really should be doing it. Like David Herrmann said it's > sometimes useful if unref/free functions automatically cope with NULL, > but ttm buffers don't seem to be of this kind. So consistency with > other ttm drivers seems better, same with all the gem_free_object > callbacks. That's fair. I'm convinced. Patches 1, 3, and 5 are also Reviewed-by: Ian Romanick > -Daniel