Messages in this thread | | | Date | Tue, 1 Mar 2022 23:25:21 +0000 | From | Wei Liu <> | Subject | Re: [PATCH v3 04/30] drivers: hv: dxgkrnl: Creation of dxgadapter object |
| |
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.
| |