On Fri, Dec 23, 2011 at 3:07 AM, Eyal Shapira <eyal@xxxxxxxxxx> wrote:
>
>
>
> On Thu, Dec 22, 2011 at 10:59 AM, Eliad Peller <eliad@xxxxxxxxxx> wrote:
>>
>> On Wed, Dec 21, 2011 at 2:06 PM, Eyal Shapira <eyal@xxxxxxxxxx> wrote:
>> > mac80211 requires that drv_sched_scan_stop
>> > will always succeed. We have a few possible errors
>> > such as OOM, failure to ELP wakeup, fail in the FW command.
>> > Instead of no-op , trigger a recovery which would
>> > mark sched scan as stopped and clear up any bad FW state.
>> >
>> > Signed-off-by: Eyal Shapira <eyal@xxxxxxxxxx>
>> > ---
>> this patch seems a bit redundant.
>>
>> > ret = wl1271_ps_elp_wakeup(wl);
>> > - if (ret < 0)
>> > + if (ret < 0) {
>> > + wl12xx_queue_recovery_work(wl);
>> > goto out;
>> > + }
>> >
>> we enqueue recovery in wl1271_ps_elp_wakeup() in most error paths.
>>
ok. agreed. what about the path of completion error in wl1271_ps_elp_wakeup() ?
Why not trigger recovery in that case as well ?
>
>>
>> > - /* FIXME: what to do if alloc'ing to stop fails? */
>> > stop = kzalloc(sizeof(*stop), GFP_KERNEL);
>> > if (!stop) {
>> > wl1271_error("failed to alloc memory to send sched scan stop");
>> > - return;
>> > + goto recovery;
>> > }
>> not sure if initiating recovery here will really help. it'll probably
>> fail as well, and good chances we are going to panic soon anyway :)
>>
Luca pointed out the same thing so I'm dropping this.
The question is what should we do with failing alloc as
op_sched_scan_stop should be void.
Due to other changes in the sched_scan_stop path in mac/cfg80211 it's
more important to
call ieee80211_sched_scan_stoppeD() as otherwise allocs done in
mac80211 won't be freed as well as the userspace
won't be notified of sched scan stop.
Options:
1. Call ieee80211_sched_scan_stopped() for any failure (OOM,
elp_wakeup, ...). This would free up the allocs in mac80211 and notify
userspace
but would leave the FW out of sync with the upper layers. The next
sched scan initiated would trigger a recovery given that the previous
sched
scan wasn't stopped.
2. No op - Just ignore kzalloc failure. Sched scan can't be stopped in
this case and the userspace won't get any completion event and alloc
in
mac80211 will remain.
3. Variation of 1 - Call stopped() but propagate to userspace through
the NL event that we couldn't really stop.
I don't think that would fly as it's more of an API change to userspace.
I understand that option #2 is what we're going for lacking a better
alternative.
Any other ideas / opinions ?
>> > ret = wl1271_cmd_send(wl, CMD_STOP_PERIODIC_SCAN, stop,
>> > sizeof(*stop), 0);
>> > +
>> > + kfree(stop);
>> > +
>> > if (ret < 0) {
>> > wl1271_error("failed to send sched scan stop command");
>> > - goto out_free;
>> > + goto recovery;
>> > }
>> >
>> wl1271_cmd_send enqueues recovery work on error as well.
>>
got it. will be dropped.
>>
>> anyway, the major thing i don't like here is handling the sched_stop
>> in a different manner than the rest of the commands.
>>
>> Eliad.
>
>
--
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
