* [PATCH] Staging: lustre: interval_tree: Convert inline function to macro @ 2015-10-31 18:45 Shivani Bhardwaj 2015-10-31 18:53 ` [Outreachy kernel] " Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Shivani Bhardwaj @ 2015-10-31 18:45 UTC (permalink / raw) To: outreachy-kernel Convert the function max_u64() to macro because macros tend to be more flexible than inline functions and here, the macro is going to be type safe so there are not going to be any side effects. Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> --- drivers/staging/lustre/lustre/ldlm/interval_tree.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/staging/lustre/lustre/ldlm/interval_tree.c b/drivers/staging/lustre/lustre/ldlm/interval_tree.c index a2ea8e5..52d0afd 100644 --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c @@ -42,6 +42,8 @@ #include "../include/obd_support.h" #include "../include/interval_tree.h" +#define max_u64(x, y) ((x > y) ? x : y) + enum { INTERVAL_RED = 0, INTERVAL_BLACK = 1 @@ -96,11 +98,6 @@ static inline int extent_equal(struct interval_node_extent *e1, return (e1->start == e2->start) && (e1->end == e2->end); } -static inline __u64 max_u64(__u64 x, __u64 y) -{ - return x > y ? x : y; -} - static struct interval_node *interval_first(struct interval_node *node) { if (!node) -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 18:45 [PATCH] Staging: lustre: interval_tree: Convert inline function to macro Shivani Bhardwaj @ 2015-10-31 18:53 ` Greg KH 2015-10-31 18:54 ` Shivani Bhardwaj 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2015-10-31 18:53 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: outreachy-kernel On Sun, Nov 01, 2015 at 12:15:44AM +0530, Shivani Bhardwaj wrote: > Convert the function max_u64() to macro because macros tend to be more > flexible than inline functions and here, the macro is going to be type > safe so there are not going to be any side effects. > > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > --- > drivers/staging/lustre/lustre/ldlm/interval_tree.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/ldlm/interval_tree.c b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > index a2ea8e5..52d0afd 100644 > --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > @@ -42,6 +42,8 @@ > #include "../include/obd_support.h" > #include "../include/interval_tree.h" > > +#define max_u64(x, y) ((x > y) ? x : y) Ick, no, there is already a _correct_ macro in the kernel for this, don't try to create another one that does not work properly for all users. And your patch is also backwards, inline functions are better than macros, don't ever convert from an inline function to a macro. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 18:53 ` [Outreachy kernel] " Greg KH @ 2015-10-31 18:54 ` Shivani Bhardwaj 2015-10-31 19:09 ` Julia Lawall 0 siblings, 1 reply; 9+ messages in thread From: Shivani Bhardwaj @ 2015-10-31 18:54 UTC (permalink / raw) To: Greg KH; +Cc: outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 1333 bytes --] On Sun, Nov 1, 2015 at 12:23 AM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Sun, Nov 01, 2015 at 12:15:44AM +0530, Shivani Bhardwaj wrote: > > Convert the function max_u64() to macro because macros tend to be more > > flexible than inline functions and here, the macro is going to be type > > safe so there are not going to be any side effects. > > > > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > > --- > > drivers/staging/lustre/lustre/ldlm/interval_tree.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > index a2ea8e5..52d0afd 100644 > > --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > @@ -42,6 +42,8 @@ > > #include "../include/obd_support.h" > > #include "../include/interval_tree.h" > > > > +#define max_u64(x, y) ((x > y) ? x : y) > > Ick, no, there is already a _correct_ macro in the kernel for this, > don't try to create another one that does not work properly for all > users. > > And your patch is also backwards, inline functions are better than > macros, don't ever convert from an inline function to a macro. > > Alright. Thanks for explaining. > thanks, > > greg k-h > [-- Attachment #2: Type: text/html, Size: 2025 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 18:54 ` Shivani Bhardwaj @ 2015-10-31 19:09 ` Julia Lawall 2015-10-31 19:12 ` Shivani Bhardwaj 0 siblings, 1 reply; 9+ messages in thread From: Julia Lawall @ 2015-10-31 19:09 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: Greg KH, outreachy-kernel [-- Attachment #1: Type: TEXT/PLAIN, Size: 1847 bytes --] On Sun, 1 Nov 2015, Shivani Bhardwaj wrote: > > > On Sun, Nov 1, 2015 at 12:23 AM, Greg KH <gregkh@linuxfoundation.org> wrote: > On Sun, Nov 01, 2015 at 12:15:44AM +0530, Shivani Bhardwaj > wrote: > > Convert the function max_u64() to macro because macros tend to > be more > > flexible than inline functions and here, the macro is going to > be type > > safe so there are not going to be any side effects. > > > > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > > --- > > drivers/staging/lustre/lustre/ldlm/interval_tree.c | 7 > ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git > a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > index a2ea8e5..52d0afd 100644 > > --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > @@ -42,6 +42,8 @@ > > #include "../include/obd_support.h" > > #include "../include/interval_tree.h" > > > > +#define max_u64(x, y) ((x > y) ? x : y) > > Ick, no, there is already a _correct_ macro in the kernel for > this, > don't try to create another one that does not work properly for > all > users. > > And your patch is also backwards, inline functions are better > than > macros, don't ever convert from an inline function to a macro. > > > Alright. Thanks for explaining. Inline functons have types, which help you find bugs. Inline functions don't risk duplicating computations. In your macro, if eg x is the larger value, then whatever x is replaced by will be evaluated twice. Consider what would happen if x is eg i++. julia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 19:09 ` Julia Lawall @ 2015-10-31 19:12 ` Shivani Bhardwaj 2015-10-31 19:15 ` Julia Lawall 0 siblings, 1 reply; 9+ messages in thread From: Shivani Bhardwaj @ 2015-10-31 19:12 UTC (permalink / raw) To: Julia Lawall; +Cc: Greg KH, outreachy-kernel [-- Attachment #1: Type: text/plain, Size: 2290 bytes --] On Sun, Nov 1, 2015 at 12:39 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Sun, 1 Nov 2015, Shivani Bhardwaj wrote: > > > > > > > On Sun, Nov 1, 2015 at 12:23 AM, Greg KH <gregkh@linuxfoundation.org> > wrote: > > On Sun, Nov 01, 2015 at 12:15:44AM +0530, Shivani Bhardwaj > > wrote: > > > Convert the function max_u64() to macro because macros tend to > > be more > > > flexible than inline functions and here, the macro is going to > > be type > > > safe so there are not going to be any side effects. > > > > > > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> > > > --- > > > drivers/staging/lustre/lustre/ldlm/interval_tree.c | 7 > > ++----- > > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > > > diff --git > > a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > > index a2ea8e5..52d0afd 100644 > > > --- a/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > > +++ b/drivers/staging/lustre/lustre/ldlm/interval_tree.c > > > @@ -42,6 +42,8 @@ > > > #include "../include/obd_support.h" > > > #include "../include/interval_tree.h" > > > > > > +#define max_u64(x, y) ((x > y) ? x : y) > > > > Ick, no, there is already a _correct_ macro in the kernel for > > this, > > don't try to create another one that does not work properly for > > all > > users. > > > > And your patch is also backwards, inline functions are better > > than > > macros, don't ever convert from an inline function to a macro. > > > > > > Alright. Thanks for explaining. > > Inline functons have types, which help you find bugs. > > Inline functions don't risk duplicating computations. In your macro, if > eg x is the larger value, then whatever x is replaced by will be evaluated > twice. Consider what would happen if x is eg i++. > > Yes, Julia. I took into consideration this case and I checked out the entire code where the function was called, there was no situation like this. Plus, I had read that inline functions are compiler dependent so I thought of making the change. Thank you for explaining. :) julia [-- Attachment #2: Type: text/html, Size: 3510 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 19:12 ` Shivani Bhardwaj @ 2015-10-31 19:15 ` Julia Lawall 2015-10-31 19:27 ` Shivani Bhardwaj 0 siblings, 1 reply; 9+ messages in thread From: Julia Lawall @ 2015-10-31 19:15 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: Greg KH, outreachy-kernel [-- Attachment #1: Type: TEXT/PLAIN, Size: 1024 bytes --] > Inline functons have types, which help you find bugs. > > Inline functions don't risk duplicating computations. In your macro, > if > eg x is the larger value, then whatever x is replaced by will be > evaluated > twice. Consider what would happen if x is eg i++. > > Yes, Julia. I took into consideration this case and I checked out the > entire code where the function was called, there was no situation like this. > Plus, I had read that inline functions are compiler dependent so I thought > of making the change. > Thank you for explaining. :) OK. Still, someone else could extend the code in some way and want to use what looks like a function, and use is in a bad way. Anyway, the best is always to find a standard kernel function that does the same thing. There are lots of them for these kind of numerical things. Also, you should set up your mailer so that it marks what you are replying to in some way. In the above, one can't see the difference between what I wrote and what you wrote. julia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 19:15 ` Julia Lawall @ 2015-10-31 19:27 ` Shivani Bhardwaj 2015-10-31 19:36 ` Julia Lawall 0 siblings, 1 reply; 9+ messages in thread From: Shivani Bhardwaj @ 2015-10-31 19:27 UTC (permalink / raw) To: Julia Lawall; +Cc: Greg KH, outreachy-kernel On Sun, Nov 1, 2015 at 12:45 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: >> Inline functons have types, which help you find bugs. >> >> Inline functions don't risk duplicating computations. In your macro, >> if >> eg x is the larger value, then whatever x is replaced by will be >> evaluated >> twice. Consider what would happen if x is eg i++. >> >> Yes, Julia. I took into consideration this case and I checked out the >> entire code where the function was called, there was no situation like this. >> Plus, I had read that inline functions are compiler dependent so I thought >> of making the change. >> Thank you for explaining. :) > > OK. Still, someone else could extend the code in some way and want to use > what looks like a function, and use is in a bad way. > > Anyway, the best is always to find a standard kernel function that does > the same thing. There are lots of them for these kind of numerical > things. > > Also, you should set up your mailer so that it marks what you are replying > to in some way. In the above, one can't see the difference between what I > wrote and what you wrote. > > julia Is it OK now? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 19:27 ` Shivani Bhardwaj @ 2015-10-31 19:36 ` Julia Lawall 2015-10-31 19:38 ` Shivani Bhardwaj 0 siblings, 1 reply; 9+ messages in thread From: Julia Lawall @ 2015-10-31 19:36 UTC (permalink / raw) To: Shivani Bhardwaj; +Cc: Greg KH, outreachy-kernel On Sun, 1 Nov 2015, Shivani Bhardwaj wrote: > On Sun, Nov 1, 2015 at 12:45 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: > >> Inline functons have types, which help you find bugs. > >> > >> Inline functions don't risk duplicating computations. In your macro, > >> if > >> eg x is the larger value, then whatever x is replaced by will be > >> evaluated > >> twice. Consider what would happen if x is eg i++. > >> > >> Yes, Julia. I took into consideration this case and I checked out the > >> entire code where the function was called, there was no situation like this. > >> Plus, I had read that inline functions are compiler dependent so I thought > >> of making the change. > >> Thank you for explaining. :) > > > > OK. Still, someone else could extend the code in some way and want to use > > what looks like a function, and use is in a bad way. > > > > Anyway, the best is always to find a standard kernel function that does > > the same thing. There are lots of them for these kind of numerical > > things. > > > > Also, you should set up your mailer so that it marks what you are replying > > to in some way. In the above, one can't see the difference between what I > > wrote and what you wrote. > > > > julia > > Is it OK now? Perfect. Thanks! julia ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH] Staging: lustre: interval_tree: Convert inline function to macro 2015-10-31 19:36 ` Julia Lawall @ 2015-10-31 19:38 ` Shivani Bhardwaj 0 siblings, 0 replies; 9+ messages in thread From: Shivani Bhardwaj @ 2015-10-31 19:38 UTC (permalink / raw) To: Julia Lawall; +Cc: Greg KH, outreachy-kernel On Sun, Nov 1, 2015 at 1:06 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Sun, 1 Nov 2015, Shivani Bhardwaj wrote: > >> On Sun, Nov 1, 2015 at 12:45 AM, Julia Lawall <julia.lawall@lip6.fr> wrote: >> >> Inline functons have types, which help you find bugs. >> >> >> >> Inline functions don't risk duplicating computations. In your macro, >> >> if >> >> eg x is the larger value, then whatever x is replaced by will be >> >> evaluated >> >> twice. Consider what would happen if x is eg i++. >> >> >> >> Yes, Julia. I took into consideration this case and I checked out the >> >> entire code where the function was called, there was no situation like this. >> >> Plus, I had read that inline functions are compiler dependent so I thought >> >> of making the change. >> >> Thank you for explaining. :) >> > >> > OK. Still, someone else could extend the code in some way and want to use >> > what looks like a function, and use is in a bad way. >> > >> > Anyway, the best is always to find a standard kernel function that does >> > the same thing. There are lots of them for these kind of numerical >> > things. >> > >> > Also, you should set up your mailer so that it marks what you are replying >> > to in some way. In the above, one can't see the difference between what I >> > wrote and what you wrote. >> > >> > julia >> >> Is it OK now? > > Perfect. Thanks! > > julia Thank you ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-31 19:38 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-31 18:45 [PATCH] Staging: lustre: interval_tree: Convert inline function to macro Shivani Bhardwaj 2015-10-31 18:53 ` [Outreachy kernel] " Greg KH 2015-10-31 18:54 ` Shivani Bhardwaj 2015-10-31 19:09 ` Julia Lawall 2015-10-31 19:12 ` Shivani Bhardwaj 2015-10-31 19:15 ` Julia Lawall 2015-10-31 19:27 ` Shivani Bhardwaj 2015-10-31 19:36 ` Julia Lawall 2015-10-31 19:38 ` Shivani Bhardwaj
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.