On 05/08/2014 04:27 PM, Volkin, Bradley D wrote:
On Thu, May 08, 2014 at 02:56:05AM -0700, 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?
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.
Hi Tvrtko,
I agree that a BUG_ON might be harsh here. I suppose we could log an
error and disable the command parser. Most command buffers would
still go through fine but HW parsing would reject some that the SW
parser might otherwise allow. That could be a bit odd if we ever did
get a failure - apps/functionality that worked the last time I booted
suddenly don't this time. The issue would be in the log though.
That would indeed be weird.
So command parser is just a more permissive whitelist, a super set of
hardware parser?
I don't have a strong preference on this. Whatever people prefer.
Initially I was thinking of just rejecting all batch buffers if such
fundamental initialisation fails. It beats BUG_ON in the sense that it
leaves the system alive and it doesn't suffer from the random subtle
behaviour like if it would just disable the command parser.
It is a bit theoretical because it is likely system won't be too healthy
anyway if such a tiny allocation fails but you never know. It's better
not to crash it if we don't have to.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
