On Tue, Mar 01, 2022 at 11:45:51AM -0800, Iouri Tarassov wrote:
[...]
> +int dxgadapter_set_vmbus(struct dxgadapter *adapter, struct hv_device *hdev)
> +{
> + int ret;
> +
> + guid_to_luid(&hdev->channel->offermsg.offer.if_instance,
> + &adapter->luid);
> + pr_debug("%s: %x:%x %p %pUb\n",
> + __func__, adapter->luid.b, adapter->luid.a, hdev->channel,
> + &hdev->channel->offermsg.offer.if_instance);
> +
> + ret = dxgvmbuschannel_init(&adapter->channel, hdev);
> + if (ret)
> + goto cleanup;
> +
> + adapter->channel.adapter = adapter;
> + adapter->hv_dev = hdev;
> +
> + ret = dxgvmb_send_open_adapter(adapter);
> + if (ret < 0) {
if (ret) is simpler?
Please be consistent regarding how you check for errors.
> + pr_err("dxgvmb_send_open_adapter failed: %d\n", ret);
> + goto cleanup;
> + }
> +
> + ret = dxgvmb_send_get_internal_adapter_info(adapter);
> + if (ret < 0)
> + pr_err("get_internal_adapter_info failed: %d", ret);
> +
> +cleanup:
> + if (ret)
> + pr_debug("err: %s %d", __func__, ret);
> + return ret;
> +}
> +
> +void dxgadapter_start(struct dxgadapter *adapter)
> +{
> + struct dxgvgpuchannel *ch = NULL;
> + struct dxgvgpuchannel *entry;
> + int ret;
> +
> + pr_debug("%s %x-%x",
> + __func__, adapter->luid.a, adapter->luid.b);
> +
> + /* Find the corresponding vGPU vm bus channel */
> + list_for_each_entry(entry, &dxgglobal->vgpu_ch_list_head,
> + vgpu_ch_list_entry) {
The mutex is not acquired in this function.
I have not checked if this function is used elsewhere. But this is a
non-static function, it should have at least a comment on the locking
requirement?
If it is not needed elsewhere, please make it static or named it
dxgadapter_start_locked.
[...]
> +}
> +
> +void dxgadapter_stop(struct dxgadapter *adapter)
Same comment applies to this function as well.
Thanks,
Wei.
