On 2018-08-02 04:04 PM, Christian König wrote:
> The kernel handles are dense and the kernel always tries to use the
> lowest free id. Use this to implement a more efficient handle table
> by using a resizeable array instead of a hash.
> 
> Signed-off-by: Christian König <[email protected]>
> 
> [...]
>  
> +drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
> +                                 void *value)
> +{
> +     if (key >= table->max_key) {
> +             uint32_t max_key = ALIGN(key, 512);

The idea here is to make the array page aligned, right? However, as is
it's only aligned to half the page size on 32-bit (and even worse on
systems where the minimum page size is e.g. 64K). Something like

        uint32_t max_key = ALIGN(key, sysconf(_SC_PAGESIZE) / sizeof(void*))

should do the trick.


Also, should handle_table_insert return an error if the table already
has a different non-NULL entry for the key?


> drm_private void handle_table_remove(struct handle_table *table, uint32_t key)
> +{
> +     table->values[key] = NULL;
> +}
> +
> +drm_private void *handle_table_lockup(struct handle_table *table, uint32_t 
> key)
> +{
> +     return table->values[key];
> +}

Typo: Should be "lookup", not "lockup".

Maybe these should check that key < table->max_key, to prevent memory
outside of the table from getting corrupted by buggy callers.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to