From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: linux-kernel@vger.kernel.org, Fenghua Yu <fenghua.yu@intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] x86: kernel: cpu: resctrl: Fix kernel-doc in pseudo_lock.c
Date: Tue, 08 Jun 2021 22:12:23 +0200 [thread overview]
Message-ID: <1711024.RBxhUqbo4a@linux.local> (raw)
In-Reply-To: <017e9a77-d17e-effd-5639-72a06abc4fc3@intel.com>
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
next prev parent reply other threads:[~2021-06-08 20:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-06-08 21:23 ` Reinette Chatre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1711024.RBxhUqbo4a@linux.local \
--to=fmdefrancesco@gmail.com \
--cc=bp@alien8.de \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=reinette.chatre@intel.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.