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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
