Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only

[Fabio M. De Francesco]

On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:
> On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> > On Tue, Mar 14, 2023 at 10:31:21PM +0100, Fabio wrote:
> > > On martedì 14 marzo 2023 21:43:40 CET Alison Schofield wrote:
> > > > On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> > > > > Module parameter, read_timeout, can only be set at the loading time.
> > > > > As
> > > > > it can only be modified once, initialize read_timeout once in the
> > > > > probe
> > > > > function.
> > > > > As a result, only use read_timeout as the last argument in
> > > > > wait_event_interruptible_timeout() call.
> > > > > 
> > > > > Same goes for write_timeout.
> > > > > 
> > > > > Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@xxxxxxxxx>
> > > > > ---
> > > > 
> > > > Looks like this is [PATCH v5] and needs a changelog.
> > > 
> > > Alison,
> > > 
> > > In fact, this is only the second patch that addresses Greg's suggested
> > > refactoring.
> > > 
> > > Khadija moved from v3 of "staging: axis-fifo: remove tabs to align
> > > arguments"
> > > to v4 of this completely independent patch. And then back to v1, because
> > > (at
> > > the time of v4) I pointed out to her that she had started working on a
> > > project that has a completely different purpose than the previous one.
> > > 
> > > The best course of action would have been to ask Greg to drop the 
previous
> > > patches and then reset the numbering of the new job to v1. Unfortunately 
I
> > > did not pay attention to how she then managed the numbering following my
> > > observation.
> > > 
> > > What would be the best course of action at this point?
> > 
> > My guess is that this patch gets ignored because it has a lower version
> > number than a previous patch.
> > 
> > Take the feedback given here, and rev to
> > [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> > 
> > Be sure the Changelog, below the --- explains the journey.
> > 
> > Changes in v5:
> > 
> > Changes in v4:
> > 
> > Changes in v3:
> 
> > Changes in v2:
> Hey Alison!

Hi Khadija,

Please put one or two blank lines between the last message you are replying 
and the new you are writing (exactly as I'm doing here between "Hey Alison!" 
and "Hi Khadija").

> Based on Nathan's feedback I am trying to recompile and send a patch
> without any warnings.

Great!

> As suggested by Fabio, I am running "make w=1 -jX" command to see if I
> get any warnings.

I suppose that "w=1" is a typo. The option is enabled with "W=1" (capital 
case, Linux and all UNIX-like are case-sensitive). 

> But it is taking a lot of time, is there any way of
> speeding it up?

What is you choice for 'X' in "-jX"?
Did you try with the exact number of logical cores?
Are you building into a VM with enough logical cores? 
If you are building into a VM, did you reserve enough RAM?

Please read carefully my questions above and try to understand your 
environment and reply, so that I can help you more effectively.

> If this doesn't work then I have to follow the steps to reproduce in lkp
> mail as you said before.

The steps to reproduce will take your precious time and use more resources. 
Again, try to respond my questions.

> After dealing with these warnings I will send a [PATCH v5], following
> your instructions above.

Sorry for inadvertently overlooking to warn you about to send a message to 
Greg and ask him to drop your first 3 + 1 patches. Now you are doing good by 
following what Alison suggested: send v5 and write the log of revisions under 
the three dashes (exactly how Alison explained).

> Kindly, let me know if I am on the wrong track.
> Thank you!
> 

I think you are in the right track.
Let's try to speed up your builds because you'll need to build again your 
kernel many, many times for future works.  

Thanks,

Fabio