[PATCH 05/16] drm/vmwgfx: Refactor resource validation hashtable to use linux/hashtable implementation.

[Thomas Hellström (Intel)]

Hi, Zack,

On 10/19/22 18:32, Zack Rusin wrote:
> On Wed, 2022-10-19 at 12:21 +0200, Thomas Hellström (Intel) wrote:
>> ⚠ External Email
>>
>> On 10/17/22 21:54, Zack Rusin wrote:
>>> From: Maaz Mombasawala <mombasawalam at vmware.com>
>>>
>>> Vmwgfx's hashtab implementation needs to be replaced with linux/hashtable
>>> to reduce maintenence burden.
>>> As part of this effort, refactor the res_ht hashtable used for resource
>>> validation during execbuf execution to use linux/hashtable implementation.
>>> This also refactors vmw_validation_context to use vmw_sw_context as the
>>> container for the hashtable, whereas before it used a vmwgfx_open_hash
>>> directly. This makes vmw_validation_context less generic, but there is
>>> no functional change since res_ht is the only instance where validation
>>> context used a hashtable in vmwgfx driver.
>> Why is a pointer to the vmw_sw_context added to the validation context,
>> rather than a pointer to the new hash table type itself? Seems like an
>> unnecessary indirection which adds a dependency on the sw context to the
>> validation context?
> Hi, Thomas. Thanks for taking a look at this! That's because the entire public
> interface of hashtable depends on it being a struct on which sizeof works rather
> than a pointer, i.e. almost the entire interface of hasthbale.h is defined and they
> all require that HASH_SIZE(hashtable) which is defined as
> #define HASH_SIZE(hashtable) (ARRAY_SIZE(hashtable))
> works on the hashtable. So the interface of hashtable.h can't operate on pointers,
> but rather needs the full struct.
>
> So the only two options are either rewriting the functions from linux/hashtable.h to
> take explicit HASH_SIZE(hashtable) or make sure that in the functions where you will
> use hashtable.h you pass the parent struct so that sizeof on the hashtable works
> correctly. I've seen both approaches in the kernel, but in this case we thought the
> latter made more sense.
>
Ah, Ok, yes, tricky one. Lots of options, none of them perfect.

Reviewed-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>