Messages in this thread | | | From | "Fabio M. De Francesco" <> | Subject | Re: [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c | Date | Tue, 08 Jun 2021 22:12:23 +0200 |
| |
On Tuesday, June 8, 2021 1:30:34 AM CEST Reinette Chatre wrote: > Hi Fabio, > Hi Reinette, > > Thank you very much for catching these. I am curious what your goal is > because when I ran a kernel-doc check on the resctrl area there were > many more warnings than are not addressed in this patch. Also, while > this patch claims to fix the kernel-doc in pseudo_lock.c there seems to > be a few more that are not addressed. > Actually this patch was just a preliminary test for checking if my contributions to this subsystem would be taken into consideration or completely ignored. That is the real reason why I just started with trying to fix only a couple of kernel-doc issues in pseudo_lock.c. > > Are you planning to submit more > patches to do a cleanup of kernel-doc or are these the only ones > bothering you for some reason? > I'd like to submit more cleanup patches of kernel-doc, because I always read carefully the kernel-doc above the functions I want to understand. I have a long term plan to study the Linux code and try to contribute the better I can. I'm into Linux developing since about two months, so I'm a newcomer and I still have a lot to learn. > > Could you please fixup the subject to conform to this area: > "x86/resctrl: Fix kernel-doc in pseudo_lock.c" > Sure. I was inadvertently using the drivers/staging convention I've used for the patches I've submitted there. > > For this subject to be accurate though it should fix all the kernel-doc > warnings found in pseudo_lock.c - or if not it would be helpful to > explain what the criteria for fixes are. I tested this by running: > $ scripts/kernel-doc -v -none arch/x86/kernel/cpu/resctrl/* > I've just run the above script and I see that there are a lot more warnings that I was expecting.
I want to fix as much as I can. Unfortunately I'm pretty sure I won't be able to fix them all, just because the inner working and the purpose of some functions are a bit obscure to me (at least until I get more knowledge of x86 architecture - it may take a lot of time because I'm also studying other subsystems at the same time).
> On 6/2/2021 3:23 PM, Fabio M. De Francesco wrote: > > Fixed sparse warnings about the descriptions of some function > > parameters. > > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> > > --- > > > > arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > > b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c index f6451abddb09..c3629db90570 100644 > > --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > > +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > > @@ -520,7 +520,7 @@ static int pseudo_lock_fn(void *_rdtgrp) > > > > /** > > > > * rdtgroup_monitor_in_progress - Test if monitoring in progress > > > > - * @r: resource group being queried > > + * @rdtgrp: resource group being queried > > > > * > > * Return: 1 if monitor groups have been created for this resource > > * group, 0 otherwise. > > > > @@ -1140,6 +1140,8 @@ static int measure_l3_residency(void *_plr) > > > > /** > > > > * pseudo_lock_measure_cycles - Trigger latency measure to pseudo-locked region > > > > + * @rdtgrp: resource group to which the pseudo-locked region belongs > > + * @sel: cache level selector > > This is not correct. A more accurate description could be: > "select which measurement to perform on pseudo-locked region" > Here it is an example of my lack of knowledge/experience. Obviously, I'll rewrite it according to your review.
To summarize: as soon as possible I'll submit a v2 patch with the kernel-doc fixes that I think I can understand. I am pretty sure that some fixes will not be to your standards and that for what regards some others I will not even be able to attempt to fix them :(
Thanks you very much for your kind reply,
Fabio > > > * > > * The measurement of latency to access a pseudo-locked region should be > > * done from a cpu that is associated with that pseudo-locked region. > > Reinette
| |