Re: [PATCH 07/11] btrfs: move to fn pointers for get/put workspaces

[Nikolay Borisov]

On 28.01.19 Ð. 23:24 Ñ., Dennis Zhou wrote:
> The previous patch added generic helpers for get_workspace() and
> put_workspace(). Now, we can migrate ownership of the workspace_manager
> to be in the compression type code as the compression code itself
> doesn't care beyond being able to get a workspace. The init/cleanup
> and get/put methods are abstracted so each compression algorithm can
> decide how they want to manage their workspaces.
> 
> Signed-off-by: Dennis Zhou <dennis@xxxxxxxxxx>

TBH I can't really see the value in this patch. IMO it doesn't make the
code more readable, on the contrary, you create algorithm-specific
wrappers over the generic function, where the sole specialization is in
the arguments passed to the generic functions. You introduce 4 more
function pointers and this affects performance negatively (albeit can't
say to what extent) due to spectre mitigations (retpolines).

I also read the follow up patches with the hopes of seeing how the code
becomes cleaner to no avail. At this point I'm really not in favor of
this particular patch.