* [linux-next:master 13298/13495] drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178)
@ 2024-07-23 18:52 Dan Carpenter
2024-07-23 18:59 ` Matthew Brost
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2024-07-23 18:52 UTC (permalink / raw)
To: oe-kbuild, Matthew Brost
Cc: lkp, oe-kbuild-all, Linux Memory Management List, Nirmoy Das
tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head: dee7f101b64219f512bb2f842227bd04c14efe30
commit: 61ac035361ae555ee5a17a7667fe96afdde3d59a [13298/13495] drm/xe: Drop xe_gt_tlb_invalidation_wait
config: i386-randconfig-141-20240722 (https://download.01.org/0day-ci/archive/20240723/202407231049.esig0Fkb-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202407231049.esig0Fkb-lkp@intel.com/
New smatch warnings:
drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178)
vim +/fence +183 drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
fc108a8b759f52 Matthew Brost 2023-01-17 159 static int send_tlb_invalidation(struct xe_guc *guc,
332dd0116c82a7 Matthew Brost 2023-01-24 160 struct xe_gt_tlb_invalidation_fence *fence,
332dd0116c82a7 Matthew Brost 2023-01-24 161 u32 *action, int len)
a9351846d94568 Matthew Brost 2023-01-17 162 {
a9351846d94568 Matthew Brost 2023-01-17 163 struct xe_gt *gt = guc_to_gt(guc);
501c4255c40935 Radhakrishna Sripada 2024-06-07 164 struct xe_device *xe = gt_to_xe(gt);
a9351846d94568 Matthew Brost 2023-01-17 165 int seqno;
a9351846d94568 Matthew Brost 2023-01-17 166 int ret;
a9351846d94568 Matthew Brost 2023-01-17 167
61ac035361ae55 Matthew Brost 2024-07-19 168 xe_gt_assert(gt, fence);
61ac035361ae55 Matthew Brost 2024-07-19 169
a9351846d94568 Matthew Brost 2023-01-17 170 /*
a9351846d94568 Matthew Brost 2023-01-17 171 * XXX: The seqno algorithm relies on TLB invalidation being processed
a9351846d94568 Matthew Brost 2023-01-17 172 * in order which they currently are, if that changes the algorithm will
a9351846d94568 Matthew Brost 2023-01-17 173 * need to be updated.
a9351846d94568 Matthew Brost 2023-01-17 174 */
565ce72e1c2d54 Matthew Auld 2023-05-24 175
a9351846d94568 Matthew Brost 2023-01-17 176 mutex_lock(&guc->ct.lock);
62ad062150c2ab Matthew Brost 2023-01-17 177 seqno = gt->tlb_invalidation.seqno;
fc108a8b759f52 Matthew Brost 2023-01-17 @178 fence->seqno = seqno;
^^^^^^^^^^^^^^^^^^^^
Dereference
501c4255c40935 Radhakrishna Sripada 2024-06-07 179 trace_xe_gt_tlb_invalidation_fence_send(xe, fence);
a9351846d94568 Matthew Brost 2023-01-17 180 action[1] = seqno;
332dd0116c82a7 Matthew Brost 2023-01-24 181 ret = xe_guc_ct_send_locked(&guc->ct, action, len,
a9351846d94568 Matthew Brost 2023-01-17 182 G2H_LEN_DW_TLB_INVALIDATE, 1);
38224c00d9c284 Matthew Brost 2023-01-24 @183 if (!ret && fence) {
^^^^^
Checked too late
35c8a964398e1c Matthew Auld 2023-07-10 184 spin_lock_irq(>->tlb_invalidation.pending_lock);
35c8a964398e1c Matthew Auld 2023-07-10 185 /*
35c8a964398e1c Matthew Auld 2023-07-10 186 * We haven't actually published the TLB fence as per
35c8a964398e1c Matthew Auld 2023-07-10 187 * pending_fences, but in theory our seqno could have already
35c8a964398e1c Matthew Auld 2023-07-10 188 * been written as we acquired the pending_lock. In such a case
35c8a964398e1c Matthew Auld 2023-07-10 189 * we can just go ahead and signal the fence here.
35c8a964398e1c Matthew Auld 2023-07-10 190 */
35c8a964398e1c Matthew Auld 2023-07-10 191 if (tlb_invalidation_seqno_past(gt, seqno)) {
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [linux-next:master 13298/13495] drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178)
2024-07-23 18:52 [linux-next:master 13298/13495] drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178) Dan Carpenter
@ 2024-07-23 18:59 ` Matthew Brost
2024-07-23 19:25 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Matthew Brost @ 2024-07-23 18:59 UTC (permalink / raw)
To: Dan Carpenter
Cc: oe-kbuild, lkp, oe-kbuild-all, Linux Memory Management List, Nirmoy Das
On Tue, Jul 23, 2024 at 01:52:46PM -0500, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head: dee7f101b64219f512bb2f842227bd04c14efe30
> commit: 61ac035361ae555ee5a17a7667fe96afdde3d59a [13298/13495] drm/xe: Drop xe_gt_tlb_invalidation_wait
> config: i386-randconfig-141-20240722 (https://download.01.org/0day-ci/archive/20240723/202407231049.esig0Fkb-lkp@intel.com/config)
> compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202407231049.esig0Fkb-lkp@intel.com/
>
> New smatch warnings:
> drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178)
>
> vim +/fence +183 drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
>
> fc108a8b759f52 Matthew Brost 2023-01-17 159 static int send_tlb_invalidation(struct xe_guc *guc,
> 332dd0116c82a7 Matthew Brost 2023-01-24 160 struct xe_gt_tlb_invalidation_fence *fence,
> 332dd0116c82a7 Matthew Brost 2023-01-24 161 u32 *action, int len)
> a9351846d94568 Matthew Brost 2023-01-17 162 {
> a9351846d94568 Matthew Brost 2023-01-17 163 struct xe_gt *gt = guc_to_gt(guc);
> 501c4255c40935 Radhakrishna Sripada 2024-06-07 164 struct xe_device *xe = gt_to_xe(gt);
> a9351846d94568 Matthew Brost 2023-01-17 165 int seqno;
> a9351846d94568 Matthew Brost 2023-01-17 166 int ret;
> a9351846d94568 Matthew Brost 2023-01-17 167
> 61ac035361ae55 Matthew Brost 2024-07-19 168 xe_gt_assert(gt, fence);
We assert the fence is not NULL here. It is invalid call with a NULL fence.
> 61ac035361ae55 Matthew Brost 2024-07-19 169
> a9351846d94568 Matthew Brost 2023-01-17 170 /*
> a9351846d94568 Matthew Brost 2023-01-17 171 * XXX: The seqno algorithm relies on TLB invalidation being processed
> a9351846d94568 Matthew Brost 2023-01-17 172 * in order which they currently are, if that changes the algorithm will
> a9351846d94568 Matthew Brost 2023-01-17 173 * need to be updated.
> a9351846d94568 Matthew Brost 2023-01-17 174 */
> 565ce72e1c2d54 Matthew Auld 2023-05-24 175
> a9351846d94568 Matthew Brost 2023-01-17 176 mutex_lock(&guc->ct.lock);
> 62ad062150c2ab Matthew Brost 2023-01-17 177 seqno = gt->tlb_invalidation.seqno;
> fc108a8b759f52 Matthew Brost 2023-01-17 @178 fence->seqno = seqno;
> ^^^^^^^^^^^^^^^^^^^^
> Dereference
>
> 501c4255c40935 Radhakrishna Sripada 2024-06-07 179 trace_xe_gt_tlb_invalidation_fence_send(xe, fence);
> a9351846d94568 Matthew Brost 2023-01-17 180 action[1] = seqno;
> 332dd0116c82a7 Matthew Brost 2023-01-24 181 ret = xe_guc_ct_send_locked(&guc->ct, action, len,
> a9351846d94568 Matthew Brost 2023-01-17 182 G2H_LEN_DW_TLB_INVALIDATE, 1);
> 38224c00d9c284 Matthew Brost 2023-01-24 @183 if (!ret && fence) {
> ^^^^^
> Checked too late
>
This check is not needed. Let me post a patch to remove this.
Thanks,
Matt
> 35c8a964398e1c Matthew Auld 2023-07-10 184 spin_lock_irq(>->tlb_invalidation.pending_lock);
> 35c8a964398e1c Matthew Auld 2023-07-10 185 /*
> 35c8a964398e1c Matthew Auld 2023-07-10 186 * We haven't actually published the TLB fence as per
> 35c8a964398e1c Matthew Auld 2023-07-10 187 * pending_fences, but in theory our seqno could have already
> 35c8a964398e1c Matthew Auld 2023-07-10 188 * been written as we acquired the pending_lock. In such a case
> 35c8a964398e1c Matthew Auld 2023-07-10 189 * we can just go ahead and signal the fence here.
> 35c8a964398e1c Matthew Auld 2023-07-10 190 */
> 35c8a964398e1c Matthew Auld 2023-07-10 191 if (tlb_invalidation_seqno_past(gt, seqno)) {
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [linux-next:master 13298/13495] drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178)
2024-07-23 18:59 ` Matthew Brost
@ 2024-07-23 19:25 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2024-07-23 19:25 UTC (permalink / raw)
To: Matthew Brost
Cc: oe-kbuild, lkp, oe-kbuild-all, Linux Memory Management List, Nirmoy Das
On Tue, Jul 23, 2024 at 06:59:07PM +0000, Matthew Brost wrote:
> On Tue, Jul 23, 2024 at 01:52:46PM -0500, Dan Carpenter wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head: dee7f101b64219f512bb2f842227bd04c14efe30
> > commit: 61ac035361ae555ee5a17a7667fe96afdde3d59a [13298/13495] drm/xe: Drop xe_gt_tlb_invalidation_wait
> > config: i386-randconfig-141-20240722 (https://download.01.org/0day-ci/archive/20240723/202407231049.esig0Fkb-lkp@intel.com/config)
> > compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > | Closes: https://lore.kernel.org/r/202407231049.esig0Fkb-lkp@intel.com/
> >
> > New smatch warnings:
> > drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178)
> >
> > vim +/fence +183 drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c
> >
> > fc108a8b759f52 Matthew Brost 2023-01-17 159 static int send_tlb_invalidation(struct xe_guc *guc,
> > 332dd0116c82a7 Matthew Brost 2023-01-24 160 struct xe_gt_tlb_invalidation_fence *fence,
> > 332dd0116c82a7 Matthew Brost 2023-01-24 161 u32 *action, int len)
> > a9351846d94568 Matthew Brost 2023-01-17 162 {
> > a9351846d94568 Matthew Brost 2023-01-17 163 struct xe_gt *gt = guc_to_gt(guc);
> > 501c4255c40935 Radhakrishna Sripada 2024-06-07 164 struct xe_device *xe = gt_to_xe(gt);
> > a9351846d94568 Matthew Brost 2023-01-17 165 int seqno;
> > a9351846d94568 Matthew Brost 2023-01-17 166 int ret;
> > a9351846d94568 Matthew Brost 2023-01-17 167
> > 61ac035361ae55 Matthew Brost 2024-07-19 168 xe_gt_assert(gt, fence);
>
> We assert the fence is not NULL here. It is invalid call with a NULL fence.
>
This assert just does a drm_WARN(). It doesn't call called BUG() or
do anything that affects flow analysis. These type of asserts are
deliberately ignored by Smatch.
With Smatch, I generally try not to print warnings about inconsistent
NULL checking when I know the extra checks are harmless. If you had
cross function analysis enabled then possibly that would silence the
warning. (This code is too new for me to have tested it on my own
system yet so I can't be sure if Smatch parses the callers correctly).
Unfortunately, cross function stuff doesn't scale well enough so we
can't use it on in the kbuild bot.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-07-23 19:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-23 18:52 [linux-next:master 13298/13495] drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c:183 send_tlb_invalidation() warn: variable dereferenced before check 'fence' (see line 178) Dan Carpenter
2024-07-23 18:59 ` Matthew Brost
2024-07-23 19:25 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox