LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH] Declare the file_operations struct as const
       [not found] ` <3634721.RBzQ2xsved@localhost.localdomain>
@ 2021-08-27  7:48   ` Fabio M. De Francesco
  2021-08-27  8:50     ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Fabio M. De Francesco @ 2021-08-27  7:48 UTC (permalink / raw)
  To: linux-staging, Krish Jain, linux-kernel

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 <krishjain02939@gmail.com>
> > 
> > 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 <krishjain02939@gmail.com>
> > ---
> > []
> 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 linux-kernel@vger.kernel.org.

Have a nice time with kernel hacking.

Thanks,

Fabio



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-27  7:48   ` [PATCH] Declare the file_operations struct as const Fabio M. De Francesco
@ 2021-08-27  8:50     ` Krish Jain
  2021-08-27 18:38       ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-27  8:50 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: linux-kernel

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
<fmdefrancesco@gmail.com> 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 <krishjain02939@gmail.com>
> > >
> > > 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 <krishjain02939@gmail.com>
> > > ---
> > > []
> > 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 linux-kernel@vger.kernel.org.
>
> Have a nice time with kernel hacking.
>
> Thanks,
>
> Fabio
>
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-27  8:50     ` Krish Jain
@ 2021-08-27 18:38       ` Krish Jain
  2021-08-27 19:46         ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-27 18:38 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: linux-kernel

So what do you think I can do?

Best Regards


On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <krishjain02939@gmail.com> 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
> <fmdefrancesco@gmail.com> 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 <krishjain02939@gmail.com>
> > > >
> > > > 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 <krishjain02939@gmail.com>
> > > > ---
> > > > []
> > > 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 linux-kernel@vger.kernel.org.
> >
> > Have a nice time with kernel hacking.
> >
> > Thanks,
> >
> > Fabio
> >
> >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-27 18:38       ` Krish Jain
@ 2021-08-27 19:46         ` Krish Jain
  2021-08-27 23:38           ` Bryan Brattlof
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-27 19:46 UTC (permalink / raw)
  To: Fabio M. De Francesco, gregkh; +Cc: linux-staging, linux-kernel

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 <krishjain02939@gmail.com> wrote:
>
> So what do you think I can do?
>
> Best Regards
>
>
> On Fri, Aug 27, 2021 at 10:50 AM Krish Jain <krishjain02939@gmail.com> 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
> > <fmdefrancesco@gmail.com> 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 <krishjain02939@gmail.com>
> > > > >
> > > > > 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 <krishjain02939@gmail.com>
> > > > > ---
> > > > > []
> > > > 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 linux-kernel@vger.kernel.org.
> > >
> > > Have a nice time with kernel hacking.
> > >
> > > Thanks,
> > >
> > > Fabio
> > >
> > >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-27 19:46         ` Krish Jain
@ 2021-08-27 23:38           ` Bryan Brattlof
  2021-08-28  9:37             ` Krish Jain
  2021-08-29  2:13             ` Krish Jain
  0 siblings, 2 replies; 35+ messages in thread
From: Bryan Brattlof @ 2021-08-27 23:38 UTC (permalink / raw)
  To: Krish Jain; +Cc: Fabio M. De Francesco, gregkh, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> 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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-27 23:38           ` Bryan Brattlof
@ 2021-08-28  9:37             ` Krish Jain
  2021-08-28  9:46               ` Greg KH
  2021-08-29  2:13             ` Krish Jain
  1 sibling, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-28  9:37 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: linux-staging, linux-kernel

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 <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> 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
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-28  9:37             ` Krish Jain
@ 2021-08-28  9:46               ` Greg KH
  2021-08-28  9:52                 ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Greg KH @ 2021-08-28  9:46 UTC (permalink / raw)
  To: Krish Jain; +Cc: Bryan Brattlof, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-28  9:46               ` Greg KH
@ 2021-08-28  9:52                 ` Krish Jain
  2021-08-28 11:13                   ` Fabio M. De Francesco
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-28  9:52 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-staging, linux-kernel

On Sat, Aug 28, 2021 at 11:46 AM Greg KH <gregkh@linuxfoundation.org> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-28  9:52                 ` Krish Jain
@ 2021-08-28 11:13                   ` Fabio M. De Francesco
  0 siblings, 0 replies; 35+ messages in thread
From: Fabio M. De Francesco @ 2021-08-28 11:13 UTC (permalink / raw)
  To: Greg KH, Krish Jain; +Cc: linux-staging, linux-kernel

On Saturday, August 28, 2021 11:52:39 AM CEST Krish Jain wrote:
> On Sat, Aug 28, 2021 at 11:46 AM Greg KH <gregkh@linuxfoundation.org> 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
> 
> 





^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-27 23:38           ` Bryan Brattlof
  2021-08-28  9:37             ` Krish Jain
@ 2021-08-29  2:13             ` Krish Jain
  2021-08-29  2:16               ` Krish Jain
  2021-08-29  6:16               ` Greg KH
  1 sibling, 2 replies; 35+ messages in thread
From: Krish Jain @ 2021-08-29  2:13 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: linux-kernel, linux-staging

On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> 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
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29  2:13             ` Krish Jain
@ 2021-08-29  2:16               ` Krish Jain
  2021-08-29  6:16               ` Greg KH
  1 sibling, 0 replies; 35+ messages in thread
From: Krish Jain @ 2021-08-29  2:16 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: linux-kernel, linux-staging

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 <krishjain02939@gmail.com> wrote:
>
> On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> 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
> >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29  2:13             ` Krish Jain
  2021-08-29  2:16               ` Krish Jain
@ 2021-08-29  6:16               ` Greg KH
  1 sibling, 0 replies; 35+ messages in thread
From: Greg KH @ 2021-08-29  6:16 UTC (permalink / raw)
  To: Krish Jain; +Cc: Bryan Brattlof, linux-kernel, linux-staging

On Sun, Aug 29, 2021 at 04:13:43AM +0200, Krish Jain wrote:
> On Sat, Aug 28, 2021 at 1:38 AM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-09-01 18:04                                     ` Krish Jain
@ 2021-09-01 20:29                                       ` Bryan Brattlof
  0 siblings, 0 replies; 35+ messages in thread
From: Bryan Brattlof @ 2021-09-01 20:29 UTC (permalink / raw)
  To: Krish Jain; +Cc: Greg KH, Fabio M. De Francesco, linux-staging, linux-kernel

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



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-09-01 17:34                                   ` Bryan Brattlof
@ 2021-09-01 18:04                                     ` Krish Jain
  2021-09-01 20:29                                       ` Bryan Brattlof
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-09-01 18:04 UTC (permalink / raw)
  To: Bryan Brattlof
  Cc: Greg KH, Fabio M. De Francesco, linux-staging, linux-kernel

On Wed, Sep 1, 2021 at 7:34 PM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-09-01 16:51                                 ` Greg KH
@ 2021-09-01 17:34                                   ` Bryan Brattlof
  2021-09-01 18:04                                     ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Bryan Brattlof @ 2021-09-01 17:34 UTC (permalink / raw)
  To: Krish Jain, Greg KH; +Cc: Fabio M. De Francesco, linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-09-01 15:34                               ` Krish Jain
@ 2021-09-01 16:51                                 ` Greg KH
  2021-09-01 17:34                                   ` Bryan Brattlof
  0 siblings, 1 reply; 35+ messages in thread
From: Greg KH @ 2021-09-01 16:51 UTC (permalink / raw)
  To: Krish Jain
  Cc: Bryan Brattlof, Fabio M. De Francesco, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-09-01 15:30                             ` Greg KH
@ 2021-09-01 15:34                               ` Krish Jain
  2021-09-01 16:51                                 ` Greg KH
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-09-01 15:34 UTC (permalink / raw)
  To: Greg KH
  Cc: Bryan Brattlof, Fabio M. De Francesco, linux-staging, linux-kernel

On Wed, Sep 1, 2021 at 5:30 PM Greg KH <gregkh@linuxfoundation.org> 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 <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-09-01 15:20                           ` Krish Jain
@ 2021-09-01 15:30                             ` Greg KH
  2021-09-01 15:34                               ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Greg KH @ 2021-09-01 15:30 UTC (permalink / raw)
  To: Krish Jain
  Cc: Bryan Brattlof, Fabio M. De Francesco, linux-staging, linux-kernel

On Wed, Sep 01, 2021 at 05:20:13PM +0200, Krish Jain wrote:
> On Wed, Sep 1, 2021 at 1:00 AM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-31 23:00                         ` Bryan Brattlof
@ 2021-09-01 15:20                           ` Krish Jain
  2021-09-01 15:30                             ` Greg KH
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-09-01 15:20 UTC (permalink / raw)
  To: Bryan Brattlof
  Cc: Fabio M. De Francesco, Greg KH, linux-staging, linux-kernel

On Wed, Sep 1, 2021 at 1:00 AM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-31 14:00                       ` Fabio M. De Francesco
@ 2021-08-31 23:00                         ` Bryan Brattlof
  2021-09-01 15:20                           ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Bryan Brattlof @ 2021-08-31 23:00 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: Krish Jain, Greg KH, linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-31 13:35                     ` Bryan Brattlof
@ 2021-08-31 14:00                       ` Fabio M. De Francesco
  2021-08-31 23:00                         ` Bryan Brattlof
  0 siblings, 1 reply; 35+ messages in thread
From: Fabio M. De Francesco @ 2021-08-31 14:00 UTC (permalink / raw)
  To: Krish Jain, Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-31  0:42                   ` Krish Jain
@ 2021-08-31 13:35                     ` Bryan Brattlof
  2021-08-31 14:00                       ` Fabio M. De Francesco
  0 siblings, 1 reply; 35+ messages in thread
From: Bryan Brattlof @ 2021-08-31 13:35 UTC (permalink / raw)
  To: Krish Jain; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> 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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-30 13:01                 ` Krish Jain
@ 2021-08-31  0:42                   ` Krish Jain
  2021-08-31 13:35                     ` Bryan Brattlof
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-31  0:42 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> wrote:
> > >
> > > On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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
> > > >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-30 12:40               ` Krish Jain
@ 2021-08-30 13:01                 ` Krish Jain
  2021-08-31  0:42                   ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-30 13:01 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> wrote:
> >
> > On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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
> > >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 22:11             ` Krish Jain
@ 2021-08-30 12:40               ` Krish Jain
  2021-08-30 13:01                 ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-30 12:40 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> wrote:
>
> On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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
> >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 21:00           ` Bryan Brattlof
@ 2021-08-29 22:11             ` Krish Jain
  2021-08-30 12:40               ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-29 22:11 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

On Sun, Aug 29, 2021 at 11:00 PM Bryan Brattlof <hello@bryanbrattlof.com> 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 <krishjain02939@gmail.com> 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
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 18:46         ` Krish Jain
@ 2021-08-29 21:00           ` Bryan Brattlof
  2021-08-29 22:11             ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Bryan Brattlof @ 2021-08-29 21:00 UTC (permalink / raw)
  To: Krish Jain; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> 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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 18:28       ` Krish Jain
@ 2021-08-29 18:46         ` Krish Jain
  2021-08-29 21:00           ` Bryan Brattlof
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-29 18:46 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> 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 <krishjain02939@gmail.com> wrote:
> >
> > On Sun, Aug 29, 2021 at 6:49 PM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 16:56     ` Krish Jain
@ 2021-08-29 18:28       ` Krish Jain
  2021-08-29 18:46         ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-29 18:28 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> wrote:
>
> On Sun, Aug 29, 2021 at 6:49 PM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 16:49   ` Bryan Brattlof
@ 2021-08-29 16:56     ` Krish Jain
  2021-08-29 18:28       ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Krish Jain @ 2021-08-29 16:56 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

On Sun, Aug 29, 2021 at 6:49 PM Bryan Brattlof <hello@bryanbrattlof.com> 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 16:20 ` Krish Jain
  2021-08-29 16:34   ` Krish Jain
@ 2021-08-29 16:49   ` Bryan Brattlof
  2021-08-29 16:56     ` Krish Jain
  1 sibling, 1 reply; 35+ messages in thread
From: Bryan Brattlof @ 2021-08-29 16:49 UTC (permalink / raw)
  To: Krish Jain; +Cc: Greg KH, linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 16:20 ` Krish Jain
@ 2021-08-29 16:34   ` Krish Jain
  2021-08-29 16:49   ` Bryan Brattlof
  1 sibling, 0 replies; 35+ messages in thread
From: Krish Jain @ 2021-08-29 16:34 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

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 <krishjain02939@gmail.com> wrote:
>
> On Sun, Aug 29, 2021 at 4:45 PM Bryan Brattlof <hello@bryanbrattlof.com> wrote:
> >
> > Hi Krish
> >
> > On this day, August 29, 2021, thus sayeth Krish Jain:
> > > On Sun, Aug 29, 2021 at 8:17 AM Greg KH <gregkh@linuxfoundation.org> 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 <hello@bryanbrattlof.com> 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/Pine.LNX.4.58.0412201646220.4112@ppc970.osdl.org/
> >
> > The old "grey-beards" here don't bite, no matter how much they and
> > everyone else say they do.
> >

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
  2021-08-29 14:45 Bryan Brattlof
@ 2021-08-29 16:20 ` Krish Jain
  2021-08-29 16:34   ` Krish Jain
  2021-08-29 16:49   ` Bryan Brattlof
  0 siblings, 2 replies; 35+ messages in thread
From: Krish Jain @ 2021-08-29 16:20 UTC (permalink / raw)
  To: Bryan Brattlof; +Cc: Greg KH, linux-staging, linux-kernel

On Sun, Aug 29, 2021 at 4:45 PM Bryan Brattlof <hello@bryanbrattlof.com> wrote:
>
> Hi Krish
>
> On this day, August 29, 2021, thus sayeth Krish Jain:
> > On Sun, Aug 29, 2021 at 8:17 AM Greg KH <gregkh@linuxfoundation.org> 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 <hello@bryanbrattlof.com> 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/Pine.LNX.4.58.0412201646220.4112@ppc970.osdl.org/
>
> The old "grey-beards" here don't bite, no matter how much they and
> everyone else say they do.
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] Declare the file_operations struct as const
@ 2021-08-29 14:45 Bryan Brattlof
  2021-08-29 16:20 ` Krish Jain
  0 siblings, 1 reply; 35+ messages in thread
From: Bryan Brattlof @ 2021-08-29 14:45 UTC (permalink / raw)
  To: Krish Jain; +Cc: Greg KH, linux-staging, linux-kernel

Hi Krish

On this day, August 29, 2021, thus sayeth Krish Jain:
> On Sun, Aug 29, 2021 at 8:17 AM Greg KH <gregkh@linuxfoundation.org> 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 <hello@bryanbrattlof.com> 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/Pine.LNX.4.58.0412201646220.4112@ppc970.osdl.org/

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


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH] Declare the file_operations struct as const
@ 2021-08-27  2:19 Krish Jain
  0 siblings, 0 replies; 35+ messages in thread
From: Krish Jain @ 2021-08-27  2:19 UTC (permalink / raw)
  To: linux-kernel

 From: Krish Jain <krishjain02939@gmail.com>

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 <krishjain02939@gmail.com>
---
 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

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2021-09-01 20:29 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAPGkw+x+B1731SL=afoSKiWwH-n-FA5YJ+WOwYdv8iyFhWk1zA@mail.gmail.com>
     [not found] ` <3634721.RBzQ2xsved@localhost.localdomain>
2021-08-27  7:48   ` [PATCH] Declare the file_operations struct as const Fabio M. De Francesco
2021-08-27  8:50     ` Krish Jain
2021-08-27 18:38       ` Krish Jain
2021-08-27 19:46         ` Krish Jain
2021-08-27 23:38           ` Bryan Brattlof
2021-08-28  9:37             ` Krish Jain
2021-08-28  9:46               ` Greg KH
2021-08-28  9:52                 ` Krish Jain
2021-08-28 11:13                   ` Fabio M. De Francesco
2021-08-29  2:13             ` Krish Jain
2021-08-29  2:16               ` Krish Jain
2021-08-29  6:16               ` Greg KH
2021-08-29 14:45 Bryan Brattlof
2021-08-29 16:20 ` Krish Jain
2021-08-29 16:34   ` Krish Jain
2021-08-29 16:49   ` Bryan Brattlof
2021-08-29 16:56     ` Krish Jain
2021-08-29 18:28       ` Krish Jain
2021-08-29 18:46         ` Krish Jain
2021-08-29 21:00           ` Bryan Brattlof
2021-08-29 22:11             ` Krish Jain
2021-08-30 12:40               ` Krish Jain
2021-08-30 13:01                 ` Krish Jain
2021-08-31  0:42                   ` Krish Jain
2021-08-31 13:35                     ` Bryan Brattlof
2021-08-31 14:00                       ` Fabio M. De Francesco
2021-08-31 23:00                         ` Bryan Brattlof
2021-09-01 15:20                           ` Krish Jain
2021-09-01 15:30                             ` Greg KH
2021-09-01 15:34                               ` Krish Jain
2021-09-01 16:51                                 ` Greg KH
2021-09-01 17:34                                   ` Bryan Brattlof
2021-09-01 18:04                                     ` Krish Jain
2021-09-01 20:29                                       ` Bryan Brattlof
  -- strict thread matches above, loose matches on Subject: below --
2021-08-27  2:19 Krish Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).