* [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer @ 2019-11-26 17:27 Amol Grover 2019-11-27 2:29 ` Joel Fernandes 0 siblings, 1 reply; 6+ messages in thread From: Amol Grover @ 2019-11-26 17:27 UTC (permalink / raw) To: Paul Moore, Eric Paris Cc: Joel Fernandes, linux-audit, linux-kernel-mentees, linux-kernel add __rcu notation to RCU protected global pointer auditd_conn Fixes multiple instances of sparse error: error: incompatible types in comparison expression (different address spaces) Signed-off-by: Amol Grover <frextrite@gmail.com> --- kernel/audit.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index da8dc0db5bd3..30e7fc9b8da2 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -102,12 +102,14 @@ struct audit_net { * This struct is RCU protected; you must either hold the RCU lock for reading * or the associated spinlock for writing. */ -static struct auditd_connection { +struct auditd_connection { struct pid *pid; u32 portid; struct net *net; struct rcu_head rcu; -} *auditd_conn = NULL; +}; +static struct auditd_connection __rcu *auditd_conn; +RCU_INIT_POINTER(auditd_conn); static DEFINE_SPINLOCK(auditd_conn_lock); /* If audit_rate_limit is non-zero, limit the rate of sending audit records -- 2.24.0 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer 2019-11-26 17:27 [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer Amol Grover @ 2019-11-27 2:29 ` Joel Fernandes 2019-11-27 5:28 ` Amol Grover 0 siblings, 1 reply; 6+ messages in thread From: Joel Fernandes @ 2019-11-27 2:29 UTC (permalink / raw) To: Amol Grover Cc: Paul Moore, linux-kernel, Eric Paris, linux-audit, linux-kernel-mentees On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote: > add __rcu notation to RCU protected global pointer auditd_conn > > Fixes multiple instances of sparse error: > error: incompatible types in comparison expression > (different address spaces) > > Signed-off-by: Amol Grover <frextrite@gmail.com> > --- > kernel/audit.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/kernel/audit.c b/kernel/audit.c > index da8dc0db5bd3..30e7fc9b8da2 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -102,12 +102,14 @@ struct audit_net { > * This struct is RCU protected; you must either hold the RCU lock for reading > * or the associated spinlock for writing. > */ > -static struct auditd_connection { > +struct auditd_connection { > struct pid *pid; > u32 portid; > struct net *net; > struct rcu_head rcu; > -} *auditd_conn = NULL; > +}; > +static struct auditd_connection __rcu *auditd_conn; > +RCU_INIT_POINTER(auditd_conn); Looks like this causes a build error. Always please build test your patches in the very least. And I also did not understand how RCU_INIT_POINTER can even be used outside of a function. In C, executable code cannot be outside functions. Is doing the following not sufficient to fix the sparse issue? thanks, - Joel ---8<----------------------- diff --git a/kernel/audit.c b/kernel/audit.c index 49b6049b26ac..c5d4b5a2dea1 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -108,8 +108,8 @@ struct auditd_connection { struct net *net; struct rcu_head rcu; }; -static struct auditd_connection __rcu *auditd_conn; -RCU_INIT_POINTER(auditd_conn); +static struct auditd_connection __rcu *auditd_conn = NULL; + static DEFINE_SPINLOCK(auditd_conn_lock); /* If audit_rate_limit is non-zero, limit the rate of sending audit records _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer 2019-11-27 2:29 ` Joel Fernandes @ 2019-11-27 5:28 ` Amol Grover 2019-11-27 15:25 ` Joel Fernandes 0 siblings, 1 reply; 6+ messages in thread From: Amol Grover @ 2019-11-27 5:28 UTC (permalink / raw) To: Joel Fernandes Cc: Paul Moore, linux-kernel, Eric Paris, linux-audit, linux-kernel-mentees On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote: > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote: > > add __rcu notation to RCU protected global pointer auditd_conn > > > > Fixes multiple instances of sparse error: > > error: incompatible types in comparison expression > > (different address spaces) > > > > Signed-off-by: Amol Grover <frextrite@gmail.com> > > --- > > kernel/audit.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index da8dc0db5bd3..30e7fc9b8da2 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -102,12 +102,14 @@ struct audit_net { > > * This struct is RCU protected; you must either hold the RCU lock for reading > > * or the associated spinlock for writing. > > */ > > -static struct auditd_connection { > > +struct auditd_connection { > > struct pid *pid; > > u32 portid; > > struct net *net; > > struct rcu_head rcu; > > -} *auditd_conn = NULL; > > +}; > > +static struct auditd_connection __rcu *auditd_conn; > > +RCU_INIT_POINTER(auditd_conn); > > Looks like this causes a build error. Always please build test your patches > in the very least. And I also did not understand how RCU_INIT_POINTER can > even be used outside of a function. In C, executable code cannot be outside > functions. > > Is doing the following not sufficient to fix the sparse issue? > > thanks, > > - Joel > > ---8<----------------------- > > diff --git a/kernel/audit.c b/kernel/audit.c > index 49b6049b26ac..c5d4b5a2dea1 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -108,8 +108,8 @@ struct auditd_connection { > struct net *net; > struct rcu_head rcu; > }; > -static struct auditd_connection __rcu *auditd_conn; > -RCU_INIT_POINTER(auditd_conn); > +static struct auditd_connection __rcu *auditd_conn = NULL; I ran a quick checkpatch and it gave me this error: ERROR: do not initialise statics to NULL So in order to fix it I decided to INIT the pointer (and failed) Should I consider this as a false positive? Thanks Amol > + > static DEFINE_SPINLOCK(auditd_conn_lock); > > /* If audit_rate_limit is non-zero, limit the rate of sending audit records _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer 2019-11-27 5:28 ` Amol Grover @ 2019-11-27 15:25 ` Joel Fernandes 2019-11-28 15:09 ` Amol Grover 0 siblings, 1 reply; 6+ messages in thread From: Joel Fernandes @ 2019-11-27 15:25 UTC (permalink / raw) To: Amol Grover Cc: Paul Moore, LKML, Eric Paris, linux-audit, linux-kernel-mentees On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <frextrite@gmail.com> wrote: > > On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote: > > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote: > > > add __rcu notation to RCU protected global pointer auditd_conn > > > > > > Fixes multiple instances of sparse error: > > > error: incompatible types in comparison expression > > > (different address spaces) > > > > > > Signed-off-by: Amol Grover <frextrite@gmail.com> > > > --- > > > kernel/audit.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index da8dc0db5bd3..30e7fc9b8da2 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -102,12 +102,14 @@ struct audit_net { > > > * This struct is RCU protected; you must either hold the RCU lock for reading > > > * or the associated spinlock for writing. > > > */ > > > -static struct auditd_connection { > > > +struct auditd_connection { > > > struct pid *pid; > > > u32 portid; > > > struct net *net; > > > struct rcu_head rcu; > > > -} *auditd_conn = NULL; > > > +}; > > > +static struct auditd_connection __rcu *auditd_conn; > > > +RCU_INIT_POINTER(auditd_conn); > > > > Looks like this causes a build error. Always please build test your patches > > in the very least. And I also did not understand how RCU_INIT_POINTER can > > even be used outside of a function. In C, executable code cannot be outside > > functions. > > > > Is doing the following not sufficient to fix the sparse issue? > > > > thanks, > > > > - Joel > > > > ---8<----------------------- > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index 49b6049b26ac..c5d4b5a2dea1 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -108,8 +108,8 @@ struct auditd_connection { > > struct net *net; > > struct rcu_head rcu; > > }; > > -static struct auditd_connection __rcu *auditd_conn; > > -RCU_INIT_POINTER(auditd_conn); > > +static struct auditd_connection __rcu *auditd_conn = NULL; > > I ran a quick checkpatch and it gave me this error: > ERROR: do not initialise statics to NULL > > So in order to fix it I decided to INIT the pointer (and failed) Well, try to understand the checkpatch error then, and do the right thing :) - Joel _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer 2019-11-27 15:25 ` Joel Fernandes @ 2019-11-28 15:09 ` Amol Grover 2019-11-29 20:08 ` Joel Fernandes 0 siblings, 1 reply; 6+ messages in thread From: Amol Grover @ 2019-11-28 15:09 UTC (permalink / raw) To: Joel Fernandes Cc: Paul Moore, LKML, Eric Paris, linux-audit, linux-kernel-mentees On Wed, Nov 27, 2019 at 10:25:57AM -0500, Joel Fernandes wrote: > On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <frextrite@gmail.com> wrote: > > > > On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote: > > > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote: > > > > add __rcu notation to RCU protected global pointer auditd_conn > > > > > > > > Fixes multiple instances of sparse error: > > > > error: incompatible types in comparison expression > > > > (different address spaces) > > > > > > > > Signed-off-by: Amol Grover <frextrite@gmail.com> > > > > --- > > > > kernel/audit.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > > index da8dc0db5bd3..30e7fc9b8da2 100644 > > > > --- a/kernel/audit.c > > > > +++ b/kernel/audit.c > > > > @@ -102,12 +102,14 @@ struct audit_net { > > > > * This struct is RCU protected; you must either hold the RCU lock for reading > > > > * or the associated spinlock for writing. > > > > */ > > > > -static struct auditd_connection { > > > > +struct auditd_connection { > > > > struct pid *pid; > > > > u32 portid; > > > > struct net *net; > > > > struct rcu_head rcu; > > > > -} *auditd_conn = NULL; > > > > +}; > > > > +static struct auditd_connection __rcu *auditd_conn; > > > > +RCU_INIT_POINTER(auditd_conn); > > > > > > Looks like this causes a build error. Always please build test your patches > > > in the very least. And I also did not understand how RCU_INIT_POINTER can > > > even be used outside of a function. In C, executable code cannot be outside > > > functions. > > > > > > Is doing the following not sufficient to fix the sparse issue? > > > > > > thanks, > > > > > > - Joel > > > > > > ---8<----------------------- > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index 49b6049b26ac..c5d4b5a2dea1 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -108,8 +108,8 @@ struct auditd_connection { > > > struct net *net; > > > struct rcu_head rcu; > > > }; > > > -static struct auditd_connection __rcu *auditd_conn; > > > -RCU_INIT_POINTER(auditd_conn); > > > +static struct auditd_connection __rcu *auditd_conn = NULL; > > > > I ran a quick checkpatch and it gave me this error: > > ERROR: do not initialise statics to NULL > > > > So in order to fix it I decided to INIT the pointer (and failed) > > Well, try to understand the checkpatch error then, and do the right thing :) I read-up a few articles about this. I think I've understood the problem. I'm sending in the ammended patch now. Sorry for this. Won't happen again. Thanks Amol > > - Joel _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer 2019-11-28 15:09 ` Amol Grover @ 2019-11-29 20:08 ` Joel Fernandes 0 siblings, 0 replies; 6+ messages in thread From: Joel Fernandes @ 2019-11-29 20:08 UTC (permalink / raw) To: Amol Grover Cc: Paul Moore, LKML, Eric Paris, linux-audit, linux-kernel-mentees On Thu, Nov 28, 2019 at 08:39:49PM +0530, Amol Grover wrote: > On Wed, Nov 27, 2019 at 10:25:57AM -0500, Joel Fernandes wrote: > > On Wed, Nov 27, 2019 at 12:29 AM Amol Grover <frextrite@gmail.com> wrote: > > > > > > On Tue, Nov 26, 2019 at 09:29:25PM -0500, Joel Fernandes wrote: > > > > On Tue, Nov 26, 2019 at 10:57:23PM +0530, Amol Grover wrote: > > > > > add __rcu notation to RCU protected global pointer auditd_conn > > > > > > > > > > Fixes multiple instances of sparse error: > > > > > error: incompatible types in comparison expression > > > > > (different address spaces) > > > > > > > > > > Signed-off-by: Amol Grover <frextrite@gmail.com> > > > > > --- > > > > > kernel/audit.c | 6 ++++-- > > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > > > index da8dc0db5bd3..30e7fc9b8da2 100644 > > > > > --- a/kernel/audit.c > > > > > +++ b/kernel/audit.c > > > > > @@ -102,12 +102,14 @@ struct audit_net { > > > > > * This struct is RCU protected; you must either hold the RCU lock for reading > > > > > * or the associated spinlock for writing. > > > > > */ > > > > > -static struct auditd_connection { > > > > > +struct auditd_connection { > > > > > struct pid *pid; > > > > > u32 portid; > > > > > struct net *net; > > > > > struct rcu_head rcu; > > > > > -} *auditd_conn = NULL; > > > > > +}; > > > > > +static struct auditd_connection __rcu *auditd_conn; > > > > > +RCU_INIT_POINTER(auditd_conn); > > > > > > > > Looks like this causes a build error. Always please build test your patches > > > > in the very least. And I also did not understand how RCU_INIT_POINTER can > > > > even be used outside of a function. In C, executable code cannot be outside > > > > functions. > > > > > > > > Is doing the following not sufficient to fix the sparse issue? > > > > > > > > thanks, > > > > > > > > - Joel > > > > > > > > ---8<----------------------- > > > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > > index 49b6049b26ac..c5d4b5a2dea1 100644 > > > > --- a/kernel/audit.c > > > > +++ b/kernel/audit.c > > > > @@ -108,8 +108,8 @@ struct auditd_connection { > > > > struct net *net; > > > > struct rcu_head rcu; > > > > }; > > > > -static struct auditd_connection __rcu *auditd_conn; > > > > -RCU_INIT_POINTER(auditd_conn); > > > > +static struct auditd_connection __rcu *auditd_conn = NULL; > > > > > > I ran a quick checkpatch and it gave me this error: > > > ERROR: do not initialise statics to NULL > > > > > > So in order to fix it I decided to INIT the pointer (and failed) > > > > Well, try to understand the checkpatch error then, and do the right thing :) > > I read-up a few articles about this. I think I've understood the problem. I'm > sending in the ammended patch now. Sorry for this. Won't happen again. Oh, no need apology. You are learning. But yeah always do a thorough job before sending patches. thanks, - Joel _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-29 20:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-26 17:27 [Linux-kernel-mentees] [PATCH] kernel: audit.c: Add __rcu notation to RCU pointer Amol Grover 2019-11-27 2:29 ` Joel Fernandes 2019-11-27 5:28 ` Amol Grover 2019-11-27 15:25 ` Joel Fernandes 2019-11-28 15:09 ` Amol Grover 2019-11-29 20:08 ` Joel Fernandes
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).