Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> writes:
> On 6 November 2013 10:47, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
>> Marek Puzyniak <marek.puzyniak@xxxxxxxxx> writes:
>>
>>> + struct ath10k *ar = file->private_data;
>>> + char *buf;
>>> +
>>> + buf = kzalloc(size, GFP_KERNEL);
>>> + if (buf == NULL)
>>> + return -ENOMEM;
>>> +
>>> + if (!ar->dfs_detector) {
>>> + len += scnprintf(buf + len, size - len, "DFS not enabled\n");
>>> + goto exit;
>>> + }
>>> +
>>> + ar->debug.dfs_pool_stats = ar->dfs_detector->get_stats(ar->dfs_detector);
>>
>> I think we need to take conf_mutex to make sure ar->dfs_detector is not
>> destroyed while we use it.
>>
> We deregister dfs_detector in ath10k_mac_unregister() so we will first
> destroy debugfs, then we don't need any mutex here. BTW I see we don't
> call debugfs_remove*() - so, currently mac clear debugfs for us -
> ieee80211_unregister_hw() -> wiphy_unregister I think. After that we
> deregister dfs_detector.
I don't have the code at hand but yeah, that sounds sensible.
>>> +static void ath10k_dfs_radar_report(struct ath10k *ar,
>>> + struct wmi_single_phyerr_rx_event *event,
>>> + struct phyerr_radar_report *rr,
>>> + u64 tsf)
>>> +{
>>> + u32 reg0, reg1, tsf32l;
>>> + struct pulse_event pe;
>>> + u64 tsf64;
>>> + u8 rssi, width;
>>
>> What about locking? Does this function assume that conf_mutex is held?
>> If yes, please document that with lockdep_assert_held(). If no, we have
>> a problem :)
>>
>> (Reads wmi.c)
>>
>> Ah, wmi events don't sleep anymore, forgot that. So we can't really use
>> conf_mutex here. I guess our options are bring back worker for wmi
>> events or use spinlock.
>
> I think we can use here spin_lock_bh(&ar->data_lock) when setting
> ar->debug.dfs_stats and when reading this from debugfs.
> But, is that really needed (in worst case we will get older values via debugfs)?
I would prefer not to have any race conditions in the driver, even if
it's just statistics. If there's only a race with statistics atomic
variables are also one option.
--
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
