Re: [PATCH v6 2/3] Bluetooth: Add le_scan_restart

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johan,

On Fri, Jan 9, 2015 at 11:26 AM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> Hi Jakub,
>
> On Wed, Jan 07, 2015, Jakub Pawlowski wrote:
>> -             cancel_delayed_work(&hdev->le_scan_disable);
>> +             if (!bt_cb(hdev->sent_cmd)->req.marker)
>> +                     cancel_delayed_work(&hdev->le_scan_disable);
>>
>>               clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
>
> Modifying the generic HCI request framework for this single purpose
> doesn't make much sense to me. Also, the details of the bt_cb(skb)->req
> struct are considered an implementation detail of the HCI request
> specific handling and should not be directly touched by other code.

Description of cb field of skbuff says it can be used by every layer:
" @cb: Control buffer. Free for use by every layer. Put private vars
here" ( http://lxr.free-electrons.com/source/include/linux/skbuff.h#L445
)
I fought I can just add something there, and use it.
I agree that this field doesn't fit struct hci_req_ctrl , but that was
only place where there are free bits, otherwise the struct gets too
big and don't fit cb field.

>
> It looks to me like this marker has the same purpose as the flag you had
> in some earlier revision of this set? For such a use-case a flag would
> still be the best option.

Yes, it have same meaning as this eariler version flag. The problem
with flag was that I can't set it when I create request, it need to be
set when the request is being executed, otherwise I can get race
condition. In case of this marker field, there is no option to have
race condition.

> However, I'm wondering whether we can remove
> the need for this completely. Can't you just let the code cancel the
> delayed work and the re-schedule it again once the scanning gets
> re-enabled (with some extra code decrease the timeout some appropriate
> amount for each time scanning gets restarted).

1. I can't get remaining time from workqueue, so I'll have to remember
when it was started.
2. When I do restart I'll still need some flag to know wether it was
my internal restart, or wether someone just stopped and then started
the scan, so I'll still need some flag, and way to set it.

I have one more idea on how to make it work, I'll send patch soon.

>
> Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux