[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)

linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed

* [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(&gt->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(&gt->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