On 08/04/2024 05:45, Ping-Ke Shih wrote:
>
>> I did not consider using a ref_cnt variable. I made the first
>> probe allocate the things and the first disconnect frees them:
>
> The allocation seems fine, but I feel we should free shared data by second
> disconnect because the other intf can still use them, no?
>
Maybe. Sure, the second disconnect can free them.
>>
>>
>> static struct usb_interface *rtl92du_get_other_intf(struct ieee80211_hw *hw)
>> {
>> struct usb_interface *intf;
>> struct usb_device *udev;
>> u8 other_interfaceindex;
>>
>> /* See SET_IEEE80211_DEV(hw, &intf->dev); in usb.c */
>> intf = container_of_const(wiphy_dev(hw->wiphy), struct usb_interface, dev);
>>
>> other_interfaceindex = 1 - intf->altsetting[0].desc.bInterfaceNumber;
>
> The value of bInterfaceNumber for two instances are 0 and 1, right? Then
> '1 - x' to get each other -- that looks a little tricky ;-)
>
The vendor driver assumes bInterfaceNumber can only be 0 or 1.
I can make it more explicit:
if (intf->altsetting[0].desc.bInterfaceNumber == 0)
other_interfaceindex = 1;
else
other_interfaceindex = 0;
>>
>> udev = interface_to_usbdev(intf);
>>
>> return usb_ifnum_to_if(udev, other_interfaceindex);
>> }
>>
>
> [...]
>
>>
>> static void rtl92du_deinit_shared_data(struct ieee80211_hw *hw)
>> {
>> struct usb_interface *other_intf = rtl92du_get_other_intf(hw);
>> struct rtl_priv *rtlpriv = rtl_priv(hw);
>>
>> if (!other_intf || usb_get_intfdata(other_intf)) {
>> /* The other interface doesn't exist or was not disconnected yet. */
>
> For the USB adaptor with single one interface, you don't have other_intf.
> Then, just free them.
>
> If the USB adaptor has two interfaces, it has both other_intf and
> usb_get_intfdata(other_intf), so you want to free them. But, I wonder if both
> interfaces can enter this branch?
>
They can't both enter this branch because after the first
disconnect usb_get_intfdata() will return NULL.
> Also as I mentioned above, how can you ensure other_intf isn't still using the
> shared data?
>
I can make the second disconnect free the shared data by
checking if usb_get_intfdata() returns NULL:
if (!other_intf || !usb_get_intfdata(other_intf)) {
/* The other interface doesn't exist or was already disconnected. */
kfree(rtlpriv->curveindex_2g);
