- Linaro-mm-sig - lists.linaro.org

Re: [PATCH 01/12] media: stm32: dcmi: Switch from __maybe_unused to pm_sleep_ptr()

by kernel test robot

Hi Alain, kernel test robot noticed the following build warnings: [auto build test WARNING on atorgue-stm32/stm32-next] [also build test WARNING on robh/for-next linus/master v6.19-rc1 next-20251219] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Alain-Volmat/media-stm32-dcm… base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next patch link: https://lore.kernel.org/r/20251218-stm32-dcmi-dma-chaining-v1-1-39948ca6cbf… patch subject: [PATCH 01/12] media: stm32: dcmi: Switch from __maybe_unused to pm_sleep_ptr() config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20251221/202512210044.xNNW6QJZ-lkp@…) compiler: arc-linux-gcc (GCC) 15.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251221/202512210044.xNNW6QJZ-lkp@…) 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(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202512210044.xNNW6QJZ-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/media/platform/st/stm32/stm32-dcmi.c:2127:12: warning: 'dcmi_resume' defined but not used [-Wunused-function] 2127 | static int dcmi_resume(struct device *dev) | ^~~~~~~~~~~ >> drivers/media/platform/st/stm32/stm32-dcmi.c:2116:12: warning: 'dcmi_suspend' defined but not used [-Wunused-function] 2116 | static int dcmi_suspend(struct device *dev) | ^~~~~~~~~~~~ vim +/dcmi_resume +2127 drivers/media/platform/st/stm32/stm32-dcmi.c 2115 > 2116 static int dcmi_suspend(struct device *dev) 2117 { 2118 /* disable clock */ 2119 pm_runtime_force_suspend(dev); 2120 2121 /* change pinctrl state */ 2122 pinctrl_pm_select_sleep_state(dev); 2123 2124 return 0; 2125 } 2126 > 2127 static int dcmi_resume(struct device *dev) 2128 { 2129 /* restore pinctl default state */ 2130 pinctrl_pm_select_default_state(dev); 2131 2132 /* clock enable */ 2133 pm_runtime_force_resume(dev); 2134 2135 return 0; 2136 } 2137 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

1 month, 3 weeks

1
0
0 0

Re: [PATCH] dma-buf: system_heap: account for system heap allocation in memcg

by Christian König

On 12/19/25 11:25, Maxime Ripard wrote: > On Mon, Dec 15, 2025 at 03:53:22PM +0100, Christian König wrote: >> On 12/15/25 14:59, Maxime Ripard wrote: ... >>>>> The shared ownership is indeed broken, but it's not more or less broken >>>>> than, say, memfd + udmabuf, and I'm sure plenty of others. >>>>> >>>>> So we really improve the common case, but only make the "advanced" >>>>> slightly more broken than it already is. >>>>> >>>>> Would you disagree? >>>> >>>> I strongly disagree. As far as I can see there is a huge chance we >>>> break existing use cases with that. >>> >>> Which ones? And what about the ones that are already broken? >> >> Well everybody that expects that driver resources are *not* accounted to memcg. > > Which is a thing only because these buffers have never been accounted > for in the first place. Yeah, completely agree. By not accounting it for such a long time we ended up with people depending on this behavior. Not nice, but that's what it is. > So I guess the conclusion is that we shouldn't > even try to do memory accounting, because someone somewhere might not > expect that one of its application would take too much RAM in the > system? Well we do need some kind of solution to the problem. Either having some setting where you say "This memcg limit is inclusive/exclusive device driver allocated memory" or have a completely separate limit for device driver allocated memory. Key point is we have both use cases, so we need to support both. >>>> There has been some work on TTM by Dave but I still haven't found time >>>> to wrap my head around all possible side effects such a change can >>>> have. >>>> >>>> The fundamental problem is that neither memcg nor the classic resource >>>> tracking (e.g. the OOM killer) has a good understanding of shared >>>> resources. >>> >>> And yet heap allocations don't necessarily have to be shared. But they >>> all have to be allocated. >>> >>>> For example you can use memfd to basically kill any process in the >>>> system because the OOM killer can't identify the process which holds >>>> the reference to the memory in question. And that is a *MUCH* bigger >>>> problem than just inaccurate memcg accounting. >>> >>> When you frame it like that, sure. Also, you can use the system heap to >>> DoS any process in the system. I'm not saying that what you're concerned >>> about isn't an issue, but let's not brush off other people legitimate >>> issues as well. >> >> Completely agree, but we should prioritize. >> >> That driver allocated memory is not memcg accounted is actually uAPI, >> e.g. that is not something which can easily change. >> >> While fixing the OOM killer looks perfectly doable and will then most >> likely also show a better path how to fix the memcg accounting. > > I don't necessarily disagree, but we don't necessarily have the same > priorities either. Your use-cases are probably quite different from > mine, and that's ok. But that's precisely why all these discussions > should be made on the ML when possible, or at least have some notes when > a discussion has happened at a conference or something. > > So far, my whole experience with this topic, despite being the only one > (afaik) sending patches about this for the last 1.5y, is that everytime > some work on this is done the answer is "oh but you shouldn't have > worked on it because we completely changed our mind", and that's pretty > frustrating. Welcome to the club :) I've already posted patches to start addressing at least the OOM killer issue ~10 years ago. Those patches were not well received because back then driver memory was negligible and the problem simply didn't hurt much. But by now we have GPUs and AI accelerators which eat up 90% of your system memory, security researchers stumbling over it and IIRC even multiple CVE numbers for some of the resulting issues... I should probably dig it up and re-send my patch set. Happy holidays, Christian. > > Maxime

1 month, 3 weeks

1
0
0 0

[PATCH mm-hotfixes 0/4] mm kernel-doc fixes

by Bagas Sanjaya

Hi, Here are kernel-doc fixes for mm subsystem, based on mm-hotfixes-unstable branch. This series is split from previous assorted kernel-doc fixes series [1] with review trailers applied. I'm also including textsearch fix since there's currently no maintainer for include/linux/textsearch.h (get_maintainer.pl only shows LKML). Enjoy! [1]: https://lore.kernel.org/linux-fsdevel/20251215113903.46555-1-bagasdotme@gma… Bagas Sanjaya (4): mm: Describe @flags parameter in memalloc_flags_save() textsearch: Describe @list member in ts_ops search mm: vmalloc: Fix up vrealloc_node_align() kernel-doc macro name mm, kfence: Describe @slab parameter in __kfence_obj_info() include/linux/kfence.h | 1 + include/linux/sched/mm.h | 1 + include/linux/textsearch.h | 1 + mm/vmalloc.c | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) base-commit: 980dbceadd50af9437257d8095d4a3606818e8c4 -- An old man doll... just what I always wanted! - Clara

1 month, 3 weeks

1
4
0 0

Re: [PATCH 05/19] dma-buf/selftests: test RCU ops and inline lock

by Christian König

On 12/11/25 16:04, Tvrtko Ursulin wrote: ... >> @@ -90,6 +73,11 @@ static int test_signaling(void *arg) >> goto err_free; >> } >> + if (rcu_dereference_protected(f->ops, true)) { >> + pr_err("Fence ops not cleared on signal\n"); >> + goto err_free; >> + } > > Bump to after the signaled check just below? Otherwise the signaled state hasn't been ascertained yet. Done. I've put it to the end of the test. >> + >> if (!dma_fence_is_signaled(f)) { >> pr_err("Fence not reporting signaled\n"); >> goto err_free; >> @@ -540,19 +528,7 @@ int dma_fence(void) >> SUBTEST(test_stub), >> SUBTEST(race_signal_callback), >> }; >> - int ret; >> pr_info("sizeof(dma_fence)=%zu\n", sizeof(struct dma_fence)); >> - >> - slab_fences = KMEM_CACHE(mock_fence, >> - SLAB_TYPESAFE_BY_RCU | > > Hm.. race_signal_callback looks like it could be depending on SLAB_TYPESAFE_BY_RCU. To you not? Hui? As far as I can see it doesn't. The race_signal_callback test just depends on the general RCU functionality of fences. Regards, Christian. > > Regards, > > Tvrtko > >> - SLAB_HWCACHE_ALIGN); >> - if (!slab_fences) >> - return -ENOMEM; >> - >> - ret = subtests(tests, NULL); >> - >> - kmem_cache_destroy(slab_fences); >> - >> - return ret; >> + return subtests(tests, NULL); >> } >

1 month, 4 weeks

1
0
0 0

[PATCH 00/14] Assorted kernel-doc fixes

by Bagas Sanjaya

Hi, Here are assorted kernel-doc fixes for 6.19 cycle. As the name implies, for the merging strategy, the patches can be taken by respective maintainers to appropriate fixes branches (targetting 6.19 of course) (e.g. for mm it will be mm-hotfixes). Enjoy! Bagas Sanjaya (14): genalloc: Describe @start_addr parameter in genpool_algo_t mm: Describe @flags parameter in memalloc_flags_save() textsearch: Describe @list member in ts_ops search mm: vmalloc: Fix up vrealloc_node_align() kernel-doc macro name mm, kfence: Describe @slab parameter in __kfence_obj_info() virtio: Describe @map and @vmap members in virtio_device struct fs: Describe @isnew parameter in ilookup5_nowait() VFS: fix __start_dirop() kernel-doc warnings drm/amd/display: Don't use kernel-doc comment in dc_register_software_state struct drm/amdgpu: Describe @AMD_IP_BLOCK_TYPE_RAS in amd_ip_block_type enum drm/gem/shmem: Describe @shmem and @size parameters drm/scheduler: Describe @result in drm_sched_job_done() drm/gpusvm: Fix drm_gpusvm_pages_valid_unlocked() kernel-doc comment net: bridge: Describe @tunnel_hash member in net_bridge_vlan_group struct drivers/gpu/drm/amd/display/dc/dc.h | 2 +- drivers/gpu/drm/amd/include/amd_shared.h | 1 + drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++- drivers/gpu/drm/drm_gpusvm.c | 4 ++-- drivers/gpu/drm/scheduler/sched_main.c | 1 + fs/inode.c | 1 + fs/namei.c | 3 ++- include/linux/genalloc.h | 1 + include/linux/kfence.h | 1 + include/linux/sched/mm.h | 1 + include/linux/textsearch.h | 1 + include/linux/virtio.h | 2 ++ mm/vmalloc.c | 2 +- net/bridge/br_private.h | 1 + 14 files changed, 18 insertions(+), 6 deletions(-) base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8 -- An old man doll... just what I always wanted! - Clara

1 month, 4 weeks

4
19
0 0

Re: [PATCH] dma-buf: really enable DMABUF_DEBUG by default on DEBUG kernels

by Christian König

On 12/17/25 11:25, Lukas Bulwahn wrote: > From: Lukas Bulwahn <lukas.bulwahn(a)redhat.com> > > The intent of commit 646013f513f3 ("dma-buf: enable DMABUF_DEBUG by default > on DEBUG kernels") is clear, but it mixes up the config option name. The > config option for kernel debugging is named DEBUG_KERNEL, not DEBUG. > > Fix up the DMABUF_DEBUG definition to use the intended name. > > Fixes: 646013f513f3 ("dma-buf: enable DMABUF_DEBUG by default on DEBUG kernels") > Signed-off-by: Lukas Bulwahn <lukas.bulwahn(a)redhat.com> Ah, yeah. I mixed up the C define vs the config option. Thanks for pointing that out. Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig > index fdd823e446cc..426c9ad3364f 100644 > --- a/drivers/dma-buf/Kconfig > +++ b/drivers/dma-buf/Kconfig > @@ -55,7 +55,7 @@ config DMABUF_MOVE_NOTIFY > config DMABUF_DEBUG > bool "DMA-BUF debug checks" > depends on DMA_SHARED_BUFFER > - default y if DEBUG > + default y if DEBUG_KERNEL > help > This option enables additional checks for DMA-BUF importers and > exporters. Specifically it validates that importers do not peek at the

1 month, 4 weeks

1
0
0 0

Re: [PATCH] dma-buf: system_heap: account for system heap allocation in memcg

by Christian König

On 12/15/25 14:59, Maxime Ripard wrote: > On Mon, Dec 15, 2025 at 02:30:47PM +0100, Christian König wrote: >> On 12/15/25 11:51, Maxime Ripard wrote: >>> Hi TJ, >>> >>> On Fri, Dec 12, 2025 at 08:25:19AM +0900, T.J. Mercier wrote: >>>> On Fri, Dec 12, 2025 at 4:31 AM Eric Chanudet <echanude(a)redhat.com> wrote: >>>>> >>>>> The system dma-buf heap lets userspace allocate buffers from the page >>>>> allocator. However, these allocations are not accounted for in memcg, >>>>> allowing processes to escape limits that may be configured. >>>>> >>>>> Pass the __GFP_ACCOUNT for our allocations to account them into memcg. >>>> >>>> We had a discussion just last night in the MM track at LPC about how >>>> shared memory accounted in memcg is pretty broken. Without a way to >>>> identify (and possibly transfer) ownership of a shared buffer, this >>>> makes the accounting of shared memory, and zombie memcg problems >>>> worse. :\ >>> >>> Are there notes or a report from that discussion anywhere? >>> >>> The way I see it, the dma-buf heaps *trivial* case is non-existent at >>> the moment and that's definitely broken. Any application can bypass its >>> cgroups limits trivially, and that's a pretty big hole in the system. >> >> Well, that is just the tip of the iceberg. >> >> Pretty much all driver interfaces doesn't account to memcg at the >> moment, all the way from alsa, over GPUs (both TTM and SHM-GEM) to >> V4L2. > > Yes, I know, and step 1 of the plan we discussed earlier this year is to > fix the heaps. > >>> The shared ownership is indeed broken, but it's not more or less broken >>> than, say, memfd + udmabuf, and I'm sure plenty of others. >>> >>> So we really improve the common case, but only make the "advanced" >>> slightly more broken than it already is. >>> >>> Would you disagree? >> >> I strongly disagree. As far as I can see there is a huge chance we >> break existing use cases with that. > > Which ones? And what about the ones that are already broken? Well everybody that expects that driver resources are *not* accounted to memcg. >> There has been some work on TTM by Dave but I still haven't found time >> to wrap my head around all possible side effects such a change can >> have. >> >> The fundamental problem is that neither memcg nor the classic resource >> tracking (e.g. the OOM killer) has a good understanding of shared >> resources. > > And yet heap allocations don't necessarily have to be shared. But they > all have to be allocated. > >> For example you can use memfd to basically kill any process in the >> system because the OOM killer can't identify the process which holds >> the reference to the memory in question. And that is a *MUCH* bigger >> problem than just inaccurate memcg accounting. > > When you frame it like that, sure. Also, you can use the system heap to > DoS any process in the system. I'm not saying that what you're concerned > about isn't an issue, but let's not brush off other people legitimate > issues as well. Completely agree, but we should prioritize. That driver allocated memory is not memcg accounted is actually uAPI, e.g. that is not something which can easily change. While fixing the OOM killer looks perfectly doable and will then most likely also show a better path how to fix the memcg accounting. Christian. > > Maxime

2 months

2
1
0 0

Re: [PATCH] dma-buf: system_heap: account for system heap allocation in memcg

by T.J. Mercier

On Mon, Dec 15, 2025 at 7:51 PM Maxime Ripard <mripard(a)redhat.com> wrote: > > Hi TJ, Hi Maxime, > On Fri, Dec 12, 2025 at 08:25:19AM +0900, T.J. Mercier wrote: > > On Fri, Dec 12, 2025 at 4:31 AM Eric Chanudet <echanude(a)redhat.com> wrote: > > > > > > The system dma-buf heap lets userspace allocate buffers from the page > > > allocator. However, these allocations are not accounted for in memcg, > > > allowing processes to escape limits that may be configured. > > > > > > Pass the __GFP_ACCOUNT for our allocations to account them into memcg. > > > > We had a discussion just last night in the MM track at LPC about how > > shared memory accounted in memcg is pretty broken. Without a way to > > identify (and possibly transfer) ownership of a shared buffer, this > > makes the accounting of shared memory, and zombie memcg problems > > worse. :\ > > Are there notes or a report from that discussion anywhere? The LPC vids haven't been clipped yet, and actually I can't even find the recorded full live stream from Hall A2 on the first day. So I don't think there's anything to look at, but I bet there's probably nothing there you don't already know. > The way I see it, the dma-buf heaps *trivial* case is non-existent at > the moment and that's definitely broken. Any application can bypass its > cgroups limits trivially, and that's a pretty big hole in the system. Agree, but if we only charge the first allocator then limits can still easily be bypassed assuming an app can cause an allocation outside of its cgroup tree. I'm not sure using static memcg limits where a significant portion of the memory can be shared is really feasible. Even with just pagecache being charged to memcgs, we're having trouble defining a static memcg limit that is really useful since it has to be high enough to accomodate occasional spikes due to shared memory that might or might not be charged (since it can only be charged to one memcg - it may be spread around or it may all get charged to one memcg). So excessive anonymous use has to get really bad before it gets punished. What I've been hearing lately is that folks are polling memory.stat or PSI or other metrics and using that to take actions (memory.reclaim / killing / adjust memory.high) at runtime rather than relying on memory.high/max behavior with a static limit. > The shared ownership is indeed broken, but it's not more or less broken > than, say, memfd + udmabuf, and I'm sure plenty of others. One thing that's worse about system heap buffers is that unlike memfd the memory isn't reclaimable. So without killing all users there's currently no way to deal with the zombie issue. Harry's proposing reparenting, but I don't think our current interfaces support that because we'd have to mess with the page structs behind system heap dmabufs to change the memcg during reparenting. Ah... but udmabuf pins the memfd pages, so you're right that memfd + udmabuf isn't worse. > So we really improve the common case, but only make the "advanced" > slightly more broken than it already is. > > Would you disagree? I think memcg limits in this case just wouldn't be usable because of what I mentioned above. In our common case the allocator is in a different cgroup tree than the real users of the buffer. > Maxime

2 months

1
0
0 0

Re: [PATCH 09/19] drm/sched: use inline locks for the drm-sched-fence

by Christian König

On 12/15/25 16:53, Tvrtko Ursulin wrote: > > On 15/12/2025 15:38, Christian König wrote: >> On 12/15/25 10:20, Tvrtko Ursulin wrote: >>> >>> On 12/12/2025 15:50, Christian König wrote: >>>> On 12/11/25 16:13, Tvrtko Ursulin wrote: >>>>> >>>>> On 11/12/2025 13:16, Christian König wrote: >>>>>> Using the inline lock is now the recommended way for dma_fence implementations. >>>>>> >>>>>> So use this approach for the scheduler fences as well just in case if >>>>>> anybody uses this as blueprint for its own implementation. >>>>>> >>>>>> Also saves about 4 bytes for the external spinlock. >>>>>> >>>>>> Signed-off-by: Christian König <christian.koenig(a)amd.com> >>>>>> --- >>>>>> drivers/gpu/drm/scheduler/sched_fence.c | 7 +++---- >>>>>> include/drm/gpu_scheduler.h | 4 ---- >>>>>> 2 files changed, 3 insertions(+), 8 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c >>>>>> index 08ccbde8b2f5..47471b9e43f9 100644 >>>>>> --- a/drivers/gpu/drm/scheduler/sched_fence.c >>>>>> +++ b/drivers/gpu/drm/scheduler/sched_fence.c >>>>>> @@ -161,7 +161,7 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, >>>>>> /* If we already have an earlier deadline, keep it: */ >>>>>> if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && >>>>>> ktime_before(fence->deadline, deadline)) { >>>>>> - spin_unlock_irqrestore(&fence->lock, flags); >>>>>> + dma_fence_unlock_irqrestore(f, flags); >>>>> >>>>> Rebase error I guess. Pull into the locking helpers patch. >>>> >>>> No that is actually completely intentional here. >>>> >>>> Previously we had a separate lock which protected both the DMA-fences as well as the deadline state. >>>> >>>> Now we turn that upside down by dropping the separate lock and protecting the deadline state with the dma_fence lock instead. >>> >>> I don't follow. The code is currently like this: >>> >>> static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, >>> ktime_t deadline) >>> { >>> struct drm_sched_fence *fence = to_drm_sched_fence(f); >>> struct dma_fence *parent; >>> unsigned long flags; >>> >>> spin_lock_irqsave(&fence->lock, flags); >>> >>> /* If we already have an earlier deadline, keep it: */ >>> if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && >>> ktime_before(fence->deadline, deadline)) { >>> spin_unlock_irqrestore(&fence->lock, flags); >>> return; >>> } >>> >>> fence->deadline = deadline; >>> set_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags); >>> >>> spin_unlock_irqrestore(&fence->lock, flags);... >>> >>> The diff changes one out of the three lock/unlock operations. Other two are changed in 3/19. All three should surely be changed in the same patch. >> >> We could change those spin_lock/unlock calls in patch #3, but I don't think that this is clean. >> >> See the code here currently uses fence->lock and patch #3 would change it to use fence->finished->lock instead. That might be the pointer at the moment, but that is just by coincident and not design. >> >> Only this change here ontop makes it intentional that we use fence->finished->lock for everything. > > Sorry I still don't follow. After 3/19 and before this 9/19 the function looks like this: > > static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, > ktime_t deadline) > { > struct drm_sched_fence *fence = to_drm_sched_fence(f); > struct dma_fence *parent; > unsigned long flags; > > dma_fence_lock_irqsave(f, flags); > > /* If we already have an earlier deadline, keep it: */ > if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && > ktime_before(fence->deadline, deadline)) { > spin_unlock_irqrestore(&fence->lock, flags); > return; > } > > fence->deadline = deadline; > set_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags); > > dma_fence_unlock_irqrestore(f, flags); > > Notice the lonely spin_unlock_irqrestore on the early return path while other two use the dma_fence_(un)lock helpers. Am I blind or how is that clean? Oh, that's what you mean. Sorry I was blind! Yeah that is clearly unintentional. Thanks, Christian. > > Regards, > > Tvrtko > >> >> Regards, >> Christian. >> >>> >>> Regards, >>> >>> Tvrtko >>> >>>> >>>> Regards, >>>> Christian. >>>> >>>>> >>>>> Regards, >>>>> >>>>> Tvrtko >>>>> >>>>>> return; >>>>>> } >>>>>> @@ -217,7 +217,6 @@ struct drm_sched_fence *drm_sched_fence_alloc(struct drm_sched_entity *entity, >>>>>> fence->owner = owner; >>>>>> fence->drm_client_id = drm_client_id; >>>>>> - spin_lock_init(&fence->lock); >>>>>> return fence; >>>>>> } >>>>>> @@ -230,9 +229,9 @@ void drm_sched_fence_init(struct drm_sched_fence *fence, >>>>>> fence->sched = entity->rq->sched; >>>>>> seq = atomic_inc_return(&entity->fence_seq); >>>>>> dma_fence_init(&fence->scheduled, &drm_sched_fence_ops_scheduled, >>>>>> - &fence->lock, entity->fence_context, seq); >>>>>> + NULL, entity->fence_context, seq); >>>>>> dma_fence_init(&fence->finished, &drm_sched_fence_ops_finished, >>>>>> - &fence->lock, entity->fence_context + 1, seq); >>>>>> + NULL, entity->fence_context + 1, seq); >>>>>> } >>>>>> module_init(drm_sched_fence_slab_init); >>>>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h >>>>>> index fb88301b3c45..b77f24a783e3 100644 >>>>>> --- a/include/drm/gpu_scheduler.h >>>>>> +++ b/include/drm/gpu_scheduler.h >>>>>> @@ -297,10 +297,6 @@ struct drm_sched_fence { >>>>>> * belongs to. >>>>>> */ >>>>>> struct drm_gpu_scheduler *sched; >>>>>> - /** >>>>>> - * @lock: the lock used by the scheduled and the finished fences. >>>>>> - */ >>>>>> - spinlock_t lock; >>>>>> /** >>>>>> * @owner: job owner for debugging >>>>>> */ >>>>> >>>> >>> >> >

2 months

1
0
0 0

Re: [PATCH 09/19] drm/sched: use inline locks for the drm-sched-fence

by Christian König

On 12/15/25 10:20, Tvrtko Ursulin wrote: > > On 12/12/2025 15:50, Christian König wrote: >> On 12/11/25 16:13, Tvrtko Ursulin wrote: >>> >>> On 11/12/2025 13:16, Christian König wrote: >>>> Using the inline lock is now the recommended way for dma_fence implementations. >>>> >>>> So use this approach for the scheduler fences as well just in case if >>>> anybody uses this as blueprint for its own implementation. >>>> >>>> Also saves about 4 bytes for the external spinlock. >>>> >>>> Signed-off-by: Christian König <christian.koenig(a)amd.com> >>>> --- >>>> drivers/gpu/drm/scheduler/sched_fence.c | 7 +++---- >>>> include/drm/gpu_scheduler.h | 4 ---- >>>> 2 files changed, 3 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c >>>> index 08ccbde8b2f5..47471b9e43f9 100644 >>>> --- a/drivers/gpu/drm/scheduler/sched_fence.c >>>> +++ b/drivers/gpu/drm/scheduler/sched_fence.c >>>> @@ -161,7 +161,7 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, >>>> /* If we already have an earlier deadline, keep it: */ >>>> if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && >>>> ktime_before(fence->deadline, deadline)) { >>>> - spin_unlock_irqrestore(&fence->lock, flags); >>>> + dma_fence_unlock_irqrestore(f, flags); >>> >>> Rebase error I guess. Pull into the locking helpers patch. >> >> No that is actually completely intentional here. >> >> Previously we had a separate lock which protected both the DMA-fences as well as the deadline state. >> >> Now we turn that upside down by dropping the separate lock and protecting the deadline state with the dma_fence lock instead. > > I don't follow. The code is currently like this: > > static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, > ktime_t deadline) > { > struct drm_sched_fence *fence = to_drm_sched_fence(f); > struct dma_fence *parent; > unsigned long flags; > > spin_lock_irqsave(&fence->lock, flags); > > /* If we already have an earlier deadline, keep it: */ > if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && > ktime_before(fence->deadline, deadline)) { > spin_unlock_irqrestore(&fence->lock, flags); > return; > } > > fence->deadline = deadline; > set_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags); > > spin_unlock_irqrestore(&fence->lock, flags);... > > The diff changes one out of the three lock/unlock operations. Other two are changed in 3/19. All three should surely be changed in the same patch. We could change those spin_lock/unlock calls in patch #3, but I don't think that this is clean. See the code here currently uses fence->lock and patch #3 would change it to use fence->finished->lock instead. That might be the pointer at the moment, but that is just by coincident and not design. Only this change here ontop makes it intentional that we use fence->finished->lock for everything. Regards, Christian. > > Regards, > > Tvrtko > >> >> Regards, >> Christian. >> >>> >>> Regards, >>> >>> Tvrtko >>> >>>> return; >>>> } >>>> @@ -217,7 +217,6 @@ struct drm_sched_fence *drm_sched_fence_alloc(struct drm_sched_entity *entity, >>>> fence->owner = owner; >>>> fence->drm_client_id = drm_client_id; >>>> - spin_lock_init(&fence->lock); >>>> return fence; >>>> } >>>> @@ -230,9 +229,9 @@ void drm_sched_fence_init(struct drm_sched_fence *fence, >>>> fence->sched = entity->rq->sched; >>>> seq = atomic_inc_return(&entity->fence_seq); >>>> dma_fence_init(&fence->scheduled, &drm_sched_fence_ops_scheduled, >>>> - &fence->lock, entity->fence_context, seq); >>>> + NULL, entity->fence_context, seq); >>>> dma_fence_init(&fence->finished, &drm_sched_fence_ops_finished, >>>> - &fence->lock, entity->fence_context + 1, seq); >>>> + NULL, entity->fence_context + 1, seq); >>>> } >>>> module_init(drm_sched_fence_slab_init); >>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h >>>> index fb88301b3c45..b77f24a783e3 100644 >>>> --- a/include/drm/gpu_scheduler.h >>>> +++ b/include/drm/gpu_scheduler.h >>>> @@ -297,10 +297,6 @@ struct drm_sched_fence { >>>> * belongs to. >>>> */ >>>> struct drm_gpu_scheduler *sched; >>>> - /** >>>> - * @lock: the lock used by the scheduled and the finished fences. >>>> - */ >>>> - spinlock_t lock; >>>> /** >>>> * @owner: job owner for debugging >>>> */ >>> >> >

2 months

1
0
0 0

Re: [PATCH] dma-buf: system_heap: account for system heap allocation in memcg

by Christian König

On 12/15/25 11:51, Maxime Ripard wrote: > Hi TJ, > > On Fri, Dec 12, 2025 at 08:25:19AM +0900, T.J. Mercier wrote: >> On Fri, Dec 12, 2025 at 4:31 AM Eric Chanudet <echanude(a)redhat.com> wrote: >>> >>> The system dma-buf heap lets userspace allocate buffers from the page >>> allocator. However, these allocations are not accounted for in memcg, >>> allowing processes to escape limits that may be configured. >>> >>> Pass the __GFP_ACCOUNT for our allocations to account them into memcg. >> >> We had a discussion just last night in the MM track at LPC about how >> shared memory accounted in memcg is pretty broken. Without a way to >> identify (and possibly transfer) ownership of a shared buffer, this >> makes the accounting of shared memory, and zombie memcg problems >> worse. :\ > > Are there notes or a report from that discussion anywhere? > > The way I see it, the dma-buf heaps *trivial* case is non-existent at > the moment and that's definitely broken. Any application can bypass its > cgroups limits trivially, and that's a pretty big hole in the system. Well, that is just the tip of the iceberg. Pretty much all driver interfaces doesn't account to memcg at the moment, all the way from alsa, over GPUs (both TTM and SHM-GEM) to V4L2. > The shared ownership is indeed broken, but it's not more or less broken > than, say, memfd + udmabuf, and I'm sure plenty of others. > > So we really improve the common case, but only make the "advanced" > slightly more broken than it already is. > > Would you disagree? I strongly disagree. As far as I can see there is a huge chance we break existing use cases with that. There has been some work on TTM by Dave but I still haven't found time to wrap my head around all possible side effects such a change can have. The fundamental problem is that neither memcg nor the classic resource tracking (e.g. the OOM killer) has a good understanding of shared resources. For example you can use memfd to basically kill any process in the system because the OOM killer can't identify the process which holds the reference to the memory in question. And that is a *MUCH* bigger problem than just inaccurate memcg accounting. Regards, Christian. > > Maxime

2 months

1
0
0 0

Re: [PATCH 14/19] drm/amdgpu: independence for the amdkfd_fence! v4

by Christian König

On 12/11/25 16:08, Philipp Stanner wrote: > On Thu, 2025-12-11 at 13:16 +0100, Christian König wrote: >> This allows amdkfd_fences to outlive the amdgpu module. >> >> v2: implement Felix suggestion to lock the fence while signaling it. >> v3: fix typos >> v4: fix return code in signal_eviction_fence >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 7 +++ >> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c | 44 +++++++++---------- >> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +- >> drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 4 +- >> 4 files changed, 31 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >> index 8bdfcde2029b..2f2b277cfaed 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h >> @@ -196,6 +196,7 @@ int kfd_debugfs_kfd_mem_limits(struct seq_file *m, void *data); >> #endif >> #if IS_ENABLED(CONFIG_HSA_AMD) >> bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm); >> +bool amdkfd_fence_signal(struct dma_fence *f); >> struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f); >> void amdgpu_amdkfd_remove_all_eviction_fences(struct amdgpu_bo *bo); >> int amdgpu_amdkfd_evict_userptr(struct mmu_interval_notifier *mni, >> @@ -210,6 +211,12 @@ bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm) >> return false; >> } >> >> +static inline >> +bool amdkfd_fence_signal(struct dma_fence *f) >> +{ >> + return false; >> +} > > Huh? What's that? > > That function seems to be just a NOP. It's return code is used nowhere, > is it? It's the dummy which is used when CONFIG_HSA_AMD isn't enabled. Not sure if it's actually used or not, but we have dummies for all functions declared in this file. > >> + >> static inline >> struct amdgpu_amdkfd_fence *to_amdgpu_amdkfd_fence(struct dma_fence *f) >> { >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c >> index 09c919f72b6c..9cd413e325f0 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c >> @@ -127,29 +127,9 @@ static bool amdkfd_fence_enable_signaling(struct dma_fence *f) >> if (!svm_range_schedule_evict_svm_bo(fence)) >> return true; >> } >> - return false; >> -} >> - >> -/** >> - * amdkfd_fence_release - callback that fence can be freed >> - * >> - * @f: dma_fence >> - * >> - * This function is called when the reference count becomes zero. >> - * Drops the mm_struct reference and RCU schedules freeing up the fence. >> - */ >> -static void amdkfd_fence_release(struct dma_fence *f) >> -{ >> - struct amdgpu_amdkfd_fence *fence = to_amdgpu_amdkfd_fence(f); >> - >> - /* Unconditionally signal the fence. The process is getting >> - * terminated. >> - */ >> - if (WARN_ON(!fence)) >> - return; /* Not an amdgpu_amdkfd_fence */ >> - >> mmdrop(fence->mm); >> - kfree_rcu(f, rcu); >> + fence->mm = NULL; >> + return false; >> } >> >> /** >> @@ -174,9 +154,27 @@ bool amdkfd_fence_check_mm(struct dma_fence *f, struct mm_struct *mm) >> return false; >> } >> >> +bool amdkfd_fence_signal(struct dma_fence *f) >> +{ >> + struct amdgpu_amdkfd_fence *fence = to_amdgpu_amdkfd_fence(f); >> + unsigned long flags; >> + bool was_signaled; >> + >> + dma_fence_lock_irqsave(f, flags); >> + if (fence->mm) { >> + mmdrop(fence->mm); >> + fence->mm = NULL; >> + } >> + was_signaled = dma_fence_is_signaled_locked(f); >> + if (!was_signaled) >> + dma_fence_signal_locked(f); >> + dma_fence_unlock_irqrestore(f, flags); >> + >> + return was_signaled; >> +} >> + >> static const struct dma_fence_ops amdkfd_fence_ops = { >> .get_driver_name = amdkfd_fence_get_driver_name, >> .get_timeline_name = amdkfd_fence_get_timeline_name, >> .enable_signaling = amdkfd_fence_enable_signaling, >> - .release = amdkfd_fence_release, >> }; >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> index bb252ec43733..2cf39e3d3fae 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c >> @@ -1173,7 +1173,7 @@ static void kfd_process_wq_release(struct work_struct *work) >> synchronize_rcu(); >> ef = rcu_access_pointer(p->ef); >> if (ef) >> - dma_fence_signal(ef); >> + amdkfd_fence_signal(ef); >> >> kfd_process_remove_sysfs(p); >> kfd_debugfs_remove_process(p); >> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >> index 97c2270f278f..0e94f3a976b1 100644 >> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c >> @@ -428,7 +428,7 @@ static void svm_range_bo_release(struct kref *kref) >> >> if (!dma_fence_is_signaled(&svm_bo->eviction_fence->base)) >> /* We're not in the eviction worker. Signal the fence. */ >> - dma_fence_signal(&svm_bo->eviction_fence->base); >> + amdkfd_fence_signal(&svm_bo->eviction_fence->base); >> dma_fence_put(&svm_bo->eviction_fence->base); >> amdgpu_bo_unref(&svm_bo->bo); >> kfree(svm_bo); >> @@ -3628,7 +3628,7 @@ static void svm_range_evict_svm_bo_worker(struct work_struct *work) >> mmap_read_unlock(mm); >> mmput(mm); >> >> - dma_fence_signal(&svm_bo->eviction_fence->base); >> + amdkfd_fence_signal(&svm_bo->eviction_fence->base); > > > And why do you do those changes and why doesn't the commit message > explain it? > > You stop signalling those fences, after all. Hui? I don't stop signaling the fences. I just delegate signaling into a separate helper function which does some extra cleanup before signaling the fence. Regards, Christian. > > > P. > >> >> /* This is the last reference to svm_bo, after svm_range_vram_node_free >> * has been called in svm_migrate_vram_to_ram >

2 months

1
0
0 0

Re: [PATCH 09/19] drm/sched: use inline locks for the drm-sched-fence

by Christian König

On 12/11/25 16:13, Tvrtko Ursulin wrote: > > On 11/12/2025 13:16, Christian König wrote: >> Using the inline lock is now the recommended way for dma_fence implementations. >> >> So use this approach for the scheduler fences as well just in case if >> anybody uses this as blueprint for its own implementation. >> >> Also saves about 4 bytes for the external spinlock. >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> --- >> drivers/gpu/drm/scheduler/sched_fence.c | 7 +++---- >> include/drm/gpu_scheduler.h | 4 ---- >> 2 files changed, 3 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c >> index 08ccbde8b2f5..47471b9e43f9 100644 >> --- a/drivers/gpu/drm/scheduler/sched_fence.c >> +++ b/drivers/gpu/drm/scheduler/sched_fence.c >> @@ -161,7 +161,7 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f, >> /* If we already have an earlier deadline, keep it: */ >> if (test_bit(DRM_SCHED_FENCE_FLAG_HAS_DEADLINE_BIT, &f->flags) && >> ktime_before(fence->deadline, deadline)) { >> - spin_unlock_irqrestore(&fence->lock, flags); >> + dma_fence_unlock_irqrestore(f, flags); > > Rebase error I guess. Pull into the locking helpers patch. No that is actually completely intentional here. Previously we had a separate lock which protected both the DMA-fences as well as the deadline state. Now we turn that upside down by dropping the separate lock and protecting the deadline state with the dma_fence lock instead. Regards, Christian. > > Regards, > > Tvrtko > >> return; >> } >> @@ -217,7 +217,6 @@ struct drm_sched_fence *drm_sched_fence_alloc(struct drm_sched_entity *entity, >> fence->owner = owner; >> fence->drm_client_id = drm_client_id; >> - spin_lock_init(&fence->lock); >> return fence; >> } >> @@ -230,9 +229,9 @@ void drm_sched_fence_init(struct drm_sched_fence *fence, >> fence->sched = entity->rq->sched; >> seq = atomic_inc_return(&entity->fence_seq); >> dma_fence_init(&fence->scheduled, &drm_sched_fence_ops_scheduled, >> - &fence->lock, entity->fence_context, seq); >> + NULL, entity->fence_context, seq); >> dma_fence_init(&fence->finished, &drm_sched_fence_ops_finished, >> - &fence->lock, entity->fence_context + 1, seq); >> + NULL, entity->fence_context + 1, seq); >> } >> module_init(drm_sched_fence_slab_init); >> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h >> index fb88301b3c45..b77f24a783e3 100644 >> --- a/include/drm/gpu_scheduler.h >> +++ b/include/drm/gpu_scheduler.h >> @@ -297,10 +297,6 @@ struct drm_sched_fence { >> * belongs to. >> */ >> struct drm_gpu_scheduler *sched; >> - /** >> - * @lock: the lock used by the scheduled and the finished fences. >> - */ >> - spinlock_t lock; >> /** >> * @owner: job owner for debugging >> */ >

2 months

1
0
0 0

Re: [PATCH 11/19] drm/amdgpu: independence for the amdgpu_fence! v2

by Christian König

On 12/11/25 16:53, Tvrtko Ursulin wrote: > > On 11/12/2025 13:16, Christian König wrote: >> This allows amdgpu_fences to outlive the amdgpu module. >> >> v2: use dma_fence_get_rcu_safe to be NULL safe here. >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 63 +++++++---------------- >> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 - >> 2 files changed, 20 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> index c7843e336310..c636347801c1 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c >> @@ -112,8 +112,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct amdgpu_fence *af, >> af->ring = ring; >> seq = ++ring->fence_drv.sync_seq; >> - dma_fence_init(fence, &amdgpu_fence_ops, >> - &ring->fence_drv.lock, >> + dma_fence_init(fence, &amdgpu_fence_ops, NULL, >> adev->fence_context + ring->idx, seq); >> amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr, >> @@ -467,7 +466,6 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring) >> timer_setup(&ring->fence_drv.fallback_timer, amdgpu_fence_fallback, 0); >> ring->fence_drv.num_fences_mask = ring->num_hw_submission * 2 - 1; >> - spin_lock_init(&ring->fence_drv.lock); >> ring->fence_drv.fences = kcalloc(ring->num_hw_submission * 2, sizeof(void *), >> GFP_KERNEL); >> @@ -654,16 +652,20 @@ void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error) >> struct amdgpu_fence_driver *drv = &ring->fence_drv; >> unsigned long flags; >> - spin_lock_irqsave(&drv->lock, flags); >> + rcu_read_lock(); >> for (unsigned int i = 0; i <= drv->num_fences_mask; ++i) { >> struct dma_fence *fence; >> - fence = rcu_dereference_protected(drv->fences[i], >> - lockdep_is_held(&drv->lock)); >> - if (fence && !dma_fence_is_signaled_locked(fence)) >> + fence = dma_fence_get_rcu(drv->fences[i]); > > dma_fence_get_rcu is not safe against passing a NULL fence in, while the existing code makes it look like drv->fence[] slot can contain NULL at this point? > > amdgpu_fence_process() is the place which can NULL the slots? Irq context? Why is that safe with no reference held from clearing the slot to operating on the fence? The slots are never cleared. It can only be that they are NULL when the driver is loaded. I've switched over to dma_fence_get_rcu_safe() where appropriated. Regards, Christian. > >> + if (!fence) >> + continue; >> + >> + dma_fence_lock_irqsave(fence, flags); >> + if (!dma_fence_is_signaled_locked(fence)) >> dma_fence_set_error(fence, error); >> + dma_fence_unlock_irqrestore(fence, flags); >> } >> - spin_unlock_irqrestore(&drv->lock, flags); >> + rcu_read_unlock(); >> } >> /** >> @@ -714,16 +716,19 @@ void amdgpu_fence_driver_guilty_force_completion(struct amdgpu_fence *af) >> seq = ring->fence_drv.sync_seq & ring->fence_drv.num_fences_mask; >> /* mark all fences from the guilty context with an error */ >> - spin_lock_irqsave(&ring->fence_drv.lock, flags); >> + rcu_read_lock(); >> do { >> last_seq++; >> last_seq &= ring->fence_drv.num_fences_mask; >> ptr = &ring->fence_drv.fences[last_seq]; >> - rcu_read_lock(); >> - unprocessed = rcu_dereference(*ptr); >> + unprocessed = dma_fence_get_rcu_safe(ptr); > > Similar concern like the above. > > Regards, > > Tvrtko >> + >> + if (!unprocessed) >> + continue; >> - if (unprocessed && !dma_fence_is_signaled_locked(unprocessed)) { >> + dma_fence_lock_irqsave(unprocessed, flags); >> + if (dma_fence_is_signaled_locked(unprocessed)) { >> fence = container_of(unprocessed, struct amdgpu_fence, base); >> if (fence == af) >> @@ -731,9 +736,10 @@ void amdgpu_fence_driver_guilty_force_completion(struct amdgpu_fence *af) >> else if (fence->context == af->context) >> dma_fence_set_error(&fence->base, -ECANCELED); >> } >> - rcu_read_unlock(); >> + dma_fence_unlock_irqrestore(unprocessed, flags); >> + dma_fence_put(unprocessed); >> } while (last_seq != seq); >> - spin_unlock_irqrestore(&ring->fence_drv.lock, flags); >> + rcu_read_unlock(); >> /* signal the guilty fence */ >> amdgpu_fence_write(ring, (u32)af->base.seqno); >> amdgpu_fence_process(ring); >> @@ -823,39 +829,10 @@ static bool amdgpu_fence_enable_signaling(struct dma_fence *f) >> return true; >> } >> -/** >> - * amdgpu_fence_free - free up the fence memory >> - * >> - * @rcu: RCU callback head >> - * >> - * Free up the fence memory after the RCU grace period. >> - */ >> -static void amdgpu_fence_free(struct rcu_head *rcu) >> -{ >> - struct dma_fence *f = container_of(rcu, struct dma_fence, rcu); >> - >> - /* free fence_slab if it's separated fence*/ >> - kfree(to_amdgpu_fence(f)); >> -} >> - >> -/** >> - * amdgpu_fence_release - callback that fence can be freed >> - * >> - * @f: fence >> - * >> - * This function is called when the reference count becomes zero. >> - * It just RCU schedules freeing up the fence. >> - */ >> -static void amdgpu_fence_release(struct dma_fence *f) >> -{ >> - call_rcu(&f->rcu, amdgpu_fence_free); >> -} >> - >> static const struct dma_fence_ops amdgpu_fence_ops = { >> .get_driver_name = amdgpu_fence_get_driver_name, >> .get_timeline_name = amdgpu_fence_get_timeline_name, >> .enable_signaling = amdgpu_fence_enable_signaling, >> - .release = amdgpu_fence_release, >> }; >> /* >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> index 7a27c6c4bb44..9cbf63454004 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h >> @@ -125,7 +125,6 @@ struct amdgpu_fence_driver { >> unsigned irq_type; >> struct timer_list fallback_timer; >> unsigned num_fences_mask; >> - spinlock_t lock; >> struct dma_fence **fences; >> }; >> >

2 months

1
0
0 0

Re: [PATCH 15/19] drm/amdgpu: independence for the amdgpu_userq_fence!

by Christian König

On 12/11/25 17:12, Tvrtko Ursulin wrote: > > On 11/12/2025 13:16, Christian König wrote: >> This allows amdgpu_userq_fences to outlive the amdgpu module. >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 13 +---- >> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 54 ++++--------------- >> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 8 --- >> 3 files changed, 11 insertions(+), 64 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> index 2dfbddcef9ab..f206297aae8b 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> @@ -3155,11 +3155,7 @@ static int __init amdgpu_init(void) >> r = amdgpu_sync_init(); >> if (r) >> - goto error_sync; >> - >> - r = amdgpu_userq_fence_slab_init(); >> - if (r) >> - goto error_fence; >> + return r; >> DRM_INFO("amdgpu kernel modesetting enabled.\n"); >> amdgpu_register_atpx_handler(); >> @@ -3176,12 +3172,6 @@ static int __init amdgpu_init(void) >> /* let modprobe override vga console setting */ >> return pci_register_driver(&amdgpu_kms_pci_driver); >> - >> -error_fence: >> - amdgpu_sync_fini(); >> - >> -error_sync: >> - return r; >> } >> static void __exit amdgpu_exit(void) >> @@ -3191,7 +3181,6 @@ static void __exit amdgpu_exit(void) >> amdgpu_unregister_atpx_handler(); >> amdgpu_acpi_release(); >> amdgpu_sync_fini(); >> - amdgpu_userq_fence_slab_fini(); >> mmu_notifier_synchronize(); >> amdgpu_xcp_drv_release(); >> } >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >> index eba9fb359047..bb19f72770b0 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c >> @@ -33,26 +33,6 @@ >> #include "amdgpu_userq_fence.h" >> static const struct dma_fence_ops amdgpu_userq_fence_ops; >> -static struct kmem_cache *amdgpu_userq_fence_slab; >> - >> -int amdgpu_userq_fence_slab_init(void) >> -{ >> - amdgpu_userq_fence_slab = kmem_cache_create("amdgpu_userq_fence", >> - sizeof(struct amdgpu_userq_fence), >> - 0, >> - SLAB_HWCACHE_ALIGN, >> - NULL); >> - if (!amdgpu_userq_fence_slab) >> - return -ENOMEM; >> - >> - return 0; >> -} >> - >> -void amdgpu_userq_fence_slab_fini(void) >> -{ >> - rcu_barrier(); > > What was this rcu_barrier() for? Cargo culted or more to it? All dma_fences are RCU protected. When they are backed by a kmem_cache you need to make sure to wait for an RCU grace period to pass before destroying the kmem_cache. Since the dma_fence framework now uses kfree_rcu that shouldn't be problematic any more. >> - kmem_cache_destroy(amdgpu_userq_fence_slab); >> -} >> static inline struct amdgpu_userq_fence *to_amdgpu_userq_fence(struct dma_fence *f) >> { >> @@ -227,7 +207,7 @@ void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv) >> static int amdgpu_userq_fence_alloc(struct amdgpu_userq_fence **userq_fence) >> { >> - *userq_fence = kmem_cache_alloc(amdgpu_userq_fence_slab, GFP_ATOMIC); >> + *userq_fence = kmalloc(sizeof(**userq_fence), GFP_ATOMIC); > This GFP_ATOMIC is suboptimal for sure being on the ioctl path. It is outside of the scope for this patch, but once my userq cleanup patches get reviewed next on my list was to try and understand this. >> return *userq_fence ? 0 : -ENOMEM; >> } >> @@ -243,12 +223,11 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq, >> if (!fence_drv) >> return -EINVAL; >> - spin_lock_init(&userq_fence->lock); >> INIT_LIST_HEAD(&userq_fence->link); >> fence = &userq_fence->base; >> userq_fence->fence_drv = fence_drv; >> - dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock, >> + dma_fence_init64(fence, &amdgpu_userq_fence_ops, NULL, >> fence_drv->context, seq); >> amdgpu_userq_fence_driver_get(fence_drv); >> @@ -318,35 +297,22 @@ static bool amdgpu_userq_fence_signaled(struct dma_fence *f) >> rptr = amdgpu_userq_fence_read(fence_drv); >> wptr = fence->base.seqno; >> - if (rptr >= wptr) >> + if (rptr >= wptr) { >> + amdgpu_userq_fence_driver_put(fence->fence_drv); > > fence_drv is in a local already. > >> + fence->fence_drv = NULL; > > amdgpu_userq_fence_get_timeline_name could now oops somehow? > >> + >> + kvfree(fence->fence_drv_array); >> + fence->fence_drv_array = NULL; > > Not sure if this is safe either. amdgpu_userq_fence_driver_process() drops its reference before it unlinks the fence from the list. Can someone external trigger the fence_is_signaled check, before the interrupt processing kicks in, which will clear fence_drv_array, and so amdgpu_userq_fence_driver_process() would oops? Oh, good question. I need to double check that. Thanks, Christian. > > Regards, > > Tvrtko > >> return true; >> + } >> return false; >> } >> -static void amdgpu_userq_fence_free(struct rcu_head *rcu) >> -{ >> - struct dma_fence *fence = container_of(rcu, struct dma_fence, rcu); >> - struct amdgpu_userq_fence *userq_fence = to_amdgpu_userq_fence(fence); >> - struct amdgpu_userq_fence_driver *fence_drv = userq_fence->fence_drv; >> - >> - /* Release the fence driver reference */ >> - amdgpu_userq_fence_driver_put(fence_drv); >> - >> - kvfree(userq_fence->fence_drv_array); >> - kmem_cache_free(amdgpu_userq_fence_slab, userq_fence); >> -} >> - >> -static void amdgpu_userq_fence_release(struct dma_fence *f) >> -{ >> - call_rcu(&f->rcu, amdgpu_userq_fence_free); >> -} >> - >> static const struct dma_fence_ops amdgpu_userq_fence_ops = { >> .get_driver_name = amdgpu_userq_fence_get_driver_name, >> .get_timeline_name = amdgpu_userq_fence_get_timeline_name, >> .signaled = amdgpu_userq_fence_signaled, >> - .release = amdgpu_userq_fence_release, >> }; >> /** >> @@ -560,7 +526,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, >> r = amdgpu_userq_fence_create(queue, userq_fence, wptr, &fence); >> if (r) { >> mutex_unlock(&userq_mgr->userq_mutex); >> - kmem_cache_free(amdgpu_userq_fence_slab, userq_fence); >> + kfree(userq_fence); >> goto put_gobj_write; >> } >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h >> index d76add2afc77..6f04782f3ea9 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h >> @@ -31,11 +31,6 @@ >> struct amdgpu_userq_fence { >> struct dma_fence base; >> - /* >> - * This lock is necessary to synchronize the >> - * userqueue dma fence operations. >> - */ >> - spinlock_t lock; >> struct list_head link; >> unsigned long fence_drv_array_count; >> struct amdgpu_userq_fence_driver *fence_drv; >> @@ -58,9 +53,6 @@ struct amdgpu_userq_fence_driver { >> char timeline_name[TASK_COMM_LEN]; >> }; >> -int amdgpu_userq_fence_slab_init(void); >> -void amdgpu_userq_fence_slab_fini(void); >> - >> void amdgpu_userq_fence_driver_get(struct amdgpu_userq_fence_driver *fence_drv); >> void amdgpu_userq_fence_driver_put(struct amdgpu_userq_fence_driver *fence_drv); >> int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, >

2 months

1
0
0 0

Re: [PATCH v2] drm: Fix object leak in DRM_IOCTL_GEM_CHANGE_HANDLE

by Christian König

On 12/12/25 14:20, Karol Wachowski wrote: > Add missing drm_gem_object_put() call when drm_gem_object_lookup() > successfully returns an object. This fixes a GEM object reference > leak that can prevent driver modules from unloading when using > prime buffers. > > Fixes: 53096728b891 ("drm: Add DRM prime interface to reassign GEM handle") > Signed-off-by: Karol Wachowski <karol.wachowski(a)linux.intel.com> > --- > Changes between v1 and v2: > - move setting ret value under if branch as suggested in review > - add Cc: stable 6.18+ Oh don't CC the stable list on the review mail directly, just add "CC: stable(a)vger.kernel.org # 6.18+" to the tags. Greg is going to complain about that :( With that done Reviewed-by: Christian König <christian.koenig(a)amd.com> and please push to drm-misc-fixes. If you don't have commit rights for drm-misc-fixes please ping me and I'm going to push that. Thanks, Christian. > --- > drivers/gpu/drm/drm_gem.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ca1956608261..bcc08a6aebf8 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1010,8 +1010,10 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, > if (!obj) > return -ENOENT; > > - if (args->handle == args->new_handle) > - return 0; > + if (args->handle == args->new_handle) { > + ret = 0; > + goto out; > + } > > mutex_lock(&file_priv->prime.lock); > > @@ -1043,6 +1045,8 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, > > out_unlock: > mutex_unlock(&file_priv->prime.lock); > +out: > + drm_gem_object_put(obj); > > return ret; > }

2 months

1
0
0 0

Re: [PATCH] drm: Fix object leak in DRM_IOCTL_GEM_CHANGE_HANDLE

by Christian König

On 12/12/25 14:02, Karol Wachowski wrote: > Add missing drm_gem_object_put() call when drm_gem_object_lookup() > successfully returns an object. This fixes a GEM object reference > leak that can prevent driver modules from unloading when using > prime buffers. Good catch. > Fixes: 53096728b891 ("drm: Add DRM prime interface to reassign GEM handle") > Signed-off-by: Karol Wachowski <karol.wachowski(a)linux.intel.com> CC: stable 6.18? > --- > drivers/gpu/drm/drm_gem.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ca1956608261..e150bc1ce65a 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -1001,7 +1001,7 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, > { > struct drm_gem_change_handle *args = data; > struct drm_gem_object *obj; > - int ret; > + int ret = 0; Please set ret explicitly in the if branch below. Always initializing return values is usually considered bad coding style. Apart from that looks good to me. Thanks, Christian. > > if (!drm_core_check_feature(dev, DRIVER_GEM)) > return -EOPNOTSUPP; > @@ -1011,7 +1011,7 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, > return -ENOENT; > > if (args->handle == args->new_handle) > - return 0; > + goto out; > > mutex_lock(&file_priv->prime.lock); > > @@ -1043,6 +1043,8 @@ int drm_gem_change_handle_ioctl(struct drm_device *dev, void *data, > > out_unlock: > mutex_unlock(&file_priv->prime.lock); > +out: > + drm_gem_object_put(obj); > > return ret; > }

2 months

1
0
0 0

Re: [PATCH 03/19] dma-buf: inline spinlock for fence protection v3

by Christian König

On 12/11/25 15:35, Tvrtko Ursulin wrote: > > Hi, > > On 11/12/2025 13:16, Christian König wrote: >> Implement per-fence spinlocks, allowing implementations to not give an >> external spinlock to protect the fence internal statei. Instead a spinlock >> embedded into the fence structure itself is used in this case. >> >> Shared spinlocks have the problem that implementations need to guarantee >> that the lock live at least as long all fences referencing them. >> >> Using a per-fence spinlock allows completely decoupling spinlock producer >> and consumer life times, simplifying the handling in most use cases. >> >> v2: improve naming, coverage and function documentation >> v3: fix one additional locking in the selftests >> >> Signed-off-by: Christian König <christian.koenig(a)amd.com> >> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com> > > I don't think I gave r-b on this one. Not just yet at least. Maybe you have missed the comments I had in the previous two rounds? I will repeat them below. I was already wondering why you gave comments and an rb but though that the comments might just be optional. Going to remove that and see on the comments below. >> @@ -365,7 +364,7 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence, >> struct dma_fence_cb *cur, *tmp; >> struct list_head cb_list; >> - lockdep_assert_held(fence->lock); >> + lockdep_assert_held(dma_fence_spinlock(fence)); >> if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >> &fence->flags))) >> @@ -412,9 +411,9 @@ void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp) >> if (WARN_ON(!fence)) >> return; >> - spin_lock_irqsave(fence->lock, flags); >> + dma_fence_lock_irqsave(fence, flags); > > For the locking wrappers I think it would be better to introduce them in a purely mechanical patch preceding this one. That is, just add the wrappers and nothing else. That doesn't fully work for all cases, but I will separate it out a bit more. >> static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm) >> { >> + struct dma_fence *fence; >> unsigned long flags; >> - spinlock_t *lock; >> /* >> * Workaround to stop racing between the fence signaling and handling >> - * the cb. The lock is static after initially setting it up, just make >> - * sure that the dma_fence structure isn't freed up. >> + * the cb. >> */ >> rcu_read_lock(); >> - lock = vm->last_tlb_flush->lock; >> + fence = dma_fence_get_rcu(vm->last_tlb_flush); > > Why does this belong here? If taking a reference fixes some race it needs to be a separate patch. If it doesn't then this patch shouldn't be adding it. The code previously assumed that the lock is global and can't go away while the function is called. When we start to use an inline lock that assumption is not true any more. But you're right that can be a separate patch. >> @@ -362,6 +368,38 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) >> } while (1); >> } >> +/** >> + * dma_fence_spinlock - return pointer to the spinlock protecting the fence >> + * @fence: the fence to get the lock from >> + * >> + * Return either the pointer to the embedded or the external spin lock. >> + */ >> +static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence) >> +{ >> + return test_bit(DMA_FENCE_FLAG_INLINE_LOCK_BIT, &fence->flags) ? >> + &fence->inline_lock : fence->extern_lock; > > Is sprinkling of conditionals better than growing the struct? Probably yes, since branch misses are cheaper than cache misses. Unless the code grows significantly on some hot path and we get instruction cache misses instead. Who knows. But let say in the commit message we considered it and decided on this solution due xyz. Sure. > > On a quick grep there is one arch where this grows the struct past a cache line anyway, but as it is PA-RISC I guess no one cares. Lets mention that in the commit message as well. Interesting, I was aware of the problems on Sparc regarding spinlocks but that PA-RISC also has something more complicated then an int is news to me. Anyway I agree it doesn't really matter. Regards, Christian. > > Regards, > > Tvrtko >> +} >> + >> +/** >> + * dma_fence_lock_irqsave - irqsave lock the fence >> + * @fence: the fence to lock >> + * @flags: where to store the CPU flags. >> + * >> + * Lock the fence, preventing it from changing to the signaled state. >> + */ >> +#define dma_fence_lock_irqsave(fence, flags) \ >> + spin_lock_irqsave(dma_fence_spinlock(fence), flags) >> + >> +/** >> + * dma_fence_unlock_irqrestore - unlock the fence and irqrestore >> + * @fence: the fence to unlock >> + * @flags the CPU flags to restore >> + * >> + * Unlock the fence, allowing it to change it's state to signaled again. >> + */ >> +#define dma_fence_unlock_irqrestore(fence, flags) \ >> + spin_unlock_irqrestore(dma_fence_spinlock(fence), flags) >> + >> #ifdef CONFIG_LOCKDEP >> bool dma_fence_begin_signalling(void); >> void dma_fence_end_signalling(bool cookie); >

2 months

1
0
0 0

Re: Independence for dma_fences! v4

by Christian König

On 12/11/25 13:33, Philipp Stanner wrote: > On Thu, 2025-12-11 at 13:16 +0100, Christian König wrote: >> Hi everyone, >> >> dma_fences have ever lived under the tyranny dictated by the module >> lifetime of their issuer, leading to crashes should anybody still holding >> a reference to a dma_fence when the module of the issuer was unloaded. >> >> The basic problem is that when buffer are shared between drivers >> dma_fence objects can leak into external drivers and stay there even >> after they are signaled. The dma_resv object for example only lazy releases >> dma_fences. >> >> So what happens is that when the module who originally created the dma_fence >> unloads the dma_fence_ops function table becomes unavailable as well and so >> any attempt to release the fence crashes the system. >> >> Previously various approaches have been discussed, including changing the >> locking semantics of the dma_fence callbacks (by me) as well as using the >> drm scheduler as intermediate layer (by Sima) to disconnect dma_fences >> from their actual users, but none of them are actually solving all problems. >> >> Tvrtko did some really nice prerequisite work by protecting the returned >> strings of the dma_fence_ops by RCU. This way dma_fence creators where >> able to just wait for an RCU grace period after fence signaling before >> they could be save to free those data structures. >> >> Now this patch set here goes a step further and protects the whole >> dma_fence_ops structure by RCU, so that after the fence signals the >> pointer to the dma_fence_ops is set to NULL when there is no wait nor >> release callback given. All functionality which use the dma_fence_ops >> reference are put inside an RCU critical section, except for the >> deprecated issuer specific wait and of course the optional release >> callback. >> >> Additional to the RCU changes the lock protecting the dma_fence state >> previously had to be allocated external. This set here now changes the >> functionality to make that external lock optional and allows dma_fences >> to use an inline lock and be self contained. >> >> v4: >> >> Rebases the whole set on upstream changes, especially the cleanup >> from Philip in patch "drm/amdgpu: independence for the amdkfd_fence!". >> >> Adding two patches which brings the DMA-fence self tests up to date. >> The first selftest changes removes the mock_wait and so actually starts >> testing the default behavior instead of some hacky implementation in the >> test. This one should probably go upstream independent of this set. >> The second drops the mock_fence as well and tests the new RCU and inline >> spinlock functionality. >> >> Especially the first patch still needs a Reviewed-by, apart from that I >> think I've addressed all review comments. >> >> The plan is to push the core DMA-buf changes to drm-misc-next and then the >> driver specific changes through the driver channels as approprite. > > This does not apply to drm-misc-next (unless I'm screwing up badly). > > Where can I apply it? I'd like to test the drm_sched changes before > this gets merged. drm-tip from a few days ago, otherwise the xe changes won't work. Regards, Christian. > > P. > >> >> Please review and comment, >> Christian. >> >> >

2 months

1
0
0 0

Re: [PATCH] dma-buf: system_heap: account for system heap allocation in memcg

by T.J. Mercier

On Fri, Dec 12, 2025 at 4:31 AM Eric Chanudet <echanude(a)redhat.com> wrote: > > The system dma-buf heap lets userspace allocate buffers from the page > allocator. However, these allocations are not accounted for in memcg, > allowing processes to escape limits that may be configured. > > Pass the __GFP_ACCOUNT for our allocations to account them into memcg. We had a discussion just last night in the MM track at LPC about how shared memory accounted in memcg is pretty broken. Without a way to identify (and possibly transfer) ownership of a shared buffer, this makes the accounting of shared memory, and zombie memcg problems worse. :\ > > Userspace components using the system heap can be constrained with, e.g: > systemd-run --user --scope -p MemoryMax=10M ... > > Signed-off-by: Eric Chanudet <echanude(a)redhat.com> > --- > drivers/dma-buf/heaps/system_heap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c > index 4c782fe33fd4..c91fcdff4b77 100644 > --- a/drivers/dma-buf/heaps/system_heap.c > +++ b/drivers/dma-buf/heaps/system_heap.c > @@ -38,10 +38,10 @@ struct dma_heap_attachment { > bool mapped; > }; > > -#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO) > +#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_ACCOUNT) > #define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ > | __GFP_NORETRY) & ~__GFP_RECLAIM) \ > - | __GFP_COMP) > + | __GFP_COMP | __GFP_ACCOUNT) > static gfp_t order_flags[] = {HIGH_ORDER_GFP, HIGH_ORDER_GFP, LOW_ORDER_GFP}; > /* > * The selection of the orders used for allocation (1MB, 64K, 4K) is designed > -- > 2.52.0 >

2 months

1
0
0 0

Independence for dma_fences! v4

by Christian König

Hi everyone, dma_fences have ever lived under the tyranny dictated by the module lifetime of their issuer, leading to crashes should anybody still holding a reference to a dma_fence when the module of the issuer was unloaded. The basic problem is that when buffer are shared between drivers dma_fence objects can leak into external drivers and stay there even after they are signaled. The dma_resv object for example only lazy releases dma_fences. So what happens is that when the module who originally created the dma_fence unloads the dma_fence_ops function table becomes unavailable as well and so any attempt to release the fence crashes the system. Previously various approaches have been discussed, including changing the locking semantics of the dma_fence callbacks (by me) as well as using the drm scheduler as intermediate layer (by Sima) to disconnect dma_fences from their actual users, but none of them are actually solving all problems. Tvrtko did some really nice prerequisite work by protecting the returned strings of the dma_fence_ops by RCU. This way dma_fence creators where able to just wait for an RCU grace period after fence signaling before they could be save to free those data structures. Now this patch set here goes a step further and protects the whole dma_fence_ops structure by RCU, so that after the fence signals the pointer to the dma_fence_ops is set to NULL when there is no wait nor release callback given. All functionality which use the dma_fence_ops reference are put inside an RCU critical section, except for the deprecated issuer specific wait and of course the optional release callback. Additional to the RCU changes the lock protecting the dma_fence state previously had to be allocated external. This set here now changes the functionality to make that external lock optional and allows dma_fences to use an inline lock and be self contained. v4: Rebases the whole set on upstream changes, especially the cleanup from Philip in patch "drm/amdgpu: independence for the amdkfd_fence!". Adding two patches which brings the DMA-fence self tests up to date. The first selftest changes removes the mock_wait and so actually starts testing the default behavior instead of some hacky implementation in the test. This one should probably go upstream independent of this set. The second drops the mock_fence as well and tests the new RCU and inline spinlock functionality. Especially the first patch still needs a Reviewed-by, apart from that I think I've addressed all review comments. The plan is to push the core DMA-buf changes to drm-misc-next and then the driver specific changes through the driver channels as approprite. Please review and comment, Christian.

2 months

1
19
0 0

[PATCH bpf 1/2] bpf: Fix truncated dmabuf iterator reads

by T.J. Mercier

If there is a large number (hundreds) of dmabufs allocated, the text output generated from dmabuf_iter_seq_show can exceed common user buffer sizes (e.g. PAGE_SIZE) necessitating multiple start/stop cycles to iterate through all dmabufs. However the dmabuf iterator currently returns NULL in dmabuf_iter_seq_start for all non-zero pos values, which results in the truncation of the output before all dmabufs are handled. After dma_buf_iter_begin / dma_buf_iter_next, the refcount of the buffer is elevated so that the BPF iterator program can run without holding any locks. When a stop occurs, instead of immediately dropping the reference on the buffer, stash a pointer to the buffer in seq->priv until either start is called or the iterator is released. This also enables the resumption of iteration without first walking through the list of dmabufs based on the pos value. Fixes: 76ea95534995 ("bpf: Add dmabuf iterator") Signed-off-by: T.J. Mercier <tjmercier(a)google.com> --- kernel/bpf/dmabuf_iter.c | 56 +++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/dmabuf_iter.c b/kernel/bpf/dmabuf_iter.c index 4dd7ef7c145c..cd500248abd9 100644 --- a/kernel/bpf/dmabuf_iter.c +++ b/kernel/bpf/dmabuf_iter.c @@ -6,10 +6,33 @@ #include <linux/kernel.h> #include <linux/seq_file.h> +struct dmabuf_iter_priv { + /* + * If this pointer is non-NULL, the buffer's refcount is elevated to + * prevent destruction between stop/start. If reading is not resumed and + * start is never called again, then dmabuf_iter_seq_fini drops the + * reference when the iterator is released. + */ + struct dma_buf *dmabuf; +}; + static void *dmabuf_iter_seq_start(struct seq_file *seq, loff_t *pos) { - if (*pos) - return NULL; + struct dmabuf_iter_priv *p = seq->private; + + if (*pos) { + struct dma_buf *dmabuf = p->dmabuf; + + if (!dmabuf) + return NULL; + + /* + * Always resume from where we stopped, regardless of the value + * of pos. + */ + p->dmabuf = NULL; + return dmabuf; + } return dma_buf_iter_begin(); } @@ -54,8 +77,11 @@ static void dmabuf_iter_seq_stop(struct seq_file *seq, void *v) { struct dma_buf *dmabuf = v; - if (dmabuf) - dma_buf_put(dmabuf); + if (dmabuf) { + struct dmabuf_iter_priv *p = seq->private; + + p->dmabuf = dmabuf; + } } static const struct seq_operations dmabuf_iter_seq_ops = { @@ -71,11 +97,27 @@ static void bpf_iter_dmabuf_show_fdinfo(const struct bpf_iter_aux_info *aux, seq_puts(seq, "dmabuf iter\n"); } +static int dmabuf_iter_seq_init(void *priv, struct bpf_iter_aux_info *aux) +{ + struct dmabuf_iter_priv *p = (struct dmabuf_iter_priv *)priv; + + p->dmabuf = NULL; + return 0; +} + +static void dmabuf_iter_seq_fini(void *priv) +{ + struct dmabuf_iter_priv *p = (struct dmabuf_iter_priv *)priv; + + if (p->dmabuf) + dma_buf_put(p->dmabuf); +} + static const struct bpf_iter_seq_info dmabuf_iter_seq_info = { .seq_ops = &dmabuf_iter_seq_ops, - .init_seq_private = NULL, - .fini_seq_private = NULL, - .seq_priv_size = 0, + .init_seq_private = dmabuf_iter_seq_init, + .fini_seq_private = dmabuf_iter_seq_fini, + .seq_priv_size = sizeof(struct dmabuf_iter_priv), }; static struct bpf_iter_reg bpf_dmabuf_reg_info = { base-commit: 30f09200cc4aefbd8385b01e41bde2e4565a6f0e -- 2.52.0.177.g9f829587af-goog

2 months

2
3
0 0

Re: [PATCH 1/2] dma-buf: Add __dma_fence_is_signaled()

by kernel test robot

Hello, kernel test robot noticed "Oops:general_protection_fault,probably_for_non-canonical_address#:#[##]SMP_KASAN_PTI" on: commit: 142f20971642be40507d3a0bc94e905e251db81a ("[PATCH 1/2] dma-buf: Add __dma_fence_is_signaled()") url: https://github.com/intel-lab-lkp/linux/commits/Philipp-Stanner/dma-buf-dma-… base: git://linuxtv.org/sailus/media_tree.git master patch link: https://lore.kernel.org/all/20251125104443.82974-2-phasta@kernel.org/ patch subject: [PATCH 1/2] dma-buf: Add __dma_fence_is_signaled() in testcase: lkvs version: lkvs-x86_64-6505b18-1_20251124 with following parameters: test: rapl-client config: x86_64-rhel-9.4-func compiler: gcc-14 test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory (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(a)intel.com> | Closes: https://lore.kernel.org/oe-lkp/202512092317.8cf778a-lkp@intel.com [ 46.662293][ T179] i915 0000:00:02.0: vgaarb: VGA decodes changed: olddecodes=io+mem,decodes=io+mem:owns=io+mem [ 46.680656][ T40] sd 1:0:0:0: [sda] Preferred minimum I/O size 4096 bytes [ 46.682302][ T59] i915 0000:00:02.0: Direct firmware load for i915/skl_dmc_ver1_27.bin failed with error -2 [ 46.698362][ T59] i915 0000:00:02.0: [drm] Failed to load DMC firmware i915/skl_dmc_ver1_27.bin (-ENOENT). Disabling runtime power management. [ 46.711256][ T59] i915 0000:00:02.0: [drm] DMC firmware homepage: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git [ 46.725941][ T12] Oops: general protection fault, probably for non-canonical address 0xdffffc01544551ba: 0000 [#1] SMP KASAN PTI [ 46.725953][ T179] ------------[ cut here ]------------ [ 46.737600][ T12] KASAN: probably user-memory-access in range [0x0000000aa22a8dd0-0x0000000aa22a8dd7] [ 46.737612][ T12] CPU: 3 UID: 0 PID: 12 Comm: kworker/u16:0 Tainted: G S I 6.18.0-rc5-00236-g142f20971642 #1 PREEMPT(voluntary) [ 46.742889][ T179] Fence 0000:00:02.0:[i915]:9:2 released with pending signals! [ 46.752212][ T12] Tainted: [S]=CPU_OUT_OF_SPEC, [I]=FIRMWARE_WORKAROUND [ 46.752217][ T12] Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.1.1 10/07/2015 [ 46.752220][ T12] Workqueue: i915 __i915_gem_free_work [i915] [ 46.765216][ T179] WARNING: CPU: 0 PID: 179 at drivers/dma-buf/dma-fence.c:555 dma_fence_release (kbuild/src/consumer/drivers/dma-buf/dma-fence.c:555 (discriminator 1)) [ 46.772517][ T12] [ 46.772521][ T12] RIP: 0010:__list_add_valid_or_report (kbuild/src/consumer/lib/list_debug.c:29) [ 46.772528][ T12] Code: 8b 98 fe 48 89 d3 48 85 d2 0f 84 af 8b 98 fe 4c 8d 62 08 48 89 fd 49 89 f5 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f 85 b7 00 00 00 4c 39 6b 08 75 43 48 b8 00 00 00 00 All code ======== 0: 8b 98 fe 48 89 d3 mov -0x2c76b702(%rax),%ebx 6: 48 85 d2 test %rdx,%rdx 9: 0f 84 af 8b 98 fe je 0xfffffffffe988bbe f: 4c 8d 62 08 lea 0x8(%rdx),%r12 13: 48 89 fd mov %rdi,%rbp 16: 49 89 f5 mov %rsi,%r13 19: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax 20: fc ff df 23: 4c 89 e2 mov %r12,%rdx 26: 48 c1 ea 03 shr $0x3,%rdx 2a:* 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) <-- trapping instruction 2e: 0f 85 b7 00 00 00 jne 0xeb 34: 4c 39 6b 08 cmp %r13,0x8(%rbx) 38: 75 43 jne 0x7d 3a: 48 rex.W 3b: b8 00 00 00 00 mov $0x0,%eax Code starting with the faulting instruction =========================================== 0: 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1) 4: 0f 85 b7 00 00 00 jne 0xc1 a: 4c 39 6b 08 cmp %r13,0x8(%rbx) e: 75 43 jne 0x53 10: 48 rex.W 11: b8 00 00 00 00 mov $0x0,%eax [ 46.779267][ T179] Modules linked in: [ 46.787293][ T12] RSP: 0018:ffffc9000010fac8 EFLAGS: 00010003 [ 46.787300][ T12] RAX: dffffc0000000000 RBX: 0000000aa22a8dcf RCX: ffffffff82f5859d [ 46.793179][ T179] platform_profile [ 46.803018][ T12] RDX: 00000001544551ba RSI: ffff88884fde5c90 RDI: ffffc9000010fb50 [ 46.803023][ T12] RBP: ffffc9000010fb50 R08: 0000000000000000 R09: ffffed1109fbcb96 [ 46.803025][ T12] R10: ffff88884fde5cb7 R11: fefefefefefefeff R12: 0000000aa22a8dd7 [ 46.805219][ T179] sd_mod [ 46.811413][ T12] R13: ffff88884fde5c90 R14: ffff88884fde5c90 R15: 0000000aa22a8dcf [ 46.811417][ T12] FS: 0000000000000000(0000) GS:ffff888848e3e000(0000) knlGS:0000000000000000 [ 46.811419][ T12] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 46.830755][ T179] intel_rapl_common [ 46.834463][ T12] CR2: 00007f3bc83ae000 CR3: 0000000868388002 CR4: 00000000003726f0 [ 46.834467][ T12] Call Trace: [ 46.834469][ T12] <TASK> [ 46.840348][ T179] snd_hda_core [ 46.848115][ T12] dma_fence_default_wait (kbuild/src/consumer/include/linux/list.h:158 (discriminator 1) kbuild/src/consumer/include/linux/list.h:177 (discriminator 1) kbuild/src/consumer/drivers/dma-buf/dma-fence.c:800 (discriminator 1)) [ 46.851758][ T179] x86_pkg_temp_thermal [ 46.859527][ T12] ? __pfx_dma_fence_default_wait (kbuild/src/consumer/drivers/dma-buf/dma-fence.c:778) The kernel config and materials to reproduce are available at: https://download.01.org/0day-ci/archive/20251209/202512092317.8cf778a-lkp@i… -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

2 months, 1 week

1
0
0 0

Re: [PATCH v18 25/42] dept: add documents for dept

by Bagas Sanjaya

On Fri, Dec 05, 2025 at 04:18:38PM +0900, Byungchul Park wrote: > Add documents describing the concept and APIs of dept. > > Signed-off-by: Byungchul Park <byungchul(a)sk.com> > --- > Documentation/dev-tools/dept.rst | 778 +++++++++++++++++++++++++++ > Documentation/dev-tools/dept_api.rst | 125 +++++ You forget to add toctree entries: ---- >8 ---- diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst index 4b8425e348abd1..02c858f5ed1fa2 100644 --- a/Documentation/dev-tools/index.rst +++ b/Documentation/dev-tools/index.rst @@ -22,6 +22,8 @@ Documentation/process/debugging/index.rst clang-format coccinelle sparse + dept + dept_api kcov gcov kasan > +Lockdep detects a deadlock by checking lock acquisition order. For > +example, a graph to track acquisition order built by lockdep might look > +like: > + > +.. literal:: > + > + A -> B - > + \ > + -> E > + / > + C -> D - > + > + where 'A -> B' means that acquisition A is prior to acquisition B > + with A still held. Use code-block directive for literal code blocks: ---- >8 ---- diff --git a/Documentation/dev-tools/dept.rst b/Documentation/dev-tools/dept.rst index 333166464543d7..8394c4ea81bc2a 100644 --- a/Documentation/dev-tools/dept.rst +++ b/Documentation/dev-tools/dept.rst @@ -10,7 +10,7 @@ Lockdep detects a deadlock by checking lock acquisition order. For example, a graph to track acquisition order built by lockdep might look like: -.. literal:: +.. code-block:: A -> B - \ @@ -25,7 +25,7 @@ Lockdep keeps adding each new acquisition order into the graph at runtime. For example, 'E -> C' will be added when the two locks have been acquired in the order, E and then C. The graph will look like: -.. literal:: +.. code-block:: A -> B - \ @@ -41,7 +41,7 @@ been acquired in the order, E and then C. The graph will look like: This graph contains a subgraph that demonstrates a loop like: -.. literal:: +.. code-block:: -> E - / \ @@ -76,7 +76,7 @@ e.g. irq context, normal process context, wq worker context, or so on. Can lockdep detect the following deadlock? -.. literal:: +.. code-block:: context X context Y context Z @@ -91,7 +91,7 @@ Can lockdep detect the following deadlock? No. What about the following? -.. literal:: +.. code-block:: context X context Y @@ -116,7 +116,7 @@ What leads a deadlock A deadlock occurs when one or multi contexts are waiting for events that will never happen. For example: -.. literal:: +.. code-block:: context X context Y context Z @@ -148,7 +148,7 @@ In terms of dependency: Dependency graph reflecting this example will look like: -.. literal:: +.. code-block:: -> C -> A -> B - / \ @@ -171,7 +171,7 @@ Introduce DEPT DEPT(DEPendency Tracker) tracks wait and event instead of lock acquisition order so as to recognize the following situation: -.. literal:: +.. code-block:: context X context Y context Z @@ -186,7 +186,7 @@ acquisition order so as to recognize the following situation: and builds up a dependency graph at runtime that is similar to lockdep. The graph might look like: -.. literal:: +.. code-block:: -> C -> A -> B - / \ @@ -199,7 +199,7 @@ DEPT keeps adding each new dependency into the graph at runtime. For example, 'B -> D' will be added when event D occurrence is a prerequisite to reaching event B like: -.. literal:: +.. code-block:: context W @@ -211,7 +211,7 @@ prerequisite to reaching event B like: After the addition, the graph will look like: -.. literal:: +.. code-block:: -> D / @@ -236,7 +236,7 @@ How DEPT works Let's take a look how DEPT works with the 1st example in the section 'Limitation of lockdep'. -.. literal:: +.. code-block:: context X context Y context Z @@ -256,7 +256,7 @@ event. Adding comments to describe DEPT's view in detail: -.. literal:: +.. code-block:: context X context Y context Z @@ -293,7 +293,7 @@ Adding comments to describe DEPT's view in detail: Let's build up dependency graph with this example. Firstly, context X: -.. literal:: +.. code-block:: context X @@ -304,7 +304,7 @@ Let's build up dependency graph with this example. Firstly, context X: There are no events to create dependency. Next, context Y: -.. literal:: +.. code-block:: context Y @@ -332,7 +332,7 @@ event A cannot be triggered if wait B cannot be awakened by event B. Therefore, we can say event A depends on event B, say, 'A -> B'. The graph will look like after adding the dependency: -.. literal:: +.. code-block:: A -> B @@ -340,7 +340,7 @@ graph will look like after adding the dependency: Lastly, context Z: -.. literal:: +.. code-block:: context Z @@ -362,7 +362,7 @@ triggered if wait A cannot be awakened by event A. Therefore, we can say event B depends on event A, say, 'B -> A'. The graph will look like after adding the dependency: -.. literal:: +.. code-block:: -> A -> B - / \ @@ -386,7 +386,7 @@ Interpret DEPT report The following is the same example in the section 'How DEPT works'. -.. literal:: +.. code-block:: context X context Y context Z @@ -425,7 +425,7 @@ We can simplify this by labeling each waiting point with [W], each point where its event's context starts with [S] and each event with [E]. This example will look like after the labeling: -.. literal:: +.. code-block:: context X context Y context Z @@ -443,7 +443,7 @@ DEPT uses the symbols [W], [S] and [E] in its report as described above. The following is an example reported by DEPT for a real problem in practice. -.. literal:: +.. code-block:: Link: https://lore.kernel.org/lkml/6383cde5-cf4b-facf-6e07-1378a485657d@I-love.SA… Link: https://lore.kernel.org/lkml/1674268856-31807-1-git-send-email-byungchul.pa… @@ -646,7 +646,7 @@ practice. Let's take a look at the summary that is the most important part. -.. literal:: +.. code-block:: --------------------------------------------------- summary @@ -669,7 +669,7 @@ Let's take a look at the summary that is the most important part. The summary shows the following scenario: -.. literal:: +.. code-block:: context A context B context ?(unknown) @@ -684,7 +684,7 @@ The summary shows the following scenario: Adding comments to describe DEPT's view in detail: -.. literal:: +.. code-block:: context A context B context ?(unknown) @@ -711,7 +711,7 @@ Adding comments to describe DEPT's view in detail: Let's build up dependency graph with this report. Firstly, context A: -.. literal:: +.. code-block:: context A @@ -735,7 +735,7 @@ unlock(&ni->ni_lock:0) depends on folio_unlock(&f1), say, The graph will look like after adding the dependency: -.. literal:: +.. code-block:: unlock(&ni->ni_lock:0) -> folio_unlock(&f1) @@ -743,7 +743,7 @@ The graph will look like after adding the dependency: Secondly, context B: -.. literal:: +.. code-block:: context B @@ -762,7 +762,7 @@ folio_unlock(&f1) depends on unlock(&ni->ni_lock:0), say, The graph will look like after adding the dependency: -.. literal:: +.. code-block:: -> unlock(&ni->ni_lock:0) -> folio_unlock(&f1) - / \ > +Limitation of lockdep > +--------------------- > + > +Lockdep deals with a deadlock by typical lock e.g. spinlock and mutex, > +that are supposed to be released within the acquisition context. > +However, when it comes to a deadlock by folio lock that is not supposed > +to be released within the acquisition context or other general > +synchronization mechanisms, lockdep doesn't work. > + > +NOTE: In this document, 'context' refers to any type of unique context > +e.g. irq context, normal process context, wq worker context, or so on. > + > +Can lockdep detect the following deadlock? > + > +.. literal:: > + > + context X context Y context Z > + > + mutex_lock A > + folio_lock B > + folio_lock B <- DEADLOCK > + mutex_lock A <- DEADLOCK > + folio_unlock B > + folio_unlock B > + mutex_unlock A > + mutex_unlock A > + > +No. What about the following? > + > +.. literal:: > + > + context X context Y > + > + mutex_lock A > + mutex_lock A <- DEADLOCK > + wait_for_complete B <- DEADLOCK > + complete B > + mutex_unlock A > + mutex_unlock A > + > +No. One unanswered question from my v17 review [1]: You explain in "How DEPT works" section how DEPT detects deadlock in the first example (the former with three contexts). Can you do the same on the second example (the latter with two contexts)? Thanks. [1]: https://lore.kernel.org/linux-doc/aN84jKyrE1BumpLj@archie.me/ -- An old man doll... just what I always wanted! - Clara

2 months, 1 week

1
0
0 0

[PATCH 1/2] dma-buf: improve sg_table debugging hack v3

by Christian König

This debugging hack is important to enforce the rule that importers should *never* touch the underlying struct page of the exporter. Instead of just mangling the page link create a copy of the sg_table but only copy over the DMA addresses and not the pages. This will cause a NULL pointer de-reference if the importer tries to touch the struct page. Still quite a hack but this at least allows the exporter to properly keeps it's sg_table intact while allowing the DMA-buf maintainer to find and fix misbehaving importers and finally switch over to using a different data structure in the future. v2: improve the hack further by using a wrapper structure and explaining the background a bit more in the commit message. v3: fix some whitespace issues, use sg_assign_page(). Signed-off-by: Christian König <christian.koenig(a)amd.com> Reviewed-by: Michael J. Ruhl <michael.j.ruhl(a)intel.com> (v1) --- drivers/dma-buf/dma-buf.c | 74 +++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 2305bb2cc1f1..944f4103b5cc 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -35,6 +35,12 @@ #include "dma-buf-sysfs-stats.h" +/* Wrapper to hide the sg_table page link from the importer */ +struct dma_buf_sg_table_wrapper { + struct sg_table *original; + struct sg_table wrapper; +}; + static inline int is_dma_buf_file(struct file *); static DEFINE_MUTEX(dmabuf_list_mutex); @@ -828,21 +834,59 @@ void dma_buf_put(struct dma_buf *dmabuf) } EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF"); -static void mangle_sg_table(struct sg_table *sg_table) +static int dma_buf_mangle_sg_table(struct sg_table **sg_table) { -#ifdef CONFIG_DMABUF_DEBUG - int i; - struct scatterlist *sg; - - /* To catch abuse of the underlying struct page by importers mix - * up the bits, but take care to preserve the low SG_ bits to - * not corrupt the sgt. The mixing is undone on unmap - * before passing the sgt back to the exporter. + struct scatterlist *to_sg, *from_sg; + struct sg_table *from = *sg_table; + struct dma_buf_sg_table_wrapper *to; + int i, ret; + + if (!IS_ENABLED(CONFIG_DMABUF_DEBUG)) + return 0; + + /* + * To catch abuse of the underlying struct page by importers copy the + * sg_table without copying the page_link and give only the copy back to + * the importer. */ - for_each_sgtable_sg(sg_table, sg, i) - sg->page_link ^= ~0xffUL; -#endif + to = kzalloc(sizeof(*to), GFP_KERNEL); + if (!to) + return -ENOMEM; + + ret = sg_alloc_table(&to->wrapper, from->nents, GFP_KERNEL); + if (ret) + goto free_to; + + to_sg = to->wrapper.sgl; + for_each_sgtable_dma_sg(from, from_sg, i) { + to_sg->offset = 0; + to_sg->length = 0; + sg_assign_page(to_sg, NULL); + sg_dma_address(to_sg) = sg_dma_address(from_sg); + sg_dma_len(to_sg) = sg_dma_len(from_sg); + to_sg = sg_next(to_sg); + } + to->original = from; + *sg_table = &to->wrapper; + return 0; + +free_to: + kfree(to); + return ret; +} + +static void dma_buf_demangle_sg_table(struct sg_table **sg_table) +{ + struct dma_buf_sg_table_wrapper *copy; + + if (!IS_ENABLED(CONFIG_DMABUF_DEBUG)) + return; + + copy = container_of(*sg_table, typeof(*copy), wrapper); + *sg_table = copy->original; + sg_free_table(&copy->wrapper); + kfree(copy); } static inline bool @@ -1139,7 +1183,9 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, if (ret < 0) goto error_unmap; } - mangle_sg_table(sg_table); + ret = dma_buf_mangle_sg_table(&sg_table); + if (ret) + goto error_unmap; if (IS_ENABLED(CONFIG_DMA_API_DEBUG)) { struct scatterlist *sg; @@ -1220,7 +1266,7 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, dma_resv_assert_held(attach->dmabuf->resv); - mangle_sg_table(sg_table); + dma_buf_demangle_sg_table(&sg_table); attach->dmabuf->ops->unmap_dma_buf(attach, sg_table, direction); if (dma_buf_pin_on_map(attach)) -- 2.43.0

2 months, 1 week

2
4
0 0

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig