Re: [PATCH] drm/i915: Use hash tables for the command parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 05/08/2014 02:02 PM, Damien Lespiau wrote:
On Thu, May 08, 2014 at 01:25:33PM +0100, Tvrtko Ursulin wrote:

On 05/08/2014 12:44 PM, Damien Lespiau wrote:
On Thu, May 08, 2014 at 10:56:05AM +0100, Tvrtko Ursulin wrote:

Hi Brad,

On 04/28/2014 04:22 PM, bradley.d.volkin@xxxxxxxxx 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?

Those number_of_cmds allocations are a bit awkward though, couldn't we
just embed the hlist_node into the desciptor struct?

Until Brad comes online, I think that is because command descriptors
to hash table entries are not 1-to-1.

Ah, I guess the common cmds are part of several hash tables. We could at
least turn the multiple allocations into one big allocation though.

Probably a problem since commands can be added dynamically.

Also from the memory use point of view, single allocations are 12/24 bytes so fit into 16/32 byte slabs. That's some wastage on the face of it, but bunching them up would make... say common plus render commands are ~30 in total, so 360/720 bytes. Which would waste 152/304 bytes (512 and 1024 slabs) vs. 120/240 bytes for individual allocations. So for 30 command array current approach is even better. :)

Maybe make a dedicated cache for struct cmd_node?

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux