From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============0157674815587406609==" MIME-Version: 1.0 From: Greg Kroah-Hartman To: kbuild-all@lists.01.org Subject: Re: [staging:staging-testing 57/111] drivers/staging/wfx/scan.c:207 wfx_scan_work() warn: inconsistent returns 'sem:&wvif->scan.lock'. Date: Thu, 10 Oct 2019 10:42:47 +0200 Message-ID: <20191010084247.GA365617@kroah.com> In-Reply-To: <18946799.ioaMs8cUNb@pc-42> List-Id: --===============0157674815587406609== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Wed, Oct 09, 2019 at 03:07:39PM +0000, Jerome Pouiller wrote: > On Wednesday 9 October 2019 09:38:31 CEST kbuild test robot wrote: > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.= git staging-testing > > head: d49d1c76b96ebf39539e93d5ab7943a01ef70e4f > > commit: 1a61af0f8cbecd1610c6fc380d0fb00f57fd43f2 [57/111] staging: wfx:= allow to scan networks > > = > > If you fix the issue, kindly add following tag > > Reported-by: kbuild test robot > > Reported-by: Dan Carpenter > > = > > New smatch warnings: > > drivers/staging/wfx/scan.c:207 wfx_scan_work() warn: inconsistent retur= ns 'sem:&wvif->scan.lock'. > > Locked on: line 201 > > Unlocked on: line 145 > > = > > # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/co= mmit/?id=3D1a61af0f8cbecd1610c6fc380d0fb00f57fd43f2 > > git remote add staging https://git.kernel.org/pub/scm/linux/kernel/git/= gregkh/staging.git > > git remote update staging > > git checkout 1a61af0f8cbecd1610c6fc380d0fb00f57fd43f2 > > vim +207 drivers/staging/wfx/scan.c > > = > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 116 void wfx_scan= _work(struct work_struct *work) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 117 { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 118 struc= t wfx_vif *wvif =3D container_of(work, struct wfx_vif, scan.work); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 119 struc= t ieee80211_channel **it; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 120 struc= t wfx_scan_params scan =3D { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 121 = .scan_req.scan_type.type =3D 0, /* Foreground */ > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 122 }; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 123 struc= t ieee80211_channel *first; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 124 int i; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 125 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 126 down(= &wvif->scan.lock); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 127 mutex= _lock(&wvif->wdev->conf_mutex); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 128 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 129 if (!= wvif->scan.req || wvif->scan.curr =3D=3D wvif->scan.end) { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 130 = if (wvif->scan.output_power !=3D wvif->wdev->output_power) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 131 = hif_set_output_power(wvif, wvif->wdev->output_power * 10); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 132 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 133 = if (wvif->scan.status < 0) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 134 = dev_warn(wvif->wdev->dev, "scan failed\n"); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 135 = else if (wvif->scan.req) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 136 = dev_dbg(wvif->wdev->dev, "scan completed\n"); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 137 = else > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 138 = dev_dbg(wvif->wdev->dev, "scan canceled\n"); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 139 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 140 = wvif->scan.req =3D NULL; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 141 = wfx_tx_unlock(wvif->wdev); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 142 = mutex_unlock(&wvif->wdev->conf_mutex); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 143 = __ieee80211_scan_completed_compat(wvif->wdev->hw, wvif->scan.status ? 1 = : 0); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 144 = up(&wvif->scan.lock); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 145 = return; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 146 } > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 147 first= =3D *wvif->scan.curr; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 148 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 149 for (= it =3D wvif->scan.curr + 1, i =3D 1; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 150 = it !=3D wvif->scan.end && i < HIF_API_MAX_NB_CHANNELS; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 151 = ++it, ++i) { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 152 = if ((*it)->band !=3D first->band) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 153 = break; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 154 = if (((*it)->flags ^ first->flags) & > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 155 = IEEE80211_CHAN_NO_IR) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 156 = break; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 157 = if (!(first->flags & IEEE80211_CHAN_NO_IR) && > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 158 = (*it)->max_power !=3D first->max_power) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 159 = break; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 160 } > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 161 scan.= scan_req.band =3D first->band; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 162 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 163 if (w= vif->scan.req->no_cck) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 164 = scan.scan_req.max_transmit_rate =3D API_RATE_INDEX_G_6MBPS; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 165 else > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 166 = scan.scan_req.max_transmit_rate =3D API_RATE_INDEX_B_1MBPS; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 167 scan.= scan_req.num_of_probe_requests =3D > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 168 = (first->flags & IEEE80211_CHAN_NO_IR) ? 0 : 2; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 169 scan.= scan_req.num_of_ssi_ds =3D wvif->scan.n_ssids; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 170 scan.= ssids =3D &wvif->scan.ssids[0]; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 171 scan.= scan_req.num_of_channels =3D it - wvif->scan.curr; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 172 scan.= scan_req.probe_delay =3D 100; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 173 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 174 scan.= ch =3D kcalloc(scan.scan_req.num_of_channels, sizeof(u8), GFP_KERNEL); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 175 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 176 if (!= scan.ch) { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 177 = wvif->scan.status =3D -ENOMEM; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 178 = goto fail; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 179 } > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 180 for (= i =3D 0; i < scan.scan_req.num_of_channels; ++i) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 181 = scan.ch[i] =3D wvif->scan.curr[i]->hw_value; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 182 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 183 if (w= vif->scan.curr[0]->flags & IEEE80211_CHAN_NO_IR) { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 184 = scan.scan_req.min_channel_time =3D 50; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 185 = scan.scan_req.max_channel_time =3D 150; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 186 } els= e { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 187 = scan.scan_req.min_channel_time =3D 10; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 188 = scan.scan_req.max_channel_time =3D 50; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 189 } > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 190 if (!= (first->flags & IEEE80211_CHAN_NO_IR) && > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 191 w= vif->scan.output_power !=3D first->max_power) { > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 192 = wvif->scan.output_power =3D first->max_power; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 193 = hif_set_output_power(wvif, wvif->scan.output_power * 10); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 194 } > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 195 wvif-= >scan.status =3D wfx_scan_start(wvif, &scan); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 196 kfree= (scan.ch); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 197 if (w= vif->scan.status) > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 198 = goto fail; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 199 wvif-= >scan.curr =3D it; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 200 mutex= _unlock(&wvif->wdev->conf_mutex); > > = > > Smatch says that we should have an: up(&wvif->scan.lock);. That seems > > reasonable, but I'm not positive that it's correct. > > = > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 201 retur= n; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 202 > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 203 fail: > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 204 wvif-= >scan.curr =3D wvif->scan.end; > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 205 mutex= _unlock(&wvif->wdev->conf_mutex); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 206 up(&w= vif->scan.lock); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 @207 sched= ule_work(&wvif->scan.work); > > 1a61af0f8cbecd J=C3=A9r=C3=B4me Pouiller 2019-09-19 208 } > = > wvif->scan.lock is unlocked when scan finish or timeout (in > wfx_scan_complete()). So, the current behavior works (even if it > could/should be written more cleanly). Yes, please rewrite this to be more "obvious" otherwise you are going to have to be answering this question for the next 10+ years... thanks, greg k-h --===============0157674815587406609==--