diff mbox series

[v17,10/18] drm/shmem-helper: Use refcount_t for vmap_use_count

Message ID 20230914232721.408581-11-dmitry.osipenko@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers | expand

Commit Message

Dmitry Osipenko Sept. 14, 2023, 11:27 p.m. UTC
Use refcount_t helper for vmap_use_count to make refcounting consistent
with pages_use_count and pages_pin_count that use refcount_t. This also
makes vmapping to benefit from the refcount_t's overflow checks.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 28 +++++++++++---------------
 include/drm/drm_gem_shmem_helper.h     |  2 +-
 2 files changed, 13 insertions(+), 17 deletions(-)

Comments

kernel test robot Sept. 26, 2023, 4:50 a.m. UTC | #1
Hello,

kernel test robot noticed "WARNING:at_drivers/gpu/drm/drm_fbdev_generic.c:#drm_fbdev_generic_helper_fb_dirty" on:

commit: e5f31d1a2da5d43187a0e1fc4d1882982ab0d9b8 ("[PATCH v17 10/18] drm/shmem-helper: Use refcount_t for vmap_use_count")
url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Osipenko/drm-gem-Change-locked-unlocked-postfix-of-drm_gem_v-unmap-function-names/20230915-073105
base: git://anongit.freedesktop.org/drm/drm drm-next
patch link: https://lore.kernel.org/all/20230914232721.408581-11-dmitry.osipenko@collabora.com/
patch subject: [PATCH v17 10/18] drm/shmem-helper: Use refcount_t for vmap_use_count

in testcase: rcuscale
version: 
with following parameters:

	runtime: 300s
	scale_type: tasks



compiler: gcc-11
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)



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 <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202309261238.bce3ce91-oliver.sang@intel.com


[   23.102568][    T8] ------------[ cut here ]------------
[   23.102576][    T8] platform vkms: Damage blitter failed: ret=-12
[ 23.102605][ T8] WARNING: CPU: 0 PID: 8 at drivers/gpu/drm/drm_fbdev_generic.c:226 drm_fbdev_generic_helper_fb_dirty (drivers/gpu/drm/drm_fbdev_generic.c:226) 
[   23.102614][    T8] Modules linked in:
[   23.102619][    T8] CPU: 0 PID: 8 Comm: kworker/0:0 Tainted: G                 N 6.6.0-rc1-00010-ge5f31d1a2da5 #1 debbcfa26ac7594ab4e1fde44696b4efa3ea485a
[   23.102624][    T8] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   23.102627][    T8] Workqueue: events drm_fb_helper_damage_work
[ 23.102631][ T8] EIP: drm_fbdev_generic_helper_fb_dirty (drivers/gpu/drm/drm_fbdev_generic.c:226) 
[ 23.102633][ T8] Code: 05 a8 49 81 c2 01 8b 40 08 8b 58 2c 85 db 75 02 8b 18 89 55 e0 e8 90 7a 04 00 8b 55 e0 52 53 50 68 d4 41 28 c2 e8 74 68 a5 ff <0f> 0b 8b 55 e0 83 c4 10 e9 7b ff ff ff 8d 76 00 8b 45 e4 8b 55 e8
All code
========
   0:	05 a8 49 81 c2       	add    $0xc28149a8,%eax
   5:	01 8b 40 08 8b 58    	add    %ecx,0x588b0840(%rbx)
   b:	2c 85                	sub    $0x85,%al
   d:	db 75 02             	(bad)  0x2(%rbp)
  10:	8b 18                	mov    (%rax),%ebx
  12:	89 55 e0             	mov    %edx,-0x20(%rbp)
  15:	e8 90 7a 04 00       	call   0x47aaa
  1a:	8b 55 e0             	mov    -0x20(%rbp),%edx
  1d:	52                   	push   %rdx
  1e:	53                   	push   %rbx
  1f:	50                   	push   %rax
  20:	68 d4 41 28 c2       	push   $0xffffffffc22841d4
  25:	e8 74 68 a5 ff       	call   0xffffffffffa5689e
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	8b 55 e0             	mov    -0x20(%rbp),%edx
  2f:	83 c4 10             	add    $0x10,%esp
  32:	e9 7b ff ff ff       	jmp    0xffffffffffffffb2
  37:	8d 76 00             	lea    0x0(%rsi),%esi
  3a:	8b 45 e4             	mov    -0x1c(%rbp),%eax
  3d:	8b 55 e8             	mov    -0x18(%rbp),%edx

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	8b 55 e0             	mov    -0x20(%rbp),%edx
   5:	83 c4 10             	add    $0x10,%esp
   8:	e9 7b ff ff ff       	jmp    0xffffffffffffff88
   d:	8d 76 00             	lea    0x0(%rsi),%esi
  10:	8b 45 e4             	mov    -0x1c(%rbp),%eax
  13:	8b 55 e8             	mov    -0x18(%rbp),%edx
[   23.102636][    T8] EAX: 00000000 EBX: ee027ab0 ECX: 00000000 EDX: 00000000
[   23.102638][    T8] ESI: c624b600 EDI: c624b738 EBP: c3585ef8 ESP: c3585ec4
[   23.102640][    T8] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010246
[   23.102647][    T8] CR0: 80050033 CR2: ffd3a000 CR3: 0295e000 CR4: 00040690
[   23.102649][    T8] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[   23.102651][    T8] DR6: fffe0ff0 DR7: 00000400
[   23.102653][    T8] Call Trace:
[ 23.102656][ T8] ? show_regs (arch/x86/kernel/dumpstack.c:479 arch/x86/kernel/dumpstack.c:465) 
[ 23.102661][ T8] ? drm_fbdev_generic_helper_fb_dirty (drivers/gpu/drm/drm_fbdev_generic.c:226) 
[ 23.102664][ T8] ? __warn (kernel/panic.c:673) 
[ 23.102668][ T8] ? drm_fbdev_generic_helper_fb_dirty (drivers/gpu/drm/drm_fbdev_generic.c:226) 
[ 23.102670][ T8] ? report_bug (lib/bug.c:201 lib/bug.c:219) 
[ 23.102677][ T8] ? exc_overflow (arch/x86/kernel/traps.c:250) 
[ 23.102681][ T8] ? handle_bug (arch/x86/kernel/traps.c:216) 
[ 23.102685][ T8] ? exc_invalid_op (arch/x86/kernel/traps.c:258 (discriminator 1)) 
[ 23.102689][ T8] ? handle_exception (arch/x86/entry/entry_32.S:1056) 
[ 23.102701][ T8] ? exc_overflow (arch/x86/kernel/traps.c:250) 
[ 23.102704][ T8] ? drm_fbdev_generic_helper_fb_dirty (drivers/gpu/drm/drm_fbdev_generic.c:226) 
[ 23.102708][ T8] ? exc_overflow (arch/x86/kernel/traps.c:250) 
[ 23.102710][ T8] ? drm_fbdev_generic_helper_fb_dirty (drivers/gpu/drm/drm_fbdev_generic.c:226) 
[ 23.102715][ T8] ? trace_hardirqs_on (kernel/trace/trace_preemptirq.c:63) 
[ 23.102720][ T8] drm_fb_helper_damage_work (drivers/gpu/drm/drm_fb_helper.c:391 drivers/gpu/drm/drm_fb_helper.c:413) 
[ 23.102723][ T8] process_one_work (kernel/workqueue.c:2635) 
[ 23.102731][ T8] worker_thread (kernel/workqueue.c:2697 kernel/workqueue.c:2784) 
[ 23.102736][ T8] kthread (kernel/kthread.c:388) 
[ 23.102740][ T8] ? process_one_work (kernel/workqueue.c:2730) 
[ 23.102743][ T8] ? kthread_complete_and_exit (kernel/kthread.c:341) 
[ 23.102747][ T8] ret_from_fork (arch/x86/kernel/process.c:153) 
[ 23.102750][ T8] ? kthread_complete_and_exit (kernel/kthread.c:341) 
[ 23.102754][ T8] ret_from_fork_asm (arch/x86/entry/entry_32.S:741) 
[ 23.102757][ T8] entry_INT80_32 (arch/x86/entry/entry_32.S:947) 
[   23.102765][    T8] irq event stamp: 249311
[ 23.102767][ T8] hardirqs last enabled at (249317): vprintk_emit (arch/x86/include/asm/irqflags.h:42 arch/x86/include/asm/irqflags.h:77 arch/x86/include/asm/irqflags.h:135 kernel/printk/printk.c:1961 kernel/printk/printk.c:2306) 
[ 23.102772][ T8] hardirqs last disabled at (249322): vprintk_emit (kernel/printk/printk.c:1940 kernel/printk/printk.c:2306) 
[ 23.102775][ T8] softirqs last enabled at (235498): __do_softirq (arch/x86/include/asm/preempt.h:27 kernel/softirq.c:400 kernel/softirq.c:582) 
[ 23.102778][ T8] softirqs last disabled at (235491): do_softirq_own_stack (arch/x86/kernel/irq_32.c:57 arch/x86/kernel/irq_32.c:147) 
[   23.102781][    T8] ---[ end trace 0000000000000000 ]---
[   23.104064][    T1] Console: switching to colour frame buffer device 128x48
[   23.189012][    T1] platform vkms: [drm] fb0: vkmsdrmfb frame buffer device



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230926/202309261238.bce3ce91-oliver.sang@intel.com
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index e33810450324..e1fcb5154209 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -144,7 +144,7 @@  void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
 	} else {
 		dma_resv_lock(shmem->base.resv, NULL);
 
-		drm_WARN_ON(obj->dev, shmem->vmap_use_count);
+		drm_WARN_ON(obj->dev, refcount_read(&shmem->vmap_use_count));
 
 		if (shmem->sgt) {
 			dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
@@ -343,23 +343,25 @@  int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
 
 		dma_resv_assert_held(shmem->base.resv);
 
-		if (shmem->vmap_use_count++ > 0) {
+		if (refcount_inc_not_zero(&shmem->vmap_use_count)) {
 			iosys_map_set_vaddr(map, shmem->vaddr);
 			return 0;
 		}
 
 		ret = drm_gem_shmem_pin_locked(shmem);
 		if (ret)
-			goto err_zero_use;
+			return ret;
 
 		if (shmem->map_wc)
 			prot = pgprot_writecombine(prot);
 		shmem->vaddr = vmap(shmem->pages, obj->size >> PAGE_SHIFT,
 				    VM_MAP, prot);
-		if (!shmem->vaddr)
+		if (!shmem->vaddr) {
 			ret = -ENOMEM;
-		else
+		} else {
 			iosys_map_set_vaddr(map, shmem->vaddr);
+			refcount_set(&shmem->vmap_use_count, 1);
+		}
 	}
 
 	if (ret) {
@@ -372,8 +374,6 @@  int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
 err_put_pages:
 	if (!obj->import_attach)
 		drm_gem_shmem_unpin_locked(shmem);
-err_zero_use:
-	shmem->vmap_use_count = 0;
 
 	return ret;
 }
@@ -401,14 +401,10 @@  void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
 	} else {
 		dma_resv_assert_held(shmem->base.resv);
 
-		if (drm_WARN_ON_ONCE(obj->dev, !shmem->vmap_use_count))
-			return;
-
-		if (--shmem->vmap_use_count > 0)
-			return;
-
-		vunmap(shmem->vaddr);
-		drm_gem_shmem_unpin_locked(shmem);
+		if (refcount_dec_and_test(&shmem->vmap_use_count)) {
+			vunmap(shmem->vaddr);
+			drm_gem_shmem_unpin_locked(shmem);
+		}
 	}
 
 	shmem->vaddr = NULL;
@@ -654,7 +650,7 @@  void drm_gem_shmem_print_info(const struct drm_gem_shmem_object *shmem,
 
 	drm_printf_indent(p, indent, "pages_pin_count=%u\n", refcount_read(&shmem->pages_pin_count));
 	drm_printf_indent(p, indent, "pages_use_count=%u\n", refcount_read(&shmem->pages_use_count));
-	drm_printf_indent(p, indent, "vmap_use_count=%u\n", shmem->vmap_use_count);
+	drm_printf_indent(p, indent, "vmap_use_count=%u\n", refcount_read(&shmem->vmap_use_count));
 	drm_printf_indent(p, indent, "vaddr=%p\n", shmem->vaddr);
 }
 EXPORT_SYMBOL_GPL(drm_gem_shmem_print_info);
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index 5b1ce7f7a39c..53dbd6a86edf 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -81,7 +81,7 @@  struct drm_gem_shmem_object {
 	 * Reference count on the virtual address.
 	 * The address are un-mapped when the count reaches zero.
 	 */
-	unsigned int vmap_use_count;
+	refcount_t vmap_use_count;
 
 	/**
 	 * @pages_mark_dirty_on_put: