On 14-2-2017 13:51, Johannes Berg wrote:
> On Tue, 2017-02-14 at 13:35 +0100, Arend Van Spriel wrote:
>> On 24-1-2017 10:40, Johannes Berg wrote:
>>>
>>>> + * @max_sched_scan_reqs: maximum number of scheduled scan
>>>> requests
>>>> that
>>>> + * the device can run concurrently.
>>>
>>> Perhaps we should get rid of WIPHY_FLAG_SUPPORTS_SCHED_SCAN and
>>> just
>>> set this to 1 for such devices? Otherwise we have two different
>>> requirements, and we need to track that 0 is an invalid value here
>>> if
>>> WIPHY_FLAG_SUPPORTS_SCHED_SCAN is set, or something like that?
>>
>> Ok. Doesn't that cause issues in user-space. Or do you only want to
>> get rid of it in cfg80211 api and report the flag to user-space when
>> max_sched_scan_reqs equals 1?
>
> WIPHY_FLAG_* aren't directly reported to userspace at all.
I noticed diving in the code. Indirectly it is used for reporting
support of sched_scan_start command. Anyway, I got rid of it.
>>> This might break older userspace - you'll have to put it in a later
>>> portion of the code.
>>>
>>> I'm also a bit surprised the attributes aren't actually optional
>>> for when sched scan isn't supported, I'd make the new one optional
>>> and I guess we can fix the others later too, if desired.
>>
>> Why would it break user-space. Is the order in which attributes are
>> added into the stream something user-space relies on.
>
> No. But there was a size limit on how much older userspace could
> process before we did the splitting.
I see. So basically adding stuff to (split_start == 0) is not wanted.
Just trying to get a clear requirement/rule here. Do we (still) know the
exact size limit?
>>>> +static struct cfg80211_sched_scan_request *
>>>> +cfg80211_find_sched_scan_req(struct cfg80211_registered_device
>>>> *rdev, u64 reqid)
>>>> +{
>>>> + struct cfg80211_sched_scan_request *pos;
>>>> +
>>>> + list_for_each_entry(pos, &rdev->sched_scan_req_list,
>>>> list) {
>>>> + if (pos->reqid == reqid)
>>>> + return pos;
>>>> + }
>>>> + return ERR_PTR(-ENOENT);
>>>> +}
>>>
>>> Here too, I guess, since you don't actually use RCU.
>>
>> So should I use RCU here? Not sure what is the better choice here.
>
> No no, I just meant to add locking assertions :)
Sure. Did that already pending your reply ;-)
Regards,
Arend
