All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c
@ 2021-06-02 22:23 Fabio M. De Francesco
  2021-06-07 23:30 ` Reinette Chatre
  0 siblings, 1 reply; 4+ messages in thread
From: Fabio M. De Francesco @ 2021-06-02 22:23 UTC (permalink / raw)
  To: linux-kernel, Fenghua Yu, Reinette Chatre, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
  Cc: Fabio M. De Francesco

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
  *
  * The measurement of latency to access a pseudo-locked region should be
  * done from a cpu that is associated with that pseudo-locked region.
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c
  2021-06-02 22:23 [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c Fabio M. De Francesco
@ 2021-06-07 23:30 ` Reinette Chatre
  2021-06-08 20:12   ` Fabio M. De Francesco
  0 siblings, 1 reply; 4+ messages in thread
From: Reinette Chatre @ 2021-06-07 23:30 UTC (permalink / raw)
  To: Fabio M. De Francesco, linux-kernel, Fenghua Yu, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin

Hi Fabio,

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. 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?

Could you please fixup the subject to conform to this area:
"x86/resctrl: Fix kernel-doc in pseudo_lock.c"

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/*

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"

>    *
>    * 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c
  2021-06-07 23:30 ` Reinette Chatre
@ 2021-06-08 20:12   ` Fabio M. De Francesco
  2021-06-08 21:23     ` Reinette Chatre
  0 siblings, 1 reply; 4+ messages in thread
From: Fabio M. De Francesco @ 2021-06-08 20:12 UTC (permalink / raw)
  To: Reinette Chatre
  Cc: linux-kernel, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin

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





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c
  2021-06-08 20:12   ` Fabio M. De Francesco
@ 2021-06-08 21:23     ` Reinette Chatre
  0 siblings, 0 replies; 4+ messages in thread
From: Reinette Chatre @ 2021-06-08 21:23 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: linux-kernel, Fenghua Yu, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, H. Peter Anvin

Hi Fabio,

On 6/8/2021 1:12 PM, Fabio M. De Francesco wrote:
> 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.

Your submissions are appreciated and will be taken into consideration.

>> 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.

Unfortunately the kernel is not consistent in this regard.
>> 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).

...

> 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 :(
> 

Thank you for giving me insight into your status and plans. Your 
approach sounds reasonable to me. When you submit fixes to parts you 
understand I can provide feedback based on my understandings to 
collaborate towards improved kernel-doc in this area.

Thank you

Reinette



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-08 21:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 22:23 [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c Fabio M. De Francesco
2021-06-07 23:30 ` Reinette Chatre
2021-06-08 20:12   ` Fabio M. De Francesco
2021-06-08 21:23     ` Reinette Chatre

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.