Messages in this thread | | | From | "Altshul, Maxim" <> | Subject | RE: [PATCH 1/2] mac80211/wlcore: Add ieee80211_hw variable to get_expected_throughput | Date | Sun, 7 Aug 2016 13:42:56 +0000 |
| |
Hi Johaness, I have prepared a patch for the issue and it is waiting for me to send it, but I feel that maybe I have not explained the previous issue well enough or I did not understand your request fully. I would like to clarify about the previous patch (the one that you applied) again:
a. The bug occurred because I have added a member called wl to the structure wl_sta, but it turned to be NULL when the function drv_get_expected_throughput was called. b. This member was NULL because it was initialized in the wrong place (sta_add instead of update_sta_state), and thus the regression has failed. c. Even so, wl_sta itself was not NULL at any point. d. This is why I have created two patches: First patch (the one that you have applied) made it easy for the driver to access hw->priv (the problematic access to hw->priv was the reason I added wl to wl_sta in the first place, which was a mistake). Second patch reverted the addition of wl member to wl_sta.
2. From what I have seen, other ops that take ieee80211_sta as a parameter do not check for sta->uploaded, which is why it feels a little odd to do it in drv_get_expected_throughput and nowhere else.
Please tell me how to proceed. If you still think that a patch is needed, I will send it right away!
Max
-----Original Message----- From: Johannes Berg [mailto:johannes@sipsolutions.net] Sent: Friday, August 05, 2016 6:34 PM To: Altshul, Maxim; linux-kernel@vger.kernel.org Cc: john.stultz@linaro.org; Kalle Valo; Eliad Peller; Machani, Yaniv Subject: Re: [PATCH 1/2] mac80211/wlcore: Add ieee80211_hw variable to get_expected_throughput
On Fri, 2016-08-05 at 13:25 +0000, Altshul, Maxim wrote: > Hi, > 1) Sorry about the change log, I will try to be clearer next time.
Just mention something about how the bug happens please, at least.
> 2+3) The issue is not that the station is not known, it's that > wl_sta->wl was null. > wl member is now completely removed from wl_sta (PATCH 2/2) and hw is > sent directly from mac80211 to the driver (so it can get hw->priv).
Right, I understand that wl_sta->wl was NULL. But the driver must have some code to assign wl_sta->wl, right? And that would be called in add_sta or sta_state. Thus the reason for the crash would be that the station wasn't actually known to the driver yet.
Even if that wasn't quite the reason here, I think we need to take it into account and check sta->uploaded before calling the driver, so I'd like you to submit a patch for that.
johannes
| |