Re: [PATCH v2] drm/sced: Add FIFO sched policy to rq
Am 03.09.22 um 04:48 schrieb Andrey Grodzovsky:
Poblem: Given many entities competing for same rq on
same scheduler an uncceptabliy long wait time for some
jobs waiting stuck in rq before being picked up are
observed (seen using GPUVis).
The issue is due to Round Robin policy used by scheduler
to pick up the next entity for execution. Under stress
of many entities and long job queus within entity some
jobs could be stack for very long time in it's entity's
queue before being popped from the queue and executed
while for other entites with samller job queues a job
might execute ealier even though that job arrived later
then the job in the long queue.
Fix:
Add FIFO selection policy to entites in RQ, chose next enitity
on rq in such order that if job on one entity arrived
ealrier then job on another entity the first job will start
executing ealier regardless of the length of the entity's job
queue.
v2:
Switch to rb tree structure for entites based on TS of
oldest job waiting in job queue of enitity. Improves next
enitity extraction to O(1). Enitity TS update
O(log(number of entites in rq))
Drop default option in module control parameter.
Signed-off-by: Andrey Grodzovsky
Tested-by: Li Yunxiang (Teddy)
[SNIP]
/**
@@ -313,6 +330,14 @@ struct drm_sched_job {
/** @last_dependency: tracks @dependencies as they signal */
unsigned long last_dependency;
+
+
+ /**
+ * @submit_ts:
+ *
+ * Marks job submit time
Maybe write something like "When the job was pushed into the entity queue."
Apart from that I leave it to Luben and you to get this stuff upstream.
Thanks,
Christian.
+ */
+ ktime_t submit_ts;
};
static inline bool drm_sched_invalidate_job(struct drm_sched_job *s_job,
@@ -501,6 +526,10 @@ void drm_sched_rq_add_entity(struct drm_sched_rq *rq,
void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
struct drm_sched_entity *entity);
+void drm_sched_rq_update_fifo(struct drm_sched_entity *entity, ktime_t ts,
+ bool remove_only);
+
+
int drm_sched_entity_init(struct drm_sched_entity *entity,
enum drm_sched_priority priority,
struct drm_gpu_scheduler **sched_list,
Re: [PATCH v2 1/2] drm/amdgpu: Move HDP remapping earlier during init
On 9/2/2022 1:09 AM, Alex Deucher wrote:
How about this? We should just move HDP remap to gmc hw_init since
that is mainly what uses it anyway.
Sorry, I missed to R-B the previous version. Did you find any problem
when common block is initialized first?
I think that patch provides a consistent IP init sequence during cold
init and resume.
Thanks,
Lijo
Alex
On Tue, Aug 30, 2022 at 2:05 PM Alex Deucher wrote:
On Tue, Aug 30, 2022 at 12:06 PM Lazar, Lijo wrote:
On 8/30/2022 8:39 PM, Alex Deucher wrote:
On Tue, Aug 30, 2022 at 10:45 AM Lazar, Lijo wrote:
On 8/30/2022 7:18 PM, Alex Deucher wrote:
On Tue, Aug 30, 2022 at 12:05 AM Lazar, Lijo wrote:
On 8/29/2022 10:20 PM, Alex Deucher wrote:
On Mon, Aug 29, 2022 at 4:18 AM Lijo Lazar wrote:
HDP flush is used early in the init sequence as part of memory controller
block initialization. Hence remapping of HDP registers needed for flush
needs to happen earlier.
This also fixes the Unsupported Request error reported through AER during
driver load. The error happens as a write happens to the remap offset
before real remapping is done.
Link:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216373data=05%7C01%7Clijo.lazar%40amd.com%7C4e59ae0f8ed54aa7c5a208da8c51aa29%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637976579623485975%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=WTcd9ImY7Oba0MT6oQ7%2B7UEBkdS6azvqgYoK%2B%2F4mJPg%3Dreserved=0
The error was unnoticed before and got visible because of the commit
referenced below. This doesn't fix anything in the commit below, rather
fixes the issue in amdgpu exposed by the commit. The reference is only
to associate this commit with below one so that both go together.
Fixes: 8795e182b02d ("PCI/portdrv: Don't disable AER reporting in
get_port_device_capability()")
Reported-by: Tom Seewald
Signed-off-by: Lijo Lazar
Cc: sta...@vger.kernel.org
How about something like the attached patch rather than these two
patches? It's a bit bigger but seems cleaner and more defensive in my
opinion.
Whenever device goes to suspend/reset and then comes back, remap offset
has to be set back to 0 to make sure it doesn't use the wrong offset
when the register assumes default values again.
To avoid the if-check in hdp_flush (which is more frequent), another way
is to initialize the remap offset to default offset during early init
and hw fini/suspend sequences. It won't be obvious (even with this
patch) as to when remap offset vs default offset is used though.
On resume, the common IP is resumed first so it will always be set.
The only case that is a problem is init because we init GMC out of
order. We could init common before GMC in amdgpu_device_ip_init(). I
think that should be fine, but I wasn't sure if there might be some
fallout from that on certain cards.
There are other places where an IP order is forced like in
amdgpu_device_ip_reinit_early_sriov(). This also may not affect this
case as remapping is not done for VF.
Agree that a better way is to have the common IP to be inited first.
How about these patches?
Looks good to me. BTW, is nbio 7.7 for an APU (in which case hdp flush
is not expected to be used)?
It would be used in some cases, e.g., GPU VM passthrough where we use
the BAR rather than the carve out.
Alex
Thanks,
Lijo
Alex
Thanks,
Lijo
Alex
Thanks,
Lijo
Alex
---
v2:
Take care of IP resume cases (Alex Deucher)
Add NULL check to nbio.funcs to cover older (GFXv8) ASICs (Felix
Kuehling)
Add more details in commit message and associate with AER patch
(Bjorn
Helgaas)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 ++
drivers/gpu/drm/amd/amdgpu/nv.c| 6 --
drivers/gpu/drm/amd/amdgpu/soc15.c | 6 --
drivers/gpu/drm/amd/amdgpu/soc21.c | 6 --
4 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ce7d117efdb5..e420118769a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2334,6 +2334,26 @@ static int amdgpu_device_init_schedulers(struct
amdgpu_device *adev)
return 0;
}
+/**
+ * amdgpu_device_prepare_ip - prepare IPs for hardware initialization
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Any common hardware initialization sequence that needs to be done before
+ * hw init of individual IPs is performed here. This is different from the
+ * 'common block' which initializes a set of IPs.
+ */
+static void amdgpu_device_prepare_ip(struct amdgpu_device *adev)
+{
+ /* Remap HDP registers to a hole in mmio space, for the purpose
+* of exposing those registers to process space. This needs to be
+* done before hw
RE: [PATCH 10/11] drm/amdgpu: add gang submit frontend v4
[AMD Official Use Only - General]
Before we finally add the gang submission frontend, is there any interface/flag
for user mode driver to detect if gang submission is supported by kernel?
Regards,
Trigger
-Original Message-
From: amd-gfx On Behalf Of Christian
K?nig
Sent: 2022年8月29日 21:18
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org
Cc: Koenig, Christian
Subject: [PATCH 10/11] drm/amdgpu: add gang submit frontend v4
Allows submitting jobs as gang which needs to run on multiple engines at the
same time.
All members of the gang get the same implicit, explicit and VM dependencies. So
no gang member will start running until everything else is ready.
The last job is considered the gang leader (usually a submission to the GFX
ring) and used for signaling output dependencies.
Each job is remembered individually as user of a buffer object, so there is no
joining of work at the end.
v2: rebase and fix review comments from Andrey and Yogesh
v3: use READ instead of BOOKKEEP for now because of VM unmaps, set gang
leader only when necessary
v4: fix order of pushing jobs and adding fences found by Trigger.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 259 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.h| 10 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 12 +-
3 files changed, 184 insertions(+), 97 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 9821299dfb49..a6e50ad5e306 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -69,6 +69,7 @@ static int amdgpu_cs_p1_ib(struct amdgpu_cs_parser *p,
unsigned int *num_ibs)
{
struct drm_sched_entity *entity;
+ unsigned int i;
int r;
r = amdgpu_ctx_get_entity(p->ctx, chunk_ib->ip_type, @@ -77,17 +78,28
@@ static int amdgpu_cs_p1_ib(struct amdgpu_cs_parser *p,
if (r)
return r;
- /* Abort if there is no run queue associated with this entity.
-* Possibly because of disabled HW IP*/
+ /*
+* Abort if there is no run queue associated with this entity.
+* Possibly because of disabled HW IP.
+*/
if (entity->rq == NULL)
return -EINVAL;
- /* Currently we don't support submitting to multiple entities */
- if (p->entity && p->entity != entity)
+ /* Check if we can add this IB to some existing job */
+ for (i = 0; i < p->gang_size; ++i) {
+ if (p->entities[i] == entity)
+ goto found;
+ }
+
+ /* If not increase the gang size if possible */
+ if (i == AMDGPU_CS_GANG_SIZE)
return -EINVAL;
- p->entity = entity;
- ++(*num_ibs);
+ p->entities[i] = entity;
+ p->gang_size = i + 1;
+
+found:
+ ++(num_ibs[i]);
return 0;
}
@@ -161,11 +173,12 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
union drm_amdgpu_cs *cs)
{
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
+ unsigned int num_ibs[AMDGPU_CS_GANG_SIZE] = { };
struct amdgpu_vm *vm = >vm;
uint64_t *chunk_array_user;
uint64_t *chunk_array;
- unsigned size, num_ibs = 0;
uint32_t uf_offset = 0;
+ unsigned int size;
int ret;
int i;
@@ -231,7 +244,7 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
if (size < sizeof(struct drm_amdgpu_cs_chunk_ib))
goto free_partial_kdata;
- ret = amdgpu_cs_p1_ib(p, p->chunks[i].kdata, _ibs);
+ ret = amdgpu_cs_p1_ib(p, p->chunks[i].kdata, num_ibs);
if (ret)
goto free_partial_kdata;
break;
@@ -268,21 +281,28 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
}
}
- ret = amdgpu_job_alloc(p->adev, num_ibs, >job, vm);
- if (ret)
- goto free_all_kdata;
+ if (!p->gang_size)
+ return -EINVAL;
- ret = drm_sched_job_init(>job->base, p->entity, >vm);
- if (ret)
- goto free_all_kdata;
+ for (i = 0; i < p->gang_size; ++i) {
+ ret = amdgpu_job_alloc(p->adev, num_ibs[i], >jobs[i], vm);
+ if (ret)
+ goto free_all_kdata;
+
+ ret = drm_sched_job_init(>jobs[i]->base, p->entities[i],
+>vm);
+ if (ret)
+ goto free_all_kdata;
+ }
+ p->gang_leader = p->jobs[p->gang_size - 1];
- if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
+ if (p->ctx->vram_lost_counter != p->gang_leader->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
}
RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp
If not sure whether the discovery table can be fulfilled the cache info in the upcoming NPI ASICs, maybe need the dummy cache info in the default clause case rather than add more and more faking some specific HW configuration and sometimes that may give misleading HW info. Regards, --Prike -Original Message- From: Zhang, Yifan Sent: Monday, September 5, 2022 9:47 AM To: Liang, Prike ; Liu, Aaron ; Huang, Tim ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian ; Huang, Ray ; Kuehling, Felix Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] It is actually a bug of discovery table which needs to be identified in NPI phase. Hopefully we don't need neither dummy nor yellow carp cache info in the future. Best Regards, Yifan -Original Message- From: Liang, Prike Sent: Monday, September 5, 2022 9:24 AM To: Zhang, Yifan ; Liu, Aaron ; Huang, Tim ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian ; Huang, Ray ; Kuehling, Felix Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp How about add a dummy cache info for the NPI product in the default case and notify user that's using the dummy cache configuration to make sure not miss correcting the HW info in the future? Regards, --Prike -Original Message- From: amd-gfx On Behalf Of Zhang, Yifan Sent: Friday, September 2, 2022 10:28 AM To: Liu, Aaron ; Huang, Tim ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] Hi Aaron, Yes, the cache info are different. But this diff is intentional to workaround the discovery table lacks cache info in GC 11.0.1 issue. The workaround will be removed after discovery table finishes integrating cache info. Given they already have a test version, we can pend this patch until they finish integration. Best Regards, Yifan -Original Message- From: Liu, Aaron Sent: Friday, September 2, 2022 9:44 AM To: Huang, Tim ; Zhang, Yifan ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] Hi Yifan, Yellow carp's cache info cannot be duplicated to GC_11_0_1. Different point to GC_11_0_1: TCP L1 Cache size is 32 GL1 Data Cache size per SA is 256 Others looks good to me -- Best Regards Aaron Liu > -Original Message- > From: amd-gfx On Behalf Of > Huang, Tim > Sent: Friday, September 2, 2022 6:44 AM > To: Zhang, Yifan ; amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Du, Xiaojian > > Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow > carp > > [Public] > > [Public] > > Reviewed-by: Tim Huang > > Best Regards, > Tim Huang > > > > -Original Message- > From: Zhang, Yifan > Sent: Thursday, September 1, 2022 3:30 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Huang, Tim > ; Du, Xiaojian ; Zhang, Yifan > > Subject: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp > > Current discovery table doesn't have cache info for GC 11.0.1, thus > can't be parsed like other GC 11, this patch to match GC 11.0.1 cache > info to yellow carp > > Signed-off-by: Yifan Zhang > --- > drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > index 24b414cff3ec..1c500bfb0b28 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > @@ -1516,11 +1516,11 @@ static int kfd_fill_gpu_cache_info(struct > kfd_dev *kdev, > case IP_VERSION(10, 3, 3): > case IP_VERSION(10, 3, 6): /* TODO: Double check these > on production silicon */ > case IP_VERSION(10, 3, 7): /* TODO: Double check these > on production silicon */ > + case IP_VERSION(11, 0, 1): /* TODO: Double check these > +on production silicon */ > pcache_info = yellow_carp_cache_info; > num_of_cache_types = > ARRAY_SIZE(yellow_carp_cache_info); > break; > case IP_VERSION(11, 0, 0): > - case IP_VERSION(11, 0, 1): > case IP_VERSION(11, 0, 2): > case IP_VERSION(11, 0, 3): > pcache_info = cache_info; > -- > 2.37.1
RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp
[Public] It is actually a bug of discovery table which needs to be identified in NPI phase. Hopefully we don't need neither dummy nor yellow carp cache info in the future. Best Regards, Yifan -Original Message- From: Liang, Prike Sent: Monday, September 5, 2022 9:24 AM To: Zhang, Yifan ; Liu, Aaron ; Huang, Tim ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian ; Huang, Ray ; Kuehling, Felix Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp How about add a dummy cache info for the NPI product in the default case and notify user that's using the dummy cache configuration to make sure not miss correcting the HW info in the future? Regards, --Prike -Original Message- From: amd-gfx On Behalf Of Zhang, Yifan Sent: Friday, September 2, 2022 10:28 AM To: Liu, Aaron ; Huang, Tim ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] Hi Aaron, Yes, the cache info are different. But this diff is intentional to workaround the discovery table lacks cache info in GC 11.0.1 issue. The workaround will be removed after discovery table finishes integrating cache info. Given they already have a test version, we can pend this patch until they finish integration. Best Regards, Yifan -Original Message- From: Liu, Aaron Sent: Friday, September 2, 2022 9:44 AM To: Huang, Tim ; Zhang, Yifan ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] Hi Yifan, Yellow carp's cache info cannot be duplicated to GC_11_0_1. Different point to GC_11_0_1: TCP L1 Cache size is 32 GL1 Data Cache size per SA is 256 Others looks good to me -- Best Regards Aaron Liu > -Original Message- > From: amd-gfx On Behalf Of > Huang, Tim > Sent: Friday, September 2, 2022 6:44 AM > To: Zhang, Yifan ; amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Du, Xiaojian > > Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow > carp > > [Public] > > [Public] > > Reviewed-by: Tim Huang > > Best Regards, > Tim Huang > > > > -Original Message- > From: Zhang, Yifan > Sent: Thursday, September 1, 2022 3:30 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Huang, Tim > ; Du, Xiaojian ; Zhang, Yifan > > Subject: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp > > Current discovery table doesn't have cache info for GC 11.0.1, thus > can't be parsed like other GC 11, this patch to match GC 11.0.1 cache > info to yellow carp > > Signed-off-by: Yifan Zhang > --- > drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > index 24b414cff3ec..1c500bfb0b28 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > @@ -1516,11 +1516,11 @@ static int kfd_fill_gpu_cache_info(struct > kfd_dev *kdev, > case IP_VERSION(10, 3, 3): > case IP_VERSION(10, 3, 6): /* TODO: Double check these > on production silicon */ > case IP_VERSION(10, 3, 7): /* TODO: Double check these > on production silicon */ > + case IP_VERSION(11, 0, 1): /* TODO: Double check these > +on production silicon */ > pcache_info = yellow_carp_cache_info; > num_of_cache_types = > ARRAY_SIZE(yellow_carp_cache_info); > break; > case IP_VERSION(11, 0, 0): > - case IP_VERSION(11, 0, 1): > case IP_VERSION(11, 0, 2): > case IP_VERSION(11, 0, 3): > pcache_info = cache_info; > -- > 2.37.1
RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp
How about add a dummy cache info for the NPI product in the default case and notify user that's using the dummy cache configuration to make sure not miss correcting the HW info in the future? Regards, --Prike -Original Message- From: amd-gfx On Behalf Of Zhang, Yifan Sent: Friday, September 2, 2022 10:28 AM To: Liu, Aaron ; Huang, Tim ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] Hi Aaron, Yes, the cache info are different. But this diff is intentional to workaround the discovery table lacks cache info in GC 11.0.1 issue. The workaround will be removed after discovery table finishes integrating cache info. Given they already have a test version, we can pend this patch until they finish integration. Best Regards, Yifan -Original Message- From: Liu, Aaron Sent: Friday, September 2, 2022 9:44 AM To: Huang, Tim ; Zhang, Yifan ; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander ; Du, Xiaojian Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp [Public] Hi Yifan, Yellow carp's cache info cannot be duplicated to GC_11_0_1. Different point to GC_11_0_1: TCP L1 Cache size is 32 GL1 Data Cache size per SA is 256 Others looks good to me -- Best Regards Aaron Liu > -Original Message- > From: amd-gfx On Behalf Of > Huang, Tim > Sent: Friday, September 2, 2022 6:44 AM > To: Zhang, Yifan ; amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Du, Xiaojian > > Subject: RE: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow > carp > > [Public] > > [Public] > > Reviewed-by: Tim Huang > > Best Regards, > Tim Huang > > > > -Original Message- > From: Zhang, Yifan > Sent: Thursday, September 1, 2022 3:30 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander ; Huang, Tim > ; Du, Xiaojian ; Zhang, Yifan > > Subject: [PATCH] drm/amdkfd: Match GC 11.0.1 cache info to yellow carp > > Current discovery table doesn't have cache info for GC 11.0.1, thus > can't be parsed like other GC 11, this patch to match GC 11.0.1 cache > info to yellow carp > > Signed-off-by: Yifan Zhang > --- > drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > index 24b414cff3ec..1c500bfb0b28 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c > @@ -1516,11 +1516,11 @@ static int kfd_fill_gpu_cache_info(struct > kfd_dev *kdev, > case IP_VERSION(10, 3, 3): > case IP_VERSION(10, 3, 6): /* TODO: Double check these > on production silicon */ > case IP_VERSION(10, 3, 7): /* TODO: Double check these > on production silicon */ > + case IP_VERSION(11, 0, 1): /* TODO: Double check these > +on production silicon */ > pcache_info = yellow_carp_cache_info; > num_of_cache_types = > ARRAY_SIZE(yellow_carp_cache_info); > break; > case IP_VERSION(11, 0, 0): > - case IP_VERSION(11, 0, 1): > case IP_VERSION(11, 0, 2): > case IP_VERSION(11, 0, 3): > pcache_info = cache_info; > -- > 2.37.1
[PATCH v6 44/57] dyndbg: extend __ddebug_add_module proto to allow packing sites
In order to actually reclaim useful blocks of memory, we need to
repack the vector of redundant site recs, not just detect the
duplicates. To allow this, extend __ddebug_add_module()s prototype by
adding:
struct _ddebug_site *packed_sites - address of empty "stack"
unsigned int *packed_base - index of Top-of-Stack
This allows dynamic_debug_init() to tell __ddebug_add_module() where
to push the unique site recs it finds while de-duplicating, and to
communicate the new TOS back for the next iteration.
Since we know we are shrinking data, we can overwrite _ddebug_sites[],
for both builtins, and loadable modules, via ddebug_add_module().
For ddebug_add_module(), which is called from kernel/module/main, the
2 args: reuse the module.sites vector, with a 0 offset. This will
allow de-duplication of the local vector.
No de-duplication is done yet, so no use of the stack.
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 506a7e2e59d6..1b57e43e9c31 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1341,7 +1341,8 @@ static void ddebug_attach_module_classes(struct
ddebug_table *dt,
* and add it to the global list.
*/
static int __ddebug_add_module(struct _ddebug_info *di, unsigned int base,
- const char *modname)
+ const char *modname, struct _ddebug_site
*packed_sites,
+ unsigned int *packed_base)
{
struct ddebug_table *dt;
int i;
@@ -1390,7 +1391,8 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
- return __ddebug_add_module(di, 0, modname);
+ unsigned int packed_base = 0;
+ return __ddebug_add_module(di, 0, modname, di->sites, _base);
}
/* helper for ddebug_dyndbg_(boot|module)_param_cb */
@@ -1506,7 +1508,7 @@ static int __init dynamic_debug_init(void)
{
struct _ddebug *iter, *iter_mod_start;
struct _ddebug_site *site, *site_mod_start;
- int ret, i, mod_sites, mod_ct;
+ int ret, i, mod_sites, mod_ct, site_base;
const char *modname;
char *cmdline;
@@ -1550,7 +1552,8 @@ static int __init dynamic_debug_init(void)
di.num_descs = mod_sites;
di.descs = iter_mod_start;
di.sites = site_mod_start;
- ret = __ddebug_add_module(, i - mod_sites, modname);
+ ret = __ddebug_add_module(, i - mod_sites, modname,
+ __start___dyndbg_sites,
_base);
if (ret)
goto out_err;
@@ -1563,11 +1566,13 @@ static int __init dynamic_debug_init(void)
di.num_descs = mod_sites;
di.descs = iter_mod_start;
di.sites = site_mod_start;
- ret = __ddebug_add_module(, i - mod_sites, modname);
+ ret = __ddebug_add_module(, i - mod_sites, modname,
+ __start___dyndbg_sites, _base);
if (ret)
goto out_err;
ddebug_init_success = 1;
+
vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in
__dyndbg section\n",
i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
(int)((i * sizeof(struct _ddebug)) >> 10));
--
2.37.2
[PATCH v6 46/57] dyndbg: drop site-> in add-module, more needed
---
lib/dynamic_debug.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 41c23ec979f4..059212df68f9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1347,7 +1347,7 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
struct ddebug_table *dt;
int i;
- v3pr_info("add-module: %s.%d sites, start: %d\n", modname,
di->num_descs, base);
+ v3pr_info("add-module: %s %d/%d sites, start: %d\n", modname,
di->num_descs, di->num_sites, base);
if (!di->num_descs) {
v3pr_info(" skip %s\n", modname);
return 0;
@@ -1375,19 +1375,20 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
if (di->classes && di->num_classes)
ddebug_attach_module_classes(dt, di->classes, di->num_classes);
+ //BUG_ON(di->num_descs != di->num_sites);
+
for (i = 0; i < di->num_descs; i++) {
- if (di->descs[i].site->_function !=
packed_sites[(*packed_base)]._function)
+ if (di->sites[i]._function !=
packed_sites[(*packed_base)]._function)
+
memcpy((void *) _sites[++(*packed_base)],
- (void *) di->descs[i].site, sizeof(struct
_ddebug_site));
- else
- di->descs[i].site = _sites[(*packed_base)];
+ (void *) >sites[i], sizeof(struct
_ddebug_site));
di->descs[i]._index = i + base;
di->descs[i]._map = *packed_base;
- v3pr_info(" %d %d %s.%s.%d - %d\n", i, *packed_base, modname,
- di->descs[i].site->_function, di->descs[i].lineno,
*packed_base);
+ v3pr_info(" %d %d %s.%s.%d\n", i, *packed_base, modname,
+ packed_sites[*packed_base]._function,
di->descs[i].lineno);
}
mutex_lock(_lock);
list_add_tail(>link, _tables);
--
2.37.2
[PATCH v6 45/57] dyndbg: de-duplicate sites
In __ddebug_add_module(), detect repeated site records (by function
name changes), and push changes onto the stack/vector passed in
from dynamic_debug_init().
For ddebug_add_module(), this transparently de-duplicates the local
sites vector (passed in recently added stack-base, and offset 0).
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 1b57e43e9c31..41c23ec979f4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1347,7 +1347,7 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
struct ddebug_table *dt;
int i;
- v3pr_info("add-module: %s.%d sites\n", modname, di->num_descs);
+ v3pr_info("add-module: %s.%d sites, start: %d\n", modname,
di->num_descs, base);
if (!di->num_descs) {
v3pr_info(" skip %s\n", modname);
return 0;
@@ -1376,11 +1376,19 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
ddebug_attach_module_classes(dt, di->classes, di->num_classes);
for (i = 0; i < di->num_descs; i++) {
- di->descs[i]._index = base + i;
- v3pr_info(" %d %d %s.%s.%d\n", i, base, modname,
- di->descs[i].site->_function, di->descs[i].lineno);
- }
+ if (di->descs[i].site->_function !=
packed_sites[(*packed_base)]._function)
+ memcpy((void *) _sites[++(*packed_base)],
+ (void *) di->descs[i].site, sizeof(struct
_ddebug_site));
+ else
+ di->descs[i].site = _sites[(*packed_base)];
+
+ di->descs[i]._index = i + base;
+ di->descs[i]._map = *packed_base;
+
+ v3pr_info(" %d %d %s.%s.%d - %d\n", i, *packed_base, modname,
+ di->descs[i].site->_function, di->descs[i].lineno,
*packed_base);
+ }
mutex_lock(_lock);
list_add_tail(>link, _tables);
mutex_unlock(_lock);
@@ -1539,7 +1547,7 @@ static int __init dynamic_debug_init(void)
iter = iter_mod_start = __start___dyndbg;
site = site_mod_start = __start___dyndbg_sites;
modname = iter->site->_modname;
- i = mod_sites = mod_ct = 0;
+ i = mod_sites = mod_ct = site_base = 0;
for (; iter < __stop___dyndbg; iter++, site++, i++, mod_sites++) {
--
2.37.2
[PATCH v6 47/57] dyndbg: demote iter->site in _init
---
lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 059212df68f9..65b0a1025ddf 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1547,7 +1547,7 @@ static int __init dynamic_debug_init(void)
}
iter = iter_mod_start = __start___dyndbg;
site = site_mod_start = __start___dyndbg_sites;
- modname = iter->site->_modname;
+ modname = site->_modname;
i = mod_sites = mod_ct = site_base = 0;
for (; iter < __stop___dyndbg; iter++, site++, i++, mod_sites++) {
--
2.37.2
[PATCH v6 38/57] dyndbg: add 2 more trace-events: pr_debug, dev_dbg
ddebug_trace() currently issues a single printk:console event.
Replace that, adding include/trace/events/dyndbg.h, which defines 2
new events:
- dyndbg:prdbg - from trace_prdbg() - if !dev
- dyndbg:devdbg - from trace_devdbg() - if !!dev
This links the legacy pr_debug API to tracefs, via dyndbg, allowing
pr_debug()s etc to add just a little more user-context to the
trace-logs, and then at users option, less syslog.
The 2 new trace_*() calls accept their caller's args respectively,
keeping the available info w/o alteration; we can't improve on
transparency. The args:
1- struct _ddebug *descriptor, giving tracefs all of dyndbg's info.
2- struct device *dev, used by trace_devdbg(), if !!dev
The trace_*() calls need the descriptor arg, the callchain prototypes
above them are extended to provide it.
The existing category param in this callchain is partially redundant;
when descriptor is available, it has the class_id.
dev_dbg(desc, dev...), if dev is true, issues a trace_devdbg(),
otherwise trace_prdbg(). This way we dont consume buffer space
storing nulls. Otherwise the events are equivalent.
Also add include/trace/events/drm.h, to create 2 events for use in
drm_dbg() and drm_devdbg(), and call them. This recapitulates the
changes described above, connecting 3-10K drm.debug callsites to
tracefs.
Signed-off-by: Jim Cromie
---
include/trace/events/dyndbg.h | 74 +++
lib/dynamic_debug.c | 73 +-
2 files changed, 111 insertions(+), 36 deletions(-)
create mode 100644 include/trace/events/dyndbg.h
diff --git a/include/trace/events/dyndbg.h b/include/trace/events/dyndbg.h
new file mode 100644
index ..e19fcb56566c
--- /dev/null
+++ b/include/trace/events/dyndbg.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM dyndbg
+
+#if !defined(_TRACE_DYNDBG_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_DYNDBG_H
+
+#include
+
+/* capture pr_debug() callsite descriptor and message */
+TRACE_EVENT(prdbg,
+ TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
+
+ TP_ARGS(desc, text, len),
+
+ TP_STRUCT__entry(
+ __field(const struct _ddebug *, desc)
+ __dynamic_array(char, msg, len + 1)
+ ),
+
+ TP_fast_assign(
+ __entry->desc = desc;
+ /*
+* Each trace entry is printed in a new line.
+* If the msg finishes with '\n', cut it off
+* to avoid blank lines in the trace.
+*/
+ if (len > 0 && (text[len - 1] == '\n'))
+ len -= 1;
+
+ memcpy(__get_str(msg), text, len);
+ __get_str(msg)[len] = 0;
+ ),
+
+ TP_printk("%s.%s %s", __entry->desc->modname,
+ __entry->desc->function, __get_str(msg))
+);
+
+/* capture dev_dbg() callsite descriptor, device, and message */
+TRACE_EVENT(devdbg,
+ TP_PROTO(const struct _ddebug *desc, const struct device *dev,
+const char *text, size_t len),
+
+ TP_ARGS(desc, dev, text, len),
+
+ TP_STRUCT__entry(
+ __field(const struct _ddebug *, desc)
+ __field(const struct device *, dev)
+ __dynamic_array(char, msg, len + 1)
+ ),
+
+ TP_fast_assign(
+ __entry->desc = desc;
+ __entry->dev = (struct device *) dev;
+ /*
+* Each trace entry is printed in a new line.
+* If the msg finishes with '\n', cut it off
+* to avoid blank lines in the trace.
+*/
+ if (len > 0 && (text[len - 1] == '\n'))
+ len -= 1;
+
+ memcpy(__get_str(msg), text, len);
+ __get_str(msg)[len] = 0;
+ ),
+
+ TP_printk("%s.%s %s", __entry->desc->modname,
+ __entry->desc->function, __get_str(msg))
+);
+
+#endif /* _TRACE_DYNDBG_H */
+
+/* This part must be outside protection */
+#include
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3c6c18f13889..0870e939f255 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -36,7 +36,9 @@
#include
#include
#include
-#include
+
+#define CREATE_TRACE_POINTS
+#include
#include
@@ -874,7 +876,9 @@ struct ddebug_trace_bufs {
static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
static DEFINE_PER_CPU(int, ddebug_trace_reserve);
-static void ddebug_trace(const char *fmt, va_list args)
+__printf(3, 0)
+static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
+const char *fmt, va_list args)
{
struct ddebug_trace_buf *buf;
[PATCH v6 49/57] dyndbg: add structs _ddebug_hdr, _ddebug_site_hdr
Add new structs _ddebug_hdr and _ddebug_site_hdr, latter for symmetry.
The job of the struct _ddebug_hdr is to:
Contain an _uplink member, which points up to the _ddebug_info
record that keeps builtin and module's dyndbg state. That record is
init'd (by dynamic_debug_init for builtins, or find_module_sections
for loadables).
Be declared in .gnu.linkonce.dyndbg section, which places it just
before the __dyndbg section; a fixed relative offset.
This allows _ddebug_map_site(desc) to cheaply compute
[desc._index], [0], then container_of() to get the header
and its ._uplink, to the _ddebug_info, which gives access to
->sites[desc->_map].
Structurally, the header has a single unnamed union (which exposes its
field names upward) containing an embedded struct _ddebug{,_site}
(this forces the size to be compatible with the array/section its
destined for), shared with our new struct _ddebug_info * _uplink
member, which is also inside an unnamed struct, to allow adding other
fields later to use the remaining space.
In dynamic_debug.c: ref the linker symbols for the headers, print them
to confirm expected linkages; things look proper. Add a header ref to
struct _ddebug_info, to facilitate the validity checks.
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 23 ++-
lib/dynamic_debug.c | 9 +
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 268e903b7c4e..f23608c38a79 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -126,8 +126,29 @@ struct ddebug_class_map {
#define NUM_TYPE_ARGS(eltype, ...) \
(sizeof((eltype[]){__VA_ARGS__}) / sizeof(eltype))
-/* encapsulate linker provided built-in (or module) dyndbg data */
+/* define header record, linker inserts it at descs[0] */
+struct _ddebug_hdr {
+ union {
+ struct _ddebug __; /* force sizeof this */
+ struct {
+ struct _ddebug_info * _uplink;
+ /* space available */
+ };
+ };
+};
+/* here for symmetry, extra storage */
+struct _ddebug_site_hdr {
+ union {
+ struct _ddebug_site __; /* force sizeof this */
+ struct {
+ struct _ddebug_info * _uplink;
+ };
+ };
+};
+
+/* encapsulate linker provided built-in (or module) dyndbg vectors */
struct _ddebug_info {
+ struct _ddebug_hdr *hdr;
struct _ddebug *descs;
struct _ddebug_site *sites;
struct ddebug_class_map *classes;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 65b0a1025ddf..91fe7fb5dda9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -49,6 +49,9 @@ extern struct _ddebug_site __stop___dyndbg_sites[];
extern struct ddebug_class_map __start___dyndbg_classes[];
extern struct ddebug_class_map __stop___dyndbg_classes[];
+extern struct _ddebug_hdr __dyndbg_header[];
+extern struct _ddebug_site_hdr __dyndbg_site_header[];
+
struct ddebug_table {
struct list_head link, maps;
const char *mod_name;
@@ -1522,6 +1525,7 @@ static int __init dynamic_debug_init(void)
char *cmdline;
struct _ddebug_info di = {
+ .hdr = __dyndbg_header,
.descs = __start___dyndbg,
.sites = __start___dyndbg_sites,
.classes = __start___dyndbg_classes,
@@ -1545,6 +1549,11 @@ static int __init dynamic_debug_init(void)
pr_err("unequal vectors: descs/sites %d/%d\n", di.num_descs,
di.num_sites);
return 1;
}
+
+ /* these 2 print the same, until _TABLE is added */
+ v2pr_info("%px %px \n", __dyndbg_header, __dyndbg_site_header);
+ v2pr_info("%px %px \n", di.descs, di.sites);
+
iter = iter_mod_start = __start___dyndbg;
site = site_mod_start = __start___dyndbg_sites;
modname = site->_modname;
--
2.37.2
[PATCH v6 34/57] dyndbg: add _DPRINTK_FLAGS_ENABLED
Distinguish the condition: _DPRINTK_FLAGS_ENABLED from the bit:
_DPRINTK_FLAGS_PRINT, and re-define former in terms of latter, in
preparation to add a 2nd bit: _DPRINTK_FLAGS_TRACE
Update JUMP_LABEL code block to check _DPRINTK_FLAGS_ENABLED symbol.
Also add a 'K' to get new symbol _DPRINTK_FLAGS_PRINTK, in order to
break any stale uses.
CC: vincent.whitchu...@axis.com
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 10 ++
lib/dynamic_debug.c | 8
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 41682278d2e8..ecd3379090c4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,7 +32,7 @@ struct _ddebug {
* writes commands to /dynamic_debug/control
*/
#define _DPRINTK_FLAGS_NONE0
-#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message using the format */
+#define _DPRINTK_FLAGS_PRINTK (1 << 0) /* printk() a message using the format
*/
#define _DPRINTK_FLAGS_INCL_MODNAME(1<<1)
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
@@ -42,8 +42,10 @@ struct _ddebug {
(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
_DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID)
+#define _DPRINTK_FLAGS_ENABLED _DPRINTK_FLAGS_PRINTK
+
#if defined DEBUG
-#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
+#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINTK
#else
#define _DPRINTK_FLAGS_DEFAULT 0
#endif
@@ -198,10 +200,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#ifdef DEBUG
#define DYNAMIC_DEBUG_BRANCH(descriptor) \
- likely(descriptor.flags & _DPRINTK_FLAGS_PRINT)
+ likely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
#else
#define DYNAMIC_DEBUG_BRANCH(descriptor) \
- unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)
+ unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
#endif
#endif /* CONFIG_JUMP_LABEL */
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 009f2ead09c1..1e9d0124a0e9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -89,7 +89,7 @@ static inline const char *trim_prefix(const char *path)
}
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
- { _DPRINTK_FLAGS_PRINT, 'p' },
+ { _DPRINTK_FLAGS_PRINTK, 'p' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
@@ -246,10 +246,10 @@ static int ddebug_change(const struct ddebug_query *query,
if (newflags == dp->flags)
continue;
#ifdef CONFIG_JUMP_LABEL
- if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(newflags & _DPRINTK_FLAGS_PRINT))
+ if (dp->flags & _DPRINTK_FLAGS_ENABLED) {
+ if (!(newflags & _DPRINTK_FLAGS_ENABLED))
static_branch_disable(>key.dd_key_true);
- } else if (newflags & _DPRINTK_FLAGS_PRINT) {
+ } else if (newflags & _DPRINTK_FLAGS_ENABLED) {
static_branch_enable(>key.dd_key_true);
}
#endif
--
2.37.2
[PATCH v6 53/57] dyndbg: add/use is_dyndbg_header then set _uplink
Add static int is_dyndbg_header(d), which verifies that the arg is
initialized as expected; that it points to the _ddebug_hdr &
_ddebug_site_hdr records initialized by DYNAMIC_DEBUG_TABLE().
That init macro sets the _uplink fields in the 2 records to point at
each other. This is an impossible situation for the regular callsite
record pairs built by *_METADATA_CLS(), so it provides a robust
verification that linkage happened as we require/depend upon.
In dynamic_debug_init(), is_dyndbg_header() validates the header, and
sets _uplink to builtin_state.
Thereafter, ddebug_map_site() can use it, and we can drop the
_ddebug.site member, and shrink the DATA footprint.
---
include/linux/dynamic_debug.h | 1 +
lib/dynamic_debug.c | 38 +--
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 23d3d2882882..ed3e1e1c08eb 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -149,6 +149,7 @@ struct _ddebug_site_hdr {
/* encapsulate linker provided built-in (or module) dyndbg vectors */
struct _ddebug_info {
struct _ddebug_hdr *hdr;
+ struct _ddebug_site_hdr *site_hdr;
struct _ddebug *descs;
struct _ddebug_site *sites;
struct ddebug_class_map *classes;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 891d70d7fed4..0a68fbfd8432 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1339,6 +1339,25 @@ static void ddebug_attach_module_classes(struct
ddebug_table *dt,
vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
}
+/*
+ * detect the hardwired loopback initialized into the header pairs'
+ * _uplink member. In dynamic_debug_init(), it verifies the presence
+ * of the header, before setting its _uplink to either _state
+ * or the module's embedded _ddebug_info. __ddebug_add_module() will
+ * also use it..
+ */
+static int is_dyndbg_header(struct _ddebug_hdr *hdr)
+{
+ struct _ddebug_site_hdr *sp;
+
+ if (!hdr || !hdr->_uplink)
+ return 0;
+
+ sp = (struct _ddebug_site_hdr *) ((struct _ddebug_hdr *)hdr)->_uplink;
+ return hdr == (struct _ddebug_hdr *)
+ ((struct _ddebug_site_hdr *)sp)->_uplink;
+}
+
/*
* Allocate a new ddebug_table for the given module
* and add it to the global list.
@@ -1351,6 +1370,15 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
int i, num_funcs = 0;
v3pr_info("add-module: %s %d/%d sites, start: %d\n", modname,
di->num_descs, di->num_sites, base);
+
+ if (is_dyndbg_header((struct _ddebug_hdr *)>descs[0])) {
+ pr_info("module header\n");
+ di->hdr = (struct _ddebug_hdr *) di->descs;
+ di->descs++;
+ di->sites++;
+ di->num_descs--;
+ di->num_sites--;
+ }
if (!di->num_descs) {
v3pr_info(" skip %s\n", modname);
return 0;
@@ -1525,6 +1553,7 @@ static int __init dynamic_debug_init(void)
struct _ddebug_info di = {
.hdr = __dyndbg_header,
+ .site_hdr = __dyndbg_site_header,
.descs = __start___dyndbg,
.sites = __start___dyndbg_sites,
.classes = __start___dyndbg_classes,
@@ -1548,11 +1577,16 @@ static int __init dynamic_debug_init(void)
pr_err("unequal vectors: descs/sites %d/%d\n", di.num_descs,
di.num_sites);
return 1;
}
-
/* these 2 print the same, until _TABLE is added */
- v2pr_info("%px %px \n", __dyndbg_header, __dyndbg_site_header);
+ v2pr_info("%px %px \n", di.hdr, __dyndbg_site_header);
v2pr_info("%px %px \n", di.descs, di.sites);
+ if (is_dyndbg_header(di.hdr)) {
+ di.hdr->_uplink = _state;
+ } else {
+ pr_err("missing header records: cannot continue!\n");
+ return 1;
+ }
iter = iter_mod_start = __start___dyndbg;
site = site_mod_start = __start___dyndbg_sites;
modname = site->_modname;
--
2.37.2
[PATCH v6 56/57] dyndbg: work ddebug_map_site
attempt container-of, broken, missing use of uplink... took that out.
ptr computations are naive, and wrong.
Deeper prob is lack of _ddebug_vec, with header and descs in same struct.
maybe.
builtin_state looks right in debugger
---
lib/dynamic_debug.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 194367bc13fb..11fea1f818a7 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -173,8 +173,25 @@ static struct ddebug_class_map
*ddebug_find_valid_class(struct ddebug_table cons
return NULL;
}
-static inline struct _ddebug_site * _ddebug_map_site(const struct _ddebug
*desc)
+/*
+ * mapping from desc to site is multi-step:
+ * - _index back to [0]
+ * - container-of to get header struct above
+ * - ._uplink field, pointing to _ddebug_info (for builtins, loadables)
+ * - di->sites[desc->_map]
+ */
+static struct _ddebug_site * _ddebug_map_site(const struct _ddebug *desc)
{
+ struct _ddebug_info *di;
+ struct _ddebug const *d0 = desc - desc->_index * sizeof(struct _ddebug);
+
+ di = (struct _ddebug_info *) d0;
+
+ v3pr_info("map_site idx:%d map:%d %s.%s di:%px site:%px ds:%px\n",
+ desc->_index, desc->_map,
+ desc->site->_modname, desc->site->_function,
+ di, desc->site, >sites[desc->_map]);
+
return desc->site;
}
#define _desc_field(desc, _fld)(desc ? (_ddebug_map_site(desc)->_fld)
: "_na_")
@@ -866,7 +883,7 @@ static char *__dynamic_emit_prefix(const struct _ddebug
*desc, char *buf)
return buf;
}
-static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
+char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
{
if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY))
return __dynamic_emit_prefix(desc, buf);
--
2.37.2
[PATCH v6 39/57] dyndbg/drm: POC add tracebits sysfs-knob
clone DRM.debug interface to DRM.tracebits: ie map bits to
drm-debug-categories, except this interface enables messages to
tracefs, not to syslog.
1- we reuse the class-map added previously.
this reflects the single source of both syslog/trace events
2- add a 2nd struct ddebug_classes_bitmap_param
refs 1, reusing it.
flags = "T", to enable trace-events on this callsite.
3- module_param_cb([2]) - does the sysfs part
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/drm_print.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index c50edbf443d3..75d0cecd7e86 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -45,6 +45,9 @@
unsigned long __drm_debug;
EXPORT_SYMBOL(__drm_debug);
+unsigned long __drm_trace;
+EXPORT_SYMBOL(__drm_trace);
+
MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug
category.\n"
"\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n"
"\t\tBit 1 (0x02) will enable DRIVER messages (drm controller code)\n"
@@ -77,6 +80,13 @@ static struct ddebug_class_param drm_debug_bitmap = {
.map = _debug_classes,
};
module_param_cb(debug, _ops_dyndbg_classes, _debug_bitmap, 0600);
+
+static struct ddebug_class_param drm_trace_bitmap = {
+ .bits = &__drm_trace,
+ .flags = "T",
+ .map = _debug_classes,
+};
+module_param_cb(tracecats, _ops_dyndbg_classes, _trace_bitmap, 0600);
#endif
void __drm_puts_coredump(struct drm_printer *p, const char *str)
--
2.37.2
[PATCH v6 50/57] dyndbg: count unique callsites
Count de-duplicated callsites. Since the _ddebug_site excludes
lineno, all callsites in a function are identical, and this
effectively counts functions in the module with callsites.
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 91fe7fb5dda9..891d70d7fed4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1348,7 +1348,7 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
unsigned int *packed_base)
{
struct ddebug_table *dt;
- int i;
+ int i, num_funcs = 0;
v3pr_info("add-module: %s %d/%d sites, start: %d\n", modname,
di->num_descs, di->num_sites, base);
if (!di->num_descs) {
@@ -1381,12 +1381,11 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
//BUG_ON(di->num_descs != di->num_sites);
for (i = 0; i < di->num_descs; i++) {
-
- if (di->sites[i]._function !=
packed_sites[(*packed_base)]._function)
-
+ if (di->sites[i]._function !=
packed_sites[(*packed_base)]._function) {
+ num_funcs++;
memcpy((void *) _sites[++(*packed_base)],
(void *) >sites[i], sizeof(struct
_ddebug_site));
-
+ }
di->descs[i]._index = i + base;
di->descs[i]._map = *packed_base;
@@ -1397,7 +1396,7 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
list_add_tail(>link, _tables);
mutex_unlock(_lock);
- vpr_info("%3u debug prints in module %s\n", di->num_descs, modname);
+ vpr_info("%3u debug prints in %d functions, in module %s\n",
di->num_descs, num_funcs, modname);
return 0;
}
--
2.37.2
[PATCH v6 36/57] dyndbg: add write-events-to-tracefs code
adds: ddebug_trace()
uses trace_console() temporarily to issue printk:console event
uses internal-ish __ftrace_trace_stack code:
4-context buffer stack, barriers per Steve Rostedt
call it from new funcs:
ddebug_printk() - print to both syslog/tracefs
ddebug_dev_printk() - dev-print to both syslog/tracefs
These handle both _DPRINTK_FLAGS_PRINTK and _DPRINTK_FLAGS_TRACE
cases, allowing to vsnprintf the message once and use it for both,
skipping past the KERN_DEBUG character for tracing.
Finally, adjust the callers: __ddebug_{pr_debug,{,net,ib}dev_dbg},
replacing printk and dev_printk with the new funcs above.
The _DPRINTK_FLAGS_TRACE flag character is 'T', so the following finds
all callsites enabled for tracing:
grep -P =p?T /proc/dynamic_debug/control
This patch,~1,~2 are basically copies of:
https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchu...@axis.com/
with a few differences:
- s/dynamic_/ddebug_/ on Vincent's additions
- __printf attrs on the _printk funcs
- reuses trace_console() event, not adding a new "printk:dyndbg" event.
next patch replaces this with 2 new events
CC: vincent.whitchu...@axis.com
Signed-off-by: Jim Cromie
---
.../admin-guide/dynamic-debug-howto.rst | 5 +-
lib/dynamic_debug.c | 156 +++---
2 files changed, 133 insertions(+), 28 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst
b/Documentation/admin-guide/dynamic-debug-howto.rst
index faa22f77847a..45b6e5697c89 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -209,8 +209,9 @@ of the characters::
The flags are::
- penables the pr_debug() callsite.
- _enables no flags.
+ pcallsite prints to syslog
+ Tcallsite issues a dyndbg:* trace-event
+ _enables no flags
Decorator flags add to the message-prefix, in order:
tInclude thread ID, or
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 1e9d0124a0e9..3c6c18f13889 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include
@@ -90,6 +91,7 @@ static inline const char *trim_prefix(const char *path)
static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINTK, 'p' },
+ { _DPRINTK_FLAGS_TRACE, 'T' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
@@ -854,6 +856,98 @@ static inline char *dynamic_emit_prefix(struct _ddebug
*desc, char *buf)
return buf;
}
+/*
+ * This code is heavily based on __ftrace_trace_stack().
+ *
+ * Allow 4 levels of nesting: normal, softirq, irq, NMI.
+ */
+#define DYNAMIC_TRACE_NESTING 4
+
+struct ddebug_trace_buf {
+ char buf[256];
+};
+
+struct ddebug_trace_bufs {
+ struct ddebug_trace_buf bufs[DYNAMIC_TRACE_NESTING];
+};
+
+static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
+static DEFINE_PER_CPU(int, ddebug_trace_reserve);
+
+static void ddebug_trace(const char *fmt, va_list args)
+{
+ struct ddebug_trace_buf *buf;
+ int bufidx;
+ int len;
+
+ preempt_disable_notrace();
+
+ bufidx = __this_cpu_inc_return(ddebug_trace_reserve) - 1;
+
+ if (WARN_ON_ONCE(bufidx > DYNAMIC_TRACE_NESTING))
+ goto out;
+
+ /* For the same reasons as in __ftrace_trace_stack(). */
+ barrier();
+
+ buf = this_cpu_ptr(ddebug_trace_bufs.bufs) + bufidx;
+
+ len = vscnprintf(buf->buf, sizeof(buf->buf), fmt, args);
+ trace_console(buf->buf, len);
+
+out:
+ /* As above. */
+ barrier();
+ __this_cpu_dec(ddebug_trace_reserve);
+ preempt_enable_notrace();
+}
+
+__printf(2, 3)
+static void ddebug_printk(unsigned int flags, const char *fmt, ...)
+{
+ if (flags & _DPRINTK_FLAGS_TRACE) {
+ va_list args;
+
+ va_start(args, fmt);
+ /*
+* All callers include the KERN_DEBUG prefix to keep the
+* vprintk case simple; strip it out for tracing.
+*/
+ ddebug_trace(fmt + strlen(KERN_DEBUG), args);
+ va_end(args);
+ }
+
+ if (flags & _DPRINTK_FLAGS_PRINTK) {
+ va_list args;
+
+ va_start(args, fmt);
+ vprintk(fmt, args);
+ va_end(args);
+ }
+}
+
+__printf(3, 4)
+static void ddebug_dev_printk(unsigned int flags, const struct device *dev,
+ const char *fmt, ...)
+{
+
+ if (flags & _DPRINTK_FLAGS_TRACE) {
+ va_list args;
+
+ va_start(args, fmt);
+ ddebug_trace(fmt, args);
+ va_end(args);
+ }
+
+ if (flags & _DPRINTK_FLAGS_PRINTK) {
+ va_list args;
+
+ va_start(args, fmt);
+
[PATCH v6 54/57] dyndbg: add .gnu.linkonce. & __dyndbg* sections in module.lds.h
For a long time now, loadable modules have tacitly linked the __dyndbg
section into the .ko, as is observable in dmesg by enabling module.c's
pr_debugs and loading a module. Recently, __dyndbg_sites was added,
following the original model.
But now, we need to explicitly name those (__dyndbg, __dyndbg_sites)
sections in order to place new .gnu.linkonce.dyndbg* sections in front
of them.
This gives us the properties we need for _ddebug_map_site() to drop
the _ddebug.site pointer:
fixed offset from &__dyndbg[N] to &__dyndbg[0]
container_of gets
header has ptr to __dyndbg_sites[]
__dyndbg_sites[_map] gives de-duplicated site recs
NOTE
HEAD~1 took headers off front of descs,sites and saved them into
_ddebug_info, this puts the gnu.linkonce.* records into those vectors.
Signed-off-by: Jim Cromie
---
include/asm-generic/module.lds.h | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/asm-generic/module.lds.h b/include/asm-generic/module.lds.h
index f210d5c1b78b..328c48dfc88c 100644
--- a/include/asm-generic/module.lds.h
+++ b/include/asm-generic/module.lds.h
@@ -4,7 +4,17 @@
/*
* can specify arch-specific sections for linking modules.
- * Empty for the asm-generic header.
+ *
+ * For loadable modules with CONFIG_DYNAMIC_DEBUG, we need to keep the
+ * 2 __dyndbg* ELF sections, which are loaded by module.c
+ *
+ * Pack the 2 __dyndbg* input sections with their respective
+ * .gnu.linkonce. header records into 2 output sections, with those
+ * header records in the 0th element.
*/
+SECTIONS {
+__dyndbg_sites : ALIGN(8) { *(.gnu.linkonce.dyndbg_site) *(__dyndbg_sites) }
+__dyndbg : ALIGN(8) { *(.gnu.linkonce.dyndbg) *(__dyndbg) }
+}
#endif /* __ASM_GENERIC_MODULE_LDS_H */
--
2.37.2
[PATCH v6 55/57] dyndbg: dynamic_debug_sites_reclaim() using free_reserved_page() WAG
STATUS:
broken with some over-free, punt and comment out late_initcall
Implement dymamic_debug_sites_reclaim(as a late_initcall) to free the
tail~47% of the builtin __dyndbg_sites[] vector back to the buddy
allocator. Code is copied from mm/init.c:free_initmem(), wo the
poisoning. Comments cargo culted wo grokking.
v3pr_info:
dyndbg: freeing range: ea0c5500-ea0c5700,
83154668-8315c480
dyndbg: ie range: ea0c5500-ea0c5700,
83154000-8315c000
dyndbg: freeing page: ea0c5540, 83155000
dyndbg: freeing page: ea0c5580, 83156000
dyndbg: freeing page: ea0c55c0, 83157000
dyndbg: freeing page: ea0c5600, 83158000
dyndbg: freeing page: ea0c5640, 83159000
dyndbg: freeing page: ea0c5680, 8315a000
dyndbg: freeing page: ea0c56c0, 8315b000
Presuming those are 4k pages, 28kb is reclaimed, which is reasonably
consistent with the numbers below; 1532/2761 compresson of 64KiB.
I have fixed one edge-case over-free, which gave me a coredump on
```cat control```, there may be (one) other.
dyndbg: 2762 prdebugs in 235 modules, 11 KiB in ddebug tables, 86 KiB \
in __dyndbg section, 64 KiB in __dyndbg_sites section
CC: da...@redhat.com# free_reserved_page() tip
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 45 ++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0a68fbfd8432..194367bc13fb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1542,6 +1542,7 @@ static int __init dynamic_debug_init_control(void)
fs_initcall(dynamic_debug_init_control);
static struct _ddebug_info builtin_state;
+static __initdata struct _ddebug_site *last_site;
static int __init dynamic_debug_init(void)
{
@@ -1622,11 +1623,12 @@ static int __init dynamic_debug_init(void)
if (ret)
goto out_err;
+ last_site = &__start___dyndbg_sites[site_base];
ddebug_init_success = 1;
- vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in
__dyndbg section\n",
-i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
-(int)((i * sizeof(struct _ddebug)) >> 10));
+ vpr_info("%d prdebugs in %d functions, %d modules, %d KiB in ddebug
tables, %d,%d kiB in __dyndbg,_sites sections\n",
+i, site_base, mod_ct, (int)((mod_ct * sizeof(struct
ddebug_table)) >> 10),
+(int)((i * sizeof(struct _ddebug)) >> 10), (int)((site_base *
sizeof(struct _ddebug_site)) >> 10));
if (di.num_classes)
v2pr_info(" %d builtin ddebug class-maps\n", di.num_classes);
@@ -1652,3 +1654,40 @@ static int __init dynamic_debug_init(void)
/* Allow early initialization for boot messages via boot param */
early_initcall(dynamic_debug_init);
+static int __init dynamic_debug_sites_reclaim(void)
+{
+ unsigned long addr, end, start;
+ /*
+* from mm/init.c:free_initmem (void) wo poisoning
+* The init section is aligned to 8k in vmlinux.lds.
+* Page align for >8k pagesizes.
+*/
+ start = (unsigned long)__start___dyndbg_sites;
+ end = (unsigned long)__stop___dyndbg_sites;
+ addr = (unsigned long)last_site;
+
+ vpr_info("fullrange: %px-%px, %lx-%lx\n",
+virt_to_page(start), virt_to_page(end), start, end);
+
+ vpr_info("freeing range: %px-%px, %lx-%lx\n",
+virt_to_page(addr), virt_to_page(end), addr, end);
+
+ addr &= PAGE_MASK;
+ addr += PAGE_SIZE;
+ end &= PAGE_MASK;
+ end += PAGE_SIZE;
+
+ vpr_info("ie range: %px-%px, %lx-%lx\n",
+virt_to_page(addr), virt_to_page(end), addr, end);
+
+ if (verbose < 2) {
+ vpr_info(" skipping reclaim cuz its broken by `cat control`\n");
+ return 0;
+ }
+ for (; addr < end; addr += PAGE_SIZE) {
+ vpr_info("freeing page: %px, %lx\n", virt_to_page(addr), addr);
+ free_reserved_page(virt_to_page(addr));
+ }
+ return 0;
+}
+//late_initcall(dynamic_debug_sites_reclaim);
--
2.37.2
[PATCH v6 48/57] dyndbg: add .gnu.linkonce slot in vmlinux.lds.h KEEPs
Add linker symbols and KEEPs for .gnu.linkonce.dyndbg and .gnu.linkonce.dyndbg_sites sections, placing them in front of their respective dyndbg and dyndbg_sites sections. This placement gives us a known relative offset (ie -1) from the start of the vector to the header, letting us use container_of to get it. The _index added previously allows determining [0] from any desc[N]. The .gnu.linkonce. collapses possible multiple declarations into a single allocation, with a single address. todo: will need similar for modules.lds.h Signed-off-by: Jim Cromie --- include/asm-generic/vmlinux.lds.h | 4 1 file changed, 4 insertions(+) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 1e7ee65e8591..20fdea9efd78 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -348,9 +348,13 @@ __start___dyndbg_classes = .; \ KEEP(*(__dyndbg_classes)) \ __stop___dyndbg_classes = .;\ + __dyndbg_header = .;\ + KEEP(*(.gnu.linkonce.dyndbg)) \ __start___dyndbg = .; \ KEEP(*(__dyndbg)) \ __stop___dyndbg = .;\ + __dyndbg_site_header = .; \ + KEEP(*(.gnu.linkonce.dyndbg_sites)) \ __start___dyndbg_sites = .; \ KEEP(*(__dyndbg_sites)) \ __stop___dyndbg_sites = .; \ -- 2.37.2
[PATCH v6 41/57] dyndbg: split repeating columns to new struct _ddebug_site
Struct _ddebug has 3 RO-data fields: _module, _file, _function, which
have significant repetition in the builtins: 4222 unique records /
8920 callsites on a recent laptop build. Thats just 47% unique, on
24/56 of the unitary record.
The quick path to excising this redundancy is:
1- split the table in 2, link 1st to 2nd (done here)
2- de-duplicate the 2nd table. (soon)
So split struct _ddebug, move the 3 fields to a new struct
_ddebug_site, and add a pointer from _ddebug to _debug_sites. The
lineno field stays in _ddebug, so all _sites in a fn are identical.
The new ptr from _ddebug -> _ddebug_site increases memory footprint,
until step 2 is completed, at which point:
old = 56* 8920
new = (56-24+8) * 8920 + 24 * 4222
IE:
DB<2> p 56 * 8920
499520
DB<3> p 40*8920 + 24 * 4222
458128
Thats 41392 saved, or ~8.3%
Further, the site pointer is just temporary scaffolding:
- descriptors are in a vector.
- new desc._idx field (from spare bits) can get us to 0.
set during _init, not by linker
- add a header record in front of vector (.gnu.linkonce.dyndbg*)
point it up to struct dyndbg_info
- dyndbg_info has .sites
- same desc._idx gets us sites[._idx]
- new desc._map field, gives sites[._map]
this allows de-duplication and packing.
Once that is done, the savings increases:
DB<7> p (56 * 8920) - (((56-24) *8920) + 24*4222)
112752 saved, or 22%
STEP 1:
dynamic_debug.h:
This cuts struct _ddebug in half, renames the top-half to
_ddebug_site, keeps __align(8) for both halves. Adds a forward decl
for a unified comment for both halves, and added _ddebug.site field to
point at a site record.
Rework DEFINE_DYNAMIC_DEBUG_METADATA_CLS macro to declare & initialize
the 2 static/private struct vars together, and link them together. It
places each struct into its own section, so the linker packs 2
parallel arrays, and links them like a ladder.
struct _ddebug_info is extended to track _ddebug_site[] just like it
does for _ddebug[] and _ddebug_classes[].
The accessor macros desc_{module,filename,function} follow the
field-moves with added '->site->' references, and return "_nope_" or
"_na_" if the desc or site are null. This makes those ptrs nullable,
and their referents recoverable (nothing tries to use this yet).
NB: the "_na_" is undone temporarily later, for dev shortcut.
Also add const to lineno field. It is set by compiler.
In struct ddebug_table, add struct _ddebug_site *sites, to treat new
vector just like the module's _ddebug[]s (its __dyndbg section, for
loadable mods). While we don't need it now, we will need it to
de-scaffold (drop the _ddebug.site).
dynamic_debug.c:
extern declarations of the section start/end symbols named and
initialized in vmlinux.lds.h
dynamic_debug_init():
Initialize global builtin_state from initialized cursor var. Trying
to do so statically gave:
"error: initializer element is not computable at load time"
Check (num-descs == num-sites), and quit early otherwise. This is an
important precondition, w/o it, we cannot really continue confidently.
I inadvertently created the situation by having __used on 1/2 of the
_ddebug{,_site} pair created by DECLARE_DYNAMIC_DEBUG_METADATA; this
created ~70/ extra site records. This "worked", but was unnerving
until I tracked it down.
Add site iterator & site_mod_start marker, recapping iter/_mod_start.
Inside the main loop, validate (site == iter->site). This is the
full/proper precondition for the expected section contents and
inter-linkage; the (num-descs == num-sites) check is just a quick
necessary-but-not-sufficient version of this.
NOTE: this check could be a BUG_ON, esp as any mismatch should have
been caught by the quick-check. ATM it is just a pr_err; Id prefer to
see errors rather than crashes.
Demotes iter->site by replacing iter->site->_module by site->_module.
This is a small step towards dropping it entirely.
vmlinux.lds.h:
add __dyndbg_sites section and start/end symbols, with the same
align(8) and KEEP as used in the __dyndbg section.
kernel/module/main.c:load_info():
Initialize _ddebug_info.sites with new section address.
Signed-off-by: Jim Cromie
---
include/asm-generic/vmlinux.lds.h | 3 +++
include/linux/dynamic_debug.h | 37 +-
kernel/module/main.c | 2 ++
lib/dynamic_debug.c | 38 +++
4 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 9b8bd5504ad9..1e7ee65e8591 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -351,6 +351,9 @@
__start___dyndbg = .; \
KEEP(*(__dyndbg)) \
__stop___dyndbg = .;\
+ __start___dyndbg_sites = .; \
+
[PATCH v6 57/57] dyndbg: fiddle with readback value on LEVEL_NAMES types
my test scripts were writing one val, reading back val+1 echo L3 > p_level_names cat p_level_names L4 fix this w a -1 offset. Signed-off-by: Jim Cromie --- lib/dynamic_debug.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 11fea1f818a7..7d458601a61b 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -830,6 +830,8 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp) return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits); case DD_CLASS_TYPE_LEVEL_NAMES: + return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl - 1); + case DD_CLASS_TYPE_LEVEL_NUM: return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl); default: -- 2.37.2
[PATCH v6 35/57] dyndbg: add _DPRINTK_FLAGS_TRACE
add new flag, and OR it into _DPRINTK_FLAGS_ENABLED definition
CC: vincent.whitchu...@axis.com
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index ecd3379090c4..04f49df308a7 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -42,7 +42,9 @@ struct _ddebug {
(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
_DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID)
-#define _DPRINTK_FLAGS_ENABLED _DPRINTK_FLAGS_PRINTK
+#define _DPRINTK_FLAGS_TRACE (1 << 5)
+#define _DPRINTK_FLAGS_ENABLED (_DPRINTK_FLAGS_PRINTK | \
+_DPRINTK_FLAGS_TRACE)
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINTK
--
2.37.2
[PATCH v6 52/57] dyndbg: add DEFINE_DYNAMIC_DEBUG_TABLE, use it tacitly RFC
This defines a new macro (tbd: tweak name) which is a special version
of DEFINE_DYNAMIC_DEBUG_METADATA_CLS(). Unlike that macro, which
declares callsites in pairs of desc/site recs.
This macro declares a pair of header records, which the linker script
then places once (.gnu.linkonce.) into the front of the respective
sections. For builtins, the .linkonce gives us a single header
preceding all ~3-5k callsites.
It uses __unused, __weak, .gnu.linkonce to avoid trouble with multiple
inclusions, and should not consume space when not linked to by a
callsite in a loadable module.
Atypically, this macro is also tacitly invoked in the header, which is
indirectly included by printk.h. This means 2 things: it mostly
insures that source files will declare it at least once, and that
multiple decls resolve to the identical storage address.
I did this with the expectation that I could let the linker compute
the offset and initialize ._index accordingly, but that resulted in
some "linker cant compute" error. Ive initialized it in
__ddebug_add_module() instead.
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 34 ++
1 file changed, 34 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index f23608c38a79..23d3d2882882 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -220,6 +220,32 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_CLASS_DFLT, fmt)
+/*
+ * DEFINE_DYNAMIC_DEBUG_TABLE(): This is a special version of
+ * DEFINE_DYNAMIC_DEBUG_METADATA(). It creates a single pair of
+ * header records, which the linker scripts place in front of their
+ * respective sections.
+ *
+ * To allow for a robust runtime test in dynamic_debug_init(), the
+ * pair of records is hardwired into a self-reference loop which can
+ * be validated before the field is altered to support _ddebug_map_site()
+ */
+#define DEFINE_DYNAMIC_DEBUG_TABLE() \
+ /* forward decl, allowing loopback pointer */ \
+ struct _ddebug_hdr __used __aligned(8) \
+ __section(".gnu.linkonce.dyndbg") \
+ _LINKONCE_dyndbg_header;\
+ struct _ddebug_site_hdr __used __aligned(8) \
+ __section(".gnu.linkonce.dyndbg_site") \
+ _LINKONCE_dyndbg_site_header = {\
+ ._uplink = (void *) &_LINKONCE_dyndbg_header\
+ }; \
+ struct _ddebug_hdr __used __aligned(8) \
+ __section(".gnu.linkonce.dyndbg") \
+ _LINKONCE_dyndbg_header = { \
+ ._uplink = (void *) &_LINKONCE_dyndbg_site_header \
+ }
+
#ifdef CONFIG_JUMP_LABEL
#ifdef DEBUG
@@ -390,4 +416,12 @@ static inline int param_get_dyndbg_classes(char *buffer,
const struct kernel_par
extern const struct kernel_param_ops param_ops_dyndbg_classes;
+#if ((defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))) \
+ && defined(KBUILD_MODNAME) && !defined(NO_DYNAMIC_DEBUG_TABLE))
+
+/* transparently invoked, except when -DNO_DYNAMIC_DEBUG_TABLE */
+DEFINE_DYNAMIC_DEBUG_TABLE();
+#endif
+
#endif
--
2.37.2
[PATCH v6 43/57] dyndbg: add _index,_map to struct _ddebug
We now have 2 vectors of data: (__dyndbg, __dyndbg_sites), known to be
equal-length, with _ddebug.site connecting them, like rungs on a
ladder. In order to drop _ddebug.site, we need to replace the
absolute ptr with a cheap relative-address computation:
struct _ddebug_site *site = _ddebug_map_site(desc);
1st step: (done here)
add ._index, ._map fields, and initialize them in __debug_add_module()
to N (and = each other), where N is each element's position in the
builtins (or loaded-module). Then _ddebug_map_site(desc) can compute
[0], and then use container_of() to go up 1 struct.
RFC: Next steps: (outlined here, not done):
1. UPLINK:
Define 2 new header structs, with _uplink fields, pointing at _ddebug_info:
struct _ddebug_site_hdr {
union {
struct _ddebug_info _uplink*;
struct _ddebug_site __; // sizeof this
};
};
struct _ddebug_hdr {
union {
struct _ddebug_info _uplink*;
struct _ddebug __; // sizeof this
};
};
the union forces the allocation to be the bigger of the 2, with the
expectation that the pointer will always fit.
2. Reserve a record in front of the __dyndbg* sections.
can be done in {module,vmlinux}.lds.h, something like:
SECTIONS {
__dyndbg_sites : ALIGN(8) { *(.gnu.linkonce.dyndbg_site) *(__dyndbg_sites) }
__dyndbg : ALIGN(8) { *(.gnu.linkonce.dyndbg) *(__dyndbg) }
}
KEEP( *(.gnu.linkonce.__dyndbg_site) *(__dyndbg_sites))
3. specialize DYNAMIC_DEBUG_METADATA_CLS as DYNAMIC_DEBUG_TABLE.
This special macro version creates header records, and puts them into
.gnu.linkonce section. Unusually (uniquely?), it is tacitly invoked
by dynamic_debug.h on behalf of all printk.h includers. This can
result in multiple "declarations" in the same scope, so macro uses
__weak and/or __unused to suppress linkage & errors.
This created mumble-RELATIVE linkage errors in a few parts of the
kernel, I worked around this by just suppressing the declaration if
cflags includes -DDYNDBG_NO_TABLE.
The up-link is init'd in 2 cases:
The file static struct _ddebug_info builtins is initialized by
dynamic_debug_init(), with the 3 ELF sections addresses+lengths
composing the builtin dyndbg-state. It will be able to also
initialize the header.UPLINK fields, as long as the storage space is
available. Then rel_map() can rely upon it to provide the ref
for a callsite's desc*, if it is enabled.
Then at runtime (assuming initialization is correct):
- use _index to get [0]
- container-of gets .UPLINK to _info
- info->sites[_index] - replaces .sites
- info->sites[_map]- allows de-duplicated vector position. M
---
include/linux/dynamic_debug.h | 3 +++
lib/dynamic_debug.c | 8
2 files changed, 11 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index c05148dab367..268e903b7c4e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -31,6 +31,8 @@ struct _ddebug {
/* format is always needed, lineno shares word with flags */
const char *format;
const unsigned lineno:16;
+ unsigned _index:14;
+ unsigned _map:14;
#define CLS_BITS 6
unsigned int class_id:CLS_BITS;
#define _DPRINTK_CLASS_DFLT((1 << CLS_BITS) - 1)
@@ -60,6 +62,7 @@ struct _ddebug {
#define _DPRINTK_FLAGS_DEFAULT 0
#endif
unsigned int flags:8;
+
#ifdef CONFIG_JUMP_LABEL
union {
struct static_key_true dd_key_true;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f1f354efed5a..506a7e2e59d6 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1344,6 +1344,7 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
const char *modname)
{
struct ddebug_table *dt;
+ int i;
v3pr_info("add-module: %s.%d sites\n", modname, di->num_descs);
if (!di->num_descs) {
@@ -1364,6 +1365,7 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
*/
dt->mod_name = modname;
dt->ddebugs = di->descs;
+ dt->sites = di->sites;
dt->num_ddebugs = di->num_descs;
INIT_LIST_HEAD(>link);
@@ -1372,6 +1374,12 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
if (di->classes && di->num_classes)
ddebug_attach_module_classes(dt, di->classes, di->num_classes);
+ for (i = 0; i < di->num_descs; i++) {
+ di->descs[i]._index = base + i;
+ v3pr_info(" %d %d %s.%s.%d\n", i, base, modname,
+ di->descs[i].site->_function, di->descs[i].lineno);
+ }
+
mutex_lock(_lock);
list_add_tail(>link, _tables);
mutex_unlock(_lock);
--
2.37.2
[PATCH v6 33/57] nouveau: WIP add 2 LEVEL_NUM classmaps for CLI, SUBDEV
clone the nvkm_printk,_,__ macro ladder into nvkm_drmdbg,_,__.
And alter the debug, trace, spam macros to use the renamed ladder.
This *sets-up* (not done yet) to remove the _subdev->debug >= (l)
condition, once the bitmap-param is wired up correctly, and figured
into dyndbg's jump-label enablement. WIP.
Then undo the 1-line change that reduced count of prdbgs from 632 to 119.
ie: s/NV_SUBDEV_DBG_##l/NV_DBG_##l/
So heres what happened: new symbol is 15 (or 10), and fails this macro
test, so gets compiled out, and the dev_dbg is excluded.
if (CONFIG_NOUVEAU_DEBUG >= (l) && _subdev->debug >= (l)) \
dev_dbg(_subdev->device->dev, "%s: "f, _subdev->name, ##a); \
I could hack this, by doing (l + base), but base is pretty distant.
OTOH, the whole CONFIG_NOUVEAU_DEBUG check could be reworked; given
that trace is minumum recommended, theres not that many callsites
elided (SPAM only iirc) at compile-time, and dyndbg means keeping them
has "zero" run-cost (and 56 bytes per callsite). So this config item
doesnt do much when DRM_USE_DYNAMIC_DEBUG=y.
So this is a useful place to stop and look around, try to guess which
trail to take..
nouveau has additional debug variables to consider:
drivers/gpu/drm/nouveau/include/nvkm/core/device.h
131:if (_device->debug >= (l)) \
drivers/gpu/drm/nouveau/include/nvkm/core/client.h
39: if (_client->debug >= NV_DBG_##l) \
drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h
54: if (CONFIG_NOUVEAU_DEBUG >= (l) && _subdev->debug >= (l)) \
This is another baby-step, that seems not to break, so lets get a
snapshot.
whats done here:
In nouveau_drm.c, do DECLARE_DYNDBG_CLASSMAP(LEVEL_NUM type) 2x more,
for cli and subdev, right after the drm DECLARE. Adjust base for
each, to share the 0..30 classid-space.
These declare class-names: NV_CLI_DBG_* NV_SUBDEV_DBG_* accordingly.
Note: class-name-space is flat and wide, so super-generic names like
INFO should be prefixed; who could predict what a generic V3 does
across all modules.
s should be included
adjusting the base to avoid each other, and the 0-10
already mapped to drm-debug-categorys (just above this addition).
In nvkm/core/debug.h, add enums to match the names, with initial
values to match the bases.
In nvkm/core/subdev.h, alter (recently added) nvkm_drmdbg_() to use
NV_SUBDEV_DBG_* instead of NV_DBG_*.
NB: in both classmaps, Ive left FATAL..WARN out, they're not really
optional the way INFO..SPAM are; dyndbg shouldn't be able to turn them off.
bash-5.1# modprobe nouveau
[ 966.107833] dyndbg: 3 debug prints in module wmi
[ 966.342188] dyndbg: class[0]: module:nouveau base:15 len:5 ty:1
[ 966.342873] dyndbg: 15: 0 NV_SUBDEV_DBG_OFF
[ 966.343352] dyndbg: 16: 1 NV_SUBDEV_DBG_INFO
[ 966.343912] dyndbg: 17: 2 NV_SUBDEV_DBG_DEBUG
[ 966.33] dyndbg: 18: 3 NV_SUBDEV_DBG_TRACE
[ 966.344938] dyndbg: 19: 4 NV_SUBDEV_DBG_SPAM
[ 966.345402] dyndbg: class[1]: module:nouveau base:10 len:5 ty:1
[ 966.346011] dyndbg: 10: 0 NV_CLI_DBG_OFF
[ 966.346477] dyndbg: 11: 1 NV_CLI_DBG_INFO
[ 966.346989] dyndbg: 12: 2 NV_CLI_DBG_DEBUG
[ 966.347442] dyndbg: 13: 3 NV_CLI_DBG_TRACE
[ 966.347875] dyndbg: 14: 4 NV_CLI_DBG_SPAM
[ 966.348284] dyndbg: class[2]: module:nouveau base:0 len:10 ty:0
[ 966.34] dyndbg: 0: 0 DRM_UT_CORE
[ 966.349310] dyndbg: 1: 1 DRM_UT_DRIVER
[ 966.349694] dyndbg: 2: 2 DRM_UT_KMS
[ 966.350083] dyndbg: 3: 3 DRM_UT_PRIME
[ 966.350482] dyndbg: 4: 4 DRM_UT_ATOMIC
[ 966.351016] dyndbg: 5: 5 DRM_UT_VBL
[ 966.351475] dyndbg: 6: 6 DRM_UT_STATE
[ 966.351899] dyndbg: 7: 7 DRM_UT_LEASE
[ 966.352309] dyndbg: 8: 8 DRM_UT_DP
[ 966.352678] dyndbg: 9: 9 DRM_UT_DRMRES
[ 966.353104] dyndbg: module:nouveau attached 3 classes
[ 966.353759] dyndbg: 119 debug prints in module nouveau
NOTE: it was 632 with previous commit, switching NV_DEBUG to use
NV_SUBDEV_DBG_DEBUG instead of NV_DBG_DEBUG may be the cause.
Signed-off-by: Jim Cromie
---
.../gpu/drm/nouveau/include/nvkm/core/debug.h | 16 +
.../drm/nouveau/include/nvkm/core/subdev.h| 17 ++
drivers/gpu/drm/nouveau/nouveau_drm.c | 7 ++
drivers/gpu/drm/nouveau/nvkm/core/subdev.c| 23 +++
4 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
b/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
index b4a9c7d991ca..6a155a23a3d1 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/core/debug.h
@@ -9,4 +9,20 @@
#define NV_DBG_TRACE5
#define NV_DBG_PARANOIA 6
#define NV_DBG_SPAM 7
+
+enum nv_cli_dbg_verbose {
+ NV_CLI_DBG_OFF = 10,
+ NV_CLI_DBG_INFO,
+ NV_CLI_DBG_DEBUG,
+ NV_CLI_DBG_TRACE,
+ NV_CLI_DBG_SPAM
+};
+enum nv_subdev_dbg_verbose {
+ NV_SUBDEV_DBG_OFF = 15,
+ NV_SUBDEV_DBG_INFO,
[PATCH v6 51/57] dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE
The next patch adds DEFINE_DYNAMIC_DEBUG_TABLE(), which broke several subtrees, including efi, vdso, and some of arch/*/boot/compressed, with various relocation errors, iirc. Avoid those problems by adding a define to suppress the "transparent" DEFINE_DYNAMIC_DEBUG_TABLE() invocation. I found the x86 problems myself, l...@intel.com found arm & sparc problems, and may yet find others. Reported-by: # on [jimc:lkp-test/dyndbg-diet] recently Signed-off-by: Jim Cromie --- arch/arm/boot/compressed/Makefile | 2 ++ arch/sparc/vdso/Makefile | 2 ++ arch/x86/boot/compressed/Makefile | 1 + arch/x86/entry/vdso/Makefile | 3 +++ arch/x86/purgatory/Makefile | 1 + drivers/firmware/efi/libstub/Makefile | 3 ++- 6 files changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index 41bcbb460fac..f2d7a3b62727 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -81,6 +81,8 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma_with_size compress-$(CONFIG_KERNEL_XZ) = xzkern_with_size compress-$(CONFIG_KERNEL_LZ4) = lz4_with_size +KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE + libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y) diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile index 77d7b9032158..58ed4d64c978 100644 --- a/arch/sparc/vdso/Makefile +++ b/arch/sparc/vdso/Makefile @@ -30,6 +30,8 @@ obj-y += $(vdso_img_objs) targets += $(vdso_img_cfiles) targets += $(vdso_img_sodbg) $(vdso_img-y:%=vdso%.so) +KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE + CPPFLAGS_vdso.lds += -P -C VDSO_LDFLAGS_vdso.lds = -m elf64_sparc -soname linux-vdso.so.1 --no-undefined \ diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index 35ce1a64068b..b76bbf9fc504 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -36,6 +36,7 @@ KBUILD_CFLAGS := -m$(BITS) -O2 $(CLANG_FLAGS) KBUILD_CFLAGS += -fno-strict-aliasing -fPIE KBUILD_CFLAGS += -Wundef KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING +KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE cflags-$(CONFIG_X86_32) := -march=i386 cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone KBUILD_CFLAGS += $(cflags-y) diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile index 12f6c4d714cd..3182728f9f4d 100644 --- a/arch/x86/entry/vdso/Makefile +++ b/arch/x86/entry/vdso/Makefile @@ -29,6 +29,9 @@ vobjs32-y := vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o vobjs32-y += vdso32/vclock_gettime.o vobjs-$(CONFIG_X86_SGX)+= vsgx.o +# avoid a x86_64_RELATIVE error +KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE + # files to link into kernel obj-y += vma.o extable.o KASAN_SANITIZE_vma.o := y diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile index 31c634a22818..8d264836ae64 100644 --- a/arch/x86/purgatory/Makefile +++ b/arch/x86/purgatory/Makefile @@ -35,6 +35,7 @@ PURGATORY_CFLAGS_REMOVE := -mcmodel=kernel PURGATORY_CFLAGS := -mcmodel=large -ffreestanding -fno-zero-initialized-in-bss -g0 PURGATORY_CFLAGS += $(DISABLE_STACKLEAK_PLUGIN) -DDISABLE_BRANCH_PROFILING PURGATORY_CFLAGS += -fno-stack-protector +PURGATORY_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE # Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That # in turn leaves some undefined symbols like __fentry__ in purgatory and not diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 3ef67431f05e..2b7e34555c5a 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -14,7 +14,8 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ \ $(call cc-disable-warning, address-of-packed-member) \ $(call cc-disable-warning, gnu) \ -fno-asynchronous-unwind-tables \ - $(CLANG_FLAGS) + $(CLANG_FLAGS) \ + -DNO_DYNAMIC_DEBUG_TABLE # arm64 uses the full KBUILD_CFLAGS so it's necessary to explicitly # disable the stackleak plugin -- 2.37.2
[PATCH v6 28/57] drm_print: refine drm_debug_enabled for jump-label
In order to use dynamic-debug's jump-label optimization in drm-debug,
its clarifying to refine drm_debug_enabled into 3 uses:
1. drm_debug_enabled - legacy, public
2. __drm_debug_enabled - optimized for dyndbg jump-label enablement.
3. _drm_debug_enabled - pr_debug instrumented, observable
1. The legacy version always checks the bits.
2. is privileged, for use by __drm_dbg(), __drm_dev_dbg(), which do an
early return unless the category is enabled. For dyndbg builds, debug
callsites are selectively "pre-enabled", so __drm_debug_enabled()
short-circuits to true there. Remaining callers of 1 may be able to
use 2, case by case.
3. is 1st wrapped in a macro, with a pr_debug, which reports each
usage in /proc/dynamic_debug/control, making it observable in the
logs. The macro lets the pr_debug see the real caller, not an inline
function.
When plugged into 1, 3 identified ~10 remaining callers of the
function, leading to the follow-on cleanup patch, and would allow
activating the pr_debugs, estimating the callrate, and the potential
savings by using the wrapper macro. It is unused ATM, but it fills
out the picture.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/drm_print.c | 4 ++--
include/drm/drm_print.h | 28
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 29a29949ad0b..cb203d63b286 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -285,7 +285,7 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
struct va_format vaf;
va_list args;
- if (!drm_debug_enabled(category))
+ if (!__drm_debug_enabled(category))
return;
va_start(args, format);
@@ -308,7 +308,7 @@ void ___drm_dbg(enum drm_debug_category category, const
char *format, ...)
struct va_format vaf;
va_list args;
- if (!drm_debug_enabled(category))
+ if (!__drm_debug_enabled(category))
return;
va_start(args, format);
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index dfdd81c3287c..7631b5fb669e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -321,11 +321,39 @@ enum drm_debug_category {
DRM_UT_DRMRES
};
+/*
+ * 3 name flavors of drm_debug_enabled:
+ * drm_debug_enabled - public/legacy, always checks bits
+ * _drm_debug_enabled - instrumented to observe call-rates, est overheads.
+ * __drm_debug_enabled - privileged - knows jump-label state, can short-circuit
+ */
static inline bool drm_debug_enabled(enum drm_debug_category category)
{
return unlikely(__drm_debug & BIT(category));
}
+/*
+ * Wrap fn in macro, so that the pr_debug sees the actual caller, not
+ * the inline fn. Using this name creates a callsite entry / control
+ * point in /proc/dynamic_debug/control.
+ */
+#define _drm_debug_enabled(category) \
+ ({ \
+ pr_debug("todo: maybe avoid via dyndbg\n"); \
+ drm_debug_enabled(category);\
+ })
+
+#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+/*
+ * dyndbg is wrapping the drm.debug API, so as to avoid the runtime
+ * bit-test overheads of drm_debug_enabled() in those api calls.
+ * In this case, executed callsites are known enabled, so true.
+ */
+#define __drm_debug_enabled(category) true
+#else
+#define __drm_debug_enabled(category) drm_debug_enabled(category)
+#endif
+
/*
* struct device based logging
*
--
2.37.2
[PATCH v6 40/57] dyndbg: abstraction macros for modname, function, filename fields
The .data table built of struct _ddebug[]s has about 40% repetition in
the "decorator" fields: modname, function, filename. The real fix is
to "normalize the db", but 1st we need a layer of accessor macros,
giving us just one field deref to alter later.
So:
define 3 field-abstraction macros: desc_{modname,function,filename}(desc).
replace all raw field refs in lib/dynamic_debug.c with them.
The macros protect against NULL pointer derefs, substituting "_na_"
otherwise. This provides a generic guard, opening options to drop the
contents of __dyndbg_sites[] opportunistically, and trim kernel/module
data. DRM, which could use only .class_id to control drm.debug, could
drop them.
The 3 macros use a common foundation: _ddebug_map_site(), which will
adapt to follow the coming table-split.
Also change field names; adding '_' prefix to insure that bare
field-refs are found and fixed. Most field uses get the macros,
except for dynamic_debug_init(), which will need to follow the
rearrangements.
NOTE: macros are private, not currently for general use.
trace/events/dyndbg.h was a candidate to use these macros, for which I
included the header. On rethink, these macros are wrong abstraction
for tracing; better to expose a dynamic_prefix_emit(1) flavor, and use
it in TP_printk to "decorate" enabled trace-logs like it does for
sys-logs.
So this patch removes the raw field-refs rather than use the macros.
I left the include to mark the intention to use privatish interfaces,
see if it draws objections or compile errs.
[1] fills caller provided char-buffer, perhaps not ideal for tracefs.
A fixed max-size-possible per-callsite (or globally) is practical, but
the struct _ddebug_site * val is probably best; it refs a prdbg's
site-rec of a builtin or loadable module, which is unique over the
pertinent lifetime, and has all the info.
WAG,TLDR: "decorating" could be defered until `cat trace`, modulo
loadable sites[] data being needed to render after module is unloaded.
no functional changes.
Signed-off-by: Jim Cromie
TP-print-dont-use-desc-foo
---
include/linux/dynamic_debug.h | 12 ++--
include/trace/events/dyndbg.h | 7 +++
lib/dynamic_debug.c | 37 ++-
3 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 04f49df308a7..e04f5b0a31e2 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -18,9 +18,9 @@ struct _ddebug {
* These fields are used to drive the user interface
* for selecting and displaying debug callsites.
*/
- const char *modname;
- const char *function;
- const char *filename;
+ const char *_modname;
+ const char *_function;
+ const char *_filename;
const char *format;
unsigned int lineno:18;
#define CLS_BITS 6
@@ -166,9 +166,9 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \
static struct _ddebug __aligned(8) \
__section("__dyndbg") name = { \
- .modname = KBUILD_MODNAME, \
- .function = __func__, \
- .filename = __FILE__, \
+ ._modname = KBUILD_MODNAME, \
+ ._function = __func__, \
+ ._filename = __FILE__, \
.format = (fmt),\
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT,\
diff --git a/include/trace/events/dyndbg.h b/include/trace/events/dyndbg.h
index e19fcb56566c..2997289c0e3f 100644
--- a/include/trace/events/dyndbg.h
+++ b/include/trace/events/dyndbg.h
@@ -6,6 +6,7 @@
#define _TRACE_DYNDBG_H
#include
+#include
/* capture pr_debug() callsite descriptor and message */
TRACE_EVENT(prdbg,
@@ -32,8 +33,7 @@ TRACE_EVENT(prdbg,
__get_str(msg)[len] = 0;
),
- TP_printk("%s.%s %s", __entry->desc->modname,
- __entry->desc->function, __get_str(msg))
+ TP_printk("%s", __get_str(msg))
);
/* capture dev_dbg() callsite descriptor, device, and message */
@@ -64,8 +64,7 @@ TRACE_EVENT(devdbg,
__get_str(msg)[len] = 0;
),
- TP_printk("%s.%s %s", __entry->desc->modname,
- __entry->desc->function, __get_str(msg))
+ TP_printk("%s", __get_str(msg))
);
#endif /* _TRACE_DYNDBG_H */
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0870e939f255..5a22708679a7 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -167,6 +167,15 @@ static struct ddebug_class_map
*ddebug_find_valid_class(struct
[PATCH v6 25/57] drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro
For CONFIG_DRM_USE_DYNAMIC_DEBUG=y, wrap __drm_dbg() & __drm_dev_dbg() in one of dyndbg's Factory macros: _dynamic_func_call_no_desc(). This adds the callsite descriptor into the code, and an entry for each into /proc/dynamic_debug/control. #> echo class DRM_UT_ATOMIC +p > /proc/dynamic_debug/control CONFIG_DRM_USE_DYNAMIC_DEBUG=y/n is configurable because of the .data footprint cost of per-callsite control; 56 bytes/site * ~2k for i915, ~4k callsites for amdgpu. This is large enough that a kernel builder might not want it. Signed-off-by: Jim Cromie --- drivers/gpu/drm/Kconfig | 12 drivers/gpu/drm/Makefile | 2 ++ include/drm/drm_print.h | 12 3 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 2f52e8941074..3b75c286f14f 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -51,6 +51,18 @@ config DRM_DEBUG_MM If in doubt, say "N". +config DRM_USE_DYNAMIC_DEBUG + bool "use dynamic debug to implement drm.debug" + default y + depends on DRM + depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE + depends on JUMP_LABEL + help + Use dynamic-debug to avoid drm_debug_enabled() runtime overheads. + Due to callsite counts in DRM drivers (~4k in amdgpu) and 56 + bytes per callsite, the .data costs can be substantial, and + are therefore configurable. + config DRM_KUNIT_TEST tristate "KUnit tests for DRM" if !KUNIT_ALL_TESTS depends on DRM && KUNIT diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 25d0ba310509..0b283e46f28b 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -3,6 +3,8 @@ # Makefile for the drm device driver. This driver provides support for the # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. +CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE + drm-y := drm_aperture.o drm_auth.o drm_cache.o \ drm_file.o drm_gem.o drm_ioctl.o \ drm_drv.o \ diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index c429c258c957..2d2cef76b5c1 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -384,8 +384,14 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category, } \ }) +#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG) #define drm_dev_dbg(dev, cat, fmt, ...)\ __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__) +#else +#define drm_dev_dbg(dev, cat, fmt, ...)\ + _dynamic_func_call_no_desc(fmt, __drm_dev_dbg, \ + dev, cat, fmt, ##__VA_ARGS__) +#endif /** * DRM_DEV_DEBUG() - Debug output for generic drm code @@ -492,7 +498,13 @@ void ___drm_dbg(enum drm_debug_category category, const char *format, ...); __printf(1, 2) void __drm_err(const char *format, ...); +#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG) #define __drm_dbg(fmt, ...)___drm_dbg(fmt, ##__VA_ARGS__) +#else +#define __drm_dbg(cat, fmt, ...) \ + _dynamic_func_call_no_desc(fmt, ___drm_dbg, \ + cat, fmt, ##__VA_ARGS__) +#endif /* Macros to make printk easier */ -- 2.37.2
[PATCH v6 16/57] dyndbg: add ddebug_attach_module_classes
Add ddebug_attach_module_classes(), call it from ddebug_add_module().
It scans the classes/section its given, finds records where the
module-name matches the module being added, and adds them to the
module's maps list. No locking here, since the record
isn't yet linked into the ddebug_tables list.
It is called indirectly from 2 sources:
- from load_module(), where it scans the module's __dyndbg_classes
section, which contains DYNAMIC_DEBUG_CLASSES definitions from just
the module.
- from dynamic_debug_init(), where all DYNAMIC_DEBUG_CLASSES
definitions of each builtin module have been packed together.
This is why ddebug_attach_module_classes() checks module-name.
NOTES
Its (highly) likely that builtin classes will be ordered by module
name (just like prdbg descriptors are in the __dyndbg section). So
the list can be replaced by a vector (ptr + length), which will work
for loaded modules too. This would imitate whats currently done for
the _ddebug descriptors.
That said, converting to vector,len is close to pointless; a small
minority of modules will ever define a class-map, and almost all of
them will have only 1 or 2 class-maps, so theres only a couple dozen
pointers to save. TODO: re-evaluate for lines removable.
Signed-off-by: Jim Cromie
---
v6
. fix compile err due to reorderd commits, and missed lkp report.
. init dt->maps in ddebug_add_module, prior to ddebug_attach_module_classes
---
lib/dynamic_debug.c | 32 +++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index fb31a1a2fc3f..b71efd0b491d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -45,7 +45,7 @@ extern struct ddebug_class_map __start___dyndbg_classes[];
extern struct ddebug_class_map __stop___dyndbg_classes[];
struct ddebug_table {
- struct list_head link;
+ struct list_head link, maps;
const char *mod_name;
unsigned int num_ddebugs;
struct _ddebug *ddebugs;
@@ -921,6 +921,32 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
};
+static void ddebug_attach_module_classes(struct ddebug_table *dt,
+struct ddebug_class_map *classes,
+int num_classes)
+{
+ struct ddebug_class_map *cm;
+ int i, j, ct = 0;
+
+ for (cm = classes, i = 0; i < num_classes; i++, cm++) {
+
+ if (!strcmp(cm->mod_name, dt->mod_name)) {
+
+ v2pr_info("class[%d]: module:%s base:%d len:%d
ty:%d\n", i,
+ cm->mod_name, cm->base, cm->length,
cm->map_type);
+
+ for (j = 0; j < cm->length; j++)
+ v3pr_info(" %d: %d %s\n", j + cm->base, j,
+ cm->class_names[j]);
+
+ list_add(>link, >maps);
+ ct++;
+ }
+ }
+ if (ct)
+ vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
+}
+
/*
* Allocate a new ddebug_table for the given module
* and add it to the global list.
@@ -952,6 +978,10 @@ static int __ddebug_add_module(struct _ddebug_info *di,
unsigned int base,
dt->num_ddebugs = di->num_descs;
INIT_LIST_HEAD(>link);
+ INIT_LIST_HEAD(>maps);
+
+ if (di->classes && di->num_classes)
+ ddebug_attach_module_classes(dt, di->classes, di->num_classes);
mutex_lock(_lock);
list_add_tail(>link, _tables);
--
2.37.2
[PATCH v6 37/57] dyndbg: add 2 trace-events: drm_debug, drm_devdbg
Recently added ddebug_trace() issues a single printk:console event.
Replace that event with 2 new ones, defined in a new header:
include/trace/events/dyndbg.h
- dyndbg:prdbg - from trace_prdbg() - if !dev
- dyndbg:devdbg - from trace_devdbg() - if !!dev
This links the legacy pr_debug API to tracefs, so pr_debug() and
dev_dbg() calls can add more debug context into the trace-logs, and
then at users option, less into syslog.
The new events allow enabling by event-type in tracefs, and dyndbg
allows individual enablement of prdbg callsites (via +T flag).
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/drm_print.c | 25 -
include/trace/events/drm.h | 54 +
2 files changed, 72 insertions(+), 7 deletions(-)
create mode 100644 include/trace/events/drm.h
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 5b93c11895bb..c50edbf443d3 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -35,6 +35,9 @@
#include
#include
+#define CREATE_TRACE_POINTS
+#include
+
/*
* __drm_debug: Enable debug output.
* Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
@@ -293,13 +296,19 @@ void __drm_dev_dbg(struct _ddebug *desc, const struct
device *dev,
vaf.fmt = format;
vaf.va =
- if (dev)
- dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
- __builtin_return_address(0), );
- else
- printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
- __builtin_return_address(0), );
-
+ if (dev) {
+ if (desc->flags & _DPRINTK_FLAGS_PRINTK)
+ dev_printk(KERN_DEBUG, dev, "[" DRM_NAME ":%ps] %pV",
+ __builtin_return_address(0), );
+ if (desc->flags & _DPRINTK_FLAGS_TRACE)
+ trace_drm_devdbg(dev, category, );
+ } else {
+ if (desc->flags & _DPRINTK_FLAGS_PRINTK)
+ printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
+ __builtin_return_address(0), );
+ if (desc->flags & _DPRINTK_FLAGS_TRACE)
+ trace_drm_debug(category, );
+ }
va_end(args);
}
EXPORT_SYMBOL(__drm_dev_dbg);
@@ -319,6 +328,8 @@ void ___drm_dbg(struct _ddebug *desc, enum
drm_debug_category category, const ch
printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
__builtin_return_address(0), );
+ trace_drm_debug(category, );
+
va_end(args);
}
EXPORT_SYMBOL(___drm_dbg);
diff --git a/include/trace/events/drm.h b/include/trace/events/drm.h
new file mode 100644
index ..589fa1e1f2c2
--- /dev/null
+++ b/include/trace/events/drm.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM drm
+
+#if !defined(_TRACE_DRM_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_DRM_H
+
+#include
+
+/* drm_debug() was called, pass its args */
+TRACE_EVENT(drm_debug,
+ TP_PROTO(int drm_debug_category, struct va_format *vaf),
+
+ TP_ARGS(drm_debug_category, vaf),
+
+ TP_STRUCT__entry(
+ __field(int, drm_debug_category)
+ __vstring(msg, vaf->fmt, vaf->va)
+ ),
+
+ TP_fast_assign(
+ __entry->drm_debug_category = drm_debug_category;
+ __assign_vstr(msg, vaf->fmt, vaf->va);
+ ),
+
+ TP_printk("%s", __get_str(msg))
+);
+
+/* drm_devdbg() was called, pass its args, preserving order */
+TRACE_EVENT(drm_devdbg,
+ TP_PROTO(const struct device *dev, int drm_debug_category, struct
va_format *vaf),
+
+ TP_ARGS(dev, drm_debug_category, vaf),
+
+ TP_STRUCT__entry(
+ __field(const struct device*, dev)
+ __field(int, drm_debug_category)
+ __vstring(msg, vaf->fmt, vaf->va)
+ ),
+
+ TP_fast_assign(
+ __entry->drm_debug_category = drm_debug_category;
+ __entry->dev = dev;
+ __assign_vstr(msg, vaf->fmt, vaf->va);
+ ),
+
+ TP_printk("cat:%d, %s %s", __entry->drm_debug_category,
+ dev_name(__entry->dev), __get_str(msg))
+);
+
+#endif /* _TRACE_DRM_H */
+
+/* This part must be outside protection */
+#include
--
2.37.2
[PATCH v6 42/57] dyndbg: shrink lineno field by 2 bits
struct _ddebug.lineno int:18 is bigger than we need:
git ls-files | grep -P '\.c$' | xargs wc -l | sort -n | tail -n20
...
22553 drivers/scsi/lpfc/lpfc_sli.c
28593 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c
46300 drivers/net/wireless/realtek/rtw88/rtw8822c_table.c
48725 drivers/net/wireless/realtek/rtw89/rtw8852a_table.c
1386670 total
...
All but one of the biggest of these are just .data tables, and have no
code, so would never need a pr_debug(). drivers/scsi/lpfc/lpfc_sli.c,
at ~22k lines, would be encodable with 15 bits, but lets just shrink
to 16 bits for now.
RFC: Is this tantamout to a "don't write files with >64kloc" policy ?
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dcc0e8cc2ef0..c05148dab367 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -30,7 +30,7 @@ struct _ddebug {
struct _ddebug_site *site;
/* format is always needed, lineno shares word with flags */
const char *format;
- const unsigned lineno:18;
+ const unsigned lineno:16;
#define CLS_BITS 6
unsigned int class_id:CLS_BITS;
#define _DPRINTK_CLASS_DFLT((1 << CLS_BITS) - 1)
--
2.37.2
[PATCH v6 21/57] dyndbg: test DECLARE_DYNDBG_CLASSMAP, sysfs nodes
Demonstrate use of DECLARE_DYNDBG_CLASSMAP macro, and expose them as
sysfs-nodes for testing.
For each of the 4 class-map-types:
- declare a class-map of that type,
- declare the enum corresponding to those class-names
- share _base across 0..30 range
- add a __pr_debug_cls() call for each class-name
- declare 2 sysnodes for each class-map
for 'p' flag, and future 'T' flag
These declarations create the following sysfs parameter interface:
:#> pwd
/sys/module/test_dynamic_debug/parameters
:#> ls
T_disjoint_bits T_disjoint_names T_level_names T_level_num do_prints
p_disjoint_bits p_disjoint_names p_level_names p_level_num
NOTES:
The local wrapper macro is an api candidate, but there are already too
many parameters. OTOH, maybe related enum should be in there too,
since it has _base inter-dependencies.
The T_* params control the (future) T flag on the same class'd
pr_debug callsites as their p* counterparts. Using them will fail,
until the dyndbg-trace patches are added in.
:#> echo 1 > T_disjoint
[ 28.792489] dyndbg: disjoint: 0x1 > test_dynamic_debug.T_D2
[ 28.793848] dyndbg: query 0: "class D2_CORE +T" mod:*
[ 28.795086] dyndbg: split into words: "class" "D2_CORE" "+T"
[ 28.796467] dyndbg: op='+'
[ 28.797148] dyndbg: unknown flag 'T'
[ 28.798021] dyndbg: flags parse failed
[ 28.798947] dyndbg: processed 1 queries, with 0 matches, 1 errs
[ 28.800378] dyndbg: bit_0: -22 matches on class: D2_CORE -> 0x1
[ 28.801959] dyndbg: test_dynamic_debug.T_D2: updated 0x0 -> 0x1
[ 28.803974] dyndbg: total matches: -22
Signed-off-by: Jim Cromie
---
lib/test_dynamic_debug.c | 125 ++-
1 file changed, 110 insertions(+), 15 deletions(-)
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index ba3882ca3e48..8dd250ad022b 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -10,57 +10,152 @@
#include
-static void do_prints(void); /* device under test */
-
-/* run tests by reading or writing sysfs node */
+/* run tests by reading or writing sysfs node: do_prints */
+static void do_prints(void); /* device under test */
static int param_set_do_prints(const char *instr, const struct kernel_param
*kp)
{
do_prints();
return 0;
}
-
static int param_get_do_prints(char *buffer, const struct kernel_param *kp)
{
do_prints();
return scnprintf(buffer, PAGE_SIZE, "did do_prints\n");
}
-
static const struct kernel_param_ops param_ops_do_prints = {
.set = param_set_do_prints,
.get = param_get_do_prints,
};
-
module_param_cb(do_prints, _ops_do_prints, NULL, 0600);
-static void do_alpha(void)
+/*
+ * Using the CLASSMAP api:
+ * - classmaps must have corresponding enum
+ * - enum symbols must match/correlate with class-name strings in the map.
+ * - base must equal enum's 1st value
+ * - multiple maps must set their base to share the 0-30 class_id space !!
+ * (build-bug-on tips welcome)
+ * Additionally, here:
+ * - tie together sysname, mapname, bitsname, flagsname
+ */
+#define DD_SYS_WRAP(_model, _flags)\
+ static unsigned long bits_##_model; \
+ static struct ddebug_class_param _flags##_model = { \
+ .bits = _##_model, \
+ .flags = #_flags, \
+ .map = _##_model, \
+ }; \
+ module_param_cb(_flags##_##_model, _ops_dyndbg_classes,
&_flags##_model, 0600)
+
+/* numeric input, independent bits */
+enum cat_disjoint_bits {
+ D2_CORE = 0,
+ D2_DRIVER,
+ D2_KMS,
+ D2_PRIME,
+ D2_ATOMIC,
+ D2_VBL,
+ D2_STATE,
+ D2_LEASE,
+ D2_DP,
+ D2_DRMRES };
+DECLARE_DYNDBG_CLASSMAP(map_disjoint_bits, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "D2_CORE",
+ "D2_DRIVER",
+ "D2_KMS",
+ "D2_PRIME",
+ "D2_ATOMIC",
+ "D2_VBL",
+ "D2_STATE",
+ "D2_LEASE",
+ "D2_DP",
+ "D2_DRMRES");
+DD_SYS_WRAP(disjoint_bits, p);
+DD_SYS_WRAP(disjoint_bits, T);
+
+/* symbolic input, independent bits */
+enum cat_disjoint_names { LOW = 11, MID, HI };
+DECLARE_DYNDBG_CLASSMAP(map_disjoint_names, DD_CLASS_TYPE_DISJOINT_NAMES, 10,
+ "LOW", "MID", "HI");
+DD_SYS_WRAP(disjoint_names, p);
+DD_SYS_WRAP(disjoint_names, T);
+
+/* numeric verbosity, V2 > V1 related */
+enum cat_level_num { V0 = 14, V1, V2, V3, V4, V5, V6, V7 };
+DECLARE_DYNDBG_CLASSMAP(map_level_num, DD_CLASS_TYPE_LEVEL_NUM, 14,
+ "V0", "V1", "V2", "V3", "V4", "V5", "V6", "V7");
+DD_SYS_WRAP(level_num, p);
[PATCH v6 24/57] drm_print: interpose drm_*dbg with forwarding macros
change drm_dev_dbg & drm_dbg to macros, which forward to the renamed
functions (with __ prefix added).
Those functions sit below the categorized layer of macros implementing
the DRM debug.category API, and implement most of it. These are good
places to insert dynamic-debug jump-label mechanics, which will allow
DRM to avoid the runtime cost of drm_debug_enabled().
no functional changes.
memory cost baseline: (unchanged)
bash-5.1# drms_load
[9.220389] dyndbg: 1 debug prints in module drm
[9.224426] ACPI: bus type drm_connector registered
[9.302192] dyndbg: 2 debug prints in module ttm
[9.305033] dyndbg: 8 debug prints in module video
[9.627563] dyndbg: 127 debug prints in module i915
[9.721505] AMD-Vi: AMD IOMMUv2 functionality not available on this system -
This is not a bug.
[ 10.091345] dyndbg: 2196 debug prints in module amdgpu
[ 10.106589] [drm] amdgpu kernel modesetting enabled.
[ 10.107270] amdgpu: CRAT table not found
[ 10.107926] amdgpu: Virtual CRAT table created for CPU
[ 10.108398] amdgpu: Topology: Add CPU node
[ 10.168507] dyndbg: 3 debug prints in module wmi
[ 10.329587] dyndbg: 3 debug prints in module nouveau
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/drm_print.c | 10 +-
include/drm/drm_print.h | 9 +++--
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ec32df35a3e3..29a29949ad0b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -279,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char
*level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
-const char *format, ...)
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+ const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -301,9 +301,9 @@ void drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
va_end(args);
}
-EXPORT_SYMBOL(drm_dev_dbg);
+EXPORT_SYMBOL(__drm_dev_dbg);
-void __drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -320,7 +320,7 @@ void __drm_dbg(enum drm_debug_category category, const char
*format, ...)
va_end(args);
}
-EXPORT_SYMBOL(__drm_dbg);
+EXPORT_SYMBOL(___drm_dbg);
void __drm_err(const char *format, ...)
{
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 668273e36c2c..c429c258c957 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -335,7 +335,7 @@ __printf(3, 4)
void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
__printf(3, 4)
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
const char *format, ...);
/**
@@ -384,6 +384,9 @@ void drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
} \
})
+#define drm_dev_dbg(dev, cat, fmt, ...)\
+ __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+
/**
* DRM_DEV_DEBUG() - Debug output for generic drm code
*
@@ -485,10 +488,12 @@ void drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
*/
__printf(2, 3)
-void __drm_dbg(enum drm_debug_category category, const char *format, ...);
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
__printf(1, 2)
void __drm_err(const char *format, ...);
+#define __drm_dbg(fmt, ...)___drm_dbg(fmt, ##__VA_ARGS__)
+
/* Macros to make printk easier */
#define _DRM_PRINTK(once, level, fmt, ...) \
--
2.37.2
[PATCH v6 32/57] nouveau: adapt NV_DEBUG, NV_ATOMIC to use DRM.debug
These 2 macros used drm_debug_enabled() on DRM_UT_{DRIVER,ATOMIC}
respectively, replace those with drm_dbg_##cat invocations.
this results in new class'd prdbg callsites:
:#> grep nouveau /proc/dynamic_debug/control | grep class | wc
1161130 15584
:#> grep nouveau /proc/dynamic_debug/control | grep class | grep DRIVER | wc
74 7049709
:#> grep nouveau /proc/dynamic_debug/control | grep class | grep ATOMIC | wc
31 3074237
:#> grep nouveau /proc/dynamic_debug/control | grep class | grep KMS | wc
11 1191638
the KMS entries are due to existing uses of drm_dbg_kms().
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/nouveau/nouveau_drv.h | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.h
b/drivers/gpu/drm/nouveau/nouveau_drv.h
index 84df5ddae4d0..3b8a76004b57 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.h
@@ -39,6 +39,7 @@
*/
#include
+#include
#include
#include
@@ -263,13 +264,16 @@ void nouveau_drm_device_remove(struct drm_device *dev);
#define NV_WARN(drm,f,a...) NV_PRINTK(warn, &(drm)->client, f, ##a)
#define NV_INFO(drm,f,a...) NV_PRINTK(info, &(drm)->client, f, ##a)
-#define NV_DEBUG(drm,f,a...) do {
\
- if (drm_debug_enabled(DRM_UT_DRIVER)) \
- NV_PRINTK(info, &(drm)->client, f, ##a); \
+#define NV_DRMDBG(cat,c,f,a...) do { \
+ struct nouveau_cli *_cli = (c); \
+ drm_dbg_##cat(_cli->drm->dev, "%s: "f, _cli->name, ##a); \
} while(0)
-#define NV_ATOMIC(drm,f,a...) do {
\
- if (drm_debug_enabled(DRM_UT_ATOMIC)) \
- NV_PRINTK(info, &(drm)->client, f, ##a); \
+
+#define NV_DEBUG(drm,f,a...) do { \
+ NV_DRMDBG(driver, &(drm)->client, f, ##a); \
+} while(0)
+#define NV_ATOMIC(drm,f,a...) do { \
+ NV_DRMDBG(atomic, &(drm)->client, f, ##a); \
} while(0)
#define NV_PRINTK_ONCE(l,c,f,a...) NV_PRINTK(l##_once,c,f, ##a)
--
2.37.2
[PATCH v6 15/57] kernel/module: add __dyndbg_classes section
Add __dyndbg_classes section, using __dyndbg as a model. Use it:
vmlinux.lds.h:
KEEP the new section, which also silences orphan section warning on
loadable modules. Add (__start_/__stop_)__dyndbg_classes linker
symbols for the c externs (below).
kernel/module/main.c:
- fill new fields in find_module_sections(), using section_objs()
- extend callchain prototypes
to pass classes, length
load_module(): pass new info to dynamic_debug_setup()
dynamic_debug_setup(): new params, pass through to ddebug_add_module()
dynamic_debug.c:
- add externs to the linker symbols.
ddebug_add_module():
- It currently builds a debug_table, and *will* find and attach classes.
dynamic_debug_init():
- add class fields to the _ddebug_info cursor var: di.
Signed-off-by: Jim Cromie
---
include/asm-generic/vmlinux.lds.h | 3 +++
include/linux/dynamic_debug.h | 2 ++
kernel/module/main.c | 2 ++
lib/dynamic_debug.c | 7 +++
4 files changed, 14 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index 7515a465ec03..9b8bd5504ad9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -345,6 +345,9 @@
*(__tracepoints)\
/* implement dynamic printk debug */\
. = ALIGN(8); \
+ __start___dyndbg_classes = .; \
+ KEEP(*(__dyndbg_classes)) \
+ __stop___dyndbg_classes = .;\
__start___dyndbg = .; \
KEEP(*(__dyndbg)) \
__stop___dyndbg = .;\
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 98dbf1d49984..9073a43a2039 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -114,7 +114,9 @@ struct ddebug_class_map {
/* encapsulate linker provided built-in (or module) dyndbg data */
struct _ddebug_info {
struct _ddebug *descs;
+ struct ddebug_class_map *classes;
unsigned int num_descs;
+ unsigned int num_classes;
};
#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 4c20bc3ff203..6aa6153aa6e0 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2113,6 +2113,8 @@ static int find_module_sections(struct module *mod,
struct load_info *info)
info->dyndbg.descs = section_objs(info, "__dyndbg",
sizeof(*info->dyndbg.descs),
>dyndbg.num_descs);
+ info->dyndbg.classes = section_objs(info, "__dyndbg_classes",
+ sizeof(*info->dyndbg.classes),
>dyndbg.num_classes);
return 0;
}
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c358ccdf4a39..fb31a1a2fc3f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -41,6 +41,8 @@
extern struct _ddebug __start___dyndbg[];
extern struct _ddebug __stop___dyndbg[];
+extern struct ddebug_class_map __start___dyndbg_classes[];
+extern struct ddebug_class_map __stop___dyndbg_classes[];
struct ddebug_table {
struct list_head link;
@@ -1079,7 +1081,9 @@ static int __init dynamic_debug_init(void)
struct _ddebug_info di = {
.descs = __start___dyndbg,
+ .classes = __start___dyndbg_classes,
.num_descs = __stop___dyndbg - __start___dyndbg,
+ .num_classes = __stop___dyndbg_classes -
__start___dyndbg_classes,
};
if (&__start___dyndbg == &__stop___dyndbg) {
@@ -1122,6 +1126,9 @@ static int __init dynamic_debug_init(void)
i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
(int)((i * sizeof(struct _ddebug)) >> 10));
+ if (di.num_classes)
+ v2pr_info(" %d builtin ddebug class-maps\n", di.num_classes);
+
/* now that ddebug tables are loaded, process all boot args
* again to find and activate queries given in dyndbg params.
* While this has already been done for known boot params, it
--
2.37.2
[PATCH v6 31/57] nouveau: change nvkm_debug/trace to use dev_dbg POC
These 2 macros formerly used dev_info, and they still check subdev->debug to gate the printing. So dyndbg control is redundant ATM (and possibly confusing, since its off by default). prdbg count is up from 3, or from 65 (with VMM_DEBUG here) [7.765379] dyndbg: 516 debug prints in module nouveau Its possible to control error, warn, info callsites too, but they're usually on, and the .data overheads on ~450 more callsites (56 bytes each) would just be wasted. $ for l in fatal error warn info debug trace spam; do echo $l; ack nvkm_$l drivers/gpu |wc; done fatal 3 19 335 error 2891956 30651 warn 84 5138860 info 14 881502 debug 3872339 40844 trace 31 2193368 spam 1 7 123 bash-5.1# echo $(( 516-65-387-31-1 )) 32 Thats approximate; not accounting #defines and doc/comment mentions. NOTE: this patch changes the log-level of the macro-issued messages from KERN_INFO to KERN_DEBUG. Adding a .kern_lvl field to struct _ddebug could fix that. RFC: dyndbg & subdev->debug Separate class-maps for each subdev are possible; except for the coordinated use of _base, each is independent, including choice of DISJOINT or LEVELS, as long as class-names don't conflict. So theres some flexibility. Signed-off-by: Jim Cromie --- drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h b/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h index 96113c8bee8c..065d07ccea87 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/subdev.h @@ -59,8 +59,8 @@ void nvkm_subdev_intr(struct nvkm_subdev *); #define nvkm_error(s,f,a...) nvkm_printk((s), ERROR,err, f, ##a) #define nvkm_warn(s,f,a...) nvkm_printk((s), WARN, notice, f, ##a) #define nvkm_info(s,f,a...) nvkm_printk((s), INFO, info, f, ##a) -#define nvkm_debug(s,f,a...) nvkm_printk((s), DEBUG, info, f, ##a) -#define nvkm_trace(s,f,a...) nvkm_printk((s), TRACE, info, f, ##a) +#define nvkm_debug(s,f,a...) nvkm_printk((s), DEBUG,dbg, f, ##a) +#define nvkm_trace(s,f,a...) nvkm_printk((s), TRACE,dbg, f, ##a) #define nvkm_spam(s,f,a...) nvkm_printk((s), SPAM,dbg, f, ##a) #define nvkm_error_ratelimited(s,f,a...) nvkm_printk((s), ERROR, err_ratelimited, f, ##a) -- 2.37.2
[PATCH v6 09/57] dyndbg: drop EXPORTed dynamic_debug_exec_queries
This exported fn is unused, and will not be needed. Lets dump it.
The export was added to let drm control pr_debugs, as part of using
them to avoid drm_debug_enabled overheads. But its better to just
implement the drm.debug bitmap interface, then its available for
everyone.
Fixes: a2d375eda771 ("dyndbg: refine export, rename to
dynamic_debug_exec_queries()")
Fixes: 4c0d77828d4f ("dyndbg: export ddebug_exec_queries")
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 9 -
lib/dynamic_debug.c | 29 -
2 files changed, 38 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index f30b01aa9fa4..8d9eec5f6d8b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -55,9 +55,6 @@ struct _ddebug {
#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
-/* exported for module authors to exercise >control */
-int dynamic_debug_exec_queries(const char *query, const char *modname);
-
int ddebug_add_module(struct _ddebug *tab, unsigned int n,
const char *modname);
extern int ddebug_remove_module(const char *mod_name);
@@ -221,12 +218,6 @@ static inline int ddebug_dyndbg_module_param_cb(char
*param, char *val,
rowsize, groupsize, buf, len, ascii); \
} while (0)
-static inline int dynamic_debug_exec_queries(const char *query, const char
*modname)
-{
- pr_warn("kernel not built with CONFIG_DYNAMIC_DEBUG_CORE\n");
- return 0;
-}
-
#endif /* !CONFIG_DYNAMIC_DEBUG_CORE */
#endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 5a849716220a..e96dc216463b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -558,35 +558,6 @@ static int ddebug_exec_queries(char *query, const char
*modname)
return nfound;
}
-/**
- * dynamic_debug_exec_queries - select and change dynamic-debug prints
- * @query: query-string described in admin-guide/dynamic-debug-howto
- * @modname: string containing module name, usually _name
- *
- * This uses the >/proc/dynamic_debug/control reader, allowing module
- * authors to modify their dynamic-debug callsites. The modname is
- * canonically struct module.mod_name, but can also be null or a
- * module-wildcard, for example: "drm*".
- */
-int dynamic_debug_exec_queries(const char *query, const char *modname)
-{
- int rc;
- char *qry; /* writable copy of query */
-
- if (!query) {
- pr_err("non-null query/command string expected\n");
- return -EINVAL;
- }
- qry = kstrndup(query, PAGE_SIZE, GFP_KERNEL);
- if (!qry)
- return -ENOMEM;
-
- rc = ddebug_exec_queries(qry, modname);
- kfree(qry);
- return rc;
-}
-EXPORT_SYMBOL_GPL(dynamic_debug_exec_queries);
-
#define PREFIX_SIZE 64
static int remaining(int wrote)
--
2.37.2
[PATCH v6 29/57] drm_print: prefer bare printk KERN_DEBUG on generic fn
drm_print.c calls pr_debug() just once, from __drm_printfn_debug(),
which is a generic/service fn. The callsite is compile-time enabled
by DEBUG in both DYNAMIC_DEBUG=y/n builds.
For dyndbg builds, reverting this callsite back to bare printk is
correcting a few anti-features:
1- callsite is generic, serves multiple drm users.
it is soft-wired on currently by #define DEBUG
could accidentally: #> echo -p > /proc/dynamic_debug/control
2- optional "decorations" by dyndbg are unhelpful/misleading here,
they describe only the generic site, not end users
IOW, 1,2 are unhelpful at best, and possibly confusing.
reverting yields a nominal data and text shrink:
textdata bss dec hex filename
462583 36604 54592 553779 87333 /kernel/drivers/gpu/drm/drm.ko
462515 36532 54592 553639 872a7 -dirty/kernel/drivers/gpu/drm/drm.ko
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/drm_print.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index cb203d63b286..ec477c44a784 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -23,8 +23,6 @@
* Rob Clark
*/
-#define DEBUG /* for pr_debug() */
-
#include
#include
@@ -185,7 +183,8 @@ EXPORT_SYMBOL(__drm_printfn_info);
void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf)
{
- pr_debug("%s %pV", p->prefix, vaf);
+ /* pr_debug callsite decorations are unhelpful here */
+ printk(KERN_DEBUG "%s %pV", p->prefix, vaf);
}
EXPORT_SYMBOL(__drm_printfn_debug);
--
2.37.2
[PATCH v6 30/57] drm_print: add _ddebug descriptor to drm_*dbg prototypes
upgrade the callchain to drm_dbg() and drm_dev_dbg(); add a struct
_ddebug ptr parameter to them, and supply that additional param by
replacing the '_no_desc' flavor of dyndbg Factory macro currently used
with the flavor that supplies the descriptor.
NOTES:
The descriptor gives these fns access to the decorator flags, but does
none of the dynamic-prefixing done by __dynamic_emit_prefix().
DRM already has conventions for logging/messaging; just tossing
optional decorations on top may not help. Instead, existing flags (or
new ones) can be used to make current conventions optional.
For CONFIG_DRM_USE_DYNAMIC_DEBUG=N, just pass null.
Note: desc->class_id is redundant with category parameter, but its
availability is dependent on desc.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/drm_print.c | 8 +---
include/drm/drm_print.h | 23 ---
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ec477c44a784..5b93c11895bb 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
@@ -278,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char
*level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
- const char *format, ...)
+void __drm_dev_dbg(struct _ddebug *desc, const struct device *dev,
+ enum drm_debug_category category, const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -287,6 +288,7 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
if (!__drm_debug_enabled(category))
return;
+ /* we know we are printing for either syslog, tracefs, or both */
va_start(args, format);
vaf.fmt = format;
vaf.va =
@@ -302,7 +304,7 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
}
EXPORT_SYMBOL(__drm_dev_dbg);
-void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(struct _ddebug *desc, enum drm_debug_category category, const
char *format, ...)
{
struct va_format vaf;
va_list args;
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 7631b5fb669e..46f14cfb401e 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -363,9 +363,10 @@ static inline bool drm_debug_enabled(enum
drm_debug_category category)
__printf(3, 4)
void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
-__printf(3, 4)
-void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
-const char *format, ...);
+struct _ddebug;
+__printf(4, 5)
+void __drm_dev_dbg(struct _ddebug *desc, const struct device *dev,
+ enum drm_debug_category category, const char *format, ...);
/**
* DRM_DEV_ERROR() - Error output.
@@ -415,11 +416,11 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
#define drm_dev_dbg(dev, cat, fmt, ...)\
- __drm_dev_dbg(dev, cat, fmt, ##__VA_ARGS__)
+ __drm_dev_dbg(NULL, dev, cat, fmt, ##__VA_ARGS__)
#else
#define drm_dev_dbg(dev, cat, fmt, ...)\
- _dynamic_func_call_no_desc(fmt, __drm_dev_dbg, \
- dev, cat, fmt, ##__VA_ARGS__)
+ _dynamic_func_call_cls(cat, fmt, __drm_dev_dbg, \
+ dev, cat, fmt, ##__VA_ARGS__)
#endif
/**
@@ -523,17 +524,17 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
* Prefer drm_device based logging over device or prink based logging.
*/
-__printf(2, 3)
-void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
+__printf(3, 4)
+void ___drm_dbg(struct _ddebug *desc, enum drm_debug_category category, const
char *format, ...);
__printf(1, 2)
void __drm_err(const char *format, ...);
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-#define __drm_dbg(fmt, ...)___drm_dbg(fmt, ##__VA_ARGS__)
+#define __drm_dbg(fmt, ...)___drm_dbg(NULL, fmt, ##__VA_ARGS__)
#else
#define __drm_dbg(cat, fmt, ...) \
- _dynamic_func_call_no_desc(fmt, ___drm_dbg, \
- cat, fmt, ##__VA_ARGS__)
+ _dynamic_func_call_cls(cat, fmt, ___drm_dbg,\
+ cat, fmt, ##__VA_ARGS__)
#endif
/* Macros to make printk easier */
--
2.37.2
[PATCH v6 18/57] doc-dyndbg: describe "class CLASS_NAME" query support
Add an explanation of the new "class CLASS_NAME" syntax and meaning, noting that the module determines if CLASS_NAME applies to it. Signed-off-by: Jim Cromie --- Documentation/admin-guide/dynamic-debug-howto.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index a89cfa083155..d8954ab05c7b 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -35,6 +35,7 @@ Dynamic debug has even more useful features: - line number (including ranges of line numbers) - module name - format string + - class name (as known/declared by each module) * Provides a debugfs control file: ``/dynamic_debug/control`` which can be read to display the complete list of known debug @@ -142,6 +143,7 @@ against. Possible keywords are::: 'file' string | 'module' string | 'format' string | +'class' string | 'line' line-range line-range ::= lineno | @@ -203,6 +205,15 @@ format format "nfsd: SETATTR" // a neater way to match a format with whitespace format 'nfsd: SETATTR' // yet another way to match a format with whitespace +class +The given class_name is validated against each module, which may +have declared a list of known class_names. If the class_name is +found for a module, callsite & class matching and adjustment +proceeds. Examples:: + + class DRM_UT_KMS# a DRM.debug category + class JUNK # silent non-match + line The given line number or range of line numbers is compared against the line number of each ``pr_debug()`` callsite. A single -- 2.37.2
[PATCH v6 10/57] dyndbg: cleanup auto vars in dynamic_debug_init
rework var-names for clarity, regularity
rename variables
- n to mod_sites - it counts sites-per-module
- entries to i - display only
- iter_start to iter_mod_start - marks start of each module's subrange
- modct to mod_ct - stylistic
new iterator var:
- site - cursor parallel to iter
1st step towards 'demotion' of iter->site, for removal later
treat vars as iters:
- drop init at top
init just above for-loop, in a textual block
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e96dc216463b..2e8ebef3bd0d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1059,11 +1059,10 @@ static int __init dynamic_debug_init_control(void)
static int __init dynamic_debug_init(void)
{
- struct _ddebug *iter, *iter_start;
- const char *modname = NULL;
+ struct _ddebug *iter, *iter_mod_start;
+ int ret, i, mod_sites, mod_ct;
+ const char *modname;
char *cmdline;
- int ret = 0;
- int n = 0, entries = 0, modct = 0;
if (&__start___dyndbg == &__stop___dyndbg) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
@@ -1074,30 +1073,32 @@ static int __init dynamic_debug_init(void)
ddebug_init_success = 1;
return 0;
}
- iter = __start___dyndbg;
+
+ iter = iter_mod_start = __start___dyndbg;
modname = iter->modname;
- iter_start = iter;
- for (; iter < __stop___dyndbg; iter++) {
- entries++;
+ i = mod_sites = mod_ct = 0;
+
+ for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
+
if (strcmp(modname, iter->modname)) {
- modct++;
- ret = ddebug_add_module(iter_start, n, modname);
+ mod_ct++;
+ ret = ddebug_add_module(iter_mod_start, mod_sites,
modname);
if (ret)
goto out_err;
- n = 0;
+
+ mod_sites = 0;
modname = iter->modname;
- iter_start = iter;
+ iter_mod_start = iter;
}
- n++;
}
- ret = ddebug_add_module(iter_start, n, modname);
+ ret = ddebug_add_module(iter_mod_start, mod_sites, modname);
if (ret)
goto out_err;
ddebug_init_success = 1;
vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in
__dyndbg section\n",
-entries, modct, (int)((modct * sizeof(struct ddebug_table)) >>
10),
-(int)((entries * sizeof(struct _ddebug)) >> 10));
+i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
+(int)((i * sizeof(struct _ddebug)) >> 10));
/* now that ddebug tables are loaded, process all boot args
* again to find and activate queries given in dyndbg params.
--
2.37.2
[PATCH v6 20/57] dyndbg: add drm.debug style (drm/parameters/debug) bitmap support
Add kernel_param_ops and callbacks to use a class-map to validate and
apply input to a sysfs-node, which allows users to control classes
defined in that class-map. This supports uses like:
echo 0x3 > /sys/module/drm/parameters/debug
IE add these:
- int param_set_dyndbg_classes()
- int param_get_dyndbg_classes()
- struct kernel_param_ops param_ops_dyndbg_classes
Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are
non-static and exported. This might be unnecessary here.
get/set use an augmented kernel_param; the arg refs a new struct
ddebug_class_param, which contains:
- A ptr to user's state-store; a union of for drm.debug,
for nouveau level debug. By ref'g the client's bit-state _var, code
coordinates with existing code (like drm_debug_enabled) which uses
it, so existing/remaining calls can work unchanged. Changing
drm.debug to a ulong allows use of BIT() etc.
- FLAGS: dyndbg.flags toggled by changes to bitmap. Usually just "p".
- MAP: a pointer to struct ddebug_classes_map, which maps those
class-names to .class_ids 0..N that the module is using. This
class-map is declared & initialized by DECLARE_DYNDBG_CLASSMAP.
- map-type: 4 enums DD_CLASS_TYPE_* select 2 input forms and 2 meanings.
numeric input:
DD_CLASS_TYPE_DISJOINT_BITS integer input, independent bits. ie: drm.debug
DD_CLASS_TYPE_LEVEL_NUM integer input, 0..N levels
classnames-list (comma separated) input:
DD_CLASS_TYPE_DISJOINT_NAMES each name affects a bit, others preserved
DD_CLASS_TYPE_LEVEL_NAMES names have level meanings, like kern_levels.h
_NAMES- comma-separated classnames (with optional +-)
_NUM - numeric input, 0-N expected
_BITS - numeric input, 0x1F bitmap form expected
_DISJOINT - bits are independent
_LEVEL- (x /sys/module/drm/parameters/debug_catnames
A naive _LEVEL_NAMES use, with one class, that sets all in the
class-map according to (x /sys/module/test_dynamic_debug/parameters/p_level_names
: problem solved
echo -L1 > /sys/module/test_dynamic_debug/parameters/p_level_names
Note this artifact:
: this is same as prev cmd (due to +/-)
echo L0 > /sys/module/test_dynamic_debug/parameters/p_level_names
: this is "even-more" off, but same wo __pr_debug_class(L0, "..").
echo -L0 > /sys/module/test_dynamic_debug/parameters/p_level_names
A stress-test/make-work usage (kid toggling a light switch):
echo +L7,L0,L7,L0,L7,L0,L7,L0,L7,L0,L7,L0,L7 \
> /sys/module/test_dynamic_debug/parameters/p_level_names
ddebug_apply_class_bitmap(): inside-fn, works on bitmaps, receives
new-bits, finds diffs vs client-bitvector holding "current" state,
and issues exec_query to commit the adjustment.
param_set_dyndbg_classes(): interface fn, sends _NAMES to
param_set_dyndbg_classnames() and returns, falls thru to handle _BITS,
_NUM internally, and calls ddebug_apply_class_bitmap(). Finishes by
updating state.
param_set_dyndbg_classnames(): handles classnames-list in loop, calls
ddebug_apply_class_bitmap for each, then updates state.
NOTES:
_LEVEL_ is overlay on _DISJOINT_; inputs are converted to a bitmask,
by the callbacks. IOW this is possible, and possibly confusing:
echo class V3 +p > control
echo class V1 -p > control
IMO thats ok, relative verbosity is an interface property.
_LEVEL_NUM maps still need class-names, even though the names are not
usable at the sysfs interface (unlike with _NAMES style). The names
are the only way to >control the classes.
- It must have a "V0" name,
something below "V1" to turn "V1" off.
__pr_debug_cls(V0,..) is printk, don't do that.
- "class names" is required at the >control interface.
- relative levels are not enforced at >control
_LEVEL_NAMES bear +/- signs, which alters the on-bit-pos by 1. IOW,
+L2 means L0,L1,L2, and -L2 means just L0,L1. This kinda spoils the
readback fidelity, since the L0 bit gets turned on by any use of any
L*, except "-L0".
All the interface uncertainty here pertains to the _NAMES features.
Nobody has actually asked for this, so its practical (if a little
tedious) to split it out.
Signed-off-by: Jim Cromie
---
. drop kp->mod->name as unneeded (build-dependent)
. param_set_dyndbg_classnames, not _class_strings
. DD_CLASS_TYPE_* name changes, per Jason
. callbacks:
extend comments on DD_CLASS_TYPE_* handling, flow notes
varname changes
. kdoc tweaks
. add stub macro: #define KP_NAME(kp) kp->name
later, add "$module." prefix when config doesn't do it
. s/struct ddebug_classes_bitmap_param/struct ddebug_class_param/
. fix levels state handling
v4 had trouble where level-type's state was fouled by conversion to
bitmap form given to apply-bitmap. fix by:
(simplify/clarify)
add old-bits, new-bits to sysfs-iface
use CLASSMAP_BITMASK more
in param_set_dyndbg_class{es,names}():
move state-saving inside switches, and tailor it to type.
this preserves lvl-state, vs -v4 which didnt.
I could "hack" in an offset, but the
[PATCH v6 26/57] drm-print.h: include dyndbg header
lkp robot told me:
>> drivers/gpu/drm/drm_ioc32.c:989:2:
error: call to undeclared function '_dynamic_func_call_cls';
ISO C99 and later do not support implicit function declarations
[-Wimplicit-function-declaration]
DRM_DEBUG("comm=\"%s\", pid=%d, dev=0x%lx, auth=%d, %s\n",
Since that macro is defined in drm_print.h, and under DRM_USE_DYN*=y
configs, invokes dyndbg-factory macros, include dynamic_debug.h from
there too, so that those configs have the definitions of all the
macros in the callchain.
This is done as a separate patch mostly to see how lkp sorts it.
Signed-off-by: Jim Cromie
---
include/drm/drm_print.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2d2cef76b5c1..f8bb3e7158c6 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
--
2.37.2
[PATCH v6 22/57] drm_print: condense enum drm_debug_category
enum drm_debug_category has 10 categories, but is initialized with
bitmasks which require 10 bits of underlying storage. By using
natural enumeration, and moving the BIT(cat) into drm_debug_enabled(),
the enum fits in 4 bits, allowing the category to be represented
directly in pr_debug callsites, via the ddebug.class_id field.
While this slightly pessimizes the bit-test in drm_debug_enabled(),
using dyndbg with JUMP_LABEL will avoid the function entirely.
NOTE: this change forecloses the possibility of doing:
drm_dbg(DRM_UT_CORE|DRM_UT_KMS, "weird 2-cat experiment")
but thats already strongly implied by the use of the enum itself; its
not a normal enum if it can be 2 values simultaneously.
Signed-off-by: Jim Cromie
---
include/drm/drm_print.h | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 22fabdeed297..b3b470440e46 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -279,49 +279,49 @@ enum drm_debug_category {
* @DRM_UT_CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c,
* drm_memory.c, ...
*/
- DRM_UT_CORE = 0x01,
+ DRM_UT_CORE,
/**
* @DRM_UT_DRIVER: Used in the vendor specific part of the driver: i915,
* radeon, ... macro.
*/
- DRM_UT_DRIVER = 0x02,
+ DRM_UT_DRIVER,
/**
* @DRM_UT_KMS: Used in the modesetting code.
*/
- DRM_UT_KMS = 0x04,
+ DRM_UT_KMS,
/**
* @DRM_UT_PRIME: Used in the prime code.
*/
- DRM_UT_PRIME= 0x08,
+ DRM_UT_PRIME,
/**
* @DRM_UT_ATOMIC: Used in the atomic code.
*/
- DRM_UT_ATOMIC = 0x10,
+ DRM_UT_ATOMIC,
/**
* @DRM_UT_VBL: Used for verbose debug message in the vblank code.
*/
- DRM_UT_VBL = 0x20,
+ DRM_UT_VBL,
/**
* @DRM_UT_STATE: Used for verbose atomic state debugging.
*/
- DRM_UT_STATE= 0x40,
+ DRM_UT_STATE,
/**
* @DRM_UT_LEASE: Used in the lease code.
*/
- DRM_UT_LEASE= 0x80,
+ DRM_UT_LEASE,
/**
* @DRM_UT_DP: Used in the DP code.
*/
- DRM_UT_DP = 0x100,
+ DRM_UT_DP,
/**
* @DRM_UT_DRMRES: Used in the drm managed resources code.
*/
- DRM_UT_DRMRES = 0x200,
+ DRM_UT_DRMRES
};
static inline bool drm_debug_enabled(enum drm_debug_category category)
{
- return unlikely(__drm_debug & category);
+ return unlikely(__drm_debug & BIT(category));
}
/*
--
2.37.2
[PATCH v6 27/57] drm-print: add drm_dbg_driver to improve namespace symmetry
drm_print defines all of these:
drm_dbg_{core,kms,prime,atomic,vbl,lease,_dp,_drmres}
but not drm_dbg_driver itself, since it was the original drm_dbg.
To improve namespace symmetry, change the drm_dbg defn to
drm_dbg_driver, and redef grandfathered name to symmetric one.
This will help with nouveau, which uses its own stack of macros to
construct calls to dev_info, dev_dbg, etc, for which adaptation means
drm_dbg_##driver constructs.
Signed-off-by: Jim Cromie
---
include/drm/drm_print.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index f8bb3e7158c6..dfdd81c3287c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -468,7 +468,7 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
#define drm_dbg_core(drm, fmt, ...)\
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_CORE, fmt, ##__VA_ARGS__)
-#define drm_dbg(drm, fmt, ...) \
+#define drm_dbg_driver(drm, fmt, ...)
\
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRIVER, fmt,
##__VA_ARGS__)
#define drm_dbg_kms(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__)
@@ -487,6 +487,7 @@ void __drm_dev_dbg(const struct device *dev, enum
drm_debug_category category,
#define drm_dbg_drmres(drm, fmt, ...) \
drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRMRES, fmt,
##__VA_ARGS__)
+#define drm_dbg(drm, fmt, ...) drm_dbg_driver(drm, fmt, ##__VA_ARGS__)
/*
* printk based logging
--
2.37.2
[PATCH v6 19/57] doc-dyndbg: edit dynamic-debug-howto for brevity, audience
Rework/modernize docs: - use /proc/dynamic_debug/control in examples its *always* there (when dyndbg is config'd), even when is not. drop talk, its a distraction here. - alias ddcmd='echo $* > /proc/dynamic_debug/control focus on args: declutter, hide boilerplate, make pwd independent. - swap sections: Viewing before Controlling. control file as Catalog. - focus on use by a system administrator add an alias to make examples more readable drop grep-101 lessons, admins know this. - use init/main.c as 1st example, thread it thru doc where useful. everybodys kernel boots, runs these. - add *prdbg* api section to the bottom of the file, its for developers more than admins. move list of api functions there. - simplify - drop extra words, phrases, sentences. - add "decorator" flags line to unify "prefix", trim fmlt descriptions CC: linux-...@vger.kernel.org Signed-off-by: Jim Cromie --- fixup-doc: trailing colons for block headers, trim fedora numbers. Bagas --- .../admin-guide/dynamic-debug-howto.rst | 235 +- 1 file changed, 117 insertions(+), 118 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index d8954ab05c7b..faa22f77847a 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -5,30 +5,19 @@ Dynamic debug Introduction -This document describes how to use the dynamic debug (dyndbg) feature. +Dynamic debug allows you to dynamically enable/disable kernel +debug-print code to obtain additional kernel information. -Dynamic debug is designed to allow you to dynamically enable/disable -kernel code to obtain additional kernel information. Currently, if -``CONFIG_DYNAMIC_DEBUG`` is set, then all ``pr_debug()``/``dev_dbg()`` and -``print_hex_dump_debug()``/``print_hex_dump_bytes()`` calls can be dynamically -enabled per-callsite. +If ``/proc/dynamic_debug/control`` exists, your kernel has dynamic +debug. You'll need root access (sudo su) to use this. -If you do not want to enable dynamic debug globally (i.e. in some embedded -system), you may set ``CONFIG_DYNAMIC_DEBUG_CORE`` as basic support of dynamic -debug and add ``ccflags := -DDYNAMIC_DEBUG_MODULE`` into the Makefile of any -modules which you'd like to dynamically debug later. +Dynamic debug provides: -If ``CONFIG_DYNAMIC_DEBUG`` is not set, ``print_hex_dump_debug()`` is just -shortcut for ``print_hex_dump(KERN_DEBUG)``. + * a Catalog of all *prdbgs* in your kernel. + ``cat /proc/dynamic_debug/control`` to see them. -For ``print_hex_dump_debug()``/``print_hex_dump_bytes()``, format string is -its ``prefix_str`` argument, if it is constant string; or ``hexdump`` -in case ``prefix_str`` is built dynamically. - -Dynamic debug has even more useful features: - - * Simple query language allows turning on and off debugging - statements by matching any combination of 0 or 1 of: + * a Simple query/command language to alter *prdbgs* by selecting on + any combination of 0 or 1 of: - source filename - function name @@ -37,107 +26,88 @@ Dynamic debug has even more useful features: - format string - class name (as known/declared by each module) - * Provides a debugfs control file: ``/dynamic_debug/control`` - which can be read to display the complete list of known debug - statements, to help guide you - -Controlling dynamic debug Behaviour -=== - -The behaviour of ``pr_debug()``/``dev_dbg()`` are controlled via writing to a -control file in the 'debugfs' filesystem. Thus, you must first mount -the debugfs filesystem, in order to make use of this feature. -Subsequently, we refer to the control file as: -``/dynamic_debug/control``. For example, if you want to enable -printing from source file ``svcsock.c``, line 1603 you simply do:: - - nullarbor:~ # echo 'file svcsock.c line 1603 +p' > - /dynamic_debug/control - -If you make a mistake with the syntax, the write will fail thus:: - - nullarbor:~ # echo 'file svcsock.c wtf 1 +p' > - /dynamic_debug/control - -bash: echo: write error: Invalid argument - -Note, for systems without 'debugfs' enabled, the control file can be -found in ``/proc/dynamic_debug/control``. - Viewing Dynamic Debug Behaviour === -You can view the currently configured behaviour of all the debug -statements via:: +You can view the currently configured behaviour in the *prdbg* catalog:: - nullarbor:~ # cat /dynamic_debug/control + :#> head -n7 /proc/dynamic_debug/control # filename:lineno [module]function flags format - net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012" - net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline : %d\012" - net/sunrpc/svc_rdma.c:340
[PATCH v6 07/57] dyndbg: let query-modname override actual module name
dyndbg's control-parser: ddebug_parse_query(), requires that search
terms: module, func, file, lineno, are used only once in a query; a
thing cannot be named both foo and bar.
The cited commit added an overriding module modname, taken from the
module loader, which is authoritative. So it set query.module 1st,
which disallowed its use in the query-string.
But now, its useful to allow a module-load to enable classes across a
whole (or part of) a subsystem at once.
# enable (dynamic-debug in) drm only
modprobe drm dyndbg="class DRM_UT_CORE +p"
# get drm_helper too
modprobe drm dyndbg="class DRM_UT_CORE module drm* +p"
# get everything that knows DRM_UT_CORE
modprobe drm dyndbg="class DRM_UT_CORE module * +p"
# also for boot-args:
drm.dyndbg="class DRM_UT_CORE module * +p"
So convert the override into a default, by filling it only when/after
the query-string omitted the module.
NB: the query class FOO handling is forthcoming.
Fixes: 8e59b5cfb9a6 dynamic_debug: add modname arg to exec_query callchain
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e5cbe603000c..5a849716220a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -385,10 +385,6 @@ static int ddebug_parse_query(char *words[], int nwords,
return -EINVAL;
}
- if (modname)
- /* support $modname.dyndbg= */
- query->module = modname;
-
for (i = 0; i < nwords; i += 2) {
char *keyword = words[i];
char *arg = words[i+1];
@@ -429,6 +425,13 @@ static int ddebug_parse_query(char *words[], int nwords,
if (rc)
return rc;
}
+ if (!query->module && modname)
+ /*
+* support $modname.dyndbg=, when
+* not given in the query itself
+*/
+ query->module = modname;
+
vpr_info_dq(query, "parsed");
return 0;
}
--
2.37.2
[PATCH v6 17/57] dyndbg: validate class FOO by checking with module
Add module-to-class validation:
#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
If a query has "class FOO", then ddebug_find_valid_class(), called
from ddebug_change(), requires that FOO is known to module X,
otherwize the query is skipped entirely for X. This protects each
module's class-space, other than the default:31.
The authors' choice of FOO is highly selective, giving isolation
and/or coordinated sharing of FOOs. For example, only DRM modules
should know and respond to DRM_UT_KMS.
So this, combined with module's opt-in declaration of known classes,
effectively privatizes the .class_id space for each module (or
coordinated set of modules).
Notes:
For all "class FOO" queries, ddebug_find_valid_class() is called, it
returns the map matching the query, and sets valid_class via an
*outvar).
If no "class FOO" is supplied, valid_class = _CLASS_DFLT. This
insures that legacy queries do not trample on new class'd callsites,
as they get added.
Also add a new column to control-file output, displaying non-default
class-name (when found) or the "unknown _id:", if it has not been
(correctly) declared with one of the declarator macros.
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 76 -
1 file changed, 68 insertions(+), 8 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b71efd0b491d..db96ded78c3f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -56,6 +56,7 @@ struct ddebug_query {
const char *module;
const char *function;
const char *format;
+ const char *class_string;
unsigned int first_lineno, last_lineno;
};
@@ -136,15 +137,33 @@ static void vpr_info_dq(const struct ddebug_query *query,
const char *msg)
fmtlen--;
}
- v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\"
lineno=%u-%u\n",
-msg,
-query->function ?: "",
-query->filename ?: "",
-query->module ?: "",
-fmtlen, query->format ?: "",
-query->first_lineno, query->last_lineno);
+ v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\"
lineno=%u-%u class=%s\n",
+ msg,
+ query->function ?: "",
+ query->filename ?: "",
+ query->module ?: "",
+ fmtlen, query->format ?: "",
+ query->first_lineno, query->last_lineno, query->class_string);
}
+static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table
const *dt,
+ const char
*class_string, int *class_id)
+{
+ struct ddebug_class_map *map;
+ int idx;
+
+ list_for_each_entry(map, >maps, link) {
+ idx = match_string(map->class_names, map->length, class_string);
+ if (idx >= 0) {
+ *class_id = idx + map->base;
+ return map;
+ }
+ }
+ *class_id = -ENOENT;
+ return NULL;
+}
+
+#define __outvar /* filled by callee */
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching
@@ -159,6 +178,8 @@ static int ddebug_change(const struct ddebug_query *query,
unsigned int newflags;
unsigned int nfound = 0;
struct flagsbuf fbuf, nbuf;
+ struct ddebug_class_map *map = NULL;
+ int __outvar valid_class;
/* search for matching ddebugs */
mutex_lock(_lock);
@@ -169,9 +190,22 @@ static int ddebug_change(const struct ddebug_query *query,
!match_wildcard(query->module, dt->mod_name))
continue;
+ if (query->class_string) {
+ map = ddebug_find_valid_class(dt, query->class_string,
_class);
+ if (!map)
+ continue;
+ } else {
+ /* constrain query, do not touch class'd callsites */
+ valid_class = _DPRINTK_CLASS_DFLT;
+ }
+
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = >ddebugs[i];
+ /* match site against query-class */
+ if (dp->class_id != valid_class)
+ continue;
+
/* match against the source filename */
if (query->filename &&
!match_wildcard(query->filename, dp->filename) &&
@@ -420,6 +454,8 @@ static int ddebug_parse_query(char *words[], int nwords,
} else if (!strcmp(keyword, "line")) {
if (parse_linerange(query, arg))
return -EINVAL;
+ } else if (!strcmp(keyword, "class")) {
+
[PATCH v6 14/57] dyndbg: add DECLARE_DYNDBG_CLASSMAP macro
Using DECLARE_DYNDBG_CLASSMAP, modules can declare up to 31 classnames.
By doing so, they authorize dyndbg to manipulate class'd prdbgs (ie:
__pr_debug_cls, and soon drm_*dbg), ala::
:#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
The macro declares and initializes a static struct ddebug_class_map::
- maps approved class-names to class_ids used in module,
by array order. forex: DRM_UT_*
- class-name vals allow validation of "class FOO" queries
using macro is opt-in
- enum class_map_type - determines interface, behavior
Each module has its own class-type and class_id space, and only known
class-names will be authorized for a manipulation. Only DRM modules
should know and respont to this:
:#> echo class DRM_UT_CORE +p > control # across all modules
pr_debugs (with default class_id) are still controllable as before.
DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, classes...) is::
_var: name of the static struct var. user passes to module_param_cb()
if they want a sysfs node.
_maptype: this is hard-coded to DD_CLASS_TYPE_DISJOINT_BITS for now.
_base: usually 0, it allows splitting 31 classes into subranges, so
that multiple classes / sysfs-nodes can share the module's
class-id space.
classes: list of class_name strings, these are mapped to class-ids
starting at _base. This class-names list must have a
corresponding ENUM, with SYMBOLS that match the literals,
and 1st enum val = _base.
enum class_map_type has 4 values, on 2 factors::
- classes are disjoint/independent vs relative/xcontrol interface
doesn't enforce the LEVELS relationship, so you could confusingly have
V3 enabled, but V1 disabled. OTOH, the control iface already allows
infinite tweaking of the underlying callsites; sysfs node readback can
only tell the user what they previously wrote.
2. All dyndbg >control reduces to a query/command, includes +/-, which
is at-root a kernel patching operation with +/- semantics. And the
_NAMES handling exposes it to the user, making it API-adjacent.
And its not just >control where +/- gets used (which is settled), the
new place is with sysfs-nodes exposing _*_NAMES classes, and here its
subtly different.
_DISJOINT_NAMES: is simple, independent
_LEVEL_NAMES: masks-on bits 0 .. N-1, N..max off
# turn on L3,L2,L1 others off
echo +L3 > /sys/module/test_dynamic_debug/parameters/p_level_names
# turn on L2,L1 others off
echo -L3 > /sys/module/test_dynamic_debug/parameters/p_level_names
IOW, the - changes the threshold-on bitpos by 1.
Alternatively, we could treat the +/- as half-duplex, where -L3 turns
off L>2 (and ignores L1), and +L2 would turn on L<=2 (and ignore others).
Signed-off-by: Jim Cromie
---
. revised DD_CLASS_TYPE_{DISJOINT,LEVEL}_* names
. reorder-enum - _NAMES feature is extra.
---
include/linux/dynamic_debug.h | 55 +++
1 file changed, 55 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 3c9690da44d9..98dbf1d49984 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -56,6 +56,61 @@ struct _ddebug {
#endif
} __attribute__((aligned(8)));
+enum class_map_type {
+ DD_CLASS_TYPE_DISJOINT_BITS,
+ /**
+* DD_CLASS_TYPE_DISJOINT_BITS: classes are independent, one per bit.
+* expecting hex input. Built for drm.debug, basis for other types.
+*/
+ DD_CLASS_TYPE_LEVEL_NUM,
+ /**
+* DD_CLASS_TYPE_LEVEL_NUM: input is numeric level, 0-N.
+* N turns on just bits N-1 .. 0, so N=0 turns all bits off.
+*/
+ DD_CLASS_TYPE_DISJOINT_NAMES,
+ /**
+* DD_CLASS_TYPE_DISJOINT_NAMES: input is a CSV of [+-]CLASS_NAMES,
+* classes are independent, like _DISJOINT_BITS.
+*/
+ DD_CLASS_TYPE_LEVEL_NAMES,
+ /**
+* DD_CLASS_TYPE_LEVEL_NAMES: input is a CSV of [+-]CLASS_NAMES,
+* intended for names like: INFO,DEBUG,TRACE, with a module prefix
+* avoid EMERG,ALERT,CRIT,ERR,WARNING: they're not debug
+*/
+};
+
+struct ddebug_class_map {
+ struct list_head link;
+ struct module *mod;
+ const char *mod_name; /* needed for builtins */
+ const char **class_names;
+ const int length;
+ const int base; /* index of 1st .class_id, allows split/shared
space */
+ enum class_map_type map_type;
+};
+
+/**
+ * DECLARE_DYNDBG_CLASSMAP - declare classnames known by a module
+ * @_var: a struct ddebug_class_map, passed to module_param_cb
+ * @_type: enum class_map_type, chooses bits/verbose, numeric/symbolic
+ * @_base: offset of 1st class-name. splits .class_id space
+ * @classes: class-names used to control class'd prdbgs
+ */
+#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...)\
+ static const char *_var##_classnames[] = { __VA_ARGS__ }; \
+ static struct ddebug_class_map
[PATCH v6 23/57] drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers.
Use DECLARE_DYNDBG_CLASSMAP across DRM:
- in .c files, since macro defines/initializes a record
- in drivers, $mod_{drv,drm,param}.c
ie where param setup is done, since a classmap is param related
- in drm/drm_print.c
since existing __drm_debug param is defined there,
and we ifdef it, and provide an elaborated alternative.
- in drm_*_helper modules:
dp/drm_dp - 1st item in makefile target
drivers/gpu/drm/drm_crtc_helper.c - random pick iirc.
Since these modules all use identical CLASSMAP declarations (ie: names
and .class_id's) they will all respond together to "class DRM_UT_*"
query-commands:
:#> echo class DRM_UT_KMS +p > /proc/dynamic_debug/control
NOTES:
This changes __drm_debug from int to ulong, so BIT() is usable on it.
DRM's enum drm_debug_category values need to sync with the index of
their respective class-names here. Then .class_id == category, and
dyndbg's class FOO mechanisms will enable drm_dbg(DRM_UT_KMS, ...).
Though DRM needs consistent categories across all modules, thats not
generally needed; modules X and Y could define FOO differently (ie a
different NAME => class_id mapping), changes are made according to
each module's private class-map.
No callsites are actually selected by this patch, since none are
class'd yet.
Signed-off-by: Jim Cromie
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 14 +
drivers/gpu/drm/display/drm_dp_helper.c | 13
drivers/gpu/drm/drm_crtc_helper.c | 13
drivers/gpu/drm/drm_print.c | 27 +++--
drivers/gpu/drm/i915/i915_params.c | 12 +++
drivers/gpu/drm/nouveau/nouveau_drm.c | 13
include/drm/drm_print.h | 3 ++-
7 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index de7144b06e93..97e184f44a52 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -38,6 +38,8 @@
#include
#include
#include
+#include
+#include
#include "amdgpu.h"
#include "amdgpu_irq.h"
@@ -185,6 +187,18 @@ int amdgpu_vcnfw_log;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
.delayed_reset_work = __DELAYED_WORK_INITIALIZER(
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
b/drivers/gpu/drm/display/drm_dp_helper.c
index 92990a3d577a..cbb9c4d6d8f2 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
@@ -40,6 +41,18 @@
#include "drm_dp_helper_internal.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
struct dp_aux_backlight {
struct backlight_device *base;
struct drm_dp_aux *aux;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index 457448cc60f7..7d86020b5244 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
@@ -50,6 +51,18 @@
#include "drm_crtc_helper_internal.h"
+DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
+ "DRM_UT_CORE",
+ "DRM_UT_DRIVER",
+ "DRM_UT_KMS",
+ "DRM_UT_PRIME",
+ "DRM_UT_ATOMIC",
+ "DRM_UT_VBL",
+ "DRM_UT_STATE",
+ "DRM_UT_LEASE",
+ "DRM_UT_DP",
+ "DRM_UT_DRMRES");
+
/**
* DOC: overview
*
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index f783d4963d4b..ec32df35a3e3 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -40,7 +40,7 @@
* __drm_debug: Enable debug output.
* Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
*/
[PATCH v6 11/57] dyndbg: gather __dyndbg[] state into struct _ddebug_info
This new struct composes the linker provided (vector,len) section,
and provides a place to add other __dyndbg[] state-data later:
descs - the vector of descriptors in __dyndbg section.
num_descs - length of the data/section.
Use it, in several different ways, as follows:
In lib/dynamic_debug.c:
ddebug_add_module(): Alter params-list, replacing 2 args (array,index)
with a struct _ddebug_info * containing them both, with room for
expansion. This helps future-proof the function prototype against the
looming addition of class-map info into the dyndbg-state, by providing
a place to add more member fields later.
NB: later add static struct _ddebug_info builtins_state declaration,
not needed yet.
ddebug_add_module() is called in 2 contexts:
In dynamic_debug_init(), declare, init a struct _ddebug_info di
auto-var to use as a cursor. Then iterate over the prdbg blocks of
the builtin modules, and update the di cursor before calling
_add_module for each.
Its called from kernel/module/main.c:load_info() for each loaded
module:
In internal.h, alter struct load_info, replacing the dyndbg array,len
fields with an embedded _ddebug_info containing them both; and
populate its members in find_module_sections().
The 2 calling contexts differ in that _init deals with contiguous
subranges of __dyndbgs[] section, packed together, while loadable
modules are added one at a time.
So rename ddebug_add_module() into outer/__inner fns, call __inner
from _init, and provide the offset into the builtin __dyndbgs[] where
the module's prdbgs reside. The cursor provides start, len of the
subrange for each. The offset will be used later to pack the results
of builtin __dyndbg_sites[] de-duplication, and is 0 and unneeded for
loadable modules,
Note:
kernel/module/main.c includes for struct
_ddeubg_info. This might be prone to include loops, since its also
included by printk.h. Nothing has broken in robot-land on this.
cc: Luis Chamberlain
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 13 +++-
kernel/module/internal.h | 4 ++--
kernel/module/main.c | 18
lib/dynamic_debug.c | 40 +++
4 files changed, 50 insertions(+), 25 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 8d9eec5f6d8b..6a2001250da1 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -51,12 +51,16 @@ struct _ddebug {
#endif
} __attribute__((aligned(8)));
-
+/* encapsulate linker provided built-in (or module) dyndbg data */
+struct _ddebug_info {
+ struct _ddebug *descs;
+ unsigned int num_descs;
+};
#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
-int ddebug_add_module(struct _ddebug *tab, unsigned int n,
- const char *modname);
+int ddebug_add_module(struct _ddebug_info *dyndbg, const char *modname);
+
extern int ddebug_remove_module(const char *mod_name);
extern __printf(2, 3)
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...);
@@ -184,8 +188,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#include
#include
-static inline int ddebug_add_module(struct _ddebug *tab, unsigned int n,
- const char *modname)
+static inline int ddebug_add_module(struct _ddebug_info *dinfo, const char
*modname)
{
return 0;
}
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 680d980a4fb2..2e2bf236f558 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -53,6 +53,7 @@ extern const struct kernel_symbol __stop___ksymtab_gpl[];
extern const s32 __start___kcrctab[];
extern const s32 __start___kcrctab_gpl[];
+#include
struct load_info {
const char *name;
/* pointer to module in temporary copy, freed at end of load_module() */
@@ -62,8 +63,7 @@ struct load_info {
Elf_Shdr *sechdrs;
char *secstrings, *strtab;
unsigned long symoffs, stroffs, init_typeoffs, core_typeoffs;
- struct _ddebug *debug;
- unsigned int num_debug;
+ struct _ddebug_info dyndbg;
bool sig_ok;
#ifdef CONFIG_KALLSYMS
unsigned long mod_kallsyms_init_off;
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a4e4d84b6f4e..4c20bc3ff203 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1598,16 +1598,16 @@ static void free_modinfo(struct module *mod)
}
}
-static void dynamic_debug_setup(struct module *mod, struct _ddebug *debug,
unsigned int num)
+static void dynamic_debug_setup(struct module *mod, struct _ddebug_info
*dyndbg)
{
- if (!debug)
+ if (!dyndbg->num_descs)
return;
- ddebug_add_module(debug, num, mod->name);
+ ddebug_add_module(dyndbg, mod->name);
}
-static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)
+static void dynamic_debug_remove(struct module *mod, struct _ddebug_info
*dyndbg)
{
-
[PATCH v6 06/57] dyndbg: use ESCAPE_SPACE for cat control
`cat control` currently does octal escape, so '\n' becomes "\012". Change this to display as "\n" instead, which reads much cleaner. :#> head -n7 /proc/dynamic_debug/control # filename:lineno [module]function flags format init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\n" init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\n" init/main.c:1424 [main]run_init_process =_ " with arguments:\n" init/main.c:1426 [main]run_init_process =_ "%s\n" init/main.c:1427 [main]run_init_process =_ " with environment:\n" init/main.c:1429 [main]run_init_process =_ "%s\n" Acked-by: Jason Baron Signed-off-by: Jim Cromie --- lib/dynamic_debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 8ff11977b8bd..e5cbe603000c 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -900,7 +900,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p) trim_prefix(dp->filename), dp->lineno, iter->table->mod_name, dp->function, ddebug_describe_flags(dp->flags, )); - seq_escape(m, dp->format, "\t\r\n\""); + seq_escape_str(m, dp->format, ESCAPE_SPACE, "\t\r\n\""); seq_puts(m, "\"\n"); return 0; -- 2.37.2
[PATCH v6 12/57] dyndbg: add class_id to pr_debug callsites
DRM issues ~10 exclusive categories of debug messages; to represent
this directly in dyndbg, add a new 6-bit field: struct _ddebug.class_id.
This gives us 64 classes, which should be more than enough.
#> echo 0x012345678 > /sys/module/drm/parameters/debug
All existing callsites are initialized with _DPRINTK_CLASS_DFLT, which
is 2^6-1. This reserves 0-62 for use in new categorized/class'd
pr_debugs, which fits perfectly with natural enums (ints: 0..N).
Thats done by extending the init macro: DEFINE_DYNAMIC_DEBUG_METADATA()
with _CLS(cls, ...) suffix, and redefing old name using extended name.
Then extend the factory macro callchain with _cls() versions to provide
the callsite.class_id, with old-names passing _DPRINTK_CLASS_DFLT.
This sets us up to create class'd prdebug callsites (class'd callsites
are those with .class_id != _DPRINTK_CLASS_DFLT).
No behavior change.
cc: Rasmus Villemoes
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 71 +++
1 file changed, 55 insertions(+), 16 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 6a2001250da1..633f4e463766 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -6,6 +6,8 @@
#include
#endif
+#include
+
/*
* An instance of this structure is created in a special
* ELF section at every dynamic debug callsite. At runtime,
@@ -21,6 +23,9 @@ struct _ddebug {
const char *filename;
const char *format;
unsigned int lineno:18;
+#define CLS_BITS 6
+ unsigned int class_id:CLS_BITS;
+#define _DPRINTK_CLASS_DFLT((1 << CLS_BITS) - 1)
/*
* The flags field controls the behaviour at the callsite.
* The bits here are changed dynamically when the user
@@ -88,7 +93,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
const struct ib_device *ibdev,
const char *fmt, ...);
-#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
+#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \
static struct _ddebug __aligned(8) \
__section("__dyndbg") name = { \
.modname = KBUILD_MODNAME, \
@@ -97,8 +102,14 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
.format = (fmt),\
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT,\
+ .class_id = cls,\
_DPRINTK_KEY_INIT \
- }
+ }; \
+ BUILD_BUG_ON_MSG(cls > _DPRINTK_CLASS_DFLT, \
+"classid value overflow")
+
+#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
+ DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_CLASS_DFLT, fmt)
#ifdef CONFIG_JUMP_LABEL
@@ -129,17 +140,34 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#endif /* CONFIG_JUMP_LABEL */
-#define __dynamic_func_call(id, fmt, func, ...) do { \
- DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \
- if (DYNAMIC_DEBUG_BRANCH(id)) \
- func(, ##__VA_ARGS__); \
-} while (0)
-
-#define __dynamic_func_call_no_desc(id, fmt, func, ...) do { \
- DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \
+/*
+ * Factory macros: ($prefix)dynamic_func_call($suffix)
+ *
+ * Lower layer (with __ prefix) gets the callsite metadata, and wraps
+ * the func inside a debug-branch/static-key construct. Upper layer
+ * (with _ prefix) does the UNIQUE_ID once, so that lower can ref the
+ * name/label multiple times, and tie the elements together.
+ * Multiple flavors:
+ * (|_cls):adds in _DPRINT_CLASS_DFLT as needed
+ * (|_no_desc):former gets callsite descriptor as 1st arg (for prdbgs)
+ */
+#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);\
if (DYNAMIC_DEBUG_BRANCH(id)) \
- func(__VA_ARGS__); \
+ func(, ##__VA_ARGS__); \
} while (0)
+#define __dynamic_func_call(id, fmt, func, ...)
\
+ __dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt, \
+ func, ##__VA_ARGS__)
+
+#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do { \
+ DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt);\
+ if (DYNAMIC_DEBUG_BRANCH(id)) \
+ func(__VA_ARGS__); \
+} while (0)
+#define __dynamic_func_call_no_desc(id, fmt, func, ...)
[PATCH v6 13/57] dyndbg: add __pr_debug_cls for testing
For selftest purposes, add __pr_debug_cls(class, fmt, ...)
I didn't think we'd need to define this, since DRM effectively has it
already in drm_dbg, drm_devdbg. But test_dynamic_debug needs it in
order to demonstrate all the moving parts.
Note the __ prefix; its not intended for general use, at least until a
need emerges. ISTM the drm.debug model (macro wrappers inserting enum
const 1st arg) is the baseline approach.
That said, nouveau might want it for easy use in its debug macros. TBD.
NB: it does require a builtin-constant class, __pr_debug_cls(i++, ...)
is disallowed by compiler.
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 633f4e463766..3c9690da44d9 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -221,6 +221,13 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
KERN_DEBUG, prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii)
+/* for test only, generally expect drm.debug style macro wrappers */
+#define __pr_debug_cls(cls, fmt, ...) do { \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(cls),\
+"expecting constant class int/enum"); \
+ dynamic_pr_debug_cls(cls, fmt, ##__VA_ARGS__); \
+ } while (0)
+
#else /* !CONFIG_DYNAMIC_DEBUG_CORE */
#include
--
2.37.2
[PATCH v6 08/57] dyndbg: add test_dynamic_debug module
Provide a simple module to allow testing DYNAMIC_DEBUG behavior. It
calls do_prints() from module-init, and with a sysfs-node.
dmesg -C
dmesg -w &
modprobe test_dynamic_debug dyndbg=+p
echo 1 > /sys/module/dynamic_debug/parameters/verbose
cat /sys/module/test_dynamic_debug/parameters/do_prints
echo module test_dynamic_debug +mftl > /proc/dynamic_debug/control
echo junk > /sys/module/test_dynamic_debug/parameters/do_prints
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
MAINTAINERS | 2 ++
lib/Kconfig.debug| 10 ++
lib/Makefile | 1 +
lib/test_dynamic_debug.c | 70
4 files changed, 83 insertions(+)
create mode 100644 lib/test_dynamic_debug.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 41762df1be0c..c4465981df31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7222,6 +7222,8 @@ M:Jason Baron
S: Maintained
F: include/linux/dynamic_debug.h
F: lib/dynamic_debug.c
+M: Jim Cromie
+F: lib/test_dynamic_debug.c
DYNAMIC INTERRUPT MODERATION
M: Tal Gilboa
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 183c37502119..5bf7897d9095 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2587,6 +2587,16 @@ config TEST_STATIC_KEYS
If unsure, say N.
+config TEST_DYNAMIC_DEBUG
+ tristate "Test DYNAMIC_DEBUG"
+ depends on DYNAMIC_DEBUG
+ help
+ This module registers a tracer callback to count enabled
+ pr_debugs in a 'do_debugging' function, then alters their
+ enablements, calls the function, and compares counts.
+
+ If unsure, say N.
+
config TEST_KMOD
tristate "kmod stress tester"
depends on m
diff --git a/lib/Makefile b/lib/Makefile
index f0a855eed891..c893114029b8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_TEST_SORT) += test_sort.o
obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
+obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
obj-$(CONFIG_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
new file mode 100644
index ..ba3882ca3e48
--- /dev/null
+++ b/lib/test_dynamic_debug.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kernel module for testing dynamic_debug
+ *
+ * Authors:
+ * Jim Cromie
+ */
+
+#define pr_fmt(fmt) "test_dd: " fmt
+
+#include
+
+static void do_prints(void); /* device under test */
+
+/* run tests by reading or writing sysfs node */
+
+static int param_set_do_prints(const char *instr, const struct kernel_param
*kp)
+{
+ do_prints();
+ return 0;
+}
+
+static int param_get_do_prints(char *buffer, const struct kernel_param *kp)
+{
+ do_prints();
+ return scnprintf(buffer, PAGE_SIZE, "did do_prints\n");
+}
+
+static const struct kernel_param_ops param_ops_do_prints = {
+ .set = param_set_do_prints,
+ .get = param_get_do_prints,
+};
+
+module_param_cb(do_prints, _ops_do_prints, NULL, 0600);
+
+static void do_alpha(void)
+{
+ pr_debug("do alpha\n");
+}
+static void do_beta(void)
+{
+ pr_debug("do beta\n");
+}
+
+static void do_prints(void)
+{
+ do_alpha();
+ do_beta();
+}
+
+static int __init test_dynamic_debug_init(void)
+{
+ pr_debug("init start\n");
+
+ do_prints();
+
+ pr_debug("init done\n");
+ return 0;
+}
+
+static void __exit test_dynamic_debug_exit(void)
+{
+ pr_debug("exiting\n");
+}
+
+module_init(test_dynamic_debug_init);
+module_exit(test_dynamic_debug_exit);
+
+MODULE_AUTHOR("Jim Cromie ");
+MODULE_LICENSE("GPL");
--
2.37.2
[PATCH v6 04/57] dyndbg: reverse module walk in cat control
/proc/dynamic_debug/control walks the prdbg catalog in "reverse",
fix this by adding new ddebug_tables to tail of list.
This puts init/main.c entries 1st, which looks more than coincidental.
no functional changes.
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8faf584f2f4b..7fb99492c16f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -970,7 +970,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
dt->ddebugs = tab;
mutex_lock(_lock);
- list_add(>link, _tables);
+ list_add_tail(>link, _tables);
mutex_unlock(_lock);
vpr_info("%3u debug prints in module %s\n", n, dt->mod_name);
--
2.37.2
[PATCH v6 05/57] dyndbg: reverse module.callsite walk in cat control
Walk the module's vector of callsites backwards; ie N..0. This
"corrects" the backwards appearance of a module's prdbg vector when
walked 0..N. I think this is due to linker mechanics, which I'm
inclined to treat as immutable, and the order is fixable in display.
No functional changes.
Combined with previous commit, which reversed tables-list, we get:
:#> head -n7 /proc/dynamic_debug/control
# filename:lineno [module]function flags format
init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\012"
init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\012"
init/main.c:1424 [main]run_init_process =_ " with arguments:\012"
init/main.c:1426 [main]run_init_process =_ "%s\012"
init/main.c:1427 [main]run_init_process =_ " with environment:\012"
init/main.c:1429 [main]run_init_process =_ "%s\012"
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 7fb99492c16f..8ff11977b8bd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -59,7 +59,7 @@ struct ddebug_query {
struct ddebug_iter {
struct ddebug_table *table;
- unsigned int idx;
+ int idx;
};
struct flag_settings {
@@ -805,13 +805,12 @@ static struct _ddebug *ddebug_iter_first(struct
ddebug_iter *iter)
{
if (list_empty(_tables)) {
iter->table = NULL;
- iter->idx = 0;
return NULL;
}
iter->table = list_entry(ddebug_tables.next,
struct ddebug_table, link);
- iter->idx = 0;
- return >table->ddebugs[iter->idx];
+ iter->idx = iter->table->num_ddebugs;
+ return >table->ddebugs[--iter->idx];
}
/*
@@ -824,15 +823,16 @@ static struct _ddebug *ddebug_iter_next(struct
ddebug_iter *iter)
{
if (iter->table == NULL)
return NULL;
- if (++iter->idx == iter->table->num_ddebugs) {
+ if (--iter->idx < 0) {
/* iterate to next table */
- iter->idx = 0;
if (list_is_last(>table->link, _tables)) {
iter->table = NULL;
return NULL;
}
iter->table = list_entry(iter->table->link.next,
struct ddebug_table, link);
+ iter->idx = iter->table->num_ddebugs;
+ --iter->idx;
}
return >table->ddebugs[iter->idx];
}
--
2.37.2
[PATCH v6 03/57] dyndbg: show both old and new in change-info
print "old => new" flag values to the info("change") message.
no functional change.
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
lib/dynamic_debug.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a56c1286ffa4..8faf584f2f4b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -156,7 +156,7 @@ static int ddebug_change(const struct ddebug_query *query,
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
- struct flagsbuf fbuf;
+ struct flagsbuf fbuf, nbuf;
/* search for matching ddebugs */
mutex_lock(_lock);
@@ -217,11 +217,12 @@ static int ddebug_change(const struct ddebug_query *query,
static_branch_enable(>key.dd_key_true);
}
#endif
+ v4pr_info("changed %s:%d [%s]%s %s => %s\n",
+ trim_prefix(dp->filename), dp->lineno,
+ dt->mod_name, dp->function,
+ ddebug_describe_flags(dp->flags, ),
+ ddebug_describe_flags(newflags, ));
dp->flags = newflags;
- v4pr_info("changed %s:%d [%s]%s =%s\n",
-trim_prefix(dp->filename), dp->lineno,
-dt->mod_name, dp->function,
-ddebug_describe_flags(dp->flags, ));
}
}
mutex_unlock(_lock);
--
2.37.2
[PATCH v6 02/57] dyndbg: fix module.dyndbg handling
For CONFIG_DYNAMIC_DEBUG=N, the ddebug_dyndbg_module_param_cb()
stub-fn is too permissive:
bash-5.1# modprobe drm JUNKdyndbg
bash-5.1# modprobe drm dyndbgJUNK
[ 42.933220] dyndbg param is supported only in CONFIG_DYNAMIC_DEBUG builds
[ 42.937484] ACPI: bus type drm_connector registered
This caused no ill effects, because unknown parameters are either
ignored by default with an "unknown parameter" warning, or ignored
because dyndbg allows its no-effect use on non-dyndbg builds.
But since the code has an explicit feedback message, it should be
issued accurately. Fix with strcmp for exact param-name match.
Reported-by: Rasmus Villemoes
Fixes: b48420c1d301 dynamic_debug: make dynamic-debug work for module
initialization
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
include/linux/dynamic_debug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dce631e678dd..f30b01aa9fa4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -201,7 +201,7 @@ static inline int ddebug_remove_module(const char *mod)
static inline int ddebug_dyndbg_module_param_cb(char *param, char *val,
const char *modname)
{
- if (strstr(param, "dyndbg")) {
+ if (!strcmp(param, "dyndbg")) {
/* avoid pr_warn(), which wants pr_fmt() fully defined */
printk(KERN_WARNING "dyndbg param is supported only in "
"CONFIG_DYNAMIC_DEBUG builds\n");
--
2.37.2
[PATCH v6 01/57] dyndbg: fix static_branch manipulation
In https://lore.kernel.org/lkml/20211209150910.ga23...@axis.com/
Vincent's patch commented on, and worked around, a bug toggling
static_branch's, when a 2nd PRINTK-ish flag was added. The bug
results in a premature static_branch_disable when the 1st of 2 flags
was disabled.
The cited commit computed newflags, but then in the JUMP_LABEL block,
failed to use that result, instead using just one of the terms in it.
Using newflags instead made the code work properly.
This is Vincents test-case, reduced. It needs the 2nd flag to
demonstrate the bug, but it's explanatory here.
pt_test() {
echo 5 > /sys/module/dynamic_debug/verbose
site="module tcp" # just one callsite
echo " $site =_ " > /proc/dynamic_debug/control # clear it
# A B ~A ~B
for flg in +T +p "-T #broke here" -p; do
echo " $site $flg " > /proc/dynamic_debug/control
done;
# A B ~B ~A
for flg in +T +p "-p #broke here" -T; do
echo " $site $flg " > /proc/dynamic_debug/control
done
}
pt_test
Fixes: 84da83a6ffc0 dyndbg: combine flags & mask into a struct, simplify with it
CC: vincent.whitchu...@axis.com
Acked-by: Jason Baron
Signed-off-by: Jim Cromie
---
.drop @stable, no exposed bug.
.add jbaron ack
---
lib/dynamic_debug.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index dd7f56af9aed..a56c1286ffa4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -211,10 +211,11 @@ static int ddebug_change(const struct ddebug_query *query,
continue;
#ifdef CONFIG_JUMP_LABEL
if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
+ if (!(newflags & _DPRINTK_FLAGS_PRINT))
static_branch_disable(>key.dd_key_true);
- } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
+ } else if (newflags & _DPRINTK_FLAGS_PRINT) {
static_branch_enable(>key.dd_key_true);
+ }
#endif
dp->flags = newflags;
v4pr_info("changed %s:%d [%s]%s =%s\n",
--
2.37.2
[PATCH v6 00/57] DYNDBG: opt-in class'd debug for modules, use in drm.
hi Greg, Jason, DRM-folk, Steven, If Im not too late for linux-next in this cycle, heres V6. Diffs are minor: - rebased onto e47eb90a0a9a (tag: next-20220901, linux-next/master) gets past Kconfig conflict, same for drm-tip. - uint debug_level, not ulong. to fit nouveau. - -1 on param-read-back, to match prev write val. - added back tracefs parts, missing from -V5 updated for tracing/events: Add __vstring() and __assign_vstr() helper macros no decorations-lite in TP_printk, do it right later. - commit-msg tweaks Theres also new RFC stuff with the potential to reduce the size of the __dyndbgs section by 20%. Not ready for prime time, or linux-next, but I hope compelling. FEATURE DESCRIPTION dyndbg provides DECLARE_DYNAMIC_DEBUG_CLASSMAP() which allows module authors to declare "good" class-names, of 4 types. DYNAMIC_DEBUG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, offset, "DRM_UT_CORE", "DRM_UT_DRIVER", "DRM_UT_KMS", "DRM_UT_PRIME", "DRM_UT_ATOMIC", "DRM_UT_VBL", "DRM_UT_STATE", "DRM_UT_LEASE", "DRM_UT_DP", "DRM_UT_DRMRES"); That usage authorizes dyndbg to set class'd pr_debugs accordingly: echo class DRM_UT_CORE +p > /proc/dynamic_debug/control echo class DRM_UT_KMS +p > /proc/dynamic_debug/control Because the DRM modules declare the same classes, they each authorize dyndbg with the same classnames, which allows dyndbg to effect changes to its selected class'd prdbgs. Opting in by using the macro effectively privatizes the limited 63-classes available per module; only modules which share classnames must coordinate their use of the common range, and they can independently use the remaining id-space. Other dyndbg filtering pertains too, so single sites can be selected. 4 DD_CLASS_TYPE_*_*s determine 2 behaviors: DISJOINT bits are independent, like drm.debug categories LEVELs3>2, turns on level-2, like nouveau debug-levels NUM/BITS numeric input, bitmap if disjoint, else 0-32. NAMES accept proper names, like DRM_UT_CORE Dyndbg provides param-callbacks which enforce those behaviors: # DISJOINT_BITS echo 0x03 > /sys/module/drm/parameters/debug # LEVEL_NUM echo 3 > /sys/module/drm/nouveau/debug-mumble* # DISJOINT_NAMES echo +DRM_UT_CORE,+DRM_UT_KMS,-DRM_UT_DRIVER > /sys/module/drm/parameters/debug_categories # LEVEL_NAMES echo NV_TRACE > /sys/module/nouveau/parameters/debug-mumble* That design choice is allowed cuz verbosity is always attached to a (user visible) interface, and theres no reason not to put the implementation there (in the callback). It also considerably simplifies things; ddebug_change can treat class_id's as disjoint, period. Jim Cromie (57): prep: dyndbg: fix static_branch manipulation dyndbg: fix module.dyndbg handling dyndbg: show both old and new in change-info dyndbg: reverse module walk in cat control dyndbg: reverse module.callsite walk in cat control dyndbg: use ESCAPE_SPACE for cat control dyndbg: let query-modname override actual module name dyndbg: add test_dynamic_debug module dyndbg: drop EXPORTed dynamic_debug_exec_queries dyndbg: cleanup auto vars in dynamic_debug_init dyndbg: gather __dyndbg[] state into struct _ddebug_info class feature: dyndbg: add class_id to pr_debug callsites dyndbg: add __pr_debug_cls for testing dyndbg: add DECLARE_DYNDBG_CLASSMAP macro kernel/module: add __dyndbg_classes section dyndbg: add ddebug_attach_module_classes dyndbg: validate class FOO by checking with module doc-dyndbg: describe "class CLASS_NAME" query support doc-dyndbg: edit dynamic-debug-howto for brevity, audience dyndbg: add drm.debug style (drm/parameters/debug) bitmap support dyndbg: test DECLARE_DYNDBG_CLASSMAP, sysfs nodes drm-use-case: drm_print: condense enum drm_debug_category drm: POC drm on dyndbg - use in core, 2 helpers, 3 drivers. drm_print: interpose drm_*dbg with forwarding macros drm_print: wrap drm_*_dbg in dyndbg descriptor factory macro drm-print.h: include dyndbg header drm-print: add drm_dbg_driver to improve namespace symmetry drm_print: refine drm_debug_enabled for jump-label drm_print: prefer bare printk KERN_DEBUG on generic fn drm_print: add _ddebug descriptor to drm_*dbg prototypes nouveau: change nvkm_debug/trace to use dev_dbg POC nouveau: adapt NV_DEBUG, NV_ATOMIC to use DRM.debug nouveau: WIP add 2 LEVEL_NUM classmaps for CLI, SUBDEV dyndbg-tracefs: dyndbg: add _DPRINTK_FLAGS_ENABLED dyndbg: add _DPRINTK_FLAGS_TRACE dyndbg: add write-events-to-tracefs code dyndbg: add 2 trace-events: drm_debug, drm_devdbg dyndbg: add 2 more trace-events: pr_debug, dev_dbg dyndbg/drm: POC add tracebits sysfs-knob
Re: [PATCH] amd: amdgpu: fix coding style issue
Am 04.09.22 um 11:26 schrieb Jingyu Wang:
This is a patch to the amdgpu_sync.c file that fixes some warnings found by the
checkpatch.pl tool
Signed-off-by: Jingyu Wang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index 504af1b93bfa..dfc787b749b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -1,5 +1,6 @@
-/*
- * Copyright 2014 Advanced Micro Devices, Inc.
+// SPDX-License-Identifier: GPL-2.0
This code is under and MIT license.
Christian.
+
+/* Copyright 2014 Advanced Micro Devices, Inc.
* All Rights Reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a
@@ -315,6 +316,7 @@ struct dma_fence *amdgpu_sync_get_fence(struct amdgpu_sync
*sync)
struct hlist_node *tmp;
struct dma_fence *f;
int i;
+
hash_for_each_safe(sync->fences, i, tmp, e, node) {
f = e->fence;
@@ -392,7 +394,7 @@ void amdgpu_sync_free(struct amdgpu_sync *sync)
{
struct amdgpu_sync_entry *e;
struct hlist_node *tmp;
- unsigned i;
+ unsigned int i;
hash_for_each_safe(sync->fences, i, tmp, e, node) {
hash_del(>node);
Re: [PATCH] drm: amd: This is a patch to the amdgpu_drv.c file that fixes some warnings and errors found by the checkpatch.pl tool
Am 04.09.22 um 10:39 schrieb Jingyu Wang: Looks mostly valid, but a few style nits and the kernel test robot complained about something as well. First of all please shorten the subject line, something like "drm/amdgpu: cleanup coding style in amdgpu_drv.c". Then provide a commit message, e.g.: "Fix everything checkpatch.pl complained about in amdgpu_drv.c." Signed-off-by: Jingyu Wang --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 40 - 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index de7144b06e93..5c2ac8123450 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -140,8 +140,8 @@ uint amdgpu_pcie_lane_cap; u64 amdgpu_cg_mask = 0x; uint amdgpu_pg_mask = 0x; uint amdgpu_sdma_phase_quantum = 32; -char *amdgpu_disable_cu = NULL; -char *amdgpu_virtual_display = NULL; +char *amdgpu_disable_cu; +char *amdgpu_virtual_display; /* * OverDrive(bit 14) disabled by default @@ -287,9 +287,9 @@ module_param_named(msi, amdgpu_msi, int, 0444); * jobs is 1. The timeout for compute is 6. */ MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default: for bare metal 1 for non-compute jobs and 6 for compute jobs; " - "for passthrough or sriov, 1 for all jobs." - " 0: keep default value. negative: infinity timeout), " - "format: for bare metal [Non-Compute] or [GFX,Compute,SDMA,Video]; " + "for passthrough or sriov, 1 for all jobs. + 0: keep default value. negative: infinity timeout), + format: for bare metal [Non-Compute] or [GFX,Compute,SDMA,Video]; " "for passthrough or sriov [all jobs] or [GFX,Compute,SDMA,Video]."); checkpatch.pl might now not complain about it, but that doesn't look correct on first glance. You now include all the newlines and empty spaces in the string which is not intentionally. Christian. module_param_string(lockup_timeout, amdgpu_lockup_timeout, sizeof(amdgpu_lockup_timeout), 0444); @@ -502,7 +502,7 @@ module_param_named(virtual_display, amdgpu_virtual_display, charp, 0444); * Set how much time allow a job hang and not drop it. The default is 0. */ MODULE_PARM_DESC(job_hang_limit, "how much time allow a job hang and not drop it (default 0)"); -module_param_named(job_hang_limit, amdgpu_job_hang_limit, int ,0444); +module_param_named(job_hang_limit, amdgpu_job_hang_limit, int, 0444); /** * DOC: lbpw (int) @@ -565,8 +565,8 @@ module_param_named(timeout_period, amdgpu_watchdog_timer.period, uint, 0644); */ #ifdef CONFIG_DRM_AMDGPU_SI -#if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE) -int amdgpu_si_support = 0; +#if IS_ENABLED(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE) +int amdgpu_si_support; MODULE_PARM_DESC(si_support, "SI support (1 = enabled, 0 = disabled (default))"); #else int amdgpu_si_support = 1; @@ -584,8 +584,8 @@ module_param_named(si_support, amdgpu_si_support, int, 0444); */ #ifdef CONFIG_DRM_AMDGPU_CIK -#if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE) -int amdgpu_cik_support = 0; +#if IS_ENABLED(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE) +int amdgpu_cik_support; MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled (default))"); #else int amdgpu_cik_support = 1; @@ -601,8 +601,8 @@ module_param_named(cik_support, amdgpu_cik_support, int, 0444); * E.g. 0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte. The default is 0 (disabled). */ MODULE_PARM_DESC(smu_memory_pool_size, - "reserve gtt for smu debug usage, 0 = disable," - "0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte"); + "reserve gtt for smu debug usage, 0 = disable, + 0x1 = 256Mbyte, 0x2 = 512Mbyte, 0x4 = 1 Gbyte, 0x8 = 2GByte"); module_param_named(smu_memory_pool_size, amdgpu_smu_memory_pool_size, uint, 0444); /** @@ -772,9 +772,9 @@ module_param(hws_gws_support, bool, 0444); MODULE_PARM_DESC(hws_gws_support, "Assume MEC2 FW supports GWS barriers (false = rely on FW version check (Default), true = force supported)"); /** - * DOC: queue_preemption_timeout_ms (int) - * queue preemption timeout in ms (1 = Minimum, 9000 = default) - */ + * DOC: queue_preemption_timeout_ms (int) + * queue preemption timeout in ms (1 = Minimum, 9000 = default) + */ int queue_preemption_timeout_ms = 9000; module_param(queue_preemption_timeout_ms, int, 0644); MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue preemption timeout in ms (1 = Minimum, 9000 = default)"); @@ -799,7 +799,7 @@ MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false = defa * DOC: no_queue_eviction_on_vm_fault (int) * If set, process queues
Re: [PATCH] drm: amd: This is a patch to the amdgpu_drv.c file that fixes some warnings and errors found by the checkpatch.pl tool
Hi Jingyu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.0-rc3 next-20220901]
[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/Jingyu-Wang/drm-amd-This-is-a-patch-to-the-amdgpu_drv-c-file-that-fixes-some-warnings-and-errors-found-by-the-checkpatch-pl-tool/20220904-165633
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: mips-randconfig-r006-20220904
(https://download.01.org/0day-ci/archive/20220904/202209042040.j7yokrbb-...@intel.com/config)
compiler: mips64el-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/639ddf37854dd71c3ee836591db7518b146ae8ae
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Jingyu-Wang/drm-amd-This-is-a-patch-to-the-amdgpu_drv-c-file-that-fixes-some-warnings-and-errors-found-by-the-checkpatch-pl-tool/20220904-165633
git checkout 639ddf37854dd71c3ee836591db7518b146ae8ae
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=mips SHELL=/bin/bash drivers/gpu/drm/amd/amdgpu/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All error/warnings (new ones prefixed by >>):
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:290:17: warning: missing terminating
>> " character
290 | "for passthrough or sriov, 1 for all jobs.
| ^
In file included from include/linux/module.h:22,
from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/drm/drm_print.h:32,
from include/drm/drm_mm.h:51,
from include/drm/drm_vma_manager.h:26,
from include/drm/drm_gem.h:40,
from drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:28:
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:290:17: error: missing terminating "
>> character
290 | "for passthrough or sriov, 1 for all jobs.
| ^~
include/linux/moduleparam.h:26:61: note: in definition of macro
'__MODULE_INFO'
26 | = __MODULE_INFO_PREFIX __stringify(tag) "=" info
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:289:1: note: in expansion of macro
'MODULE_PARM_DESC'
289 | MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default:
for bare metal 1 for non-compute jobs and 6 for compute jobs; "
| ^~~~
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:291:17: error: expected ',' or ';'
>> before numeric constant
291 | 0: keep default value. negative: infinity timeout),
| ^
include/linux/moduleparam.h:26:61: note: in definition of macro
'__MODULE_INFO'
26 | = __MODULE_INFO_PREFIX __stringify(tag) "=" info
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:289:1: note: in expansion of macro
'MODULE_PARM_DESC'
289 | MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default:
for bare metal 1 for non-compute jobs and 6 for compute jobs; "
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:292:83: warning: missing terminating
" character
292 | format: for bare metal [Non-Compute] or
[GFX,Compute,SDMA,Video]; "
|
^
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:292:83: error: missing terminating "
character
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:293:17: error: expected identifier
>> or '(' before string constant
293 | "for passthrough or sriov [all jobs] or
[GFX,Compute,SDMA,Video].");
|
^~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:604:9: warning: missing terminating
" character
604 | "reserve gtt for smu debug usage, 0 = disable,
| ^
drivers/gpu/drm/amd/
Re: [PATCH] drm: amd: This is a patch to the amdgpu_drv.c file that fixes some warnings and errors found by the checkpatch.pl tool
Hi Jingyu,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.0-rc3 next-20220901]
[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/Jingyu-Wang/drm-amd-This-is-a-patch-to-the-amdgpu_drv-c-file-that-fixes-some-warnings-and-errors-found-by-the-checkpatch-pl-tool/20220904-165633
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: arc-allyesconfig
(https://download.01.org/0day-ci/archive/20220904/202209041839.rob9xu3v-...@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/639ddf37854dd71c3ee836591db7518b146ae8ae
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Jingyu-Wang/drm-amd-This-is-a-patch-to-the-amdgpu_drv-c-file-that-fixes-some-warnings-and-errors-found-by-the-checkpatch-pl-tool/20220904-165633
git checkout 639ddf37854dd71c3ee836591db7518b146ae8ae
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/amd/amdgpu/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:290:17: warning: missing terminating
>> " character
290 | "for passthrough or sriov, 1 for all jobs.
| ^
In file included from include/linux/module.h:22,
from include/linux/device/driver.h:21,
from include/linux/device.h:32,
from include/drm/drm_print.h:32,
from include/drm/drm_mm.h:51,
from include/drm/drm_vma_manager.h:26,
from include/drm/drm_gem.h:40,
from drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:28:
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:290:17: error: missing terminating "
character
290 | "for passthrough or sriov, 1 for all jobs.
| ^~
include/linux/moduleparam.h:26:61: note: in definition of macro
'__MODULE_INFO'
26 | = __MODULE_INFO_PREFIX __stringify(tag) "=" info
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:289:1: note: in expansion of macro
'MODULE_PARM_DESC'
289 | MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default:
for bare metal 1 for non-compute jobs and 6 for compute jobs; "
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:291:17: error: expected ',' or ';'
before numeric constant
291 | 0: keep default value. negative: infinity timeout),
| ^
include/linux/moduleparam.h:26:61: note: in definition of macro
'__MODULE_INFO'
26 | = __MODULE_INFO_PREFIX __stringify(tag) "=" info
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:289:1: note: in expansion of macro
'MODULE_PARM_DESC'
289 | MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (default:
for bare metal 1 for non-compute jobs and 6 for compute jobs; "
| ^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:292:83: warning: missing terminating
" character
292 | format: for bare metal [Non-Compute] or
[GFX,Compute,SDMA,Video]; "
|
^
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:292:83: error: missing terminating "
character
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:293:17: error: expected identifier
or '(' before string constant
293 | "for passthrough or sriov [all jobs] or
[GFX,Compute,SDMA,Video].");
|
^~
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:604:9: warning: missing terminating
" character
604 | "reserve gtt for smu debug usage, 0 = disable,
| ^
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:605:76: warning:
