From: Colin Ian King <colin.king@canonical.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
intel-gfx <intel-gfx@lists.freedesktop.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"Daniel Vetter" <daniel.vetter@ffwll.ch>
Subject: re: drm/i915/selftests: Prepare gtt tests for obj->mm.lock removal
Date: Thu, 1 Apr 2021 14:59:19 +0100 [thread overview]
Message-ID: <5fac4ebb-e0aa-d628-1457-3feffea3b891@canonical.com> (raw)
Hi,
Static analysis with Coverity on Linux-next has detected a potential
issue with the following commit:
commit 480ae79537b28f30ef6e07b7de69a9ae2599daa7
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date: Tue Mar 23 16:50:49 2021 +0100
drm/i915/selftests: Prepare gtt tests for obj->mm.lock removal
The analysis by Coverity is as follows:
145 static int igt_ppgtt_alloc(void *arg)
146 {
147 struct drm_i915_private *dev_priv = arg;
148 struct i915_ppgtt *ppgtt;
1. var_decl: Declaring variable ww without initializer.
149 struct i915_gem_ww_ctx ww;
150 u64 size, last, limit;
151 int err = 0;
152
153 /* Allocate a ppggt and try to fill the entire range */
154
2. Condition !(dev_priv->__info.ppgtt_type != INTEL_PPGTT_NONE),
taking false branch.
155 if (!HAS_PPGTT(dev_priv))
156 return 0;
157
158 ppgtt = i915_ppgtt_create(&dev_priv->gt);
3. Condition IS_ERR(ppgtt), taking false branch.
159 if (IS_ERR(ppgtt))
160 return PTR_ERR(ppgtt);
161
4. Condition !ppgtt->vm.allocate_va_range, taking true branch.
162 if (!ppgtt->vm.allocate_va_range)
5. Jumping to label err_ppgtt_cleanup.
163 goto err_ppgtt_cleanup;
164
165 /*
166 * While we only allocate the page tables here and so we could
167 * address a much larger GTT than we could actually fit into
168 * RAM, a practical limit is the amount of physical pages in
the system.
169 * This should ensure that we do not run into the oomkiller
during
170 * the test and take down the machine wilfully.
171 */
172 limit = totalram_pages() << PAGE_SHIFT;
173 limit = min(ppgtt->vm.total, limit);
174
175 i915_gem_ww_ctx_init(&ww, false);
176retry:
177 err = i915_vm_lock_objects(&ppgtt->vm, &ww);
178 if (err)
179 goto err_ppgtt_cleanup;
180
181 /* Check we can allocate the entire range */
182 for (size = 4096; size <= limit; size <<= 2) {
183 struct i915_vm_pt_stash stash = {};
184
185 err = i915_vm_alloc_pt_stash(&ppgtt->vm, &stash, size);
186 if (err)
187 goto err_ppgtt_cleanup;
188
189 err = i915_vm_pin_pt_stash(&ppgtt->vm, &stash);
190 if (err) {
191 i915_vm_free_pt_stash(&ppgtt->vm, &stash);
192 goto err_ppgtt_cleanup;
193 }
194
195 ppgtt->vm.allocate_va_range(&ppgtt->vm, &stash, 0, size);
196 cond_resched();
197
198 ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
199
200 i915_vm_free_pt_stash(&ppgtt->vm, &stash);
201 }
202
203 /* Check we can incrementally allocate the entire range */
204 for (last = 0, size = 4096; size <= limit; last = size, size
<<= 2) {
205 struct i915_vm_pt_stash stash = {};
206
207 err = i915_vm_alloc_pt_stash(&ppgtt->vm, &stash, size
- last);
208 if (err)
209 goto err_ppgtt_cleanup;
210
211 err = i915_vm_pin_pt_stash(&ppgtt->vm, &stash);
212 if (err) {
213 i915_vm_free_pt_stash(&ppgtt->vm, &stash);
214 goto err_ppgtt_cleanup;
215 }
216
217 ppgtt->vm.allocate_va_range(&ppgtt->vm, &stash,
218 last, size - last);
219 cond_resched();
220
221 i915_vm_free_pt_stash(&ppgtt->vm, &stash);
222 }
223
224 err_ppgtt_cleanup:
6. Condition err == -35, taking false branch.
225 if (err == -EDEADLK) {
226 err = i915_gem_ww_ctx_backoff(&ww);
227 if (!err)
228 goto retry;
229 }
7. uninit_use_in_call: Using uninitialized value ww.contended when
calling i915_gem_ww_ctx_fini.
Uninitialized pointer read (UNINIT)
8. uninit_use_in_call: Using uninitialized value ww.ctx.acquired when
calling i915_gem_ww_ctx_fini.
230 i915_gem_ww_ctx_fini(&ww);
231
232 i915_vm_put(&ppgtt->vm);
233 return err;
234 }
Coverity is reporting use of uninitialized values in (lines 230. Not
sure what the best fix is for this, so I'm reporting this as a potential
issue.
Colin
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
reply other threads:[~2021-04-01 13:59 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=5fac4ebb-e0aa-d628-1457-3feffea3b891@canonical.com \
--to=colin.king@canonical.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=thomas.hellstrom@linux.intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).