2023-03-16 12:56:34

by Khadija Kamran

[permalink] [raw]
Subject: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

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.

Convert datatpe of read_timeout from 'int' to 'long int' because
implicit conversion of 'long int' to 'int' in statement 'read_timeout =
MAX_SCHEDULE_TIMEOUT' results in an overflow warning.

Perform same steps formodule parameter, write_timeout.

Suggested-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Khadija Kamran <[email protected]>
---

Changes in v5:
- Convert timeout's datatype from int to long.
Changes in v4:
- Initialize timeouts once as suggested by Greg; this automatically
fixes the indentation problems.
- Change the subject and description.
Changes in v3:
- Fix grammatical mistakes
- Do not change the second argument's indentation in split lines
Changes in v2:
- Instead of matching alignment to open parenthesis, align second and
the last argument instead.
- Change the subject to 'remove tabs to align arguments'.
- Use imperative language in subject and description

drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index dfd2b357f484..d667dc80df47 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -103,17 +103,17 @@
* globals
* ----------------------------
*/
-static int read_timeout = 1000; /* ms to wait before read() times out */
-static int write_timeout = 1000; /* ms to wait before write() times out */
+static long read_timeout = 1000; /* ms to wait before read() times out */
+static long write_timeout = 1000; /* ms to wait before write() times out */

/* ----------------------------
* module command-line arguments
* ----------------------------
*/

-module_param(read_timeout, int, 0444);
+module_param(read_timeout, long, 0444);
MODULE_PARM_DESC(read_timeout, "ms to wait before blocking read() timing out; set to -1 for no timeout");
-module_param(write_timeout, int, 0444);
+module_param(write_timeout, long, 0444);
MODULE_PARM_DESC(write_timeout, "ms to wait before blocking write() timing out; set to -1 for no timeout");

/* ----------------------------
@@ -384,9 +384,7 @@ static ssize_t axis_fifo_read(struct file *f, char __user *buf,
mutex_lock(&fifo->read_lock);
ret = wait_event_interruptible_timeout(fifo->read_queue,
ioread32(fifo->base_addr + XLLF_RDFO_OFFSET),
- (read_timeout >= 0) ?
- msecs_to_jiffies(read_timeout) :
- MAX_SCHEDULE_TIMEOUT);
+ read_timeout);

if (ret <= 0) {
if (ret == 0) {
@@ -528,9 +526,7 @@ static ssize_t axis_fifo_write(struct file *f, const char __user *buf,
ret = wait_event_interruptible_timeout(fifo->write_queue,
ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
>= words_to_write,
- (write_timeout >= 0) ?
- msecs_to_jiffies(write_timeout) :
- MAX_SCHEDULE_TIMEOUT);
+ write_timeout);

if (ret <= 0) {
if (ret == 0) {
@@ -815,6 +811,16 @@ static int axis_fifo_probe(struct platform_device *pdev)
char *device_name;
int rc = 0; /* error return value */

+ if (read_timeout >= 0)
+ read_timeout = msecs_to_jiffies(read_timeout);
+ else
+ read_timeout = MAX_SCHEDULE_TIMEOUT;
+
+ if (write_timeout >= 0)
+ write_timeout = msecs_to_jiffies(write_timeout);
+ else
+ write_timeout = MAX_SCHEDULE_TIMEOUT;
+
/* ----------------------------
* init wrapper device
* ----------------------------
--
2.34.1



2023-03-16 13:02:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 05:56:02PM +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.
>
> Convert datatpe of read_timeout from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
>
> Perform same steps formodule parameter, write_timeout.
>
> Suggested-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Khadija Kamran <[email protected]>
> ---

I do not think you built this version as I get the following build
errors when I apply it and build:

In file included from ./include/linux/kernel.h:29,
from drivers/staging/axis-fifo/axis-fifo.c:17:
drivers/staging/axis-fifo/axis-fifo.c: In function ‘axis_fifo_init’:
./include/linux/kern_levels.h:5:25: error: format ‘%i’ expects argument of type ‘int’, but argument 2 has type ‘long int’ [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
./include/linux/printk.h:427:25: note: in definition of macro ‘printk_index_wrap’
427 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
./include/linux/printk.h:528:9: note: in expansion of macro ‘printk’
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
./include/linux/kern_levels.h:14:25: note: in expansion of macro ‘KERN_SOH’
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~~~~~
./include/linux/printk.h:528:16: note: in expansion of macro ‘KERN_INFO’
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro ‘pr_info’
957 | pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
| ^~~~~~~
./include/linux/kern_levels.h:5:25: error: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘long int’ [-Werror=format=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
./include/linux/printk.h:427:25: note: in definition of macro ‘printk_index_wrap’
427 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
./include/linux/printk.h:528:9: note: in expansion of macro ‘printk’
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
./include/linux/kern_levels.h:14:25: note: in expansion of macro ‘KERN_SOH’
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~~~~~
./include/linux/printk.h:528:16: note: in expansion of macro ‘KERN_INFO’
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro ‘pr_info’
957 | pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
| ^~~~~~~
cc1: all warnings being treated as errors


Please always test-build your patches.

thanks,

greg k-h

2023-03-16 13:04:44

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 05:56:02PM +0500, Khadija Kamran wrote:
> @@ -815,6 +811,16 @@ static int axis_fifo_probe(struct platform_device *pdev)
> char *device_name;
> int rc = 0; /* error return value */
>
> + if (read_timeout >= 0)
> + read_timeout = msecs_to_jiffies(read_timeout);
> + else
> + read_timeout = MAX_SCHEDULE_TIMEOUT;
> +
> + if (write_timeout >= 0)
> + write_timeout = msecs_to_jiffies(write_timeout);
> + else
> + write_timeout = MAX_SCHEDULE_TIMEOUT;

Also, this change needs to go into the axis_fifo_init() function, not
the driver probe as now the print message in there is not correct.

thanks,

greg k-h

2023-03-16 14:09:03

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

Hi Khadija,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url: https://github.com/intel-lab-lkp/linux/commits/Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230316-205814
patch link: https://lore.kernel.org/r/ZBMR4s8xyHGqMm72%40khadija-virtual-machine
patch subject: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230316/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7e359baa2318b55e12a0c099b75fbd0d799907cf
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230316-205814
git checkout 7e359baa2318b55e12a0c099b75fbd0d799907cf
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/staging/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from include/linux/kernel.h:29,
from drivers/staging/axis-fifo/axis-fifo.c:17:
drivers/staging/axis-fifo/axis-fifo.c: In function 'axis_fifo_init':
>> include/linux/kern_levels.h:5:25: warning: format '%i' expects argument of type 'int', but argument 2 has type 'long int' [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
427 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
include/linux/printk.h:528:9: note: in expansion of macro 'printk'
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
include/linux/kern_levels.h:14:25: note: in expansion of macro 'KERN_SOH'
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~~~~~
include/linux/printk.h:528:16: note: in expansion of macro 'KERN_INFO'
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro 'pr_info'
957 | pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
| ^~~~~~~
include/linux/kern_levels.h:5:25: warning: format '%i' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
427 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
include/linux/printk.h:528:9: note: in expansion of macro 'printk'
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
include/linux/kern_levels.h:14:25: note: in expansion of macro 'KERN_SOH'
14 | #define KERN_INFO KERN_SOH "6" /* informational */
| ^~~~~~~~
include/linux/printk.h:528:16: note: in expansion of macro 'KERN_INFO'
528 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro 'pr_info'
957 | pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
| ^~~~~~~


vim +5 include/linux/kern_levels.h

314ba3520e513a Joe Perches 2012-07-30 4
04d2c8c83d0e3a Joe Perches 2012-07-30 @5 #define KERN_SOH "\001" /* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
04d2c8c83d0e3a Joe Perches 2012-07-30 7

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-03-16 14:38:11

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

Khadija,

Just saw your v5 patch and Greg's two replies.

For v6 you will need to change the subject to "[PATCH v6] staging: axis-fifo:
initialize timeouts in init only" to indicate that you are doing assignments
in axis_fifo_init().

Don't forget to extend the version log with "Changes in v6:" and clarify that
v5 had a different "Object" (you should probably also add a link to the v5
patch in lore: https://lore.kernel.org/lkml /ZBMR4s8xyHGqMm72@khadija-virtual-
machine/). When the "Subject" changes, readers may not find the previous
versions easily.

On gioved? 16 marzo 2023 13:56:02 CET 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

Substitute "probe" with "init".

> function.
>
> As a result, only use read_timeout as the last argument in
> wait_event_interruptible_timeout() call.

This two sentences are not much clear. I'd merge and rework:

"Initialize the module parameters read_timeout and write_timeout once in
init().

Module parameters can only be set once and cannot be modified later, so we
don't need to evaluate them again when passing the parameters to
wait_event_interruptible_timeout()."

>
> Convert datatpe

s/datatpe/type/

> of read_timeout

of {read,write}_timeout

> from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow warning.

We don't care too much about the warning themselves: I mean, it overflows and
you must avoid it to happen (as you are doing with the changes of types), not
merely be interested in avoiding the warning. "[] results in an overflow." is
all we care about.

Add also the previous paragraph in the last part of the commit message.

> Perform same steps formodule parameter, write_timeout.

And instead delete the this last phrase.

> Suggested-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Khadija Kamran <[email protected]>
> ---
>
> Changes in v5:
> - Convert timeout's datatype from int to long.
> Changes in v4:
> - Initialize timeouts once as suggested by Greg; this automatically
> fixes the indentation problems.
> - Change the subject and description.
> Changes in v3:
> - Fix grammatical mistakes
> - Do not change the second argument's indentation in split lines
> Changes in v2:
> - Instead of matching alignment to open parenthesis, align second and
> the last argument instead.
> - Change the subject to 'remove tabs to align arguments'.
> - Use imperative language in subject and description
>
> drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c
> b/drivers/staging/axis-fifo/axis-fifo.c index dfd2b357f484..d667dc80df47
> 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -103,17 +103,17 @@
> * globals
> * ----------------------------
> */
> -static int read_timeout = 1000; /* ms to wait before read() times out */
> -static int write_timeout = 1000; /* ms to wait before write() times out */
> +static long read_timeout = 1000; /* ms to wait before read() times out */
> +static long write_timeout = 1000; /* ms to wait before write() times out */
>
> /* ----------------------------
> * module command-line arguments
> * ----------------------------
> */
>
> -module_param(read_timeout, int, 0444);
> +module_param(read_timeout, long, 0444);
> MODULE_PARM_DESC(read_timeout, "ms to wait before blocking read() timing
out;
> set to -1 for no timeout"); -module_param(write_timeout, int, 0444);
> +module_param(write_timeout, long, 0444);
> MODULE_PARM_DESC(write_timeout, "ms to wait before blocking write() timing
> out; set to -1 for no timeout");
>
> /* ----------------------------
> @@ -384,9 +384,7 @@ static ssize_t axis_fifo_read(struct file *f, char
__user
> *buf, mutex_lock(&fifo->read_lock);
> ret = wait_event_interruptible_timeout(fifo->read_queue,
> ioread32(fifo->base_addr + XLLF_RDFO_OFFSET),
> - (read_timeout >= 0) ?
> - msecs_to_jiffies(read_timeout) :
> - MAX_SCHEDULE_TIMEOUT);
> + read_timeout);
>
> if (ret <= 0) {
> if (ret == 0) {
> @@ -528,9 +526,7 @@ static ssize_t axis_fifo_write(struct file *f, const
char
> __user *buf, ret = wait_event_interruptible_timeout(fifo->write_queue,
> ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
>
> >= words_to_write,

What is this? You haven't yet compiled your patch.
Any further problems with enabling axis-fifo as a module?

Fabio

>
> - (write_timeout >= 0) ?
> - msecs_to_jiffies(write_timeout) :
> - MAX_SCHEDULE_TIMEOUT);
> + write_timeout);
>
> if (ret <= 0) {
> if (ret == 0) {
> @@ -815,6 +811,16 @@ static int axis_fifo_probe(struct platform_device
*pdev)
> char *device_name;
> int rc = 0; /* error return value */
>
> + if (read_timeout >= 0)
> + read_timeout = msecs_to_jiffies(read_timeout);
> + else
> + read_timeout = MAX_SCHEDULE_TIMEOUT;
> +
> + if (write_timeout >= 0)
> + write_timeout = msecs_to_jiffies(write_timeout);
> + else
> + write_timeout = MAX_SCHEDULE_TIMEOUT;
> +
> /* ----------------------------
> * init wrapper device
> * ----------------------------
> --
> 2.34.1





2023-03-16 15:10:04

by Khadija Kamran

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> Khadija,
>
> Just saw your v5 patch and Greg's two replies.
>
> For v6 you will need to change the subject to "[PATCH v6] staging: axis-fifo:
> initialize timeouts in init only" to indicate that you are doing assignments
> in axis_fifo_init().
>
> Don't forget to extend the version log with "Changes in v6:" and clarify that
> v5 had a different "Object" (you should probably also add a link to the v5
> patch in lore: https://lore.kernel.org/lkml /ZBMR4s8xyHGqMm72@khadija-virtual-
> machine/). When the "Subject" changes, readers may not find the previous
> versions easily.
>
> On gioved? 16 marzo 2023 13:56:02 CET 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
>
> Substitute "probe" with "init".
>
> > function.
> >
> > As a result, only use read_timeout as the last argument in
> > wait_event_interruptible_timeout() call.
>
> This two sentences are not much clear. I'd merge and rework:
>
> "Initialize the module parameters read_timeout and write_timeout once in
> init().
>
> Module parameters can only be set once and cannot be modified later, so we
> don't need to evaluate them again when passing the parameters to
> wait_event_interruptible_timeout()."
>
> >
> > Convert datatpe
>
> s/datatpe/type/
>
> > of read_timeout
>
> of {read,write}_timeout
>
> > from 'int' to 'long int' because
> > implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
>
> We don't care too much about the warning themselves: I mean, it overflows and
> you must avoid it to happen (as you are doing with the changes of types), not
> merely be interested in avoiding the warning. "[] results in an overflow." is
> all we care about.
>

Hey Fabio!
Thank you for your feedback. I have undertood it and will make sure to
send them in the next PATCH v6.

> Add also the previous paragraph in the last part of the commit message.
>
> > Perform same steps formodule parameter, write_timeout.
>
> And instead delete the this last phrase.
>

Can you please explain the above feedback. I am confused. What should I
use instead of this last phrase?

> > Suggested-by: Greg Kroah-Hartman <[email protected]>
> > Signed-off-by: Khadija Kamran <[email protected]>
> > ---
> >
> > Changes in v5:
> > - Convert timeout's datatype from int to long.
> > Changes in v4:
> > - Initialize timeouts once as suggested by Greg; this automatically
> > fixes the indentation problems.
> > - Change the subject and description.
> > Changes in v3:
> > - Fix grammatical mistakes
> > - Do not change the second argument's indentation in split lines
> > Changes in v2:
> > - Instead of matching alignment to open parenthesis, align second and
> > the last argument instead.
> > - Change the subject to 'remove tabs to align arguments'.
> > - Use imperative language in subject and description
> >
> > drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > 1 file changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/axis-fifo/axis-fifo.c
> > b/drivers/staging/axis-fifo/axis-fifo.c index dfd2b357f484..d667dc80df47
> > 100644
> > --- a/drivers/staging/axis-fifo/axis-fifo.c
> > +++ b/drivers/staging/axis-fifo/axis-fifo.c
> > @@ -103,17 +103,17 @@
> > * globals
> > * ----------------------------
> > */
> > -static int read_timeout = 1000; /* ms to wait before read() times out */
> > -static int write_timeout = 1000; /* ms to wait before write() times out */
> > +static long read_timeout = 1000; /* ms to wait before read() times out */
> > +static long write_timeout = 1000; /* ms to wait before write() times out */
> >
> > /* ----------------------------
> > * module command-line arguments
> > * ----------------------------
> > */
> >
> > -module_param(read_timeout, int, 0444);
> > +module_param(read_timeout, long, 0444);
> > MODULE_PARM_DESC(read_timeout, "ms to wait before blocking read() timing
> out;
> > set to -1 for no timeout"); -module_param(write_timeout, int, 0444);
> > +module_param(write_timeout, long, 0444);
> > MODULE_PARM_DESC(write_timeout, "ms to wait before blocking write() timing
> > out; set to -1 for no timeout");
> >
> > /* ----------------------------
> > @@ -384,9 +384,7 @@ static ssize_t axis_fifo_read(struct file *f, char
> __user
> > *buf, mutex_lock(&fifo->read_lock);
> > ret = wait_event_interruptible_timeout(fifo->read_queue,
> > ioread32(fifo->base_addr + XLLF_RDFO_OFFSET),
> > - (read_timeout >= 0) ?
> > - msecs_to_jiffies(read_timeout) :
> > - MAX_SCHEDULE_TIMEOUT);
> > + read_timeout);
> >
> > if (ret <= 0) {
> > if (ret == 0) {
> > @@ -528,9 +526,7 @@ static ssize_t axis_fifo_write(struct file *f, const
> char
> > __user *buf, ret = wait_event_interruptible_timeout(fifo->write_queue,
> > ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
> >
> > >= words_to_write,
>
> What is this? You haven't yet compiled your patch.
> Any further problems with enabling axis-fifo as a module?
>


Sorry, my bad. Instead of fixing the menuconfig I used this command to
remove the warnings:
make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
I thought it is compiling my module correctly.
But I am working on your feedback. And before sending my next patch I
will make sure to compile it properly.


> Fabio
>
> >
> > - (write_timeout >= 0) ?
> > - msecs_to_jiffies(write_timeout) :
> > - MAX_SCHEDULE_TIMEOUT);
> > + write_timeout);
> >
> > if (ret <= 0) {
> > if (ret == 0) {
> > @@ -815,6 +811,16 @@ static int axis_fifo_probe(struct platform_device
> *pdev)
> > char *device_name;
> > int rc = 0; /* error return value */
> >
> > + if (read_timeout >= 0)
> > + read_timeout = msecs_to_jiffies(read_timeout);
> > + else
> > + read_timeout = MAX_SCHEDULE_TIMEOUT;
> > +
> > + if (write_timeout >= 0)
> > + write_timeout = msecs_to_jiffies(write_timeout);
> > + else
> > + write_timeout = MAX_SCHEDULE_TIMEOUT;
> > +
> > /* ----------------------------
> > * init wrapper device
> > * ----------------------------
> > --
> > 2.34.1
>
>
>
>

2023-03-16 15:12:41

by Khadija Kamran

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

Hey Greg,
Thank you for the feedback, I am working on it.
I have some compilation problems currently. I'll make sure to fix them
before sending the next patch.

Regards,
Khadija


2023-03-16 16:18:25

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> > Khadija,
> >
> > Just saw your v5 patch and Greg's two replies.
> >
> > For v6 you will need to change the subject to "[PATCH v6] staging:
> > axis-fifo:
> > initialize timeouts in init only" to indicate that you are doing
assignments
> > in axis_fifo_init().
> >
> > Don't forget to extend the version log with "Changes in v6:" and clarify
> > that
> > v5 had a different "Object" (you should probably also add a link to the v5
> > patch in lore: https://lore.kernel.org/lkml
> > /ZBMR4s8xyHGqMm72@khadija-virtual- machine/). When the "Subject" changes,
> > readers may not find the previous versions easily.
> >
> > On gioved? 16 marzo 2023 13:56:02 CET 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
> >
> > Substitute "probe" with "init".
> >
> > > function.
> > >
> > > As a result, only use read_timeout as the last argument in
> > > wait_event_interruptible_timeout() call.
> >
> > This two sentences are not much clear. I'd merge and rework:
> >
> > "Initialize the module parameters read_timeout and write_timeout once in
> > init().
> >
> > Module parameters can only be set once and cannot be modified later, so we
> > don't need to evaluate them again when passing the parameters to
> > wait_event_interruptible_timeout()."
> >
> > > Convert datatpe
> >
> > s/datatpe/type/
> >
> > > of read_timeout
> >
> > of {read,write}_timeout
> >
> > > from 'int' to 'long int' because
> > > implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> > > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> >
> > We don't care too much about the warning themselves: I mean, it overflows
> > and
> > you must avoid it to happen (as you are doing with the changes of types),
> > not
> > merely be interested in avoiding the warning. "[] results in an overflow."
> > is
> > all we care about.
>
> Hey Fabio!
> Thank you for your feedback. I have understood it and will make sure to
> send them in the next PATCH v6.

Great to hear it!

> > Add also the previous paragraph in the last part of the commit message.
> >
> > > Perform same steps formodule parameter, write_timeout.
> >
> > And instead delete the this last phrase.
>
> Can you please explain the above feedback. I am confused. What should I
> use instead of this last phrase?

Sorry, I made a typo in the sentence above and that may confuse you :-(

I just intended to suggest to delete "Perform same steps formodule parameter,
write_timeout.".

In the previous lines I suggested you to merge and rework your entire commit
message. If you like it you are left with the following text (that I put for
you between two '"'):

"Initialize the module parameters read_timeout and write_timeout once in
init().

Module parameters can only be set once and cannot be modified later, so we
don't need to evaluate them again when passing the parameters to
wait_event_interruptible_timeout().

Convert the type of {read,write}_timeout from 'int' to 'long int' because
implicit conversion of 'long int' to 'int' in statement 'read_timeout =
MAX_SCHEDULE_TIMEOUT' results in an overflow.".

Just three small sentences are all you need (and don't forget to change the
Subject - "probe()" -> "init()".

I hope I have been clearer this time.
If not, please ask for further clarification.

Thanks,

Fabio

> > > Suggested-by: Greg Kroah-Hartman <[email protected]>
> > > Signed-off-by: Khadija Kamran <[email protected]>
> > > ---
> > >
> > > Changes in v5:
> > > - Convert timeout's datatype from int to long.
> > >
> > > Changes in v4:
> > > - Initialize timeouts once as suggested by Greg; this automatically
> > >
> > > fixes the indentation problems.
> > >
> > > - Change the subject and description.
> > >
> > > Changes in v3:
> > > - Fix grammatical mistakes
> > > - Do not change the second argument's indentation in split lines
> > >
> > > Changes in v2:
> > > - Instead of matching alignment to open parenthesis, align second and
> > >
> > > the last argument instead.
> > >
> > > - Change the subject to 'remove tabs to align arguments'.
> > > - Use imperative language in subject and description
> > >
> > > drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > > 1 file changed, 16 insertions(+), 10 deletions(-)

[snip]

> > >
> > > >= words_to_write,
> >
> > What is this? You haven't yet compiled your patch.
> > Any further problems with enabling axis-fifo as a module?
>
> Sorry, my bad. Instead of fixing the menuconfig I used this command to
> remove the warnings:
> make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> I thought it is compiling my module correctly.
> But I am working on your feedback. And before sending my next patch I
> will make sure to compile it properly.

When you are done with build, install, and final reboot to test if your module
can "modprobe" or "insmod" (i.e. link with the running custom kernel you
built, installed and boot), try to compare the output of the following
commands:

# uname -a
Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC 2023
(44ca817) x86_64 x86_64 x86_64 GNU/Linux

AND

# modinfo <name of the module you are testing here>

I'm running "modinfo kvm" (but showing only two of many lines):

# modinfo kvm
filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/kvm.ko.zst
vermagic: 6.2.2-1-default SMP preempt mod_unload modversions

Can you see that the kernel in "uname -a" and the filename and vermagic have
the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel and
inserted the appropriate "kvm" module.

Furthermore, before rebooting your custom kernel, you may also look at the
directory in the Kernel where you compiled your module and search for "*.o"
"*mod*" and "*.ko" files. If you have them, you built your module properly.

Thanks,

Fabio




2023-03-16 18:17:31

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On gioved? 16 marzo 2023 17:17:47 CET Fabio M. De Francesco wrote:
> On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> > > Khadija,
> > >
> > > Just saw your v5 patch and Greg's two replies.
> > >
> > > For v6 you will need to change the subject to "[PATCH v6] staging:
> > > axis-fifo:
> > > initialize timeouts in init only" to indicate that you are doing
>
> assignments
>
> > > in axis_fifo_init().
> > >
> > > Don't forget to extend the version log with "Changes in v6:" and clarify
> > > that
> > > v5 had a different "Object" (you should probably also add a link to the
v5
> > > patch in lore: https://lore.kernel.org/lkml
> > > /ZBMR4s8xyHGqMm72@khadija-virtual- machine/). When the "Subject"
changes,
> > > readers may not find the previous versions easily.
> > >
> > > On gioved? 16 marzo 2023 13:56:02 CET 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
> > >
> > > Substitute "probe" with "init".
> > >
> > > > function.
> > > >
> > > > As a result, only use read_timeout as the last argument in
> > > > wait_event_interruptible_timeout() call.
> > >
> > > This two sentences are not much clear. I'd merge and rework:
> > >
> > > "Initialize the module parameters read_timeout and write_timeout once in
> > > init().
> > >
> > > Module parameters can only be set once and cannot be modified later, so
we
> > > don't need to evaluate them again when passing the parameters to
> > > wait_event_interruptible_timeout()."
> > >
> > > > Convert datatpe
> > >
> > > s/datatpe/type/
> > >
> > > > of read_timeout
> > >
> > > of {read,write}_timeout
> > >
> > > > from 'int' to 'long int' because
> > > > implicit conversion of 'long int' to 'int' in statement 'read_timeout
=
> > > > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> > >
> > > We don't care too much about the warning themselves: I mean, it
overflows
> > > and
> > > you must avoid it to happen (as you are doing with the changes of
types),
> > > not
> > > merely be interested in avoiding the warning. "[] results in an
overflow."
> > > is
> > > all we care about.
> >
> > Hey Fabio!
> > Thank you for your feedback. I have understood it and will make sure to
> > send them in the next PATCH v6.
>
> Great to hear it!
>
> > > Add also the previous paragraph in the last part of the commit message.
> > >
> > > > Perform same steps formodule parameter, write_timeout.
> > >
> > > And instead delete the this last phrase.
> >
> > Can you please explain the above feedback. I am confused. What should I
> > use instead of this last phrase?
>
> Sorry, I made a typo in the sentence above and that may confuse you :-(
>
> I just intended to suggest to delete "Perform same steps formodule
parameter,
> write_timeout.".
>
> In the previous lines I suggested you to merge and rework your entire commit
> message. If you like it you are left with the following text (that I put for
> you between two '"'):
>
> "Initialize the module parameters read_timeout and write_timeout once in
> init().
>
> Module parameters can only be set once and cannot be modified later, so we
> don't need to evaluate them again when passing the parameters to
> wait_event_interruptible_timeout().
>
> Convert the type of {read,write}_timeout from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow.".
>
> Just three small sentences are all you need (and don't forget to change the
> Subject - "probe()" -> "init()".
>
> I hope I have been clearer this time.
> If not, please ask for further clarification.
>
> Thanks,
>
> Fabio

I put my signature here, but it was a mistake.
Did you see the rest of the message below?

> > > > Suggested-by: Greg Kroah-Hartman <[email protected]>
> > > > Signed-off-by: Khadija Kamran <[email protected]>
> > > > ---
> > > >
> > > > Changes in v5:
> > > > - Convert timeout's datatype from int to long.
> > > >
> > > > Changes in v4:
> > > > - Initialize timeouts once as suggested by Greg; this automatically
> > > >
> > > > fixes the indentation problems.
> > > >
> > > > - Change the subject and description.
> > > >
> > > > Changes in v3:
> > > > - Fix grammatical mistakes
> > > > - Do not change the second argument's indentation in split lines
> > > >
> > > > Changes in v2:
> > > > - Instead of matching alignment to open parenthesis, align second and
> > > >
> > > > the last argument instead.
> > > >
> > > > - Change the subject to 'remove tabs to align arguments'.
> > > > - Use imperative language in subject and description
> > > >
> > > > drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > > > 1 file changed, 16 insertions(+), 10 deletions(-)
>
> [snip]
>
> > > > >= words_to_write,
> > >
> > > What is this? You haven't yet compiled your patch.

This unnecessary objection was due to that split I was talking about in the v6
of your patch. At first glance it looked to me like a remnant of some mistake
with copy-pasting. Sorry, I should have looked at it more carefully.

> > > Any further problems with enabling axis-fifo as a module?
> > Sorry, my bad. Instead of fixing the menuconfig I used this command to
> > remove the warnings:
> > make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> > I thought it is compiling my module correctly.
> > But I am working on your feedback. And before sending my next patch I
> > will make sure to compile it properly.
>
> When you are done with build, install, and final reboot to test if your
module
> can "modprobe" or "insmod" (i.e. link with the running custom kernel you
> built, installed and boot), try to compare the output of the following
> commands:
>
> # uname -a
> Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC
2023
> (44ca817) x86_64 x86_64 x86_64 GNU/Linux
>
> AND
>
> # modinfo <name of the module you are testing here>
>
> I'm running "modinfo kvm" (but showing only two of many lines):
>
> # modinfo kvm
> filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/kvm.ko.zst
> vermagic: 6.2.2-1-default SMP preempt mod_unload modversions
>
> Can you see that the kernel in "uname -a" and the filename and vermagic have
> the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
and
> inserted the appropriate "kvm" module.
>
> Furthermore, before rebooting your custom kernel, you may also look at the
> directory in the Kernel where you compiled your module and search for "*.o"
> "*mod*" and "*.ko" files. If you have them, you built your module properly.
>
> Thanks,
>
> Fabio

Fabio




2023-03-16 18:35:19

by Khadija Kamran

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> > > Khadija,
> > >
> > > Just saw your v5 patch and Greg's two replies.
> > >
> > > For v6 you will need to change the subject to "[PATCH v6] staging:
> > > axis-fifo:
> > > initialize timeouts in init only" to indicate that you are doing
> assignments
> > > in axis_fifo_init().
> > >
> > > Don't forget to extend the version log with "Changes in v6:" and clarify
> > > that
> > > v5 had a different "Object" (you should probably also add a link to the v5
> > > patch in lore: https://lore.kernel.org/lkml
> > > /ZBMR4s8xyHGqMm72@khadija-virtual- machine/). When the "Subject" changes,
> > > readers may not find the previous versions easily.
> > >
> > > On gioved? 16 marzo 2023 13:56:02 CET 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
> > >
> > > Substitute "probe" with "init".
> > >
> > > > function.
> > > >
> > > > As a result, only use read_timeout as the last argument in
> > > > wait_event_interruptible_timeout() call.
> > >
> > > This two sentences are not much clear. I'd merge and rework:
> > >
> > > "Initialize the module parameters read_timeout and write_timeout once in
> > > init().
> > >
> > > Module parameters can only be set once and cannot be modified later, so we
> > > don't need to evaluate them again when passing the parameters to
> > > wait_event_interruptible_timeout()."
> > >
> > > > Convert datatpe
> > >
> > > s/datatpe/type/
> > >
> > > > of read_timeout
> > >
> > > of {read,write}_timeout
> > >
> > > > from 'int' to 'long int' because
> > > > implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> > > > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> > >
> > > We don't care too much about the warning themselves: I mean, it overflows
> > > and
> > > you must avoid it to happen (as you are doing with the changes of types),
> > > not
> > > merely be interested in avoiding the warning. "[] results in an overflow."
> > > is
> > > all we care about.
> >
> > Hey Fabio!
> > Thank you for your feedback. I have understood it and will make sure to
> > send them in the next PATCH v6.
>
> Great to hear it!
>
> > > Add also the previous paragraph in the last part of the commit message.
> > >
> > > > Perform same steps formodule parameter, write_timeout.
> > >
> > > And instead delete the this last phrase.
> >
> > Can you please explain the above feedback. I am confused. What should I
> > use instead of this last phrase?
>
> Sorry, I made a typo in the sentence above and that may confuse you :-(
>
> I just intended to suggest to delete "Perform same steps formodule parameter,
> write_timeout.".
>
> In the previous lines I suggested you to merge and rework your entire commit
> message. If you like it you are left with the following text (that I put for
> you between two '"'):
>
> "Initialize the module parameters read_timeout and write_timeout once in
> init().
>
> Module parameters can only be set once and cannot be modified later, so we
> don't need to evaluate them again when passing the parameters to
> wait_event_interruptible_timeout().
>
> Convert the type of {read,write}_timeout from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow.".
>
> Just three small sentences are all you need (and don't forget to change the
> Subject - "probe()" -> "init()".
>
> I hope I have been clearer this time.
> If not, please ask for further clarification.
>
> Thanks,
>
> Fabio
>
> > > > Suggested-by: Greg Kroah-Hartman <[email protected]>
> > > > Signed-off-by: Khadija Kamran <[email protected]>
> > > > ---
> > > >
> > > > Changes in v5:
> > > > - Convert timeout's datatype from int to long.
> > > >
> > > > Changes in v4:
> > > > - Initialize timeouts once as suggested by Greg; this automatically
> > > >
> > > > fixes the indentation problems.
> > > >
> > > > - Change the subject and description.
> > > >
> > > > Changes in v3:
> > > > - Fix grammatical mistakes
> > > > - Do not change the second argument's indentation in split lines
> > > >
> > > > Changes in v2:
> > > > - Instead of matching alignment to open parenthesis, align second and
> > > >
> > > > the last argument instead.
> > > >
> > > > - Change the subject to 'remove tabs to align arguments'.
> > > > - Use imperative language in subject and description
> > > >
> > > > drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > > > 1 file changed, 16 insertions(+), 10 deletions(-)
>
> [snip]
>
> > > >
> > > > >= words_to_write,
> > >
> > > What is this? You haven't yet compiled your patch.
> > > Any further problems with enabling axis-fifo as a module?
> >
> > Sorry, my bad. Instead of fixing the menuconfig I used this command to
> > remove the warnings:
> > make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> > I thought it is compiling my module correctly.
> > But I am working on your feedback. And before sending my next patch I
> > will make sure to compile it properly.
>

Hey Fabio!

Hope you are doing well. After spending a lot of time on this I am
stuck now. Kindly help me resolve this issueor understand it better.

Following your instructions I deleted my config file and copied one from
the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
a module.

I then ran the following commands:
- make drivers/staging/axis-fifo/
- sudo make modules_install install(this command took hours) :'(

> When you are done with build, install, and final reboot to test if your module
> can "modprobe" or "insmod" (i.e. link with the running custom kernel you
> built, installed and boot), try to compare the output of the following
> commands:
>
> # uname -a
> Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC 2023
> (44ca817) x86_64 x86_64 x86_64 GNU/Linux
>

The above command works

> AND
>
> # modinfo <name of the module you are testing here>
>

On running 'modinfo axis-fifo' I get error saying module axis-fifo not
found.

> I'm running "modinfo kvm" (but showing only two of many lines):
>
> # modinfo kvm
> filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/kvm.ko.zst
> vermagic: 6.2.2-1-default SMP preempt mod_unload modversions
>
> Can you see that the kernel in "uname -a" and the filename and vermagic have
> the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel and
> inserted the appropriate "kvm" module.
>
> Furthermore, before rebooting your custom kernel, you may also look at the
> directory in the Kernel where you compiled your module and search for "*.o"
> "*mod*" and "*.ko" files. If you have them, you built your module properly.
>

There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
the axis-fifo directory.

Kindly help me with this.

Regards,
Khadija


> Thanks,
>
> Fabio
>
>
>

2023-03-16 20:07:22

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On gioved? 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:

[snip]

> Hey Fabio!
>
> Hope you are doing well. After spending a lot of time on this I am
> stuck now. Kindly help me resolve this issue or understand it better.
>
> Following your instructions I deleted my config file and copied one from
> the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> a module.
>
> I then ran the following commands:
> - make drivers/staging/axis-fifo/

No, this is not the right command... you are not invoking the linker to make
the .ko object.

Use "make M=drivers/staging/axis-fifo/"
or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level '1'
warning and run on your 2 * 4 logical cores).

> - sudo make modules_install install(this command took hours) :'(

This is odd, it shouldn't :-/

As I said in another message, I'll set aside some time to help you check if
you need to fine tune your VM and Hypervisor configuration.

I'm returning on the same subject we have been talked about because you said
at least twice that your builds and install are too slow. We'll try to
diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for the
first days of next week - I'll send an invite).

> > When you are done with build, install, and final reboot to test if your
> > module can "modprobe" or "insmod" (i.e. link with the running custom
kernel
> > you built, installed and boot), try to compare the output of the following
> > commands:
> >
> > # uname -a
> > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC
> > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
>
> The above command works
>
> > AND
> >
> > # modinfo <name of the module you are testing here>
>
> On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> found.

Try again after building with "M=drivers/staging" (as said above). Don't
forget to run "make modules_install install" and then reboot into your custom
built Kernel, not the distribution's kernel.

While you are there, run "lsmod" to see all loaded modules. Pick one randomly
from the output list and run "modinfo name_of_the_module_you_want_info_about".

> > I'm running "modinfo kvm" (but showing only two of many lines):
> >
> > # modinfo kvm
> > filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
kvm.ko.zst
> > vermagic: 6.2.2-1-default SMP preempt mod_unload modversions
> >
> > Can you see that the kernel in "uname -a" and the filename and vermagic
have
> > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
> > and inserted the appropriate "kvm" module.
> >
> > Furthermore, before rebooting your custom kernel, you may also look at the
> > directory in the Kernel where you compiled your module and search for
"*.o"
> > "*mod*" and "*.ko" files. If you have them, you built your module
properly.
>
> There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> the axis-fifo directory.
>
> Kindly help me with this.
>
> Regards,
> Khadija
>
> > Thanks,
> >
> > Fabio

Let me know if this time it works.

Fabio

P.S.: Have you had time to read that "Linux Kernel Module Programming" guide I
sent you the link of? You can find a lot of information about modules there.
I'd strongly recommend you to read it.



2023-03-16 20:17:59

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On gioved? 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:

[snip]

> > When you are done with build, install, and final reboot to test if your
> > module can "modprobe" or "insmod" (i.e. link with the running custom
kernel
> > you built, installed and boot), try to compare the output of the following
> > commands:
> >
> > # uname -a
> > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC
> > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
>
> The above command works
>
> > AND
> >
> > # modinfo <name of the module you are testing here>
>
> On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> found.

I built axis-fifo with your changes and then I ran "make install
modules_install" in a QEMU/KVM x86_32 VM that I use on a Linux host for
testing my patches (Linux on Linux).

tweed32:~ # uname -a
Linux tweed32 6.3.0-rc2-x86-32-debug+ #32 SMP PREEMPT_DYNAMIC Thu Mar 16
18:09:49 CET 2023 i686 athlon i386 GNU/Linux

tweed32:~ # modinfo axis-fifo
filename: /lib/modules/6.3.0-rc2-x86-32-debug+/kernel/drivers/staging/
axis-fifo/axis-fifo.ko
description: Xilinx AXI-Stream FIFO v4.1 IP core driver
author: Jacob Feder <[email protected]>
license: GPL
srcversion: EBF46AD6851EAAE67D1000B
alias: of:N*T*Cxlnx,axi-fifo-mm-s-4.1C*
alias: of:N*T*Cxlnx,axi-fifo-mm-s-4.1
depends:
staging: Y
retpoline: Y
intree: Y
name: axis_fifo
vermagic: 6.3.0-rc2-x86-32-debug+ SMP preempt mod_unload modversions K7
parm: read_timeout:ms to wait before blocking read() timing out; set
to -1 for no timeout (long)
parm: write_timeout:ms to wait before blocking write() timing out;
set to -1 for no timeout (long)

Do you see the "parm" lines? What's the type of read and write_timeout?

Fabio



2023-03-17 07:00:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

Hi Khadija,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url: https://github.com/intel-lab-lkp/linux/commits/Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230316-205814
patch link: https://lore.kernel.org/r/ZBMR4s8xyHGqMm72%40khadija-virtual-machine
patch subject: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
config: hexagon-randconfig-r033-20230312 (https://download.01.org/0day-ci/archive/20230317/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7e359baa2318b55e12a0c099b75fbd0d799907cf
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230316-205814
git checkout 7e359baa2318b55e12a0c099b75fbd0d799907cf
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/staging/axis-fifo/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from drivers/staging/axis-fifo/axis-fifo.c:25:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/staging/axis-fifo/axis-fifo.c:25:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/staging/axis-fifo/axis-fifo.c:25:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
>> drivers/staging/axis-fifo/axis-fifo.c:958:3: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
read_timeout, write_timeout);
^~~~~~~~~~~~
include/linux/printk.h:528:34: note: expanded from macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:455:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:427:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:958:17: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
read_timeout, write_timeout);
^~~~~~~~~~~~~
include/linux/printk.h:528:34: note: expanded from macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:455:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:427:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
8 warnings generated.


vim +958 drivers/staging/axis-fifo/axis-fifo.c

4a965c5f89decd Jacob Feder 2018-07-22 954
4a965c5f89decd Jacob Feder 2018-07-22 955 static int __init axis_fifo_init(void)
4a965c5f89decd Jacob Feder 2018-07-22 956 {
4a965c5f89decd Jacob Feder 2018-07-22 957 pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
4a965c5f89decd Jacob Feder 2018-07-22 @958 read_timeout, write_timeout);
4a965c5f89decd Jacob Feder 2018-07-22 959 return platform_driver_register(&axis_fifo_driver);
4a965c5f89decd Jacob Feder 2018-07-22 960 }
4a965c5f89decd Jacob Feder 2018-07-22 961

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-03-20 06:00:58

by Khadija Kamran

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 09:07:09PM +0100, Fabio M. De Francesco wrote:
> On gioved? 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > > On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
>
> [snip]
>
> > Hey Fabio!
> >
> > Hope you are doing well. After spending a lot of time on this I am
> > stuck now. Kindly help me resolve this issue or understand it better.
> >
> > Following your instructions I deleted my config file and copied one from
> > the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> > and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> > a module.
> >
> > I then ran the following commands:
> > - make drivers/staging/axis-fifo/
>

Hey Fabio!
Sorry for reaching out to you very late. For the past two days I have
had problems with my Virtual Machine. It is stuck at boot, and this
happened after it accidentally shut down. I have tried to resolve this
problem but nothing is working. Currently I have boot into an older
kernel version from the GRUB menu.

I am also sharing the error here:
Gave up waiting for root device. Common problems:
-Boot args (cat /proc/cmdline)
-Check rootdelay= (did the system wait long enough?)
-Missing modules (cat /proc/modules; ls /dev)
ALERT! UUID=718ed077-947d-4018-80ad-59825678e81d does not exist.
Dropping to a shell!
BusyBox v1.27.2 (Ubuntu 1:1.27.2-2ubuntu3.2) built-in shell (ash)
Enter 'help' for a list of built-in commands.

I have Windows10 installed and I have created Ubunutu 22.04.1 VM on VMWare with
13GB RAM and 2 processors(4 cores each).

(initramfs)_
> No, this is not the right command... you are not invoking the linker to make
> the .ko object.
>
> Use "make M=drivers/staging/axis-fifo/"
> or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level '1'
> warning and run on your 2 * 4 logical cores).
>

This command gives error saying:
scripts/Makefile.build:41: /drivers/staging/axis-fifo/Makefile:
No such file or directory
make[1]: *** No rule to make target '/drivers/staging/axis-fifo/Makefile'. Stop.
make: *** [Makefile:2028: /drivers/staging/axis-fifo] Error 2

> > - sudo make modules_install install(this command took hours) :'(
>
> This is odd, it shouldn't :-/
>
> As I said in another message, I'll set aside some time to help you check if
> you need to fine tune your VM and Hypervisor configuration.
>
> I'm returning on the same subject we have been talked about because you said
> at least twice that your builds and install are too slow. We'll try to
> diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for the
> first days of next week - I'll send an invite).
>

Yes I would love that. Kindly help me with this.
Thank you!

Regards,
Khadija Kamran

> > > When you are done with build, install, and final reboot to test if your
> > > module can "modprobe" or "insmod" (i.e. link with the running custom
> kernel
> > > you built, installed and boot), try to compare the output of the following
> > > commands:
> > >
> > > # uname -a
> > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC
> > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> >
> > The above command works
> >
> > > AND
> > >
> > > # modinfo <name of the module you are testing here>
> >
> > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > found.
>
> Try again after building with "M=drivers/staging" (as said above). Don't
> forget to run "make modules_install install" and then reboot into your custom
> built Kernel, not the distribution's kernel.
>
> While you are there, run "lsmod" to see all loaded modules. Pick one randomly
> from the output list and run "modinfo name_of_the_module_you_want_info_about".
>
> > > I'm running "modinfo kvm" (but showing only two of many lines):
> > >
> > > # modinfo kvm
> > > filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
> kvm.ko.zst
> > > vermagic: 6.2.2-1-default SMP preempt mod_unload modversions
> > >
> > > Can you see that the kernel in "uname -a" and the filename and vermagic
> have
> > > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
> > > and inserted the appropriate "kvm" module.
> > >
> > > Furthermore, before rebooting your custom kernel, you may also look at the
> > > directory in the Kernel where you compiled your module and search for
> "*.o"
> > > "*mod*" and "*.ko" files. If you have them, you built your module
> properly.
> >
> > There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> > the axis-fifo directory.
> >
> > Kindly help me with this.
> >
> > Regards,
> > Khadija
> >
> > > Thanks,
> > >
> > > Fabio
>
> Let me know if this time it works.
>
> Fabio
>
> P.S.: Have you had time to read that "Linux Kernel Module Programming" guide I
> sent you the link of? You can find a lot of information about modules there.
> I'd strongly recommend you to read it.
>
>

2023-03-20 06:04:43

by Khadija Kamran

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 09:17:52PM +0100, Fabio M. De Francesco wrote:
> On gioved? 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
>
> [snip]
>
> > > When you are done with build, install, and final reboot to test if your
> > > module can "modprobe" or "insmod" (i.e. link with the running custom
> kernel
> > > you built, installed and boot), try to compare the output of the following
> > > commands:
> > >
> > > # uname -a
> > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC
> > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> >
> > The above command works
> >
> > > AND
> > >
> > > # modinfo <name of the module you are testing here>
> >
> > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > found.
>
> I built axis-fifo with your changes and then I ran "make install
> modules_install" in a QEMU/KVM x86_32 VM that I use on a Linux host for
> testing my patches (Linux on Linux).
>
> tweed32:~ # uname -a
> Linux tweed32 6.3.0-rc2-x86-32-debug+ #32 SMP PREEMPT_DYNAMIC Thu Mar 16
> 18:09:49 CET 2023 i686 athlon i386 GNU/Linux
>

Hey Fabio!

The following command is not working for me. I think I have mentioned
this before, it says, module axis-fifo not found.

Thank you!

Regards,
Khadija

> tweed32:~ # modinfo axis-fifo
> filename: /lib/modules/6.3.0-rc2-x86-32-debug+/kernel/drivers/staging/
> axis-fifo/axis-fifo.ko
> description: Xilinx AXI-Stream FIFO v4.1 IP core driver
> author: Jacob Feder <[email protected]>
> license: GPL
> srcversion: EBF46AD6851EAAE67D1000B
> alias: of:N*T*Cxlnx,axi-fifo-mm-s-4.1C*
> alias: of:N*T*Cxlnx,axi-fifo-mm-s-4.1
> depends:
> staging: Y
> retpoline: Y
> intree: Y
> name: axis_fifo
> vermagic: 6.3.0-rc2-x86-32-debug+ SMP preempt mod_unload modversions K7
> parm: read_timeout:ms to wait before blocking read() timing out; set
> to -1 for no timeout (long)
> parm: write_timeout:ms to wait before blocking write() timing out;
> set to -1 for no timeout (long)
>
> Do you see the "parm" lines? What's the type of read and write_timeout?
>
> Fabio
>
>

2023-03-20 06:37:58

by Khadija Kamran

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On Thu, Mar 16, 2023 at 09:07:09PM +0100, Fabio M. De Francesco wrote:
> On gioved? 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > > On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
>
> [snip]
>
> > Hey Fabio!
> >
> > Hope you are doing well. After spending a lot of time on this I am
> > stuck now. Kindly help me resolve this issue or understand it better.
> >
> > Following your instructions I deleted my config file and copied one from
> > the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> > and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> > a module.
> >
> > I then ran the following commands:
> > - make drivers/staging/axis-fifo/
>
> No, this is not the right command... you are not invoking the linker to make
> the .ko object.
>
> Use "make M=drivers/staging/axis-fifo/"
> or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level '1'
> warning and run on your 2 * 4 logical cores).
>
> > - sudo make modules_install install(this command took hours) :'(
>
> This is odd, it shouldn't :-/
>
> As I said in another message, I'll set aside some time to help you check if
> you need to fine tune your VM and Hypervisor configuration.
>
> I'm returning on the same subject we have been talked about because you said
> at least twice that your builds and install are too slow. We'll try to
> diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for the
> first days of next week - I'll send an invite).
>
> > > When you are done with build, install, and final reboot to test if your
> > > module can "modprobe" or "insmod" (i.e. link with the running custom
> kernel
> > > you built, installed and boot), try to compare the output of the following
> > > commands:
> > >
> > > # uname -a
> > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13 UTC
> > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> >
> > The above command works
> >
> > > AND
> > >
> > > # modinfo <name of the module you are testing here>
> >
> > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > found.
>
> Try again after building with "M=drivers/staging" (as said above). Don't
> forget to run "make modules_install install" and then reboot into your custom
> built Kernel, not the distribution's kernel.
>
> While you are there, run "lsmod" to see all loaded modules. Pick one randomly
> from the output list and run "modinfo name_of_the_module_you_want_info_about".
>
> > > I'm running "modinfo kvm" (but showing only two of many lines):
> > >
> > > # modinfo kvm
> > > filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
> kvm.ko.zst
> > > vermagic: 6.2.2-1-default SMP preempt mod_unload modversions
> > >
> > > Can you see that the kernel in "uname -a" and the filename and vermagic
> have
> > > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
> > > and inserted the appropriate "kvm" module.
> > >
> > > Furthermore, before rebooting your custom kernel, you may also look at the
> > > directory in the Kernel where you compiled your module and search for
> "*.o"
> > > "*mod*" and "*.ko" files. If you have them, you built your module
> properly.
> >
> > There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> > the axis-fifo directory.
> >
> > Kindly help me with this.
> >
> > Regards,
> > Khadija
> >
> > > Thanks,
> > >
> > > Fabio
>
> Let me know if this time it works.
>
> Fabio
>
> P.S.: Have you had time to read that "Linux Kernel Module Programming" guide I
> sent you the link of? You can find a lot of information about modules there.
> I'd strongly recommend you to read it.
>

Fabio,

I have not read it yet. But it is in my mind and as soon as I get some
free time I will start reading it :)

Thank you!

>

2023-03-20 14:10:07

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only

On luned? 20 marzo 2023 07:00:28 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 09:07:09PM +0100, Fabio M. De Francesco wrote:
> > On gioved? 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > > > On gioved? 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> > [snip]
> >
> > > Hey Fabio!
> > >
> > > Hope you are doing well. After spending a lot of time on this I am
> > > stuck now. Kindly help me resolve this issue or understand it better.
> > >
> > > Following your instructions I deleted my config file and copied one from
> > > the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> > > and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> > > a module.
> > >
> > > I then ran the following commands:
> > > - make drivers/staging/axis-fifo/
>
> Hey Fabio!
> Sorry for reaching out to you very late. For the past two days I have
> had problems with my Virtual Machine. It is stuck at boot, and this
> happened after it accidentally shut down. I have tried to resolve this
> problem but nothing is working. Currently I have boot into an older
> kernel version from the GRUB menu.
>
> I am also sharing the error here:
> Gave up waiting for root device. Common problems:
> -Boot args (cat /proc/cmdline)
> -Check rootdelay= (did the system wait long enough?)
> -Missing modules (cat /proc/modules; ls /dev)
> ALERT! UUID=718ed077-947d-4018-80ad-59825678e81d does not exist.
> Dropping to a shell!
> BusyBox v1.27.2 (Ubuntu 1:1.27.2-2ubuntu3.2) built-in shell (ash)
> Enter 'help' for a list of built-in commands.
>
> I have Windows10 installed and I have created Ubunutu 22.04.1 VM on VMWare
> with 13GB RAM and 2 processors(4 cores each).
>
> (initramfs)_
>
> > No, this is not the right command... you are not invoking the linker to
make
> > the .ko object.
> >
> > Use "make M=drivers/staging/axis-fifo/"
> > or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level
> > '1' warning and run on your 2 * 4 logical cores).
>
> This command gives error saying:
> scripts/Makefile.build:41: /drivers/staging/axis-fifo/Makefile:
> No such file or directory
> make[1]: *** No rule to make target '/drivers/staging/axis-fifo/Makefile'.
> Stop. make: *** [Makefile:2028: /drivers/staging/axis-fifo] Error 2
>
> > > - sudo make modules_install install(this command took hours) :'(
> >
> > This is odd, it shouldn't :-/
> >
> > As I said in another message, I'll set aside some time to help you check
if
> > you need to fine tune your VM and Hypervisor configuration.
> >
> > I'm returning on the same subject we have been talked about because you
said
> > at least twice that your builds and install are too slow. We'll try to
> > diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for
the
> > first days of next week - I'll send an invite).
>
> Yes I would love that. Kindly help me with this.
> Thank you!
>
> Regards,
> Khadija Kamran
>
> > > > When you are done with build, install, and final reboot to test if
your
> > > > module can "modprobe" or "insmod" (i.e. link with the running custom
> >
> > kernel
> >
> > > > you built, installed and boot), try to compare the output of the
> > > > following
> > > > commands:
> > > >
> > > > # uname -a
> > > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar 9 06:06:13
> > > > UTC
> > > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> > >
> > > The above command works
> > >
> > > > AND
> > > >
> > > > # modinfo <name of the module you are testing here>
> > >
> > > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > > found.
> >
> > Try again after building with "M=drivers/staging" (as said above). Don't
> > forget to run "make modules_install install" and then reboot into your
> > custom
> > built Kernel, not the distribution's kernel.
> >
> > While you are there, run "lsmod" to see all loaded modules. Pick one
> > randomly
> > from the output list and run "modinfo
> > name_of_the_module_you_want_info_about".>
> > > > I'm running "modinfo kvm" (but showing only two of many lines):
> > > >
> > > > # modinfo kvm
> > > > filename: /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
> >
> > kvm.ko.zst
> >
> > > > vermagic: 6.2.2-1-default SMP preempt mod_unload modversions
> > > >
> > > > Can you see that the kernel in "uname -a" and the filename and
vermagic
> >
> > have
> >
> > > > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right
> > > > Kernel
> > > > and inserted the appropriate "kvm" module.
> > > >
> > > > Furthermore, before rebooting your custom kernel, you may also look at
> > > > the
> > > > directory in the Kernel where you compiled your module and search for
> >
> > "*.o"
> >
> > > > "*mod*" and "*.ko" files. If you have them, you built your module
> >
> > properly.
> >
> > > There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> > > the axis-fifo directory.
> > >
> > > Kindly help me with this.
> > >
> > > Regards,
> > > Khadija
> > >
> > > > Thanks,
> > > >
> > > > Fabio
> >
> > Let me know if this time it works.
> >
> > Fabio
> >
> > P.S.: Have you had time to read that "Linux Kernel Module Programming"
guide
> > I sent you the link of? You can find a lot of information about modules
> > there. I'd strongly recommend you to read it.

Khadija,

I think that you are trying to skip some passages. Let me explain better...

Soon after copying from /boot your new .config you should run "make
menuconfig" or "make oldconfig" to re-create the necessary dependence and
other files which are required to build properly.

I'd suggest "make menuconfig" so that you can check if you enabled correctly
the module(s) you are interested to build and other built-in options you may
optionally need.

Then you must rebuild and install. So...

1) "make menuconfig" (don't forget to save before exiting),
2) "make W=1 -j8" (because you have 8 logical cores reserved to your VMWare
VM, correct?).
3) "make modules_install install".

At this point check that you have a fresh kernel image in /boot, that /boot/
grub/grub.conf contains a new entry with your freshly built custom kernel so
that you can boot it from selecting the proper entry in the menu that you'll
see after reboot. Also check that you have axis-fifo.ko under /lib/modules/
<the new kernel version>/build/drivers/staging/axis-fifo/.

For example in my VM for test I see axis-fifo with...

ls -l /lib/modules/6.3.0-rc2-x86-32-debug+/build/drivers/staging/axis-fifo/
total 564
-rw-r--r-- 1 1001 fsgqa 196 Mar 16 20:57 .Module.symvers.cmd
-rw-r--r-- 1 1001 fsgqa 275 Mar 17 15:39 .axis-fifo.ko.cmd
-rw-r--r-- 1 1001 fsgqa 183 Mar 12 20:29 .axis-fifo.mod.cmd
-rw-r--r-- 1 1001 fsgqa 50093 Mar 17 15:39 .axis-fifo.mod.o.cmd
-rw-r--r-- 1 1001 fsgqa 61110 Mar 17 15:37 .axis-fifo.o.cmd
-rw-r--r-- 1 1001 fsgqa 147 Mar 17 15:37 .modules.order.cmd
-rw-r--r-- 1 1001 fsgqa 0 Mar 16 20:57 Module.symvers
-rw-r--r-- 1 1001 fsgqa 255748 Mar 17 15:39 axis-fifo.ko
-rw-r--r-- 1 1001 fsgqa 38 Mar 12 20:29 axis-fifo.mod
-rw-r--r-- 1 1001 fsgqa 2354 Mar 17 15:38 axis-fifo.mod.c
-rw-r--r-- 1 1001 fsgqa 44604 Mar 17 15:39 axis-fifo.mod.o
-rw-r--r-- 1 1001 fsgqa 128156 Mar 17 15:37 axis-fifo.o
-rw-r--r-- 1 1001 fsgqa 38 Mar 17 15:37 modules.order

(as you can see, the last time I built and installed the kernel and the axis-
fifo driver's module was March 17, 15:39).

Please notice that my kernel has that "-x86-32-debug+" suffix after the kernel
version but this depends on a special option I need to set to differentiate
"debug" kernels from "production" kernels. You may have something else that
your distribution appends.

If you can't find the module run again "ls -l" one level up, like in /lib/
modules/6.3.0-rc2-x86-32-debug+/build/drivers/staging/ to see if at least you
have any of the drivers/staging/ modules. Go one more layer up if you don't
find them.

If everything I cited above is where it is supposed to be and grub.conf is
properly configured with the new entry, reboot and enjoy your module (try
modprobe, modinfo, lsmod, and the likes).

Fabio

Fabio