Message ID | fe31efd659622839c7f7bc2890d9e3411bbfa7cd.1671952191.git.drv@mailo.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | convert i915_active.count from atomic_t to refcount_t | expand |
Greeting, FYI, we noticed WARNING:at_lib/refcount.c:#refcount_warn_saturate due to commit (built with gcc-11): commit: 22c498b9fea46946f06b30a699b496116c36dddb ("[PATCH v2 1/2] drm/i915: convert i915_active.count from atomic_t to refcount_t") url: https://github.com/intel-lab-lkp/linux/commits/Deepak-R-Varma/drm-i915-convert-i915_active-count-from-atomic_t-to-refcount_t/20221225-154850 base: git://anongit.freedesktop.org/drm-intel for-linux-next patch link: https://lore.kernel.org/all/fe31efd659622839c7f7bc2890d9e3411bbfa7cd.1671952191.git.drv@mailo.com/ patch subject: [PATCH v2 1/2] drm/i915: convert i915_active.count from atomic_t to refcount_t in testcase: hwsim version: hwsim-x86_64-717e5d7-1_20220525 with following parameters: test: group-17 on test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): If you fix the issue, kindly add following tag | Reported-by: kernel test robot <oliver.sang@intel.com> | Link: https://lore.kernel.org/oe-lkp/202212281735.20cb559d-oliver.sang@intel.com [ 34.427757][ T207] ------------[ cut here ]------------ [ 34.438445][ T207] refcount_t: addition on 0; use-after-free. [ 34.444241][ T207] WARNING: CPU: 1 PID: 207 at lib/refcount.c:25 refcount_warn_saturate+0xdd/0x140 [ 34.449776][ T402] raid6: avx2x4 gen() 27042 MB/s [ 34.453208][ T207] Modules linked in: raid6_pq(+) zstd_compress x86_pkg_temp_thermal libcrc32c intel_powerclamp coretemp i915(+) sd_mod t10_pi ipmi_devintf ipmi_msghandler kvm_intel crc64_rocksoft_generic crc64_rocksoft crc64 drm_buddy kvm sg irqbypass intel_gtt crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 drm_display_helper mei_wdt ttm rapl drm_kms_helper mei_me ahci syscopyarea intel_cstate wmi_bmof sysfillrect video i2c_i801 libahci intel_uncore mei libata sysimgblt i2c_smbus intel_pch_thermal wmi intel_pmc_core acpi_pad drm fuse ip_tables [ 34.474778][ T402] raid6: avx2x2 gen() 28719 MB/s [ 34.508560][ T207] CPU: 1 PID: 207 Comm: systemd-udevd Tainted: G S I 6.1.0-rc6-01446-g22c498b9fea4 #1 [ 34.508566][ T207] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.1.1 10/07/2015 [ 34.508568][ T207] RIP: 0010:refcount_warn_saturate+0xdd/0x140 [ 34.529776][ T402] raid6: avx2x1 gen() 23869 MB/s [ 34.532014][ T207] Code: ca 91 08 03 01 e8 01 78 37 01 0f 0b eb 9d 80 3d b9 91 08 03 00 75 94 48 c7 c7 80 23 c2 83 c6 05 a9 91 08 03 01 e8 e1 77 37 01 <0f> 0b e9 7a ff ff ff 80 3d 93 91 08 03 00 0f 85 6d ff ff ff 48 c7 [ 34.532017][ T207] RSP: 0018:ffffc900009671c8 EFLAGS: 00010082 [ 34.537880][ T402] raid6: using algorithm avx2x2 gen() 28719 MB/s [ 34.542794][ T207] [ 34.542795][ T207] RAX: 0000000000000000 RBX: ffff88884b15b410 RCX: 0000000000000000 [ 34.542798][ T207] RDX: 0000000000000027 RSI: 0000000000000004 RDI: fffff5200012ce2b [ 34.542799][ T207] RBP: 0000000000000002 R08: 0000000000000001 R09: ffff8887c98a76eb [ 34.542801][ T207] R10: ffffed10f9314edd R11: 0000000000000001 R12: ffff88884b15b438 [ 34.578775][ T402] raid6: .... xor() 18702 MB/s, rmw enabled [ 34.583984][ T207] R13: ffffc90000967200 R14: 0000000000000001 R15: ffffc90000967298 [ 34.583986][ T207] FS: 00007f0bc66948c0(0000) GS:ffff8887c9880000(0000) knlGS:0000000000000000 [ 34.591744][ T402] raid6: using avx2x2 recovery algorithm [ 34.599493][ T207] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 34.599497][ T207] CR2: 00007fe21aecfd95 CR3: 0000000861710002 CR4: 00000000003706e0 [ 34.599498][ T207] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 34.599499][ T207] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 34.607256][ T402] initcall init_module+0x0/0x1000 [raid6_pq] returned 0 after 174058 usecs [ 34.612941][ T207] Call Trace: [ 34.612945][ T207] <TASK> [ 34.612946][ T207] __i915_active_activate+0x185/0x190 [i915] [ 34.684563][ T207] ? node_retire+0xf0/0xf0 [i915] [ 34.689583][ T207] i915_active_acquire+0x230/0x2a0 [i915] [ 34.695268][ T207] ? i915_active_acquire_if_busy+0x130/0x130 [i915] [ 34.701820][ T207] ? __mutex_lock_interruptible_slowpath+0x10/0x10 [ 34.708137][ T207] ? kmem_cache_alloc+0x16e/0x2f0 [ 34.712980][ T207] i915_vma_pin_ww+0x495/0x1240 [i915] [ 34.718455][ T207] ? vma_invalidate_tlb+0xd0/0xd0 [i915] [ 34.724088][ T207] ? allocate_slab+0x116/0x290 [ 34.728679][ T207] ? __cond_resched+0x1c/0x90 [ 34.733178][ T207] __i915_ggtt_pin+0x97/0x1f0 [i915] [ 34.738462][ T207] i915_ggtt_pin+0x161/0x360 [i915] [ 34.743645][ T207] ? __i915_ggtt_pin+0x1f0/0x1f0 [i915] [ 34.749212][ T207] intel_gt_init+0x110/0x640 [i915] [ 34.754380][ T207] i915_gem_init+0x23e/0x480 [i915] [ 34.759556][ T207] i915_driver_probe+0x1bd/0x6c0 [i915] [ 34.765056][ T207] ? __mutex_unlock_slowpath+0x2a0/0x2a0 [ 34.771529][ T207] ? i915_print_iommu_status+0x40/0x40 [i915] [ 34.777560][ T207] ? drm_privacy_screen_get+0x281/0x2f0 [drm] [ 34.783471][ T207] i915_pci_probe+0x20b/0x4c0 [i915] [ 34.788730][ T207] ? i915_pci_remove+0x80/0x80 [i915] [ 34.794060][ T207] ? _raw_read_unlock_irqrestore+0x40/0x40 [ 34.799671][ T207] ? __cond_resched+0x1c/0x90 [ 34.804169][ T207] ? i915_pci_remove+0x80/0x80 [i915] [ 34.809515][ T207] local_pci_probe+0xdc/0x170 [ 34.814007][ T207] pci_call_probe+0x15f/0x500 [ 34.818498][ T207] ? _raw_spin_lock+0x81/0xd0 [ 34.822990][ T207] ? pci_pm_suspend_noirq+0x8c0/0x8c0 [ 34.828172][ T207] ? pci_assign_irq+0x81/0x280 [ 34.832753][ T207] ? pci_match_device+0x351/0x6a0 [ 34.837589][ T207] ? kernfs_put+0x18/0x30 [ 34.841742][ T207] pci_device_probe+0xee/0x230 [ 34.846320][ T207] really_probe+0x1da/0xa60 [ 34.850643][ T207] __driver_probe_device+0x2a1/0x460 [ 34.855739][ T207] driver_probe_device+0x49/0x120 [ 34.860578][ T207] __driver_attach+0x178/0x4b0 [ 34.865160][ T207] ? __device_attach_driver+0x260/0x260 [ 34.870513][ T207] bus_for_each_dev+0x11b/0x1a0 [ 34.875179][ T207] ? subsys_dev_iter_exit+0x10/0x10 [ 34.880188][ T207] ? __kmem_cache_alloc_node+0x17e/0x2a0 [ 34.885628][ T207] ? klist_add_tail+0x12f/0x270 [ 34.890296][ T207] bus_add_driver+0x423/0x570 [ 34.894792][ T207] driver_register+0x20f/0x390 [ 34.899372][ T207] i915_init+0x54/0x15d [i915] [ 34.904112][ T207] ? 0xffffffffa17f5000 [ 34.908086][ T207] do_one_initcall+0x87/0x2c0 [ 34.912582][ T207] ? trace_event_raw_event_initcall_level+0x190/0x190 [ 34.919141][ T207] ? kasan_poison+0x1f/0x40 [ 34.923463][ T207] ? kasan_unpoison+0x23/0x50 [ 34.927957][ T207] do_init_module+0x18a/0x620 [ 34.932455][ T207] load_module+0x1c34/0x2450 [ 34.936863][ T207] ? post_relocation+0x370/0x370 [ 34.941617][ T207] ? __x64_sys_fspick+0x290/0x290 [ 34.946455][ T207] ? do_mmap+0x709/0x10c0 [ 34.950610][ T207] ? __do_sys_finit_module+0x113/0x1b0 [ 34.955888][ T207] __do_sys_finit_module+0x113/0x1b0 [ 34.960991][ T207] ? __ia32_sys_init_module+0xa0/0xa0 [ 34.966186][ T207] ? fput+0x19/0x120 [ 34.969912][ T207] do_syscall_64+0x35/0x80 [ 34.974158][ T207] entry_SYSCALL_64_after_hwframe+0x5e/0xc8 [ 34.979868][ T207] RIP: 0033:0x7f0bc6b4d9b9 [ 34.984103][ T207] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a7 54 0c 00 f7 d8 64 89 01 48 [ 35.003389][ T207] RSP: 002b:00007ffedd49dcf8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 35.011584][ T207] RAX: ffffffffffffffda RBX: 0000560156f63b90 RCX: 00007f0bc6b4d9b9 [ 35.019346][ T207] RDX: 0000000000000000 RSI: 00007f0bc6cd8e2d RDI: 0000000000000016 [ 35.027105][ T207] RBP: 0000000000020000 R08: 0000000000000000 R09: 0000560156f63d90 [ 35.034868][ T207] R10: 0000000000000016 R11: 0000000000000246 R12: 00007f0bc6cd8e2d [ 35.042630][ T207] R13: 0000000000000000 R14: 0000560156f63e10 R15: 0000560156f63b90 [ 35.050392][ T207] </TASK> [ 35.053249][ T207] ---[ end trace 0000000000000000 ]--- To reproduce: git clone https://github.com/intel/lkp-tests.git cd lkp-tests sudo bin/lkp install job.yaml # job file is attached in this email bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run sudo bin/lkp run generated-yaml-file # if come across any failure that blocks the test, # please remove ~/.lkp and /lkp dir to run from a clean state.
On 25-12-2022 13:17, Deepak R Varma wrote: > The refcount_* APIs are designed to address known issues with the > atomic_t APIs for reference counting. They provide following distinct > advantages: > - protect the reference counters from overflow/underflow > - avoid use-after-free errors > - provide improved memory ordering guarantee schemes > - neater and safer. > Hence, convert the atomic_t count member variable and associated > atomic_*() API calls to equivalent refcount_t type and refcount_*() API > calls. > > This patch proposal address the following warnings generated by > the atomic_as_refcounter.cocci coccinelle script > atomic_add_unless > > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > Please note: > 1. Proposed changes are compile tested only. > 2. This patch 1/2 is required to be applied before patch 2/2 due to > interdependency. > > Changes in v2: > 1. Patch added to the patch series. > 2. Handle build issues Reported-by: kernel test robot <lkp@intel.com> > Earlier a standalone patch was sent for the i915 base driver only. The > Kernel Test Robot reported build failure for additional atomic_*() calls > specific to i915 debugging support when enabled. This version now includes > those changes as well. > > > drivers/gpu/drm/i915/i915_active.c | 28 +++++++++++++----------- > drivers/gpu/drm/i915/i915_active.h | 6 ++--- > drivers/gpu/drm/i915/i915_active_types.h | 4 ++-- > 3 files changed, 20 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c > index 7412abf166a8..5e58d8b1e947 100644 > --- a/drivers/gpu/drm/i915/i915_active.c > +++ b/drivers/gpu/drm/i915/i915_active.c > @@ -92,14 +92,14 @@ static void debug_active_init(struct i915_active *ref) > static void debug_active_activate(struct i915_active *ref) > { > lockdep_assert_held(&ref->tree_lock); > - if (!atomic_read(&ref->count)) /* before the first inc */ > + if (!refcount_read(&ref->count)) /* before the first inc */ > debug_object_activate(ref, &active_debug_desc); > } > > static void debug_active_deactivate(struct i915_active *ref) > { > lockdep_assert_held(&ref->tree_lock); > - if (!atomic_read(&ref->count)) /* after the last dec */ > + if (!refcount_read(&ref->count)) /* after the last dec */ > debug_object_deactivate(ref, &active_debug_desc); > } > > @@ -133,7 +133,7 @@ __active_retire(struct i915_active *ref) > GEM_BUG_ON(i915_active_is_idle(ref)); > > /* return the unused nodes to our slabcache -- flushing the allocator */ > - if (!atomic_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, flags)) > + if (!refcount_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, &flags)) > return; > > GEM_BUG_ON(rcu_access_pointer(ref->excl.fence)); > @@ -179,8 +179,8 @@ active_work(struct work_struct *wrk) > { > struct i915_active *ref = container_of(wrk, typeof(*ref), work); > > - GEM_BUG_ON(!atomic_read(&ref->count)); > - if (atomic_add_unless(&ref->count, -1, 1)) > + GEM_BUG_ON(!refcount_read(&ref->count)); > + if (refcount_dec_not_one(&ref->count)) I'm not sure if this is correct here, I assume we should be adding instead here its decrementing ? > return; > > __active_retire(ref); > @@ -189,8 +189,8 @@ active_work(struct work_struct *wrk) > static void > active_retire(struct i915_active *ref) > { > - GEM_BUG_ON(!atomic_read(&ref->count)); > - if (atomic_add_unless(&ref->count, -1, 1)) > + GEM_BUG_ON(!refcount_read(&ref->count)); > + if (refcount_dec_not_one(&ref->count)) > return; > > if (ref->flags & I915_ACTIVE_RETIRE_SLEEPS) { > @@ -354,7 +354,7 @@ void __i915_active_init(struct i915_active *ref, > ref->cache = NULL; > > init_llist_head(&ref->preallocated_barriers); > - atomic_set(&ref->count, 0); > + refcount_set(&ref->count, 0); > __mutex_init(&ref->mutex, "i915_active", mkey); > __i915_active_fence_init(&ref->excl, NULL, excl_retire); > INIT_WORK(&ref->work, active_work); > @@ -445,7 +445,7 @@ int i915_active_add_request(struct i915_active *ref, struct i915_request *rq) > > if (replace_barrier(ref, active)) { > RCU_INIT_POINTER(active->fence, NULL); > - atomic_dec(&ref->count); > + refcount_dec(&ref->count); > } > if (!__i915_active_fence_set(active, fence)) > __i915_active_acquire(ref); > @@ -488,14 +488,16 @@ i915_active_set_exclusive(struct i915_active *ref, struct dma_fence *f) > bool i915_active_acquire_if_busy(struct i915_active *ref) > { > debug_active_assert(ref); > - return atomic_add_unless(&ref->count, 1, 0); > + return refcount_add_not_zero(1, &ref->count); > } > > static void __i915_active_activate(struct i915_active *ref) > { > spin_lock_irq(&ref->tree_lock); /* __active_retire() */ > - if (!atomic_fetch_inc(&ref->count)) > + if (!refcount_inc_not_zero(&ref->count)) { > + refcount_inc(&ref->count); > debug_active_activate(ref); > + } > spin_unlock_irq(&ref->tree_lock); > } > > @@ -757,7 +759,7 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence, > void i915_active_fini(struct i915_active *ref) > { > debug_active_fini(ref); > - GEM_BUG_ON(atomic_read(&ref->count)); > + GEM_BUG_ON(refcount_read(&ref->count)); > GEM_BUG_ON(work_pending(&ref->work)); > mutex_destroy(&ref->mutex); > > @@ -927,7 +929,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref, > > first = first->next; > > - atomic_dec(&ref->count); > + refcount_dec(&ref->count); > intel_engine_pm_put(barrier_to_engine(node)); > > kmem_cache_free(slab_cache, node); > diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h > index 7eb44132183a..116c7c28466a 100644 > --- a/drivers/gpu/drm/i915/i915_active.h > +++ b/drivers/gpu/drm/i915/i915_active.h > @@ -193,14 +193,14 @@ void i915_active_release(struct i915_active *ref); > > static inline void __i915_active_acquire(struct i915_active *ref) > { > - GEM_BUG_ON(!atomic_read(&ref->count)); > - atomic_inc(&ref->count); > + GEM_BUG_ON(!refcount_read(&ref->count)); > + refcount_inc(&ref->count); > } > > static inline bool > i915_active_is_idle(const struct i915_active *ref) > { > - return !atomic_read(&ref->count); > + return !refcount_read(&ref->count); > } > > void i915_active_fini(struct i915_active *ref); > diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h > index b02a78ac87db..152a3a25d9f7 100644 > --- a/drivers/gpu/drm/i915/i915_active_types.h > +++ b/drivers/gpu/drm/i915/i915_active_types.h > @@ -7,7 +7,7 @@ > #ifndef _I915_ACTIVE_TYPES_H_ > #define _I915_ACTIVE_TYPES_H_ > > -#include <linux/atomic.h> > +#include <linux/refcount.h> > #include <linux/dma-fence.h> > #include <linux/llist.h> > #include <linux/mutex.h> > @@ -23,7 +23,7 @@ struct i915_active_fence { > struct active_node; > > struct i915_active { > - atomic_t count; > + refcount_t count; > struct mutex mutex; > > spinlock_t tree_lock; > -- > 2.34.1 > > Regards, ~Praveen.
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 7412abf166a8..5e58d8b1e947 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -92,14 +92,14 @@ static void debug_active_init(struct i915_active *ref) static void debug_active_activate(struct i915_active *ref) { lockdep_assert_held(&ref->tree_lock); - if (!atomic_read(&ref->count)) /* before the first inc */ + if (!refcount_read(&ref->count)) /* before the first inc */ debug_object_activate(ref, &active_debug_desc); } static void debug_active_deactivate(struct i915_active *ref) { lockdep_assert_held(&ref->tree_lock); - if (!atomic_read(&ref->count)) /* after the last dec */ + if (!refcount_read(&ref->count)) /* after the last dec */ debug_object_deactivate(ref, &active_debug_desc); } @@ -133,7 +133,7 @@ __active_retire(struct i915_active *ref) GEM_BUG_ON(i915_active_is_idle(ref)); /* return the unused nodes to our slabcache -- flushing the allocator */ - if (!atomic_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, flags)) + if (!refcount_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, &flags)) return; GEM_BUG_ON(rcu_access_pointer(ref->excl.fence)); @@ -179,8 +179,8 @@ active_work(struct work_struct *wrk) { struct i915_active *ref = container_of(wrk, typeof(*ref), work); - GEM_BUG_ON(!atomic_read(&ref->count)); - if (atomic_add_unless(&ref->count, -1, 1)) + GEM_BUG_ON(!refcount_read(&ref->count)); + if (refcount_dec_not_one(&ref->count)) return; __active_retire(ref); @@ -189,8 +189,8 @@ active_work(struct work_struct *wrk) static void active_retire(struct i915_active *ref) { - GEM_BUG_ON(!atomic_read(&ref->count)); - if (atomic_add_unless(&ref->count, -1, 1)) + GEM_BUG_ON(!refcount_read(&ref->count)); + if (refcount_dec_not_one(&ref->count)) return; if (ref->flags & I915_ACTIVE_RETIRE_SLEEPS) { @@ -354,7 +354,7 @@ void __i915_active_init(struct i915_active *ref, ref->cache = NULL; init_llist_head(&ref->preallocated_barriers); - atomic_set(&ref->count, 0); + refcount_set(&ref->count, 0); __mutex_init(&ref->mutex, "i915_active", mkey); __i915_active_fence_init(&ref->excl, NULL, excl_retire); INIT_WORK(&ref->work, active_work); @@ -445,7 +445,7 @@ int i915_active_add_request(struct i915_active *ref, struct i915_request *rq) if (replace_barrier(ref, active)) { RCU_INIT_POINTER(active->fence, NULL); - atomic_dec(&ref->count); + refcount_dec(&ref->count); } if (!__i915_active_fence_set(active, fence)) __i915_active_acquire(ref); @@ -488,14 +488,16 @@ i915_active_set_exclusive(struct i915_active *ref, struct dma_fence *f) bool i915_active_acquire_if_busy(struct i915_active *ref) { debug_active_assert(ref); - return atomic_add_unless(&ref->count, 1, 0); + return refcount_add_not_zero(1, &ref->count); } static void __i915_active_activate(struct i915_active *ref) { spin_lock_irq(&ref->tree_lock); /* __active_retire() */ - if (!atomic_fetch_inc(&ref->count)) + if (!refcount_inc_not_zero(&ref->count)) { + refcount_inc(&ref->count); debug_active_activate(ref); + } spin_unlock_irq(&ref->tree_lock); } @@ -757,7 +759,7 @@ int i915_sw_fence_await_active(struct i915_sw_fence *fence, void i915_active_fini(struct i915_active *ref) { debug_active_fini(ref); - GEM_BUG_ON(atomic_read(&ref->count)); + GEM_BUG_ON(refcount_read(&ref->count)); GEM_BUG_ON(work_pending(&ref->work)); mutex_destroy(&ref->mutex); @@ -927,7 +929,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref, first = first->next; - atomic_dec(&ref->count); + refcount_dec(&ref->count); intel_engine_pm_put(barrier_to_engine(node)); kmem_cache_free(slab_cache, node); diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h index 7eb44132183a..116c7c28466a 100644 --- a/drivers/gpu/drm/i915/i915_active.h +++ b/drivers/gpu/drm/i915/i915_active.h @@ -193,14 +193,14 @@ void i915_active_release(struct i915_active *ref); static inline void __i915_active_acquire(struct i915_active *ref) { - GEM_BUG_ON(!atomic_read(&ref->count)); - atomic_inc(&ref->count); + GEM_BUG_ON(!refcount_read(&ref->count)); + refcount_inc(&ref->count); } static inline bool i915_active_is_idle(const struct i915_active *ref) { - return !atomic_read(&ref->count); + return !refcount_read(&ref->count); } void i915_active_fini(struct i915_active *ref); diff --git a/drivers/gpu/drm/i915/i915_active_types.h b/drivers/gpu/drm/i915/i915_active_types.h index b02a78ac87db..152a3a25d9f7 100644 --- a/drivers/gpu/drm/i915/i915_active_types.h +++ b/drivers/gpu/drm/i915/i915_active_types.h @@ -7,7 +7,7 @@ #ifndef _I915_ACTIVE_TYPES_H_ #define _I915_ACTIVE_TYPES_H_ -#include <linux/atomic.h> +#include <linux/refcount.h> #include <linux/dma-fence.h> #include <linux/llist.h> #include <linux/mutex.h> @@ -23,7 +23,7 @@ struct i915_active_fence { struct active_node; struct i915_active { - atomic_t count; + refcount_t count; struct mutex mutex; spinlock_t tree_lock;
The refcount_* APIs are designed to address known issues with the atomic_t APIs for reference counting. They provide following distinct advantages: - protect the reference counters from overflow/underflow - avoid use-after-free errors - provide improved memory ordering guarantee schemes - neater and safer. Hence, convert the atomic_t count member variable and associated atomic_*() API calls to equivalent refcount_t type and refcount_*() API calls. This patch proposal address the following warnings generated by the atomic_as_refcounter.cocci coccinelle script atomic_add_unless Signed-off-by: Deepak R Varma <drv@mailo.com> --- Please note: 1. Proposed changes are compile tested only. 2. This patch 1/2 is required to be applied before patch 2/2 due to interdependency. Changes in v2: 1. Patch added to the patch series. 2. Handle build issues Reported-by: kernel test robot <lkp@intel.com> Earlier a standalone patch was sent for the i915 base driver only. The Kernel Test Robot reported build failure for additional atomic_*() calls specific to i915 debugging support when enabled. This version now includes those changes as well. drivers/gpu/drm/i915/i915_active.c | 28 +++++++++++++----------- drivers/gpu/drm/i915/i915_active.h | 6 ++--- drivers/gpu/drm/i915/i915_active_types.h | 4 ++-- 3 files changed, 20 insertions(+), 18 deletions(-) -- 2.34.1