From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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 [thread overview]
Message-ID: <20191010084247.GA365617@kroah.com> (raw)
In-Reply-To: <18946799.ioaMs8cUNb@pc-42>
[-- Attachment #1: Type: text/plain, Size: 10321 bytes --]
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 <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > New smatch warnings:
> > drivers/staging/wfx/scan.c:207 wfx_scan_work() warn: inconsistent returns 'sem:&wvif->scan.lock'.
> > Locked on: line 201
> > Unlocked on: line 145
> >
> > # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id=1a61af0f8cbecd1610c6fc380d0fb00f57fd43f2
> > 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érôme Pouiller 2019-09-19 116 void wfx_scan_work(struct work_struct *work)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 117 {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 118 struct wfx_vif *wvif = container_of(work, struct wfx_vif, scan.work);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 119 struct ieee80211_channel **it;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 120 struct wfx_scan_params scan = {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 121 .scan_req.scan_type.type = 0, /* Foreground */
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 122 };
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 123 struct ieee80211_channel *first;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 124 int i;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 125
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 126 down(&wvif->scan.lock);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 127 mutex_lock(&wvif->wdev->conf_mutex);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 128
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 129 if (!wvif->scan.req || wvif->scan.curr == wvif->scan.end) {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 130 if (wvif->scan.output_power != wvif->wdev->output_power)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 131 hif_set_output_power(wvif, wvif->wdev->output_power * 10);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 132
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 133 if (wvif->scan.status < 0)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 134 dev_warn(wvif->wdev->dev, "scan failed\n");
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 135 else if (wvif->scan.req)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 136 dev_dbg(wvif->wdev->dev, "scan completed\n");
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 137 else
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 138 dev_dbg(wvif->wdev->dev, "scan canceled\n");
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 139
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 140 wvif->scan.req = NULL;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 141 wfx_tx_unlock(wvif->wdev);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 142 mutex_unlock(&wvif->wdev->conf_mutex);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 143 __ieee80211_scan_completed_compat(wvif->wdev->hw, wvif->scan.status ? 1 : 0);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 144 up(&wvif->scan.lock);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 145 return;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 146 }
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 147 first = *wvif->scan.curr;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 148
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 149 for (it = wvif->scan.curr + 1, i = 1;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 150 it != wvif->scan.end && i < HIF_API_MAX_NB_CHANNELS;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 151 ++it, ++i) {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 152 if ((*it)->band != first->band)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 153 break;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 154 if (((*it)->flags ^ first->flags) &
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 155 IEEE80211_CHAN_NO_IR)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 156 break;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 157 if (!(first->flags & IEEE80211_CHAN_NO_IR) &&
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 158 (*it)->max_power != first->max_power)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 159 break;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 160 }
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 161 scan.scan_req.band = first->band;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 162
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 163 if (wvif->scan.req->no_cck)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 164 scan.scan_req.max_transmit_rate = API_RATE_INDEX_G_6MBPS;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 165 else
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 166 scan.scan_req.max_transmit_rate = API_RATE_INDEX_B_1MBPS;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 167 scan.scan_req.num_of_probe_requests =
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 168 (first->flags & IEEE80211_CHAN_NO_IR) ? 0 : 2;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 169 scan.scan_req.num_of_ssi_ds = wvif->scan.n_ssids;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 170 scan.ssids = &wvif->scan.ssids[0];
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 171 scan.scan_req.num_of_channels = it - wvif->scan.curr;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 172 scan.scan_req.probe_delay = 100;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 173
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 174 scan.ch = kcalloc(scan.scan_req.num_of_channels, sizeof(u8), GFP_KERNEL);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 175
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 176 if (!scan.ch) {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 177 wvif->scan.status = -ENOMEM;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 178 goto fail;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 179 }
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 180 for (i = 0; i < scan.scan_req.num_of_channels; ++i)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 181 scan.ch[i] = wvif->scan.curr[i]->hw_value;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 182
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 183 if (wvif->scan.curr[0]->flags & IEEE80211_CHAN_NO_IR) {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 184 scan.scan_req.min_channel_time = 50;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 185 scan.scan_req.max_channel_time = 150;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 186 } else {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 187 scan.scan_req.min_channel_time = 10;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 188 scan.scan_req.max_channel_time = 50;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 189 }
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 190 if (!(first->flags & IEEE80211_CHAN_NO_IR) &&
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 191 wvif->scan.output_power != first->max_power) {
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 192 wvif->scan.output_power = first->max_power;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 193 hif_set_output_power(wvif, wvif->scan.output_power * 10);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 194 }
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 195 wvif->scan.status = wfx_scan_start(wvif, &scan);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 196 kfree(scan.ch);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 197 if (wvif->scan.status)
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 198 goto fail;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 199 wvif->scan.curr = it;
> > 1a61af0f8cbecd Jérôme 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érôme Pouiller 2019-09-19 201 return;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 202
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 203 fail:
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 204 wvif->scan.curr = wvif->scan.end;
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 205 mutex_unlock(&wvif->wdev->conf_mutex);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 206 up(&wvif->scan.lock);
> > 1a61af0f8cbecd Jérôme Pouiller 2019-09-19 @207 schedule_work(&wvif->scan.work);
> > 1a61af0f8cbecd Jérôme 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
parent reply other threads:[~2019-10-10 8:42 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <18946799.ioaMs8cUNb@pc-42>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191010084247.GA365617@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=kbuild-all@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.