[PATCH] eventfs: Fix kerneldoc of eventfs_remove_rec()

[PATCH] eventfs: Fix kerneldoc of eventfs_remove_rec()

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The eventfs_remove_rec() had some missing parameters in the kerneldoc
comment above it. Also, rephrase the description a bit more to have a bit
more correct grammar.

Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use eventfs_inode");
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202310052216.4SgqasWo-lkp@intel.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/event_inode.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 5a3cc5394294..1c28e013201f 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -977,9 +977,11 @@ static void free_rcu_ei(struct rcu_head *head)
 /**
  * eventfs_remove_rec - remove eventfs dir or file from list
  * @ei: eventfs_inode to be removed.
+ * @head: the list head to place the deleted @ei and children
+ * @level: prevent recursion from going more than 3 levels deep.
  *
- * This function recursively remove eventfs_inode which
- * contains info of file or dir.
+ * This function recursively removes eventfs_inodes which
+ * contains info of files and/or directories.
  */
 static void eventfs_remove_rec(struct eventfs_inode *ei, struct list_head *head, int level)
 {
-- 
2.42.0

Re: [PATCH] eventfs: Fix kerneldoc of eventfs_remove_rec()

[Mukesh Ojha]

On 10/30/2023 9:45 PM, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> The eventfs_remove_rec() had some missing parameters in the kerneldoc
> comment above it. Also, rephrase the description a bit more to have a bit
> more correct grammar.
> 
> Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use eventfs_inode");
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202310052216.4SgqasWo-lkp@intel.com/
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>

-Mukesh
> ---
>   fs/tracefs/event_inode.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index 5a3cc5394294..1c28e013201f 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -977,9 +977,11 @@ static void free_rcu_ei(struct rcu_head *head)
>   /**
>    * eventfs_remove_rec - remove eventfs dir or file from list
>    * @ei: eventfs_inode to be removed.
> + * @head: the list head to place the deleted @ei and children
> + * @level: prevent recursion from going more than 3 levels deep.
>    *
> - * This function recursively remove eventfs_inode which
> - * contains info of file or dir.
> + * This function recursively removes eventfs_inodes which
> + * contains info of files and/or directories.
>    */
>   static void eventfs_remove_rec(struct eventfs_inode *ei, struct list_head *head, int level)
>   {

Re: [PATCH] eventfs: Fix kerneldoc of eventfs_remove_rec()

[Steven Rostedt]

On Mon, 30 Oct 2023 21:57:13 +0530
Mukesh Ojha <quic_mojha@quicinc.com> wrote:

> On 10/30/2023 9:45 PM, Steven Rostedt wrote:
> > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > 
> > The eventfs_remove_rec() had some missing parameters in the kerneldoc
> > comment above it. Also, rephrase the description a bit more to have a bit
> > more correct grammar.
> > 
> > Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use eventfs_inode");
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202310052216.4SgqasWo-lkp@intel.com/
> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>  
> 
> Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>

Hi Mukesh!

First, I want to thank you for your reviews. We certainly need more
reviewers.

But I need to also state that "Reviewed-by" tags should not be sent so
lightly. The only times a Reviewed-by tag should be sent is if you
participated in the discussion of the code, you have authored some of
the code that is being modified, or are marked as a reviewer of the code in
the MAINTAINERS file.

For example, you added to the discussion here:

   https://lore.kernel.org/all/65dcdd9c-a75b-4fe7-bdcf-471a5602db20@linaro.org/

And adding your Reviewed-by tag is appropriate.

But when a maintainer receives a Reviewed-by from someone they don't know,
without any discussion in the patch, it may make that maintainer think that
the person sending the Reviewed-by is only out to get listed in the LWN
"Reviewed-by" count.

I review other developers' code all the time, and unless the code touches
something I worked on or I'm marked as a reviewer in the MAINTAINERS file,
I do not send a Reviewed-by tag unless I added some input to the patch in
question.

My advice to you is to keep up the reviewing, I appreciate it (I really
do!), but don't send out Reviewed-by tags unless you are marked as a
reviewer of the code, or participated in a discussion on that code.

Thanks,

-- Steve

Re: [PATCH] eventfs: Fix kerneldoc of eventfs_remove_rec()

[Mukesh Ojha]

On 11/2/2023 1:30 AM, Steven Rostedt wrote:
> On Mon, 30 Oct 2023 21:57:13 +0530
> Mukesh Ojha <quic_mojha@quicinc.com> wrote:
> 
>> On 10/30/2023 9:45 PM, Steven Rostedt wrote:
>>> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>>>
>>> The eventfs_remove_rec() had some missing parameters in the kerneldoc
>>> comment above it. Also, rephrase the description a bit more to have a bit
>>> more correct grammar.
>>>
>>> Fixes: 5790b1fb3d672 ("eventfs: Remove eventfs_file and just use eventfs_inode");
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202310052216.4SgqasWo-lkp@intel.com/
>>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>>
>> Reviewed-by: Mukesh Ojha <quic_mojha@quicinc.com>
> 
> Hi Mukesh!
> 
> First, I want to thank you for your reviews. We certainly need more
> reviewers.
> 
> But I need to also state that "Reviewed-by" tags should not be sent so
> lightly. The only times a Reviewed-by tag should be sent is if you
> participated in the discussion of the code, you have authored some of
> the code that is being modified, or are marked as a reviewer of the code in
> the MAINTAINERS file.

Thanks Steven for writing a suggestion note for me.

I will try to participate and take this in a good way..but i thought
for easier change where there is no discussion is needed., it is fine
to add if you have spent time in checking the code and change is proper.

> 
> For example, you added to the discussion here:
> 
>     https://lore.kernel.org/all/65dcdd9c-a75b-4fe7-bdcf-471a5602db20@linaro.org/
> 
> And adding your Reviewed-by tag is appropriate.
> 
> But when a maintainer receives a Reviewed-by from someone they don't know,
> without any discussion in the patch, it may make that maintainer think that
> the person sending the Reviewed-by is only out to get listed in the LWN
> "Reviewed-by" count.

I understand..

> 
> I review other developers' code all the time, and unless the code touches
> something I worked on or I'm marked as a reviewer in the MAINTAINERS file,
> I do not send a Reviewed-by tag unless I added some input to the patch in
> question.

Will keep this in mind.
To get involve in LKML discussion, Sending Reviewed-by may not be
important but the discussions/engagement/contribution is .
> 
> My advice to you is to keep up the reviewing, I appreciate it (I really
> do!), but don't send out Reviewed-by tags unless you are marked as a
> reviewer of the code, or participated in a discussion on that code.

Sure, thanks, will try to do my bit.

-Mukesh

> 
> Thanks,
> 
> -- Steve
> 

Re: [PATCH] eventfs: Fix kerneldoc of eventfs_remove_rec()

[Steven Rostedt]

On Thu, 2 Nov 2023 12:05:33 +0530
Mukesh Ojha <quic_mojha@quicinc.com> wrote:

> I will try to participate and take this in a good way..but i thought
> for easier change where there is no discussion is needed., it is fine
> to add if you have spent time in checking the code and change is proper.

If it's easy then automated bots will likely catch any issues. No need to
say you looked at it too. Otherwise we'll get 20 Reviewed-by tags on
comment changes.

A Reviewed-by tag has much more meaning when the code being reviewed is not
trivial, where questions about correctness is needed. In other words, if
something doesn't look quite right, ask. If it is, and the author explains
the reason it is, the fact that the explanation is now documented in the
archives is useful.

Thanks,

-- Steve