From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tvrtko Ursulin Subject: Re: [PATCH] drm/i915: Use hash tables for the command parser Date: Thu, 08 May 2014 10:56:05 +0100 Message-ID: <536B54B5.4010904@linux.intel.com> References: <1398698528-25256-1-git-send-email-bradley.d.volkin@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 3599F89CB5 for ; Thu, 8 May 2014 02:56:08 -0700 (PDT) In-Reply-To: <1398698528-25256-1-git-send-email-bradley.d.volkin@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: bradley.d.volkin@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Hi Brad, On 04/28/2014 04:22 PM, bradley.d.volkin@intel.com wrote: [snip] > - BUG_ON(!validate_cmds_sorted(ring)); > + BUG_ON(!validate_cmds_sorted(ring, cmd_tables, cmd_table_count)); > BUG_ON(!validate_regs_sorted(ring)); > + > + BUG_ON(init_hash_table(ring, cmd_tables, cmd_table_count)); Is a BUG_ON a bit harsh since the above fails only on ENOMEM condition? If the concern is not allowing any command execution if parser setup has failed, it would be nicer to the system as whole to just keep rejecting everything, but let the rest of the kernel live to enable debug or whatever? I know it won't happen almost ever so it's a minor point really. I just dislike actively hosing the whole system if it is avoidable. Regards, Tvrtko