lkml.org 
[lkml]   [2022]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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.

    \
     
     \ /
      Last update: 2022-03-02 00:25    [W:5.543 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site