2021-08-27 02:20:59

by Krish Jain

[permalink] [raw]
Subject: [PATCH] Declare the file_operations struct as const

From: Krish Jain <[email protected]>

Declare the file_operations struct as const as done elsewhere in the
kernel, as there are no modifications to its fields.

Signed-off-by: Krish Jain <[email protected]>
---
drivers/staging/android/ashmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index ddbde3f8430e..ec18f2ddd66c 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -377,7 +377,7 @@ ashmem_vmfile_get_unmapped_area(struct file *file,
unsigned long addr,

static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
{
- static struct file_operations vmfile_fops;
+ const static struct file_operations vmfile_fops;
struct ashmem_area *asma = file->private_data;
int ret = 0;

--
2.25.1


2021-08-27 07:49:07

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Friday, August 27, 2021 8:49:30 AM CEST Fabio M. De Francesco wrote:
> On Friday, August 27, 2021 3:59:28 AM CEST Krish Jain wrote:
> > From: Krish Jain <[email protected]>
> >
> > Declare the file_operations struct as const as done elsewhere in the
> > kernel, as there are no modifications to its fields.
> >
> > Signed-off-by: Krish Jain <[email protected]>
> > ---
> > []
> Are you sure that it works? I wouldn't be.
> You didn't build this file. Please build your changes before submitting patches.
>
> Furthermore, please always rebase to the current version of the staging tree.
>
> Finally, please use the class modifier "static" as the first keyword of a declaration/definition
> as it is done everywhere in the kernel (see "grep -rn "static const" drivers/staging/" and
> then switch to "const static" and read the output).

Please don't misunderstand me: as far as I can see this is your first patch and
(I'm pretty sure I can speak for everyone else about this) you are very welcome
to staging and to kernel hacking :)

However, before posting further works, you'd better read at least the following
documents:

https://www.kernel.org/doc/html/latest/process/4.Coding.html
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

And please don't forget to always CC [email protected].

Have a nice time with kernel hacking.

Thanks,

Fabio


2021-08-27 08:52:52

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Hi, yes, this is my first time programming at this low level. And yes,
I read both docs now. Furthermore the issue is that my current
hardware can't handle building the kernel, it barely managed to
survive the first build after 2 hours so I don't know how I can. If I
change it to static const would it fix the issue and build
successfully? If not what would be the error message, then I can
debug. Thanks

On Fri, Aug 27, 2021 at 9:48 AM Fabio M. De Francesco
<[email protected]> wrote:
>
> On Friday, August 27, 2021 8:49:30 AM CEST Fabio M. De Francesco wrote:
> > On Friday, August 27, 2021 3:59:28 AM CEST Krish Jain wrote:
> > > From: Krish Jain <[email protected]>
> > >
> > > Declare the file_operations struct as const as done elsewhere in the
> > > kernel, as there are no modifications to its fields.
> > >
> > > Signed-off-by: Krish Jain <[email protected]>
> > > ---
> > > []
> > Are you sure that it works? I wouldn't be.
> > You didn't build this file. Please build your changes before submitting patches.
> >
> > Furthermore, please always rebase to the current version of the staging tree.
> >
> > Finally, please use the class modifier "static" as the first keyword of a declaration/definition
> > as it is done everywhere in the kernel (see "grep -rn "static const" drivers/staging/" and
> > then switch to "const static" and read the output).
>
> Please don't misunderstand me: as far as I can see this is your first patch and
> (I'm pretty sure I can speak for everyone else about this) you are very welcome
> to staging and to kernel hacking :)
>
> However, before posting further works, you'd better read at least the following
> documents:
>
> https://www.kernel.org/doc/html/latest/process/4.Coding.html
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> And please don't forget to always CC [email protected].
>
> Have a nice time with kernel hacking.
>
> Thanks,
>
> Fabio
>
>

2021-08-27 18:40:36

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

So what do you think I can do?

Best Regards


On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <[email protected]> wrote:
>
> Hi, yes, this is my first time programming at this low level. And yes,
> I read both docs now. Furthermore the issue is that my current
> hardware can't handle building the kernel, it barely managed to
> survive the first build after 2 hours so I don't know how I can. If I
> change it to static const would it fix the issue and build
> successfully? If not what would be the error message, then I can
> debug. Thanks
>
> On Fri, Aug 27, 2021 at 9:48 AM Fabio M. De Francesco
> <[email protected]> wrote:
> >
> > On Friday, August 27, 2021 8:49:30 AM CEST Fabio M. De Francesco wrote:
> > > On Friday, August 27, 2021 3:59:28 AM CEST Krish Jain wrote:
> > > > From: Krish Jain <[email protected]>
> > > >
> > > > Declare the file_operations struct as const as done elsewhere in the
> > > > kernel, as there are no modifications to its fields.
> > > >
> > > > Signed-off-by: Krish Jain <[email protected]>
> > > > ---
> > > > []
> > > Are you sure that it works? I wouldn't be.
> > > You didn't build this file. Please build your changes before submitting patches.
> > >
> > > Furthermore, please always rebase to the current version of the staging tree.
> > >
> > > Finally, please use the class modifier "static" as the first keyword of a declaration/definition
> > > as it is done everywhere in the kernel (see "grep -rn "static const" drivers/staging/" and
> > > then switch to "const static" and read the output).
> >
> > Please don't misunderstand me: as far as I can see this is your first patch and
> > (I'm pretty sure I can speak for everyone else about this) you are very welcome
> > to staging and to kernel hacking :)
> >
> > However, before posting further works, you'd better read at least the following
> > documents:
> >
> > https://www.kernel.org/doc/html/latest/process/4.Coding.html
> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> >
> > And please don't forget to always CC [email protected].
> >
> > Have a nice time with kernel hacking.
> >
> > Thanks,
> >
> > Fabio
> >
> >

2021-08-27 19:47:24

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

I unfortunately forgot to add Greg to this thread. Doing so now. I
apologize for the confusion, if any. This patch was regarding the
staging tree's file android/ashmem.c and declaring the file_operations
struct as const as done elsewhere in the kernel, because there are no
modifications to its fields.

Warm Regards


On Fri, Aug 27, 2021 at 8:38 PM Krish Jain <[email protected]> wrote:
>
> So what do you think I can do?
>
> Best Regards
>
>
> On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <[email protected]> wrote:
> >
> > Hi, yes, this is my first time programming at this low level. And yes,
> > I read both docs now. Furthermore the issue is that my current
> > hardware can't handle building the kernel, it barely managed to
> > survive the first build after 2 hours so I don't know how I can. If I
> > change it to static const would it fix the issue and build
> > successfully? If not what would be the error message, then I can
> > debug. Thanks
> >
> > On Fri, Aug 27, 2021 at 9:48 AM Fabio M. De Francesco
> > <[email protected]> wrote:
> > >
> > > On Friday, August 27, 2021 8:49:30 AM CEST Fabio M. De Francesco wrote:
> > > > On Friday, August 27, 2021 3:59:28 AM CEST Krish Jain wrote:
> > > > > From: Krish Jain <[email protected]>
> > > > >
> > > > > Declare the file_operations struct as const as done elsewhere in the
> > > > > kernel, as there are no modifications to its fields.
> > > > >
> > > > > Signed-off-by: Krish Jain <[email protected]>
> > > > > ---
> > > > > []
> > > > Are you sure that it works? I wouldn't be.
> > > > You didn't build this file. Please build your changes before submitting patches.
> > > >
> > > > Furthermore, please always rebase to the current version of the staging tree.
> > > >
> > > > Finally, please use the class modifier "static" as the first keyword of a declaration/definition
> > > > as it is done everywhere in the kernel (see "grep -rn "static const" drivers/staging/" and
> > > > then switch to "const static" and read the output).
> > >
> > > Please don't misunderstand me: as far as I can see this is your first patch and
> > > (I'm pretty sure I can speak for everyone else about this) you are very welcome
> > > to staging and to kernel hacking :)
> > >
> > > However, before posting further works, you'd better read at least the following
> > > documents:
> > >
> > > https://www.kernel.org/doc/html/latest/process/4.Coding.html
> > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> > >
> > > And please don't forget to always CC [email protected].
> > >
> > > Have a nice time with kernel hacking.
> > >
> > > Thanks,
> > >
> > > Fabio
> > >
> > >

2021-08-27 23:42:33

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Hi Krish!

I'm sure someone has said something by now, however "top posting", where
you reply to emails by writing on the top can make things hard to read
on the mailing lists. The conversation is upside down when reading.

https://en.wikipedia.org/wiki/Posting_style#Top-posting

Next time try writing underneath the text your referring to like this:
Don't worry we're all learning here :)

On this day, August 27, 2021 thus sayeth Krish Jain:
> I unfortunately forgot to add Greg to this thread. Doing so now. I
> apologize for the confusion, if any. This patch was regarding the
> staging tree's file android/ashmem.c and declaring the file_operations
> struct as const as done elsewhere in the kernel, because there are no
> modifications to its fields.
>
> Warm Regards
>

Things can be a little deceiving in the kernel. That's why testing your
changes before you submit them can be helpful.

But don't worry too much if you break something, there are countless
bots trying to break the kernel every day. It usually means you're
learning when you break something.

>
> On Fri, Aug 27, 2021 at 8:38 PM Krish Jain <[email protected]> wrote:
> >
> > So what do you think I can do?
> >
> > Best Regards
> >

That's a tough one as it really depends on your situation. When I first
started programming I had a *really* old (even for that time) laptop
that couldn't do much. It wasn't ideal but I found I could connect
through ssh to a virtual machine my university lent me to "learn to
code".

I have no idea what your situation is like. Though having a second
computer to compile code while I wrote worked for me.

> >
> > On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <[email protected]> wrote:
> > >
> > > Hi, yes, this is my first time programming at this low level. And yes,
> > > I read both docs now. Furthermore the issue is that my current
> > > hardware can't handle building the kernel, it barely managed to
> > > survive the first build after 2 hours so I don't know how I can. If I
> > > change it to static const would it fix the issue and build
> > > successfully? If not what would be the error message, then I can
> > > debug. Thanks
> > >

As for your patch, I built the driver using:

$ make CCFLAGS=-Werror W=1 M=drivers/staging/android

Which produced the following error:


drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
380 | const static struct file_operations vmfile_fops;
| ^~~~~
drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
431 | vmfile_fops = *vmfile->f_op;
| ^
drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
| ^
drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
433 | vmfile_fops.get_unmapped_area =
| ^
make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
make: *** [Makefile:1851: drivers/staging/android] Error 2


You shouldn't need to compile the entire kernel. That may be why your
computer is having a hard time?

Hope this helps :) and it was nice to meet you Krish
~Bryan

2021-08-28 09:40:01

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Hi. Thanks for your response. Changing to "const static" would fix
the first error but looking at the second error indicates that it
can't be a const, right? So checkpatch.pl was wrong?


Best Regards

On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
>
> Hi Krish!
>
> I'm sure someone has said something by now, however "top posting", where
> you reply to emails by writing on the top can make things hard to read
> on the mailing lists. The conversation is upside down when reading.
>
> https://en.wikipedia.org/wiki/Posting_style#Top-posting
>
> Next time try writing underneath the text your referring to like this:
> Don't worry we're all learning here :)
>
> On this day, August 27, 2021 thus sayeth Krish Jain:
> > I unfortunately forgot to add Greg to this thread. Doing so now. I
> > apologize for the confusion, if any. This patch was regarding the
> > staging tree's file android/ashmem.c and declaring the file_operations
> > struct as const as done elsewhere in the kernel, because there are no
> > modifications to its fields.
> >
> > Warm Regards
> >
>
> Things can be a little deceiving in the kernel. That's why testing your
> changes before you submit them can be helpful.
>
> But don't worry too much if you break something, there are countless
> bots trying to break the kernel every day. It usually means you're
> learning when you break something.
>
> >
> > On Fri, Aug 27, 2021 at 8:38 PM Krish Jain <[email protected]> wrote:
> > >
> > > So what do you think I can do?
> > >
> > > Best Regards
> > >
>
> That's a tough one as it really depends on your situation. When I first
> started programming I had a *really* old (even for that time) laptop
> that couldn't do much. It wasn't ideal but I found I could connect
> through ssh to a virtual machine my university lent me to "learn to
> code".
>
> I have no idea what your situation is like. Though having a second
> computer to compile code while I wrote worked for me.
>
> > >
> > > On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <[email protected]> wrote:
> > > >
> > > > Hi, yes, this is my first time programming at this low level. And yes,
> > > > I read both docs now. Furthermore the issue is that my current
> > > > hardware can't handle building the kernel, it barely managed to
> > > > survive the first build after 2 hours so I don't know how I can. If I
> > > > change it to static const would it fix the issue and build
> > > > successfully? If not what would be the error message, then I can
> > > > debug. Thanks
> > > >
>
> As for your patch, I built the driver using:
>
> $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
>
> Which produced the following error:
>
>
> drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> 380 | const static struct file_operations vmfile_fops;
> | ^~~~~
> drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> 431 | vmfile_fops = *vmfile->f_op;
> | ^
> drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> | ^
> drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> 433 | vmfile_fops.get_unmapped_area =
> | ^
> make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> make: *** [Makefile:1851: drivers/staging/android] Error 2
>
>
> You shouldn't need to compile the entire kernel. That may be why your
> computer is having a hard time?
>
> Hope this helps :) and it was nice to meet you Krish
> ~Bryan
>

2021-08-28 09:48:39

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Sat, Aug 28, 2021 at 11:37:50AM +0200, Krish Jain wrote:
> Hi. Thanks for your response. Changing to "const static" would fix
> the first error but looking at the second error indicates that it
> can't be a const, right? So checkpatch.pl was wrong?

checkpatch.pl is a perl script that does its best here. You always have
to then look at the code itself to see if what it is asking you to do is
correct.

And you always have to at the very least, test build your changes to
verify that they do not break anything.

thanks,

greg k-h

2021-08-28 09:55:12

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Sat, Aug 28, 2021 at 11:46 AM Greg KH <[email protected]> wrote:
>
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top


Now I get it. I've never used this style of email ever before so am a
novice. Forgive me. Also I didn't get what you mean should I include
quotations after my reply?

> On Sat, Aug 28, 2021 at 11:37:50AM +0200, Krish Jain wrote:
> > Hi. Thanks for your response. Changing to "const static" would fix
> > the first error but looking at the second error indicates that it
> > can't be a const, right? So checkpatch.pl was wrong?
>
> checkpatch.pl is a perl script that does its best here. You always have
> to then look at the code itself to see if what it is asking you to do is
> correct.
>
> And you always have to at the very least, test build your changes to
> verify that they do not break anything.
>
> thanks,
>
> greg k-h

Thank you so much. I didn't realize that I could have tested it by
just building the driver and not the entire kernel. Anyway, I'd still
love to learn more and contribute to the kernel. Where can I find
"small fixes" I can make?


Warm Regards,

Krish

2021-08-28 11:15:08

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Saturday, August 28, 2021 11:52:39 AM CEST Krish Jain wrote:
> On Sat, Aug 28, 2021 at 11:46 AM Greg KH <[email protected]> wrote:
> >
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> >
> > A: No.
> > Q: Should I include quotations after my reply?
> >
> > http://daringfireball.net/2007/07/on_top
>
>
> Now I get it. I've never used this style of email ever before so am a
> novice. Forgive me. Also I didn't get what you mean should I include
> quotations after my reply?

No, Krish. Greg placed the lines you read above only to let you understand
how much confusion we get with top-posting :)

> > On Sat, Aug 28, 2021 at 11:37:50AM +0200, Krish Jain wrote:
> > > Hi. Thanks for your response. Changing to "const static" would fix
> > > the first error but looking at the second error indicates that it
> > > can't be a const, right? So checkpatch.pl was wrong?

You misunderstood my first message in reply to your patch:

(1) that structure shouldn't be "const". You broke the build with that so I
guessed that you didn't build the driver;

(2) You should place "static" as the first keyword of a declaration. It doesn't
change the semantics, but it is better style and so it is used in the kernel.

> > checkpatch.pl is a perl script that does its best here. You always have
> > to then look at the code itself to see if what it is asking you to do is
> > correct.
> >
> > And you always have to at the very least, test build your changes to
> > verify that they do not break anything.
> >
> > thanks,
> >
> > greg k-h
>
> Thank you so much. I didn't realize that I could have tested it by
> just building the driver and not the entire kernel. Anyway, I'd still
> love to learn more and contribute to the kernel. Where can I find
> "small fixes" I can make?

Don't expect that someone here says to you which "fixes" you should
address. Read the code and find them on your own. Get hints from
checkpatch.pl and the other static analyzers too (Sparse, Coccinelle,
make that-driver-you-chose W=1, etc..).

Thanks,

Fabio
> Warm Regards,
>
> Krish
>
>




2021-08-29 02:15:36

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
>
> Hi Krish!
>
> I'm sure someone has said something by now, however "top posting", where
> you reply to emails by writing on the top can make things hard to read
> on the mailing lists. The conversation is upside down when reading.
>
> https://en.wikipedia.org/wiki/Posting_style#Top-posting
>
> Next time try writing underneath the text your referring to like this:
> Don't worry we're all learning here :)
>
> On this day, August 27, 2021 thus sayeth Krish Jain:
> > I unfortunately forgot to add Greg to this thread. Doing so now. I
> > apologize for the confusion, if any. This patch was regarding the
> > staging tree's file android/ashmem.c and declaring the file_operations
> > struct as const as done elsewhere in the kernel, because there are no
> > modifications to its fields.
> >
> > Warm Regards
> >
>
> Things can be a little deceiving in the kernel. That's why testing your
> changes before you submit them can be helpful.
>
> But don't worry too much if you break something, there are countless
> bots trying to break the kernel every day. It usually means you're
> learning when you break something.
>
> >
> > On Fri, Aug 27, 2021 at 8:38 PM Krish Jain <[email protected]> wrote:
> > >
> > > So what do you think I can do?
> > >
> > > Best Regards
> > >
>
> That's a tough one as it really depends on your situation. When I first
> started programming I had a *really* old (even for that time) laptop
> that couldn't do much. It wasn't ideal but I found I could connect
> through ssh to a virtual machine my university lent me to "learn to
> code".
>
> I have no idea what your situation is like. Though having a second
> computer to compile code while I wrote worked for me.
>
> > >
> > > On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <[email protected]> wrote:
> > > >
> > > > Hi, yes, this is my first time programming at this low level. And yes,
> > > > I read both docs now. Furthermore the issue is that my current
> > > > hardware can't handle building the kernel, it barely managed to
> > > > survive the first build after 2 hours so I don't know how I can. If I
> > > > change it to static const would it fix the issue and build
> > > > successfully? If not what would be the error message, then I can
> > > > debug. Thanks
> > > >
>
> As for your patch, I built the driver using:
>
> $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
>
> Which produced the following error:
>
>
> drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> 380 | const static struct file_operations vmfile_fops;
> | ^~~~~
> drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> 431 | vmfile_fops = *vmfile->f_op;
> | ^
> drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> | ^
> drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> 433 | vmfile_fops.get_unmapped_area =
> | ^
> make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> make: *** [Makefile:1851: drivers/staging/android] Error 2
>

Hi, this seems very useful and I tried this myself just now. I don't
get any errors that you do though. When I hit enter I just get a new
shell prompt. What am I doing wrong? Probably a silly mistake. I ran
make CCFLAGS=-Werror M=drivers/staging/android/.


- Krish

> You shouldn't need to compile the entire kernel. That may be why your
> computer is having a hard time?
>
> Hope this helps :) and it was nice to meet you Krish
> ~Bryan
>

2021-08-29 02:19:14

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Sorry, I meant I ran "make CCFLAGS=-Werror W=1
M=drivers/staging/android" and I get no error.

On Sun, Aug 29, 2021 at 4:13 AM Krish Jain <[email protected]> wrote:
>
> On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
> >
> > Hi Krish!
> >
> > I'm sure someone has said something by now, however "top posting", where
> > you reply to emails by writing on the top can make things hard to read
> > on the mailing lists. The conversation is upside down when reading.
> >
> > https://en.wikipedia.org/wiki/Posting_style#Top-posting
> >
> > Next time try writing underneath the text your referring to like this:
> > Don't worry we're all learning here :)
> >
> > On this day, August 27, 2021 thus sayeth Krish Jain:
> > > I unfortunately forgot to add Greg to this thread. Doing so now. I
> > > apologize for the confusion, if any. This patch was regarding the
> > > staging tree's file android/ashmem.c and declaring the file_operations
> > > struct as const as done elsewhere in the kernel, because there are no
> > > modifications to its fields.
> > >
> > > Warm Regards
> > >
> >
> > Things can be a little deceiving in the kernel. That's why testing your
> > changes before you submit them can be helpful.
> >
> > But don't worry too much if you break something, there are countless
> > bots trying to break the kernel every day. It usually means you're
> > learning when you break something.
> >
> > >
> > > On Fri, Aug 27, 2021 at 8:38 PM Krish Jain <[email protected]> wrote:
> > > >
> > > > So what do you think I can do?
> > > >
> > > > Best Regards
> > > >
> >
> > That's a tough one as it really depends on your situation. When I first
> > started programming I had a *really* old (even for that time) laptop
> > that couldn't do much. It wasn't ideal but I found I could connect
> > through ssh to a virtual machine my university lent me to "learn to
> > code".
> >
> > I have no idea what your situation is like. Though having a second
> > computer to compile code while I wrote worked for me.
> >
> > > >
> > > > On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <[email protected]> wrote:
> > > > >
> > > > > Hi, yes, this is my first time programming at this low level. And yes,
> > > > > I read both docs now. Furthermore the issue is that my current
> > > > > hardware can't handle building the kernel, it barely managed to
> > > > > survive the first build after 2 hours so I don't know how I can. If I
> > > > > change it to static const would it fix the issue and build
> > > > > successfully? If not what would be the error message, then I can
> > > > > debug. Thanks
> > > > >
> >
> > As for your patch, I built the driver using:
> >
> > $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
> >
> > Which produced the following error:
> >
> >
> > drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> > drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> > 380 | const static struct file_operations vmfile_fops;
> > | ^~~~~
> > drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> > 431 | vmfile_fops = *vmfile->f_op;
> > | ^
> > drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> > 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> > | ^
> > drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> > 433 | vmfile_fops.get_unmapped_area =
> > | ^
> > make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> > make: *** [Makefile:1851: drivers/staging/android] Error 2
> >
>
> Hi, this seems very useful and I tried this myself just now. I don't
> get any errors that you do though. When I hit enter I just get a new
> shell prompt. What am I doing wrong? Probably a silly mistake. I ran
> make CCFLAGS=-Werror M=drivers/staging/android/.
>
>
> - Krish
>
> > You shouldn't need to compile the entire kernel. That may be why your
> > computer is having a hard time?
> >
> > Hope this helps :) and it was nice to meet you Krish
> > ~Bryan
> >

2021-08-29 06:21:09

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Sun, Aug 29, 2021 at 04:13:43AM +0200, Krish Jain wrote:
> On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
> > As for your patch, I built the driver using:
> >
> > $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
> >
> > Which produced the following error:
> >
> >
> > drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> > drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> > 380 | const static struct file_operations vmfile_fops;
> > | ^~~~~
> > drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> > 431 | vmfile_fops = *vmfile->f_op;
> > | ^
> > drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> > 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> > | ^
> > drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> > 433 | vmfile_fops.get_unmapped_area =
> > | ^
> > make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> > make: *** [Makefile:1851: drivers/staging/android] Error 2
> >
>
> Hi, this seems very useful and I tried this myself just now. I don't
> get any errors that you do though. When I hit enter I just get a new
> shell prompt. What am I doing wrong? Probably a silly mistake. I ran
> make CCFLAGS=-Werror M=drivers/staging/android/.

Are you sure the file is being built at all? You usually have to select
the proper configuration option to enable that driver as well.

thanks,

greg k-h

2021-08-29 14:47:44

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Hi Krish

On this day, August 29, 2021, thus sayeth Krish Jain:
> On Sun, Aug 29, 2021 at 8:17 AM Greg KH <[email protected]> wrote:
> >
> > On Sun, Aug 29, 2021 at 04:13:43AM +0200, Krish Jain wrote:
> > > On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
> > > > As for your patch, I built the driver using:
> > > >
> > > > $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
> > > >
> > > > Which produced the following error:
> > > >
> > > >
> > > > drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> > > > drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> > > > 380 | const static struct file_operations vmfile_fops;
> > > > | ^~~~~
> > > > drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> > > > 431 | vmfile_fops = *vmfile->f_op;
> > > > | ^
> > > > drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> > > > 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> > > > | ^
> > > > drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> > > > 433 | vmfile_fops.get_unmapped_area =
> > > > | ^
> > > > make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> > > > make: *** [Makefile:1851: drivers/staging/android] Error 2
> > > >
> > >
> > > Hi, this seems very useful and I tried this myself just now. I don't
> > > get any errors that you do though. When I hit enter I just get a new
> > > shell prompt. What am I doing wrong? Probably a silly mistake. I ran
> > > make CCFLAGS=-Werror M=drivers/staging/android/.
> >
> > Are you sure the file is being built at all? You usually have to select
> > the proper configuration option to enable that driver as well.
>
>
> Hi, what option do you mean? I already ran make allmodconfig and sudo
> make modules_install install and then make "CCFLAGS=-Werror W=1
> M=drivers/staging/android/" and now I do get output but one line
> "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> not have dependencies or modversions. You may get many unresolved
> symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> M=drivers/staging/android/" and that outputted the following:
>

Most of the answers you're asking for are going to get vague responses
(if any) on the mailing lists. The idea being (and I agree with) that
giving out the answers will steal your opportunity to explore and learn
the material yourself.

Yes, it would be faster if we told you the answer, but ultimately, we
would be doing a disservice to you.

Besides, more times than not we (me especially) don't have the answer.

With that said, I will give a (generous) hint. :)

***

It looks like your having trouble with Kconfig. Have a look at:

https://www.kernel.org/doc/html/latest/kbuild/makefiles.html

and:

https://www.kernel.org/doc/html/latest/kbuild/modules.html

Also, you shouldn't need to install anything if you're just testing
whether a module builds. Especially when 'sudo' and 'install' are
involved.

>
> test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> echo >&2; \
> echo >&2 " ERROR: Kernel configuration is invalid."; \
> echo >&2 " include/generated/autoconf.h or
> include/config/auto.conf are missing.";\
> echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> to fix it."; \
>

If I were you I would be asking:

"What is a Kernel configuration file? and how did I corrupt mine?"

https://www.kernel.org/doc/html/latest/kbuild/kconfig.html

>
> echo >&2 ; \
> /bin/false)
> make -f ./scripts/Makefile.build obj=drivers/staging/android \
> single-build= \
> need-builtin=1 need-modorder=1
> sh ./scripts/modules-check.sh drivers/staging/android/modules.order
> make -f ./scripts/Makefile.modpost
> WARNING: Symbol version dump "Module.symvers" is missing.
> Modules may not have dependencies or modversions.
> You may get many unresolved symbol warnings.
> make -f ./scripts/Makefile.modfinal
>

"What is Module.symvers?"

If you're reading the links I gave, you should know this by now. :)
Check out chapter 6 in "Building External Modules"

>
> I followed this and ran make oldconfig && make prepare but all that is
> outputted is again "WARNING: Symbol version dump "Module.symvers" is
> missing. Modules may not have dependencies or modversions. You may get
> many unresolved symbol warnings." Then I just tried sudo make
> modules_install install again and what was outputted was:
>

Again, Be *VERY* careful running commands you do not understand.
Especially when the words 'sudo' and 'install' are in the same
command.

>
> sed: can't read modules.order: No such file or directory
> make[1]: *** [Makefile:1494: __modinst_pre] Error 2
> make: *** [Makefile:351: __build_one_by_one] Error 2
>

"What does modules.order do?"

https://www.kernel.org/doc/html/latest/kbuild/kbuild.html

>
> Any ideas? I've been stuck on debugging for hours to no avail. Please
> enlighten me on where I am messing up.
>

All I can say is the kernels' documentation is the greatest thing to
have when joining the community. The search bar is *fantastic*.

https://www.kernel.org/doc/html/latest/index.html

Together with the collective history of ~30 years of developer's
conversations on Linux Kernel Mailing List (LMKL).

https://lore.kernel.org/lkml/

Along with our ability to see the full commit history at any time by:

$ git grep <regexp>

In the end, all of the questions you have are in the documentation. I
can't stress enough how appreciative we should be of contributers to
Documentation/ and people like Jonathan Corbet who take the time to
document how all of this stuff works in such a straight forward way.

Withholding simple answers like this may seem unfair, but the juice is
absolutely worth the squeeze the next time you run into another issue.

And trust me when I say you *will* run into another issue. We wouldn't
be here if there weren't constant issues to solve.

We've all been where you're at. Even Greg, he may deny this, but I'm
sure he has been.

I'm excited to see what you learn.
~Bryan

***

Just as an aside, for any other lurkers out there: This was the email
that gave me confidence that I could join this community and that
sending a patch, no matter how small, is welcome.

https://lore.kernel.org/lkml/[email protected]/

The old "grey-beards" here don't bite, no matter how much they and
everyone else say they do.

2021-08-29 16:26:34

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Sun, Aug 29, 2021 at 4:45 PM Bryan Brattlof <[email protected]> wrote:
>
> Hi Krish
>
> On this day, August 29, 2021, thus sayeth Krish Jain:
> > On Sun, Aug 29, 2021 at 8:17 AM Greg KH <[email protected]> wrote:
> > >
> > > On Sun, Aug 29, 2021 at 04:13:43AM +0200, Krish Jain wrote:
> > > > On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
> > > > > As for your patch, I built the driver using:
> > > > >
> > > > > $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
> > > > >
> > > > > Which produced the following error:
> > > > >
> > > > >
> > > > > drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> > > > > drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> > > > > 380 | const static struct file_operations vmfile_fops;
> > > > > | ^~~~~
> > > > > drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> > > > > 431 | vmfile_fops = *vmfile->f_op;
> > > > > | ^
> > > > > drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> > > > > 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> > > > > | ^
> > > > > drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> > > > > 433 | vmfile_fops.get_unmapped_area =
> > > > > | ^
> > > > > make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> > > > > make: *** [Makefile:1851: drivers/staging/android] Error 2
> > > > >
> > > >
> > > > Hi, this seems very useful and I tried this myself just now. I don't
> > > > get any errors that you do though. When I hit enter I just get a new
> > > > shell prompt. What am I doing wrong? Probably a silly mistake. I ran
> > > > make CCFLAGS=-Werror M=drivers/staging/android/.
> > >
> > > Are you sure the file is being built at all? You usually have to select
> > > the proper configuration option to enable that driver as well.
> >
> >
> > Hi, what option do you mean? I already ran make allmodconfig and sudo
> > make modules_install install and then make "CCFLAGS=-Werror W=1
> > M=drivers/staging/android/" and now I do get output but one line
> > "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> > not have dependencies or modversions. You may get many unresolved
> > symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> > M=drivers/staging/android/" and that outputted the following:
> >
>
> Most of the answers you're asking for are going to get vague responses
> (if any) on the mailing lists. The idea being (and I agree with) that
> giving out the answers will steal your opportunity to explore and learn
> the material yourself.
>
> Yes, it would be faster if we told you the answer, but ultimately, we
> would be doing a disservice to you.
>
> Besides, more times than not we (me especially) don't have the answer.
>
> With that said, I will give a (generous) hint. :)
>

Hi. Do I have to build the kernel once before this works? Or can I
just build a module directly?



Best Regards


>
> It looks like your having trouble with Kconfig. Have a look at:
>
> https://www.kernel.org/doc/html/latest/kbuild/makefiles.html
>
> and:
>
> https://www.kernel.org/doc/html/latest/kbuild/modules.html
>
> Also, you shouldn't need to install anything if you're just testing
> whether a module builds. Especially when 'sudo' and 'install' are
> involved.
>
> >
> > test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> > echo >&2; \
> > echo >&2 " ERROR: Kernel configuration is invalid."; \
> > echo >&2 " include/generated/autoconf.h or
> > include/config/auto.conf are missing.";\
> > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > to fix it."; \
> >
>
> If I were you I would be asking:
>
> "What is a Kernel configuration file? and how did I corrupt mine?"
>
> https://www.kernel.org/doc/html/latest/kbuild/kconfig.html
>
> >
> > echo >&2 ; \
> > /bin/false)
> > make -f ./scripts/Makefile.build obj=drivers/staging/android \
> > single-build= \
> > need-builtin=1 need-modorder=1
> > sh ./scripts/modules-check.sh drivers/staging/android/modules.order
> > make -f ./scripts/Makefile.modpost
> > WARNING: Symbol version dump "Module.symvers" is missing.
> > Modules may not have dependencies or modversions.
> > You may get many unresolved symbol warnings.
> > make -f ./scripts/Makefile.modfinal
> >
>
> "What is Module.symvers?"
>
> If you're reading the links I gave, you should know this by now. :)
> Check out chapter 6 in "Building External Modules"
>
> >
> > I followed this and ran make oldconfig && make prepare but all that is
> > outputted is again "WARNING: Symbol version dump "Module.symvers" is
> > missing. Modules may not have dependencies or modversions. You may get
> > many unresolved symbol warnings." Then I just tried sudo make
> > modules_install install again and what was outputted was:
> >
>
> Again, Be *VERY* careful running commands you do not understand.
> Especially when the words 'sudo' and 'install' are in the same
> command.
>
> >
> > sed: can't read modules.order: No such file or directory
> > make[1]: *** [Makefile:1494: __modinst_pre] Error 2
> > make: *** [Makefile:351: __build_one_by_one] Error 2
> >
>
> "What does modules.order do?"
>
> https://www.kernel.org/doc/html/latest/kbuild/kbuild.html
>
> >
> > Any ideas? I've been stuck on debugging for hours to no avail. Please
> > enlighten me on where I am messing up.
> >
>
> All I can say is the kernels' documentation is the greatest thing to
> have when joining the community. The search bar is *fantastic*.
>
> https://www.kernel.org/doc/html/latest/index.html
>
> Together with the collective history of ~30 years of developer's
> conversations on Linux Kernel Mailing List (LMKL).
>
> https://lore.kernel.org/lkml/
>
> Along with our ability to see the full commit history at any time by:
>
> $ git grep <regexp>
>
> In the end, all of the questions you have are in the documentation. I
> can't stress enough how appreciative we should be of contributers to
> Documentation/ and people like Jonathan Corbet who take the time to
> document how all of this stuff works in such a straight forward way.
>
> Withholding simple answers like this may seem unfair, but the juice is
> absolutely worth the squeeze the next time you run into another issue.
>
> And trust me when I say you *will* run into another issue. We wouldn't
> be here if there weren't constant issues to solve.
>
> We've all been where you're at. Even Greg, he may deny this, but I'm
> sure he has been.
>
> I'm excited to see what you learn.
> ~Bryan
>
> ***
>
> Just as an aside, for any other lurkers out there: This was the email
> that gave me confidence that I could join this community and that
> sending a patch, no matter how small, is welcome.
>
> https://lore.kernel.org/lkml/[email protected]/
>
> The old "grey-beards" here don't bite, no matter how much they and
> everyone else say they do.
>

2021-08-29 16:37:07

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

And I scoured the kernel.org website and I can't seem to find how my
.config file is incorrect. I let make oldconfig && make prepare
generate it for me. I am really confused. Can you give me another
hint? I still get


$ make CCFLAGS=-Werror W=1 M=drivers/staging/android/ V=1


test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
echo >&2; \
echo >&2 " ERROR: Kernel configuration is invalid."; \
echo >&2 " include/generated/autoconf.h or
include/config/auto.conf are missing.";\
echo >&2 " Run 'make oldconfig && make prepare' on kernel src
to fix it."; \
echo >&2 ; \
/bin/false)
make -f ./scripts/Makefile.build obj=drivers/staging/android \
single-build= \
need-builtin=1 need-modorder=1
sh ./scripts/modules-check.sh drivers/staging/android/modules.order
make -f ./scripts/Makefile.modpost
WARNING: Symbol version dump "Module.symvers" is missing.
Modules may not have dependencies or modversions.
You may get many unresolved symbol warnings.
make -f ./scripts/Makefile.modfinal



- Krish

On Sun, Aug 29, 2021 at 6:20 PM Krish Jain <[email protected]> wrote:
>
> On Sun, Aug 29, 2021 at 4:45 PM Bryan Brattlof <[email protected]> wrote:
> >
> > Hi Krish
> >
> > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > On Sun, Aug 29, 2021 at 8:17 AM Greg KH <[email protected]> wrote:
> > > >
> > > > On Sun, Aug 29, 2021 at 04:13:43AM +0200, Krish Jain wrote:
> > > > > On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <[email protected]> wrote:
> > > > > > As for your patch, I built the driver using:
> > > > > >
> > > > > > $ make CCFLAGS=-Werror W=1 M=drivers/staging/android
> > > > > >
> > > > > > Which produced the following error:
> > > > > >
> > > > > >
> > > > > > drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’:
> > > > > > drivers/staging/android/ashmem.c:380:2: warning: ‘static’ is not at beginning of declaration [-Wold-style-declaration]
> > > > > > 380 | const static struct file_operations vmfile_fops;
> > > > > > | ^~~~~
> > > > > > drivers/staging/android/ashmem.c:431:16: error: assignment of read-only variable ‘vmfile_fops’
> > > > > > 431 | vmfile_fops = *vmfile->f_op;
> > > > > > | ^
> > > > > > drivers/staging/android/ashmem.c:432:21: error: assignment of member ‘mmap’ in read-only object
> > > > > > 432 | vmfile_fops.mmap = ashmem_vmfile_mmap;
> > > > > > | ^
> > > > > > drivers/staging/android/ashmem.c:433:34: error: assignment of member ‘get_unmapped_area’ in read-only object
> > > > > > 433 | vmfile_fops.get_unmapped_area =
> > > > > > | ^
> > > > > > make[1]: *** [scripts/Makefile.build:271: drivers/staging/android/ashmem.o] Error 1
> > > > > > make: *** [Makefile:1851: drivers/staging/android] Error 2
> > > > > >
> > > > >
> > > > > Hi, this seems very useful and I tried this myself just now. I don't
> > > > > get any errors that you do though. When I hit enter I just get a new
> > > > > shell prompt. What am I doing wrong? Probably a silly mistake. I ran
> > > > > make CCFLAGS=-Werror M=drivers/staging/android/.
> > > >
> > > > Are you sure the file is being built at all? You usually have to select
> > > > the proper configuration option to enable that driver as well.
> > >
> > >
> > > Hi, what option do you mean? I already ran make allmodconfig and sudo
> > > make modules_install install and then make "CCFLAGS=-Werror W=1
> > > M=drivers/staging/android/" and now I do get output but one line
> > > "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> > > not have dependencies or modversions. You may get many unresolved
> > > symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> > > M=drivers/staging/android/" and that outputted the following:
> > >
> >
> > Most of the answers you're asking for are going to get vague responses
> > (if any) on the mailing lists. The idea being (and I agree with) that
> > giving out the answers will steal your opportunity to explore and learn
> > the material yourself.
> >
> > Yes, it would be faster if we told you the answer, but ultimately, we
> > would be doing a disservice to you.
> >
> > Besides, more times than not we (me especially) don't have the answer.
> >
> > With that said, I will give a (generous) hint. :)
> >
>
> Hi. Do I have to build the kernel once before this works? Or can I
> just build a module directly?
>
>
>
> Best Regards
>
>
> >
> > It looks like your having trouble with Kconfig. Have a look at:
> >
> > https://www.kernel.org/doc/html/latest/kbuild/makefiles.html
> >
> > and:
> >
> > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> >
> > Also, you shouldn't need to install anything if you're just testing
> > whether a module builds. Especially when 'sudo' and 'install' are
> > involved.
> >
> > >
> > > test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> > > echo >&2; \
> > > echo >&2 " ERROR: Kernel configuration is invalid."; \
> > > echo >&2 " include/generated/autoconf.h or
> > > include/config/auto.conf are missing.";\
> > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > to fix it."; \
> > >
> >
> > If I were you I would be asking:
> >
> > "What is a Kernel configuration file? and how did I corrupt mine?"
> >
> > https://www.kernel.org/doc/html/latest/kbuild/kconfig.html
> >
> > >
> > > echo >&2 ; \
> > > /bin/false)
> > > make -f ./scripts/Makefile.build obj=drivers/staging/android \
> > > single-build= \
> > > need-builtin=1 need-modorder=1
> > > sh ./scripts/modules-check.sh drivers/staging/android/modules.order
> > > make -f ./scripts/Makefile.modpost
> > > WARNING: Symbol version dump "Module.symvers" is missing.
> > > Modules may not have dependencies or modversions.
> > > You may get many unresolved symbol warnings.
> > > make -f ./scripts/Makefile.modfinal
> > >
> >
> > "What is Module.symvers?"
> >
> > If you're reading the links I gave, you should know this by now. :)
> > Check out chapter 6 in "Building External Modules"
> >
> > >
> > > I followed this and ran make oldconfig && make prepare but all that is
> > > outputted is again "WARNING: Symbol version dump "Module.symvers" is
> > > missing. Modules may not have dependencies or modversions. You may get
> > > many unresolved symbol warnings." Then I just tried sudo make
> > > modules_install install again and what was outputted was:
> > >
> >
> > Again, Be *VERY* careful running commands you do not understand.
> > Especially when the words 'sudo' and 'install' are in the same
> > command.
> >
> > >
> > > sed: can't read modules.order: No such file or directory
> > > make[1]: *** [Makefile:1494: __modinst_pre] Error 2
> > > make: *** [Makefile:351: __build_one_by_one] Error 2
> > >
> >
> > "What does modules.order do?"
> >
> > https://www.kernel.org/doc/html/latest/kbuild/kbuild.html
> >
> > >
> > > Any ideas? I've been stuck on debugging for hours to no avail. Please
> > > enlighten me on where I am messing up.
> > >
> >
> > All I can say is the kernels' documentation is the greatest thing to
> > have when joining the community. The search bar is *fantastic*.
> >
> > https://www.kernel.org/doc/html/latest/index.html
> >
> > Together with the collective history of ~30 years of developer's
> > conversations on Linux Kernel Mailing List (LMKL).
> >
> > https://lore.kernel.org/lkml/
> >
> > Along with our ability to see the full commit history at any time by:
> >
> > $ git grep <regexp>
> >
> > In the end, all of the questions you have are in the documentation. I
> > can't stress enough how appreciative we should be of contributers to
> > Documentation/ and people like Jonathan Corbet who take the time to
> > document how all of this stuff works in such a straight forward way.
> >
> > Withholding simple answers like this may seem unfair, but the juice is
> > absolutely worth the squeeze the next time you run into another issue.
> >
> > And trust me when I say you *will* run into another issue. We wouldn't
> > be here if there weren't constant issues to solve.
> >
> > We've all been where you're at. Even Greg, he may deny this, but I'm
> > sure he has been.
> >
> > I'm excited to see what you learn.
> > ~Bryan
> >
> > ***
> >
> > Just as an aside, for any other lurkers out there: This was the email
> > that gave me confidence that I could join this community and that
> > sending a patch, no matter how small, is welcome.
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > The old "grey-beards" here don't bite, no matter how much they and
> > everyone else say they do.
> >

2021-08-29 16:50:32

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On this day, August 29, 2021, thus sayeth Krish Jain:
> > >
> > > Hi, what option do you mean? I already ran make allmodconfig and sudo
> > > make modules_install install and then make "CCFLAGS=-Werror W=1
> > > M=drivers/staging/android/" and now I do get output but one line
> > > "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> > > not have dependencies or modversions. You may get many unresolved
> > > symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> > > M=drivers/staging/android/" and that outputted the following:
> > >
> >
> > Most of the answers you're asking for are going to get vague responses
> > (if any) on the mailing lists. The idea being (and I agree with) that
> > giving out the answers will steal your opportunity to explore and learn
> > the material yourself.
> >
> > Yes, it would be faster if we told you the answer, but ultimately, we
> > would be doing a disservice to you.
> >
> > Besides, more times than not we (me especially) don't have the answer.
> >
> > With that said, I will give a (generous) hint. :)
> >
>
> Hi. Do I have to build the kernel once before this works? Or can I
> just build a module directly?
>

Again, do not allow others to rob you of learning how to solve these
issues yourself. I *strongly* encourage you to familiarize yourself with
the Kernel Build System in the Documentation.

https://www.kernel.org/doc/html/latest/kbuild/modules.html

Specifically the first paragraph of "2. How to Build External Modules"

It may seem like a lot for such a simple issue but it *is* worth it.
~Bryan

2021-08-29 16:58:49

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Sun, Aug 29, 2021 at 6:49 PM Bryan Brattlof <[email protected]> wrote:
>
> On this day, August 29, 2021, thus sayeth Krish Jain:
> > > >
> > > > Hi, what option do you mean? I already ran make allmodconfig and sudo
> > > > make modules_install install and then make "CCFLAGS=-Werror W=1
> > > > M=drivers/staging/android/" and now I do get output but one line
> > > > "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> > > > not have dependencies or modversions. You may get many unresolved
> > > > symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> > > > M=drivers/staging/android/" and that outputted the following:
> > > >
> > >
> > > Most of the answers you're asking for are going to get vague responses
> > > (if any) on the mailing lists. The idea being (and I agree with) that
> > > giving out the answers will steal your opportunity to explore and learn
> > > the material yourself.
> > >
> > > Yes, it would be faster if we told you the answer, but ultimately, we
> > > would be doing a disservice to you.
> > >
> > > Besides, more times than not we (me especially) don't have the answer.
> > >
> > > With that said, I will give a (generous) hint. :)
> > >
> >
> > Hi. Do I have to build the kernel once before this works? Or can I
> > just build a module directly?
> >
>
> Again, do not allow others to rob you of learning how to solve these
> issues yourself. I *strongly* encourage you to familiarize yourself with
> the Kernel Build System in the Documentation.
>
> https://www.kernel.org/doc/html/latest/kbuild/modules.html
>
> Specifically the first paragraph of "2. How to Build External Modules"
>
> It may seem like a lot for such a simple issue but it *is* worth it.
> ~Bryan
>



That section says


"To build external modules, *you must have a prebuilt kernel
available* that contains the configuration and header files used in
the build. Also, the kernel must have been built with modules enabled.
If you are using a distribution kernel, there will be a package for
the kernel you are running provided by your distribution.

An alternative is to use the “make” target “modules_prepare.” This
will make sure the kernel contains the information required. The
target exists solely as a simple way to prepare a kernel source tree
for building external modules.

NOTE: “modules_prepare” will not build Module.symvers even if
CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
executed to make module versioning work.*"

So I am just trying to confirm with you whether I have to first build
the kernel with like "make" or not? As you can imagine my hardware
takes *very* long to build a kernel as I did in my last attempt so I
am asking whether it is needed. Hope you understand.

Best Regards

2021-08-29 18:30:38

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Basically it says "you must have a prebuilt kernel available that
contains the configuration and header files used in the build." Since
for the staging kernel "make oldconfig" asked me for more
configurations apart from my old configuration file (as it reads the
existing .config file that was used for an old kernel and prompts the
user for options in the current kernel source that are not found in
the file) . So I *don't* currently have a prebuilt kernel that
contains all the configuration in my staging kernel's .config file. So
do I have to build the kernel once before I can just build the module
with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?


Thanks again

On Sun, Aug 29, 2021 at 6:56 PM Krish Jain <[email protected]> wrote:
>
> On Sun, Aug 29, 2021 at 6:49 PM Bryan Brattlof <[email protected]> wrote:
> >
> > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > > >
> > > > > Hi, what option do you mean? I already ran make allmodconfig and sudo
> > > > > make modules_install install and then make "CCFLAGS=-Werror W=1
> > > > > M=drivers/staging/android/" and now I do get output but one line
> > > > > "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> > > > > not have dependencies or modversions. You may get many unresolved
> > > > > symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> > > > > M=drivers/staging/android/" and that outputted the following:
> > > > >
> > > >
> > > > Most of the answers you're asking for are going to get vague responses
> > > > (if any) on the mailing lists. The idea being (and I agree with) that
> > > > giving out the answers will steal your opportunity to explore and learn
> > > > the material yourself.
> > > >
> > > > Yes, it would be faster if we told you the answer, but ultimately, we
> > > > would be doing a disservice to you.
> > > >
> > > > Besides, more times than not we (me especially) don't have the answer.
> > > >
> > > > With that said, I will give a (generous) hint. :)
> > > >
> > >
> > > Hi. Do I have to build the kernel once before this works? Or can I
> > > just build a module directly?
> > >
> >
> > Again, do not allow others to rob you of learning how to solve these
> > issues yourself. I *strongly* encourage you to familiarize yourself with
> > the Kernel Build System in the Documentation.
> >
> > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> >
> > Specifically the first paragraph of "2. How to Build External Modules"
> >
> > It may seem like a lot for such a simple issue but it *is* worth it.
> > ~Bryan
> >
>
>
>
> That section says
>
>
> "To build external modules, *you must have a prebuilt kernel
> available* that contains the configuration and header files used in
> the build. Also, the kernel must have been built with modules enabled.
> If you are using a distribution kernel, there will be a package for
> the kernel you are running provided by your distribution.
>
> An alternative is to use the “make” target “modules_prepare.” This
> will make sure the kernel contains the information required. The
> target exists solely as a simple way to prepare a kernel source tree
> for building external modules.
>
> NOTE: “modules_prepare” will not build Module.symvers even if
> CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> executed to make module versioning work.*"
>
> So I am just trying to confirm with you whether I have to first build
> the kernel with like "make" or not? As you can imagine my hardware
> takes *very* long to build a kernel as I did in my last attempt so I
> am asking whether it is needed. Hope you understand.
>
> Best Regards

2021-08-29 18:48:27

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Keeping you updated. Small win. The "Symbol version dump
"Module.symvers" is missing. " error disappeared. Now I still don't
know why

ERROR: Kernel configuration is invalid."; \
echo >&2 " include/generated/autoconf.h or
include/config/auto.conf are missing.";\
echo >&2 " Run 'make oldconfig && make prepare' on kernel src
to fix it."; \


is still present.

How can I fix this?


Best Regards

On Sun, Aug 29, 2021 at 8:28 PM Krish Jain <[email protected]> wrote:
>
> Basically it says "you must have a prebuilt kernel available that
> contains the configuration and header files used in the build." Since
> for the staging kernel "make oldconfig" asked me for more
> configurations apart from my old configuration file (as it reads the
> existing .config file that was used for an old kernel and prompts the
> user for options in the current kernel source that are not found in
> the file) . So I *don't* currently have a prebuilt kernel that
> contains all the configuration in my staging kernel's .config file. So
> do I have to build the kernel once before I can just build the module
> with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?
>
>
> Thanks again
>
> On Sun, Aug 29, 2021 at 6:56 PM Krish Jain <[email protected]> wrote:
> >
> > On Sun, Aug 29, 2021 at 6:49 PM Bryan Brattlof <[email protected]> wrote:
> > >
> > > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > > > >
> > > > > > Hi, what option do you mean? I already ran make allmodconfig and sudo
> > > > > > make modules_install install and then make "CCFLAGS=-Werror W=1
> > > > > > M=drivers/staging/android/" and now I do get output but one line
> > > > > > "WARNING: Symbol version dump "Module.symvers" is missing. Modules may
> > > > > > not have dependencies or modversions. You may get many unresolved
> > > > > > symbol warnings." . Then I tried "make CCFLAGS=-Werror V=1
> > > > > > M=drivers/staging/android/" and that outputted the following:
> > > > > >
> > > > >
> > > > > Most of the answers you're asking for are going to get vague responses
> > > > > (if any) on the mailing lists. The idea being (and I agree with) that
> > > > > giving out the answers will steal your opportunity to explore and learn
> > > > > the material yourself.
> > > > >
> > > > > Yes, it would be faster if we told you the answer, but ultimately, we
> > > > > would be doing a disservice to you.
> > > > >
> > > > > Besides, more times than not we (me especially) don't have the answer.
> > > > >
> > > > > With that said, I will give a (generous) hint. :)
> > > > >
> > > >
> > > > Hi. Do I have to build the kernel once before this works? Or can I
> > > > just build a module directly?
> > > >
> > >
> > > Again, do not allow others to rob you of learning how to solve these
> > > issues yourself. I *strongly* encourage you to familiarize yourself with
> > > the Kernel Build System in the Documentation.
> > >
> > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > >
> > > Specifically the first paragraph of "2. How to Build External Modules"
> > >
> > > It may seem like a lot for such a simple issue but it *is* worth it.
> > > ~Bryan
> > >
> >
> >
> >
> > That section says
> >
> >
> > "To build external modules, *you must have a prebuilt kernel
> > available* that contains the configuration and header files used in
> > the build. Also, the kernel must have been built with modules enabled.
> > If you are using a distribution kernel, there will be a package for
> > the kernel you are running provided by your distribution.
> >
> > An alternative is to use the “make” target “modules_prepare.” This
> > will make sure the kernel contains the information required. The
> > target exists solely as a simple way to prepare a kernel source tree
> > for building external modules.
> >
> > NOTE: “modules_prepare” will not build Module.symvers even if
> > CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> > executed to make module versioning work.*"
> >
> > So I am just trying to confirm with you whether I have to first build
> > the kernel with like "make" or not? As you can imagine my hardware
> > takes *very* long to build a kernel as I did in my last attempt so I
> > am asking whether it is needed. Hope you understand.
> >
> > Best Regards

2021-08-29 21:01:57

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On this day, August 29, 2021, thus sayeth Krish Jain:
> Keeping you updated. Small win. The "Symbol version dump
> "Module.symvers" is missing. " error disappeared. Now I still don't
> know why
>

Whoop! Any win, no matter their size, always feel great. I ran around
the house yesterday after cross compiling DOOM! for an armel chip. It's
that "win" feeling you get that keeps me involved.

It is important that you find out why though. What is the importance to
having Module.symvers? and why is it a WARNING and not an ERROR?

What would happen if we didn't have the proper symbols when compiling or
installing this driver?

How and what generates the Module.symvers file when we *do* need it?

How can we turn this warning off when we don't need it?

This is covered in chapter "6. Module Versioning"

https://www.kernel.org/doc/html/latest/kbuild/modules.html

>
> ERROR: Kernel configuration is invalid."; \
> echo >&2 " include/generated/autoconf.h or
> include/config/auto.conf are missing.";\
> echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> to fix it."; \
>
>
> is still present.
>
> How can I fix this?
>

Are there any other 'make *config' options we could try?

What does 'make prepare' even do?

Why do we even need a configuration file?

https://www.kernel.org/doc/html/latest/kbuild/kconfig.html

>
> Best Regards
>
> On Sun, Aug 29, 2021 at 8:28 PM Krish Jain <[email protected]> wrote:
> >
> > Basically it says "you must have a prebuilt kernel available that
> > contains the configuration and header files used in the build." Since
> > for the staging kernel "make oldconfig" asked me for more
> > configurations apart from my old configuration file (as it reads the
> > existing .config file that was used for an old kernel and prompts the
> > user for options in the current kernel source that are not found in
> > the file) . So I *don't* currently have a prebuilt kernel that
> > contains all the configuration in my staging kernel's .config file. So
> > do I have to build the kernel once before I can just build the module
> > with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?
> >

What do all these other configuration settings turn on and off anyway?

Do we really need CONFIG_INFINIBAND turned on if we're working in the
drivers/staging tree of the kernel?

What would we gain from having a compiled kernel if we want to test a
single staging driver?

If you found what Module.symvers does, you should know this.

> > > >
> > > > Again, do not allow others to rob you of learning how to solve these
> > > > issues yourself. I *strongly* encourage you to familiarize yourself with
> > > > the Kernel Build System in the Documentation.
> > > >
> > > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > > >
> > > > Specifically the first paragraph of "2. How to Build External Modules"
> > > >
> > > > It may seem like a lot for such a simple issue but it *is* worth it.
> > > > ~Bryan
> > > >
> > >
> > > That section says
> > >
> > >
> > > "To build external modules, *you must have a prebuilt kernel
> > > available* that contains the configuration and header files used in
> > > the build. Also, the kernel must have been built with modules enabled.
> > > If you are using a distribution kernel, there will be a package for
> > > the kernel you are running provided by your distribution.
> > >
> > > An alternative is to use the “make” target “modules_prepare.” This
> > > will make sure the kernel contains the information required. The
> > > target exists solely as a simple way to prepare a kernel source tree
> > > for building external modules.
> > >
> > > NOTE: “modules_prepare” will not build Module.symvers even if
> > > CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> > > executed to make module versioning work.*"
> > >
> > > So I am just trying to confirm with you whether I have to first build
> > > the kernel with like "make" or not? As you can imagine my hardware
> > > takes *very* long to build a kernel as I did in my last attempt so I
> > > am asking whether it is needed. Hope you understand.
> > >

I understand. Though I still don't wish to rob you of this opportunity.

Your ability to come up with these questions and answer them yourself is
what will make you a better programmer and developer.

Don't get me wrong. Greg knows all too well the garbage I can shovel his
way. It's not about knowing the answer. It about knowing how to find the
answer yourself.

~Bryan

2021-08-29 22:13:11

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <[email protected]> wrote:
>
> On this day, August 29, 2021, thus sayeth Krish Jain:
> > Keeping you updated. Small win. The "Symbol version dump
> > "Module.symvers" is missing. " error disappeared. Now I still don't
> > know why
> >
>
> Whoop! Any win, no matter their size, always feel great. I ran around
> the house yesterday after cross compiling DOOM! for an armel chip. It's
> that "win" feeling you get that keeps me involved.
>
> It is important that you find out why though. What is the importance to
> having Module.symvers? and why is it a WARNING and not an ERROR?

When a module is loaded/used, the values contained in the kernel are
compared with similar values in the module; if they are not equal, the
kernel refuses to load the module. I don't need it in my case.

> What would happen if we didn't have the proper symbols when compiling or
> installing this driver?
> How and what generates the Module.symvers file when we *do* need it?

The kernel would refuse to load the module.





> How can we turn this warning off when we don't need it?
>
> This is covered in chapter "6. Module Versioning"
>
> https://www.kernel.org/doc/html/latest/kbuild/modules.html
>
> >
> > ERROR: Kernel configuration is invalid."; \
> > echo >&2 " include/generated/autoconf.h or
> > include/config/auto.conf are missing.";\
> > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > to fix it."; \
> >
> >
> > is still present.
> >
> > How can I fix this?
> >
>
> Are there any other 'make *config' options we could try?

Yes, like main menuconfig. I tried it but it still doesn't work.

> What does 'make prepare' even do?


Prepares for different architectures etc.


> Why do we even need a configuration file?
>
> https://www.kernel.org/doc/html/latest/kbuild/kconfig.html
>
> >
> > Best Regards
> >
> > On Sun, Aug 29, 2021 at 8:28 PM Krish Jain <[email protected]> wrote:
> > >
> > > Basically it says "you must have a prebuilt kernel available that
> > > contains the configuration and header files used in the build." Since
> > > for the staging kernel "make oldconfig" asked me for more
> > > configurations apart from my old configuration file (as it reads the
> > > existing .config file that was used for an old kernel and prompts the
> > > user for options in the current kernel source that are not found in
> > > the file) . So I *don't* currently have a prebuilt kernel that
> > > contains all the configuration in my staging kernel's .config file. So
> > > do I have to build the kernel once before I can just build the module
> > > with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?
> > >
>
> What do all these other configuration settings turn on and off anyway?
>
> Do we really need CONFIG_INFINIBAND turned on if we're working in the
> drivers/staging tree of the kernel?


No, we don't. I removed it.

> What would we gain from having a compiled kernel if we want to test a
> single staging driver?

No need to compile the entire kernel I guess for my use case. But
after all this reading :( I still don't get why " sudo make
CCFLAGS=-Werror W=1 M=drivers/staging/android/ V=1" worked for you
but not for me. I still get the following errors


test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
echo >&2; \
echo >&2 " ERROR: Kernel configuration is invalid."; \
echo >&2 " include/generated/autoconf.h or
include/config/auto.conf are missing.";\
echo >&2 " Run 'make oldconfig && make prepare' on kernel src
to fix it."; \
echo >&2 ; \
/bin/false)
.....


How can I fix this?




> If you found what Module.symvers does, you should know this.
>
> > > > >
> > > > > Again, do not allow others to rob you of learning how to solve these
> > > > > issues yourself. I *strongly* encourage you to familiarize yourself with
> > > > > the Kernel Build System in the Documentation.
> > > > >
> > > > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > > > >
> > > > > Specifically the first paragraph of "2. How to Build External Modules"
> > > > >
> > > > > It may seem like a lot for such a simple issue but it *is* worth it.
> > > > > ~Bryan
> > > > >
> > > >
> > > > That section says
> > > >
> > > >
> > > > "To build external modules, *you must have a prebuilt kernel
> > > > available* that contains the configuration and header files used in
> > > > the build. Also, the kernel must have been built with modules enabled.
> > > > If you are using a distribution kernel, there will be a package for
> > > > the kernel you are running provided by your distribution.
> > > >
> > > > An alternative is to use the “make” target “modules_prepare.” This
> > > > will make sure the kernel contains the information required. The
> > > > target exists solely as a simple way to prepare a kernel source tree
> > > > for building external modules.
> > > >
> > > > NOTE: “modules_prepare” will not build Module.symvers even if
> > > > CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> > > > executed to make module versioning work.*"
> > > >
> > > > So I am just trying to confirm with you whether I have to first build
> > > > the kernel with like "make" or not? As you can imagine my hardware
> > > > takes *very* long to build a kernel as I did in my last attempt so I
> > > > am asking whether it is needed. Hope you understand.
> > > >
>
> I understand. Though I still don't wish to rob you of this opportunity.
>
> Your ability to come up with these questions and answer them yourself is
> what will make you a better programmer and developer.
>
> Don't get me wrong. Greg knows all too well the garbage I can shovel his
> way. It's not about knowing the answer. It about knowing how to find the
> answer yourself.
>
> ~Bryan
>

2021-08-30 12:41:41

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Hi.

https://pastebin.com/NuvqMUWu is the link to the .config file.
The error I get is https://imgur.com/gkwh7Sb .


Best Regards


On Mon, Aug 30, 2021 at 12:11 AM Krish Jain <[email protected]> wrote:
>
> On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <[email protected]> wrote:
> >
> > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > Keeping you updated. Small win. The "Symbol version dump
> > > "Module.symvers" is missing. " error disappeared. Now I still don't
> > > know why
> > >
> >
> > Whoop! Any win, no matter their size, always feel great. I ran around
> > the house yesterday after cross compiling DOOM! for an armel chip. It's
> > that "win" feeling you get that keeps me involved.
> >
> > It is important that you find out why though. What is the importance to
> > having Module.symvers? and why is it a WARNING and not an ERROR?
>
> When a module is loaded/used, the values contained in the kernel are
> compared with similar values in the module; if they are not equal, the
> kernel refuses to load the module. I don't need it in my case.
>
> > What would happen if we didn't have the proper symbols when compiling or
> > installing this driver?
> > How and what generates the Module.symvers file when we *do* need it?
>
> The kernel would refuse to load the module.
>
>
>
>
>
> > How can we turn this warning off when we don't need it?
> >
> > This is covered in chapter "6. Module Versioning"
> >
> > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> >
> > >
> > > ERROR: Kernel configuration is invalid."; \
> > > echo >&2 " include/generated/autoconf.h or
> > > include/config/auto.conf are missing.";\
> > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > to fix it."; \
> > >
> > >
> > > is still present.
> > >
> > > How can I fix this?
> > >
> >
> > Are there any other 'make *config' options we could try?
>
> Yes, like main menuconfig. I tried it but it still doesn't work.
>
> > What does 'make prepare' even do?
>
>
> Prepares for different architectures etc.
>
>
> > Why do we even need a configuration file?
> >
> > https://www.kernel.org/doc/html/latest/kbuild/kconfig.html
> >
> > >
> > > Best Regards
> > >
> > > On Sun, Aug 29, 2021 at 8:28 PM Krish Jain <[email protected]> wrote:
> > > >
> > > > Basically it says "you must have a prebuilt kernel available that
> > > > contains the configuration and header files used in the build." Since
> > > > for the staging kernel "make oldconfig" asked me for more
> > > > configurations apart from my old configuration file (as it reads the
> > > > existing .config file that was used for an old kernel and prompts the
> > > > user for options in the current kernel source that are not found in
> > > > the file) . So I *don't* currently have a prebuilt kernel that
> > > > contains all the configuration in my staging kernel's .config file. So
> > > > do I have to build the kernel once before I can just build the module
> > > > with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?
> > > >
> >
> > What do all these other configuration settings turn on and off anyway?
> >
> > Do we really need CONFIG_INFINIBAND turned on if we're working in the
> > drivers/staging tree of the kernel?
>
>
> No, we don't. I removed it.
>
> > What would we gain from having a compiled kernel if we want to test a
> > single staging driver?
>
> No need to compile the entire kernel I guess for my use case. But
> after all this reading :( I still don't get why " sudo make
> CCFLAGS=-Werror W=1 M=drivers/staging/android/ V=1" worked for you
> but not for me. I still get the following errors
>
>
> test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> echo >&2; \
> echo >&2 " ERROR: Kernel configuration is invalid."; \
> echo >&2 " include/generated/autoconf.h or
> include/config/auto.conf are missing.";\
> echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> to fix it."; \
> echo >&2 ; \
> /bin/false)
> .....
>
>
> How can I fix this?
>
>
>
>
> > If you found what Module.symvers does, you should know this.
> >
> > > > > >
> > > > > > Again, do not allow others to rob you of learning how to solve these
> > > > > > issues yourself. I *strongly* encourage you to familiarize yourself with
> > > > > > the Kernel Build System in the Documentation.
> > > > > >
> > > > > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > > > > >
> > > > > > Specifically the first paragraph of "2. How to Build External Modules"
> > > > > >
> > > > > > It may seem like a lot for such a simple issue but it *is* worth it.
> > > > > > ~Bryan
> > > > > >
> > > > >
> > > > > That section says
> > > > >
> > > > >
> > > > > "To build external modules, *you must have a prebuilt kernel
> > > > > available* that contains the configuration and header files used in
> > > > > the build. Also, the kernel must have been built with modules enabled.
> > > > > If you are using a distribution kernel, there will be a package for
> > > > > the kernel you are running provided by your distribution.
> > > > >
> > > > > An alternative is to use the “make” target “modules_prepare.” This
> > > > > will make sure the kernel contains the information required. The
> > > > > target exists solely as a simple way to prepare a kernel source tree
> > > > > for building external modules.
> > > > >
> > > > > NOTE: “modules_prepare” will not build Module.symvers even if
> > > > > CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> > > > > executed to make module versioning work.*"
> > > > >
> > > > > So I am just trying to confirm with you whether I have to first build
> > > > > the kernel with like "make" or not? As you can imagine my hardware
> > > > > takes *very* long to build a kernel as I did in my last attempt so I
> > > > > am asking whether it is needed. Hope you understand.
> > > > >
> >
> > I understand. Though I still don't wish to rob you of this opportunity.
> >
> > Your ability to come up with these questions and answer them yourself is
> > what will make you a better programmer and developer.
> >
> > Don't get me wrong. Greg knows all too well the garbage I can shovel his
> > way. It's not about knowing the answer. It about knowing how to find the
> > answer yourself.
> >
> > ~Bryan
> >

2021-08-30 13:10:07

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

One sec this got even more confusing. I saw
https://stackoverflow.com/questions/23184181/error-while-running-make-install-include-generated-autoconf-h-or-include-confi
and it says

-------
You are including the V=1 which causes Make to show the commands as
they're being run. From the looks of it, you're not actually seeing
the error itself, but you're seeing the test that it's running to
check if those files exist:

test -e include/generated/autoconf.h -a -e include/config/auto.conf ||
( \ ... echo error messages here ... \ )

That test is being run, and if it fails, it would echo those messages
to standard error, which it's not. If your module isn't building it's
probably due to some other issue.
------

So where is it going all wrong? Messing up the file ashmem.c does not
print the errors.


Best Regards

On Mon, Aug 30, 2021 at 2:40 PM Krish Jain <[email protected]> wrote:
>
> Hi.
>
> https://pastebin.com/NuvqMUWu is the link to the .config file.
> The error I get is https://imgur.com/gkwh7Sb .
>
>
> Best Regards
>
>
> On Mon, Aug 30, 2021 at 12:11 AM Krish Jain <[email protected]> wrote:
> >
> > On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <[email protected]> wrote:
> > >
> > > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > > Keeping you updated. Small win. The "Symbol version dump
> > > > "Module.symvers" is missing. " error disappeared. Now I still don't
> > > > know why
> > > >
> > >
> > > Whoop! Any win, no matter their size, always feel great. I ran around
> > > the house yesterday after cross compiling DOOM! for an armel chip. It's
> > > that "win" feeling you get that keeps me involved.
> > >
> > > It is important that you find out why though. What is the importance to
> > > having Module.symvers? and why is it a WARNING and not an ERROR?
> >
> > When a module is loaded/used, the values contained in the kernel are
> > compared with similar values in the module; if they are not equal, the
> > kernel refuses to load the module. I don't need it in my case.
> >
> > > What would happen if we didn't have the proper symbols when compiling or
> > > installing this driver?
> > > How and what generates the Module.symvers file when we *do* need it?
> >
> > The kernel would refuse to load the module.
> >
> >
> >
> >
> >
> > > How can we turn this warning off when we don't need it?
> > >
> > > This is covered in chapter "6. Module Versioning"
> > >
> > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > >
> > > >
> > > > ERROR: Kernel configuration is invalid."; \
> > > > echo >&2 " include/generated/autoconf.h or
> > > > include/config/auto.conf are missing.";\
> > > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > > to fix it."; \
> > > >
> > > >
> > > > is still present.
> > > >
> > > > How can I fix this?
> > > >
> > >
> > > Are there any other 'make *config' options we could try?
> >
> > Yes, like main menuconfig. I tried it but it still doesn't work.
> >
> > > What does 'make prepare' even do?
> >
> >
> > Prepares for different architectures etc.
> >
> >
> > > Why do we even need a configuration file?
> > >
> > > https://www.kernel.org/doc/html/latest/kbuild/kconfig.html
> > >
> > > >
> > > > Best Regards
> > > >
> > > > On Sun, Aug 29, 2021 at 8:28 PM Krish Jain <[email protected]> wrote:
> > > > >
> > > > > Basically it says "you must have a prebuilt kernel available that
> > > > > contains the configuration and header files used in the build." Since
> > > > > for the staging kernel "make oldconfig" asked me for more
> > > > > configurations apart from my old configuration file (as it reads the
> > > > > existing .config file that was used for an old kernel and prompts the
> > > > > user for options in the current kernel source that are not found in
> > > > > the file) . So I *don't* currently have a prebuilt kernel that
> > > > > contains all the configuration in my staging kernel's .config file. So
> > > > > do I have to build the kernel once before I can just build the module
> > > > > with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?
> > > > >
> > >
> > > What do all these other configuration settings turn on and off anyway?
> > >
> > > Do we really need CONFIG_INFINIBAND turned on if we're working in the
> > > drivers/staging tree of the kernel?
> >
> >
> > No, we don't. I removed it.
> >
> > > What would we gain from having a compiled kernel if we want to test a
> > > single staging driver?
> >
> > No need to compile the entire kernel I guess for my use case. But
> > after all this reading :( I still don't get why " sudo make
> > CCFLAGS=-Werror W=1 M=drivers/staging/android/ V=1" worked for you
> > but not for me. I still get the following errors
> >
> >
> > test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> > echo >&2; \
> > echo >&2 " ERROR: Kernel configuration is invalid."; \
> > echo >&2 " include/generated/autoconf.h or
> > include/config/auto.conf are missing.";\
> > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > to fix it."; \
> > echo >&2 ; \
> > /bin/false)
> > .....
> >
> >
> > How can I fix this?
> >
> >
> >
> >
> > > If you found what Module.symvers does, you should know this.
> > >
> > > > > > >
> > > > > > > Again, do not allow others to rob you of learning how to solve these
> > > > > > > issues yourself. I *strongly* encourage you to familiarize yourself with
> > > > > > > the Kernel Build System in the Documentation.
> > > > > > >
> > > > > > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > > > > > >
> > > > > > > Specifically the first paragraph of "2. How to Build External Modules"
> > > > > > >
> > > > > > > It may seem like a lot for such a simple issue but it *is* worth it.
> > > > > > > ~Bryan
> > > > > > >
> > > > > >
> > > > > > That section says
> > > > > >
> > > > > >
> > > > > > "To build external modules, *you must have a prebuilt kernel
> > > > > > available* that contains the configuration and header files used in
> > > > > > the build. Also, the kernel must have been built with modules enabled.
> > > > > > If you are using a distribution kernel, there will be a package for
> > > > > > the kernel you are running provided by your distribution.
> > > > > >
> > > > > > An alternative is to use the “make” target “modules_prepare.” This
> > > > > > will make sure the kernel contains the information required. The
> > > > > > target exists solely as a simple way to prepare a kernel source tree
> > > > > > for building external modules.
> > > > > >
> > > > > > NOTE: “modules_prepare” will not build Module.symvers even if
> > > > > > CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> > > > > > executed to make module versioning work.*"
> > > > > >
> > > > > > So I am just trying to confirm with you whether I have to first build
> > > > > > the kernel with like "make" or not? As you can imagine my hardware
> > > > > > takes *very* long to build a kernel as I did in my last attempt so I
> > > > > > am asking whether it is needed. Hope you understand.
> > > > > >
> > >
> > > I understand. Though I still don't wish to rob you of this opportunity.
> > >
> > > Your ability to come up with these questions and answer them yourself is
> > > what will make you a better programmer and developer.
> > >
> > > Don't get me wrong. Greg knows all too well the garbage I can shovel his
> > > way. It's not about knowing the answer. It about knowing how to find the
> > > answer yourself.
> > >
> > > ~Bryan
> > >

2021-08-31 00:43:49

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

Hi, could someone help with this? Still stuck. Maybe someone else has
some insight into this issue too? Or Greg or Bryan.

Thanks

On Mon, Aug 30, 2021 at 3:01 PM Krish Jain <[email protected]> wrote:
>
> One sec this got even more confusing. I saw
> https://stackoverflow.com/questions/23184181/error-while-running-make-install-include-generated-autoconf-h-or-include-confi
> and it says
>
> -------
> You are including the V=1 which causes Make to show the commands as
> they're being run. From the looks of it, you're not actually seeing
> the error itself, but you're seeing the test that it's running to
> check if those files exist:
>
> test -e include/generated/autoconf.h -a -e include/config/auto.conf ||
> ( \ ... echo error messages here ... \ )
>
> That test is being run, and if it fails, it would echo those messages
> to standard error, which it's not. If your module isn't building it's
> probably due to some other issue.
> ------
>
> So where is it going all wrong? Messing up the file ashmem.c does not
> print the errors.
>
>
> Best Regards
>
> On Mon, Aug 30, 2021 at 2:40 PM Krish Jain <[email protected]> wrote:
> >
> > Hi.
> >
> > https://pastebin.com/NuvqMUWu is the link to the .config file.
> > The error I get is https://imgur.com/gkwh7Sb .
> >
> >
> > Best Regards
> >
> >
> > On Mon, Aug 30, 2021 at 12:11 AM Krish Jain <[email protected]> wrote:
> > >
> > > On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <[email protected]> wrote:
> > > >
> > > > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > > > Keeping you updated. Small win. The "Symbol version dump
> > > > > "Module.symvers" is missing. " error disappeared. Now I still don't
> > > > > know why
> > > > >
> > > >
> > > > Whoop! Any win, no matter their size, always feel great. I ran around
> > > > the house yesterday after cross compiling DOOM! for an armel chip. It's
> > > > that "win" feeling you get that keeps me involved.
> > > >
> > > > It is important that you find out why though. What is the importance to
> > > > having Module.symvers? and why is it a WARNING and not an ERROR?
> > >
> > > When a module is loaded/used, the values contained in the kernel are
> > > compared with similar values in the module; if they are not equal, the
> > > kernel refuses to load the module. I don't need it in my case.
> > >
> > > > What would happen if we didn't have the proper symbols when compiling or
> > > > installing this driver?
> > > > How and what generates the Module.symvers file when we *do* need it?
> > >
> > > The kernel would refuse to load the module.
> > >
> > >
> > >
> > >
> > >
> > > > How can we turn this warning off when we don't need it?
> > > >
> > > > This is covered in chapter "6. Module Versioning"
> > > >
> > > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > > >
> > > > >
> > > > > ERROR: Kernel configuration is invalid."; \
> > > > > echo >&2 " include/generated/autoconf.h or
> > > > > include/config/auto.conf are missing.";\
> > > > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > > > to fix it."; \
> > > > >
> > > > >
> > > > > is still present.
> > > > >
> > > > > How can I fix this?
> > > > >
> > > >
> > > > Are there any other 'make *config' options we could try?
> > >
> > > Yes, like main menuconfig. I tried it but it still doesn't work.
> > >
> > > > What does 'make prepare' even do?
> > >
> > >
> > > Prepares for different architectures etc.
> > >
> > >
> > > > Why do we even need a configuration file?
> > > >
> > > > https://www.kernel.org/doc/html/latest/kbuild/kconfig.html
> > > >
> > > > >
> > > > > Best Regards
> > > > >
> > > > > On Sun, Aug 29, 2021 at 8:28 PM Krish Jain <[email protected]> wrote:
> > > > > >
> > > > > > Basically it says "you must have a prebuilt kernel available that
> > > > > > contains the configuration and header files used in the build." Since
> > > > > > for the staging kernel "make oldconfig" asked me for more
> > > > > > configurations apart from my old configuration file (as it reads the
> > > > > > existing .config file that was used for an old kernel and prompts the
> > > > > > user for options in the current kernel source that are not found in
> > > > > > the file) . So I *don't* currently have a prebuilt kernel that
> > > > > > contains all the configuration in my staging kernel's .config file. So
> > > > > > do I have to build the kernel once before I can just build the module
> > > > > > with "make CCFLAGS=-Werror W=1 M=drivers/staging/android" ?
> > > > > >
> > > >
> > > > What do all these other configuration settings turn on and off anyway?
> > > >
> > > > Do we really need CONFIG_INFINIBAND turned on if we're working in the
> > > > drivers/staging tree of the kernel?
> > >
> > >
> > > No, we don't. I removed it.
> > >
> > > > What would we gain from having a compiled kernel if we want to test a
> > > > single staging driver?
> > >
> > > No need to compile the entire kernel I guess for my use case. But
> > > after all this reading :( I still don't get why " sudo make
> > > CCFLAGS=-Werror W=1 M=drivers/staging/android/ V=1" worked for you
> > > but not for me. I still get the following errors
> > >
> > >
> > > test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> > > echo >&2; \
> > > echo >&2 " ERROR: Kernel configuration is invalid."; \
> > > echo >&2 " include/generated/autoconf.h or
> > > include/config/auto.conf are missing.";\
> > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > to fix it."; \
> > > echo >&2 ; \
> > > /bin/false)
> > > .....
> > >
> > >
> > > How can I fix this?
> > >
> > >
> > >
> > >
> > > > If you found what Module.symvers does, you should know this.
> > > >
> > > > > > > >
> > > > > > > > Again, do not allow others to rob you of learning how to solve these
> > > > > > > > issues yourself. I *strongly* encourage you to familiarize yourself with
> > > > > > > > the Kernel Build System in the Documentation.
> > > > > > > >
> > > > > > > > https://www.kernel.org/doc/html/latest/kbuild/modules.html
> > > > > > > >
> > > > > > > > Specifically the first paragraph of "2. How to Build External Modules"
> > > > > > > >
> > > > > > > > It may seem like a lot for such a simple issue but it *is* worth it.
> > > > > > > > ~Bryan
> > > > > > > >
> > > > > > >
> > > > > > > That section says
> > > > > > >
> > > > > > >
> > > > > > > "To build external modules, *you must have a prebuilt kernel
> > > > > > > available* that contains the configuration and header files used in
> > > > > > > the build. Also, the kernel must have been built with modules enabled.
> > > > > > > If you are using a distribution kernel, there will be a package for
> > > > > > > the kernel you are running provided by your distribution.
> > > > > > >
> > > > > > > An alternative is to use the “make” target “modules_prepare.” This
> > > > > > > will make sure the kernel contains the information required. The
> > > > > > > target exists solely as a simple way to prepare a kernel source tree
> > > > > > > for building external modules.
> > > > > > >
> > > > > > > NOTE: “modules_prepare” will not build Module.symvers even if
> > > > > > > CONFIG_MODVERSIONS is set; therefore, *a full kernel build needs to be
> > > > > > > executed to make module versioning work.*"
> > > > > > >
> > > > > > > So I am just trying to confirm with you whether I have to first build
> > > > > > > the kernel with like "make" or not? As you can imagine my hardware
> > > > > > > takes *very* long to build a kernel as I did in my last attempt so I
> > > > > > > am asking whether it is needed. Hope you understand.
> > > > > > >
> > > >
> > > > I understand. Though I still don't wish to rob you of this opportunity.
> > > >
> > > > Your ability to come up with these questions and answer them yourself is
> > > > what will make you a better programmer and developer.
> > > >
> > > > Don't get me wrong. Greg knows all too well the garbage I can shovel his
> > > > way. It's not about knowing the answer. It about knowing how to find the
> > > > answer yourself.
> > > >
> > > > ~Bryan
> > > >

2021-08-31 13:39:32

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On this day, August 31, 2021, thus sayeth Krish Jain:
> Hi, could someone help with this? Still stuck. Maybe someone else has
> some insight into this issue too? Or Greg or Bryan.
>
> Thanks
>

Just another friendly reminder about top posting. I liked Greg's link to
John Gruber's opinion. TLDR: It's poor form.

http://daringfireball.net/2007/07/on_top

Ignoring these bits of advice send signals to the community that you
don't value the time we donate to you.

>
> On Mon, Aug 30, 2021 at 3:01 PM Krish Jain <[email protected]> wrote:
> >
> > One sec this got even more confusing. I saw
> > https://stackoverflow.com/questions/23184181/error-while-running-make-install-include-generated-autoconf-h-or-include-confi
> > and it says
> >
> > -------
> > You are including the V=1 which causes Make to show the commands as
> > they're being run. From the looks of it, you're not actually seeing
> > the error itself, but you're seeing the test that it's running to
> > check if those files exist:
> >
> > test -e include/generated/autoconf.h -a -e include/config/auto.conf ||
> > ( \ ... echo error messages here ... \ )
> >
> > That test is being run, and if it fails, it would echo those messages
> > to standard error, which it's not. If your module isn't building it's
> > probably due to some other issue.
> > ------
> >
> > So where is it going all wrong? Messing up the file ashmem.c does not
> > print the errors.
> >

I have no clue where you got V=1 from. Are you sure you need it?

As for "So where is it going all wrong?"

Like Greg said a few emails ago:

"Are you sure the file is being built at all? You usually have to
select the proper configuration option to enable that driver as well."

After a response from Greg like that, I would ask myself: How do I
select the proper configuration options? and How do I know if the proper
one is set?

Having had the same issue as you when I first started, I will try to
save you some time. Read the documentation, it is faster than you think
and will give you more information than I can give you over email:

https://www.kernel.org/doc/html/latest/kbuild/makefiles.html

> > >
> > > https://pastebin.com/NuvqMUWu is the link to the .config file.
> > > The error I get is https://imgur.com/gkwh7Sb .
> > >

Your .config will be different to everyone else. I have different
hardware to you, Greg or Fabio. And I will need different modules and
options enabled. I have no idea if your .config file is correct.

This error seems to indicate you're missing your Module.symvers file,
not your .config file. It's hard to tell with the V=1 option set.

> > > >
> > > > When a module is loaded/used, the values contained in the kernel are
> > > > compared with similar values in the module; if they are not equal, the
> > > > kernel refuses to load the module. I don't need it in my case.
> > > >
> > > > > What would happen if we didn't have the proper symbols when compiling or
> > > > > installing this driver?
> > > > > How and what generates the Module.symvers file when we *do* need it?
> > > >
> > > > The kernel would refuse to load the module.
> > > >

Excellent! This has nothing to do with test building our shared memory
system. We can either turn the warning off, find a way to generate the
file, or fake that we've generated the file.

Though it's important you know the impact of your decision.

> > > > > >
> > > > > > ERROR: Kernel configuration is invalid."; \
> > > > > > echo >&2 " include/generated/autoconf.h or
> > > > > > include/config/auto.conf are missing.";\
> > > > > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > > > > to fix it."; \
> > > > > >
> > > > > >
> > > > > > is still present.
> > > > > >
> > > > > > How can I fix this?
> > > > > >

After everything above, (See what we mean about top posting?) Are you
sure you're getting this ERROR?

> > > > >
> > > > > Are there any other 'make *config' options we could try?
> > > >
> > > > Yes, like main menuconfig. I tried it but it still doesn't work.
> > > >

There are more (faster) options. Though 'menuconfig' would be a great
option to select specific modules you would like to build.

> > > > >
> > > > > Do we really need CONFIG_INFINIBAND turned on if we're working in the
> > > > > drivers/staging tree of the kernel?
> > > >
> > > > No, we don't. I removed it.
> > > >

"removed it" makes me think your editing your .config file by hand.
It's not wrong to do so, just be sure you know what you're doing.

> > > >
> > > > > What would we gain from having a compiled kernel if we want to test a
> > > > > single staging driver?
> > > >
> > > > No need to compile the entire kernel I guess for my use case. But
> > > > after all this reading :( I still don't get why " sudo make
> > > > CCFLAGS=-Werror W=1 M=drivers/staging/android/ V=1" worked for you
> > > > but not for me. I still get the following errors
> > > >
> > > >
> > > > test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
> > > > echo >&2; \
> > > > echo >&2 " ERROR: Kernel configuration is invalid."; \
> > > > echo >&2 " include/generated/autoconf.h or
> > > > include/config/auto.conf are missing.";\
> > > > echo >&2 " Run 'make oldconfig && make prepare' on kernel src
> > > > to fix it."; \
> > > > echo >&2 ; \
> > > > /bin/false)
> > > > .....
> > > >
> > > >
> > > > How can I fix this?
> > > >

My advice would be to familiarize yourself with how modules are built
with "Linux Kernel Makefiles" section in the documentation.

https://www.kernel.org/doc/html/latest/kbuild/makefiles.html

It describes everything in greater detail than I could ever give here.

If anyone else wants to jump in here, feel free.
~Bryan

2021-08-31 14:01:33

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Tuesday, August 31, 2021 3:35:51 PM CEST Bryan Brattlof wrote:
> On this day, August 31, 2021, thus sayeth Krish Jain:
> > Hi, could someone help with this? Still stuck. Maybe someone else has
> > some insight into this issue too? Or Greg or Bryan.
> >
> > Thanks

> [...]

> If anyone else wants to jump in here, feel free.
> ~Bryan

Hi Bryan,

No, I don't want to jump in here because you already did everything that
would be conceivable to do.

I just want to *really* thank you for the hard work you got involved and that
you carried out with one of the highest levels of professionalism (and
patience :)) very few of us could ever equal (not I, for sure).

I thank you also not for the technical hints you gave to Krish, instead for
your your choice "to not rob [you] Krish the opportunity to learn".

Actually I was tempted to write something like "first do this, than that, and
finally run this tool". But I was able to desist, by learning from you how
people should be helped for real.

Most of us here should learn by your attitude.

Thanks again, seriously.

Regards,

Fabio



2021-08-31 23:01:52

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On this day, August 31, 2021, thus sayeth Fabio M. De Francesco:
>
> I just want to *really* thank you for the hard work you got involved and that
> you carried out with one of the highest levels of professionalism (and
> patience :)) very few of us could ever equal (not I, for sure).
>
> I thank you also not for the technical hints you gave to Krish, instead for
> your your choice "to not rob [you] Krish the opportunity to learn".
>
> Actually I was tempted to write something like "first do this, than that, and
> finally run this tool". But I was able to desist, by learning from you how
> people should be helped for real.
>
> Most of us here should learn by your attitude.
>
> Thanks again, seriously.
>

Thank you for such kind words, Fabio.

I was very lucky to be, and still am, surrounded by people who
demonstrated this idea to me when I was young. I am very happy to see
others here see how beneficial and helpful (in the long term) learning
this way can be.

I'm grateful to have found and be a part of this community.
~Bryan

2021-09-01 15:25:53

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Wed, Sep 1, 2021 at 1:00 AM Bryan Brattlof <[email protected]> wrote:
>
> On this day, August 31, 2021, thus sayeth Fabio M. De Francesco:
> >
> > I just want to *really* thank you for the hard work you got involved and that
> > you carried out with one of the highest levels of professionalism (and
> > patience :)) very few of us could ever equal (not I, for sure).
> >
> > I thank you also not for the technical hints you gave to Krish, instead for
> > your your choice "to not rob [you] Krish the opportunity to learn".
> >
> > Actually I was tempted to write something like "first do this, than that, and
> > finally run this tool". But I was able to desist, by learning from you how
> > people should be helped for real.
> >
> > Most of us here should learn by your attitude.
> >
> > Thanks again, seriously.
> >
>
> Thank you for such kind words, Fabio.
>
> I was very lucky to be, and still am, surrounded by people who
> demonstrated this idea to me when I was young. I am very happy to see
> others here see how beneficial and helpful (in the long term) learning
> this way can be.
>
> I'm grateful to have found and be a part of this community.
> ~Bryan
>



Interesting.

"make drivers/staging/android/ " works now (finally!) and shows me the
errors when I mess up in the file ashmem.c for example.
Furthermore, " make CCFLAGS=-Werror W=1 drivers/staging/android/ "
outputs the same errors too just more verbose. So it works completely
now, However, "make CCFLAGS=-Werror W=1 M=drivers/staging/android/
" just takes to new prompt line and does not output anything. Do you
know why?


Thanks

2021-09-01 15:32:19

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Wed, Sep 01, 2021 at 05:20:13PM +0200, Krish Jain wrote:
> On Wed, Sep 1, 2021 at 1:00 AM Bryan Brattlof <[email protected]> wrote:
> >
> > On this day, August 31, 2021, thus sayeth Fabio M. De Francesco:
> > >
> > > I just want to *really* thank you for the hard work you got involved and that
> > > you carried out with one of the highest levels of professionalism (and
> > > patience :)) very few of us could ever equal (not I, for sure).
> > >
> > > I thank you also not for the technical hints you gave to Krish, instead for
> > > your your choice "to not rob [you] Krish the opportunity to learn".
> > >
> > > Actually I was tempted to write something like "first do this, than that, and
> > > finally run this tool". But I was able to desist, by learning from you how
> > > people should be helped for real.
> > >
> > > Most of us here should learn by your attitude.
> > >
> > > Thanks again, seriously.
> > >
> >
> > Thank you for such kind words, Fabio.
> >
> > I was very lucky to be, and still am, surrounded by people who
> > demonstrated this idea to me when I was young. I am very happy to see
> > others here see how beneficial and helpful (in the long term) learning
> > this way can be.
> >
> > I'm grateful to have found and be a part of this community.
> > ~Bryan
> >
>
>
>
> Interesting.
>
> "make drivers/staging/android/ " works now (finally!) and shows me the
> errors when I mess up in the file ashmem.c for example.
> Furthermore, " make CCFLAGS=-Werror W=1 drivers/staging/android/ "
> outputs the same errors too just more verbose. So it works completely
> now, However, "make CCFLAGS=-Werror W=1 M=drivers/staging/android/
> " just takes to new prompt line and does not output anything. Do you
> know why?

"M=pathname" is different than "pathname", you are asking for different
things to happen here, so depending on your kernel configuration,
different things will be built (or not built).

And don't mes with CCFLAGS settings for building the kernel unless you
_really_ know what you are doing. For staging tree work, it's not
advised at all.

good luck!

greg k-h

2021-09-01 17:38:41

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On this day, September 1, 2021, thus sayeth Greg KH:
> On Wed, Sep 01, 2021 at 05:34:36PM +0200, Krish Jain wrote:
> >
> > Can you tell me why this is the case?
>
> Again, it depends on your kernel configuration file as to what will, or
> will not, be built.
>
> If you have some things set as modules, they can be built as a module,
> but the ashmem code can not be built as a module, so you would never
> build it if you did the above line.
>
> Here, look at this sequence, starting with a tree that does nothing if I
> do a simple 'make' in it, as the whole kernel is already built, and
> ashmem is enabled in the kernel configuration
>
> $ grep ASHMEM .config
> CONFIG_ASHMEM=y
> $ make
> $
>
> So, let's change the time stamp on the ashmem.c file and see what gets
> built if you use the M= option:
>
> $ touch drivers/staging/android/ashmem.c
> $ make M=drivers/staging/android
> MODPOST drivers/staging/android/Module.symvers
> $
>
> Nothing gets built as ashmem is NOT a module, and M= only builds any
> modules in the directory you specified.
>
> But, if you tell make to just build the whole subdirectory, no matter
> what the setting is, it will be built:
>
> $ make drivers/staging/android/
> CALL scripts/checksyscalls.sh
> CALL scripts/atomic/check-atomics.sh
> DESCEND objtool
> CC drivers/staging/android/ashmem.o
> AR drivers/staging/android/built-in.a
> $
>
> So that's the difference, "M=" builds modules in that directory, but if
> you tell it to build the subdir, everything in there that needs to be
> built, will be built.
>
> Be careful about your kernel configuration, that is the key for what
> will, and will not, be built.
>

Ouch...

I want to *really* apologize to you Krish for introducing so much
confusion while you, and apparently I, am still learning. And for your
persistence with seeking the correct answer here Krish.

I did not notice that this could only be build as a built-in object.
Thank you Greg for pointing out my mistake, and I apologize for dragging
this out longer than it had to and the frustration this caused.

It seems I will be reading the documentation again, along with Greg's
book recommendation, "Linux Kernel in a Nutshell" over this merge
window.

Thank you again Krish and Greg
~Bryan

2021-09-01 18:10:08

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Wed, Sep 1, 2021 at 7:34 PM Bryan Brattlof <[email protected]> wrote:
>
> On this day, September 1, 2021, thus sayeth Greg KH:
> > On Wed, Sep 01, 2021 at 05:34:36PM +0200, Krish Jain wrote:
> > >
> > > Can you tell me why this is the case?
> >
> > Again, it depends on your kernel configuration file as to what will, or
> > will not, be built.
> >
> > If you have some things set as modules, they can be built as a module,
> > but the ashmem code can not be built as a module, so you would never
> > build it if you did the above line.
> >
> > Here, look at this sequence, starting with a tree that does nothing if I
> > do a simple 'make' in it, as the whole kernel is already built, and
> > ashmem is enabled in the kernel configuration
> >
> > $ grep ASHMEM .config
> > CONFIG_ASHMEM=y
> > $ make
> > $
> >
> > So, let's change the time stamp on the ashmem.c file and see what gets
> > built if you use the M= option:
> >
> > $ touch drivers/staging/android/ashmem.c
> > $ make M=drivers/staging/android
> > MODPOST drivers/staging/android/Module.symvers
> > $
> >
> > Nothing gets built as ashmem is NOT a module, and M= only builds any
> > modules in the directory you specified.
> >
> > But, if you tell make to just build the whole subdirectory, no matter
> > what the setting is, it will be built:
> >
> > $ make drivers/staging/android/
> > CALL scripts/checksyscalls.sh
> > CALL scripts/atomic/check-atomics.sh
> > DESCEND objtool
> > CC drivers/staging/android/ashmem.o
> > AR drivers/staging/android/built-in.a
> > $
> >
> > So that's the difference, "M=" builds modules in that directory, but if
> > you tell it to build the subdir, everything in there that needs to be
> > built, will be built.
> >
> > Be careful about your kernel configuration, that is the key for what
> > will, and will not, be built.
> >
>
> Ouch...
>
> I want to *really* apologize to you Krish for introducing so much
> confusion while you, and apparently I, am still learning. And for your
> persistence with seeking the correct answer here Krish.
>
> I did not notice that this could only be build as a built-in object.
> Thank you Greg for pointing out my mistake, and I apologize for dragging
> this out longer than it had to and the frustration this caused.
>
> It seems I will be reading the documentation again, along with Greg's
> book recommendation, "Linux Kernel in a Nutshell" over this merge
> window.
>
> Thank you again Krish and Greg
> ~Bryan
>

Yes, lots of reading to do :) . I had a look at the book and it seems
better than the documentation too, I don't know, maybe the writing
style? Love it, Greg. Lastly just out of curiosity, Bryan, if this
can only be built as a built-in object then how come "As for your
patch, I built the driver using:

$ make CCFLAGS=-Werror W=1 M=drivers/staging/android"

got you the errors that I desired? Aren't you building as a module here?


Warm Regards

2021-09-01 20:07:51

by Krish Jain

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Wed, Sep 1, 2021 at 5:30 PM Greg KH <[email protected]> wrote:
>
> On Wed, Sep 01, 2021 at 05:20:13PM +0200, Krish Jain wrote:
> > On Wed, Sep 1, 2021 at 1:00 AM Bryan Brattlof <[email protected]> wrote:
> > >
> > > On this day, August 31, 2021, thus sayeth Fabio M. De Francesco:
> > > >
> > > > I just want to *really* thank you for the hard work you got involved and that
> > > > you carried out with one of the highest levels of professionalism (and
> > > > patience :)) very few of us could ever equal (not I, for sure).
> > > >
> > > > I thank you also not for the technical hints you gave to Krish, instead for
> > > > your your choice "to not rob [you] Krish the opportunity to learn".
> > > >
> > > > Actually I was tempted to write something like "first do this, than that, and
> > > > finally run this tool". But I was able to desist, by learning from you how
> > > > people should be helped for real.
> > > >
> > > > Most of us here should learn by your attitude.
> > > >
> > > > Thanks again, seriously.
> > > >
> > >
> > > Thank you for such kind words, Fabio.
> > >
> > > I was very lucky to be, and still am, surrounded by people who
> > > demonstrated this idea to me when I was young. I am very happy to see
> > > others here see how beneficial and helpful (in the long term) learning
> > > this way can be.
> > >
> > > I'm grateful to have found and be a part of this community.
> > > ~Bryan
> > >
> >
> >
> >
> > Interesting.
> >
> > "make drivers/staging/android/ " works now (finally!) and shows me the
> > errors when I mess up in the file ashmem.c for example.
> > Furthermore, " make CCFLAGS=-Werror W=1 drivers/staging/android/ "
> > outputs the same errors too just more verbose. So it works completely
> > now, However, "make CCFLAGS=-Werror W=1 M=drivers/staging/android/
> > " just takes to new prompt line and does not output anything. Do you
> > know why?
>
> "M=pathname" is different than "pathname", you are asking for different
> things to happen here, so depending on your kernel configuration,
> different things will be built (or not built).
>
> And don't mes with CCFLAGS settings for building the kernel unless you
> _really_ know what you are doing. For staging tree work, it's not
> advised at all.
>
> good luck!
>
> greg k-h


Oh ok, thanks Greg. I only attempted to use "make CCFLAGS=-Werror W=1
M=drivers/staging/android/" as that's the command Bryan used earlier
and it worked.

"As for your patch, I built the driver using:

$ make CCFLAGS=-Werror W=1 M=drivers/staging/android"


Can you tell me why this is the case? And whether I can just start
working on an android driver patch and testing whether the android
driver builds successfully using "make drivers/staging/android/ "
instead of that command, would it be the same effectively?

Best Regards,

Thanks so much for helping a high schooler learn his way around the kernel

2021-09-01 20:17:04

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On Wed, Sep 01, 2021 at 05:34:36PM +0200, Krish Jain wrote:
> Oh ok, thanks Greg. I only attempted to use "make CCFLAGS=-Werror W=1
> M=drivers/staging/android/" as that's the command Bryan used earlier
> and it worked.
>
> "As for your patch, I built the driver using:
>
> $ make CCFLAGS=-Werror W=1 M=drivers/staging/android"
>
>
> Can you tell me why this is the case?

Again, it depends on your kernel configuration file as to what will, or
will not, be built.

If you have some things set as modules, they can be built as a module,
but the ashmem code can not be built as a module, so you would never
build it if you did the above line.

Here, look at this sequence, starting with a tree that does nothing if I
do a simple 'make' in it, as the whole kernel is already built, and
ashmem is enabled in the kernel configuration

$ grep ASHMEM .config
CONFIG_ASHMEM=y
$ make
$

So, let's change the time stamp on the ashmem.c file and see what gets
built if you use the M= option:

$ touch drivers/staging/android/ashmem.c
$ make M=drivers/staging/android
MODPOST drivers/staging/android/Module.symvers
$

Nothing gets built as ashmem is NOT a module, and M= only builds any
modules in the directory you specified.

But, if you tell make to just build the whole subdirectory, no matter
what the setting is, it will be built:

$ make drivers/staging/android/
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CC drivers/staging/android/ashmem.o
AR drivers/staging/android/built-in.a
$

So that's the difference, "M=" builds modules in that directory, but if
you tell it to build the subdir, everything in there that needs to be
built, will be built.

Be careful about your kernel configuration, that is the key for what
will, and will not, be built.

Perhaps you should look at the book, "Linux Kernel in a Nutshell" that
is free online. It talks all about building and configuring a kernel.
Parts of it are out of date, but the general ideas are good.

hope this helps,

greg k-h

2021-09-01 22:46:01

by Bryan Brattlof

[permalink] [raw]
Subject: Re: [PATCH] Declare the file_operations struct as const

On this day, September 1, 2021, thus sayeth Krish Jain:
>
> Yes, lots of reading to do :) . I had a look at the book and it seems
> better than the documentation too, I don't know, maybe the writing
> style? Love it, Greg. Lastly just out of curiosity, Bryan, if this
> can only be built as a built-in object then how come "As for your
> patch, I built the driver using:
>
> $ make CCFLAGS=-Werror W=1 M=drivers/staging/android"
>
> got you the errors that I desired? Aren't you building as a module here?
>

That's a good question Krish!

The command I sent you does not build built-in modules or create the
output I sent you!

As Greg said, ASHMEM cannot be built as a loadable module!

It is odd, when retracing my steps just now, I must have known at some
point that ASHMEM was a built-in module as menuconfig will not let you
select the <M> or loadable module option. The only thing I can think I
did was build the module without using the M= option, copied the error
message, then retyped the wrong 'make' command I had used to produce it.

What is *very* embarrassing is I had multiple opportunities to catch my
mistake. Somewhere between building your patch and writing the email I
truly lost the critical piece "this is a built-in module".

Even the CCFLAGS command Greg talked about is not a great command to be
using! I should not have sent you CCFLAGS or the less worse KCFLAGS that
I should not be using. Both really have no need here in drivers/staging/
and only add to the confusion.

I will say, for your next patch that I am eagerly waiting for, the W=1
option is a good way to catch subtle errors that Greg may ask you to fix
and resend. I got one thing right :/

I don't know, I truly lost my marbles on this one.

I apologize again for my goof, it must have been very frustrating.
~Bryan