* [PATCH v3 0/3] staging: lustre: include: replace macros by functions
@ 2015-04-02 15:16 Aya Mahfouz
2015-04-02 15:18 ` [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev Aya Mahfouz
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Aya Mahfouz @ 2015-04-02 15:16 UTC (permalink / raw)
To: outreachy-kernel
Replace two macros with static inline functions that return values.
Inline functions are preferred because compilers are aware of them and
can optimize the calls. In addition, if macros are treated like
functions they can introduce bugs into the code.
Some minor fixes were done in patches 1 and 2 of the series. That
is why the patchset is resent.
Aya Mahfouz (3):
staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev
staging: lustre: include: replace OBD_CHECK_DEV_ACTIVE by
obd_check_dev_active
staging: lustre: include: remove unused macros
drivers/staging/lustre/lustre/include/obd_class.h | 76 +++++++++++++++--------
1 file changed, 50 insertions(+), 26 deletions(-)
--
2.1.0
--
Kind Regards,
Aya Saif El-yazal Mahfouz
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev
2015-04-02 15:16 [PATCH v3 0/3] staging: lustre: include: replace macros by functions Aya Mahfouz
@ 2015-04-02 15:18 ` Aya Mahfouz
2015-04-02 16:49 ` [Outreachy kernel] " Greg KH
2015-04-02 15:20 ` [PATCH v3 2/3] staging: lustre: include: replace OBD_CHECK_DEV_ACTIVE by obd_check_dev_active Aya Mahfouz
2015-04-02 15:21 ` [PATCH v3 3/3] staging: lustre: include: remove unused macros Aya Mahfouz
2 siblings, 1 reply; 6+ messages in thread
From: Aya Mahfouz @ 2015-04-02 15:18 UTC (permalink / raw)
To: outreachy-kernel
Static inline functions are preferred over macros. Hence, the function
obd_check_dev was introduced. obd_check_dev replaces the macro
OBD_CHECK_DEV. All functions that call obd_check_dev store the return
values and return them if they represent an error code.
Some of the changes were carried out manually and others were
done using coccinelle.
Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
v2: removed unneeded parenthesis around the variable obd.
v3: v2 was sent as v1. Fixed that mistake.
drivers/staging/lustre/lustre/include/obd_class.h | 33 ++++++++++++++++++-----
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index 5314f6f..76f55d3 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -341,6 +341,15 @@ do { \
} \
} while (0)
+static inline int obd_check_dev(struct obd_device *obd)
+{
+ if (!obd) {
+ CERROR("NULL device\n");
+ return -ENODEV;
+ }
+ return 0;
+}
+
/* ensure obd_setup and !obd_stopping */
#define OBD_CHECK_DEV_ACTIVE(obd) \
do { \
@@ -594,7 +603,9 @@ static inline int obd_precleanup(struct obd_device *obd,
int rc;
DECLARE_LU_VARS(ldt, d);
- OBD_CHECK_DEV(obd);
+ rc = obd_check_dev(obd);
+ if (rc)
+ return rc;
ldt = obd->obd_type->typ_lu;
d = obd->obd_lu_dev;
if (ldt != NULL && d != NULL) {
@@ -620,7 +631,9 @@ static inline int obd_cleanup(struct obd_device *obd)
int rc;
DECLARE_LU_VARS(ldt, d);
- OBD_CHECK_DEV(obd);
+ rc = obd_check_dev(obd);
+ if (rc)
+ return rc;
ldt = obd->obd_type->typ_lu;
d = obd->obd_lu_dev;
@@ -668,7 +681,9 @@ obd_process_config(struct obd_device *obd, int datalen, void *data)
int rc;
DECLARE_LU_VARS(ldt, d);
- OBD_CHECK_DEV(obd);
+ rc = obd_check_dev(obd);
+ if (rc)
+ return rc;
obd->obd_process_conf = 1;
ldt = obd->obd_type->typ_lu;
@@ -1280,7 +1295,9 @@ static inline int obd_notify(struct obd_device *obd,
{
int rc;
- OBD_CHECK_DEV(obd);
+ rc = obd_check_dev(obd);
+ if (rc)
+ return rc;
/* the check for async_recov is a complete hack - I'm hereby
overloading the meaning to also mean "this was called from
@@ -1381,7 +1398,11 @@ static inline int obd_health_check(const struct lu_env *env,
static inline int obd_register_observer(struct obd_device *obd,
struct obd_device *observer)
{
- OBD_CHECK_DEV(obd);
+ int rc;
+
+ rc = obd_check_dev(obd);
+ if (rc)
+ return rc;
down_write(&obd->obd_observer_link_sem);
if (obd->obd_observer && observer) {
up_write(&obd->obd_observer_link_sem);
@@ -1389,7 +1410,7 @@ static inline int obd_register_observer(struct obd_device *obd,
}
obd->obd_observer = observer;
up_write(&obd->obd_observer_link_sem);
- return 0;
+ return rc;
}
#if 0
--
2.1.0
--
Kind Regards,
Aya Saif El-yazal Mahfouz
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] staging: lustre: include: replace OBD_CHECK_DEV_ACTIVE by obd_check_dev_active
2015-04-02 15:16 [PATCH v3 0/3] staging: lustre: include: replace macros by functions Aya Mahfouz
2015-04-02 15:18 ` [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev Aya Mahfouz
@ 2015-04-02 15:20 ` Aya Mahfouz
2015-04-02 15:21 ` [PATCH v3 3/3] staging: lustre: include: remove unused macros Aya Mahfouz
2 siblings, 0 replies; 6+ messages in thread
From: Aya Mahfouz @ 2015-04-02 15:20 UTC (permalink / raw)
To: outreachy-kernel
Static inline functions are preferred over macros. The inline function
obd_check_dev_active is introduced to replace OBD_CHECK_DEV_ACTIVE.
All functions that call obd_check_dev_active store the return values
and return them if they represent an error code.
Some of the changes were carried out manually while others were done
using coccinelle.
Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
v2: removed unneeded parenthesis around the variable obd.
v3: v2 was sent as v1. Fixed that mistake.
drivers/staging/lustre/lustre/include/obd_class.h | 29 +++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index 76f55d3..a6dd147 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -361,6 +361,19 @@ do { \
} \
} while (0)
+static inline int obd_check_dev_active(struct obd_device *obd)
+{
+ int rc;
+
+ rc = obd_check_dev(obd);
+ if (rc)
+ return rc;
+ if (!obd->obd_set_up || obd->obd_stopping) {
+ CERROR("Device %d not setup\n", obd->obd_minor);
+ return -ENODEV;
+ }
+ return rc;
+}
#if defined (CONFIG_PROC_FS)
#define OBD_COUNTER_OFFSET(op) \
@@ -901,7 +914,9 @@ static inline int obd_add_conn(struct obd_import *imp, struct obd_uuid *uuid,
struct obd_device *obd = imp->imp_obd;
int rc;
- OBD_CHECK_DEV_ACTIVE(obd);
+ rc = obd_check_dev_active(obd);
+ if (rc)
+ return rc;
OBD_CHECK_DT_OP(obd, add_conn, -EOPNOTSUPP);
OBD_COUNTER_INCREMENT(obd, add_conn);
@@ -914,7 +929,9 @@ static inline int obd_del_conn(struct obd_import *imp, struct obd_uuid *uuid)
struct obd_device *obd = imp->imp_obd;
int rc;
- OBD_CHECK_DEV_ACTIVE(obd);
+ rc = obd_check_dev_active(obd);
+ if (rc)
+ return rc;
OBD_CHECK_DT_OP(obd, del_conn, -EOPNOTSUPP);
OBD_COUNTER_INCREMENT(obd, del_conn);
@@ -948,7 +965,9 @@ static inline int obd_connect(const struct lu_env *env,
__u64 ocf = data ? data->ocd_connect_flags : 0; /* for post-condition
* check */
- OBD_CHECK_DEV_ACTIVE(obd);
+ rc = obd_check_dev_active(obd);
+ if (rc)
+ return rc;
OBD_CHECK_DT_OP(obd, connect, -EOPNOTSUPP);
OBD_COUNTER_INCREMENT(obd, connect);
@@ -970,7 +989,9 @@ static inline int obd_reconnect(const struct lu_env *env,
__u64 ocf = d ? d->ocd_connect_flags : 0; /* for post-condition
* check */
- OBD_CHECK_DEV_ACTIVE(obd);
+ rc = obd_check_dev_active(obd);
+ if (rc)
+ return rc;
OBD_CHECK_DT_OP(obd, reconnect, 0);
OBD_COUNTER_INCREMENT(obd, reconnect);
--
2.1.0
--
Kind Regards,
Aya Saif El-yazal Mahfouz
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] staging: lustre: include: remove unused macros
2015-04-02 15:16 [PATCH v3 0/3] staging: lustre: include: replace macros by functions Aya Mahfouz
2015-04-02 15:18 ` [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev Aya Mahfouz
2015-04-02 15:20 ` [PATCH v3 2/3] staging: lustre: include: replace OBD_CHECK_DEV_ACTIVE by obd_check_dev_active Aya Mahfouz
@ 2015-04-02 15:21 ` Aya Mahfouz
2 siblings, 0 replies; 6+ messages in thread
From: Aya Mahfouz @ 2015-04-02 15:21 UTC (permalink / raw)
To: outreachy-kernel
OBD_CHECK_DEV and OBD_CHECK_DEV_ACTIVE have been replaced by static
inline functions. They are removed since they are not used anymore.
Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
v2: No changes. The patch is part of a patchset, so it was resent with
them.
v3: same as v2.
drivers/staging/lustre/lustre/include/obd_class.h | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
index a6dd147..b61e64d 100644
--- a/drivers/staging/lustre/lustre/include/obd_class.h
+++ b/drivers/staging/lustre/lustre/include/obd_class.h
@@ -333,14 +333,6 @@ void obdo_le_to_cpu(struct obdo *dobdo, struct obdo *sobdo);
/* Ensure obd_setup: used for cleanup which must be called
while obd is stopping */
-#define OBD_CHECK_DEV(obd) \
-do { \
- if (!(obd)) { \
- CERROR("NULL device\n"); \
- return -ENODEV; \
- } \
-} while (0)
-
static inline int obd_check_dev(struct obd_device *obd)
{
if (!obd) {
@@ -351,16 +343,6 @@ static inline int obd_check_dev(struct obd_device *obd)
}
/* ensure obd_setup and !obd_stopping */
-#define OBD_CHECK_DEV_ACTIVE(obd) \
-do { \
- OBD_CHECK_DEV(obd); \
- if (!(obd)->obd_set_up || (obd)->obd_stopping) { \
- CERROR("Device %d not setup\n", \
- (obd)->obd_minor); \
- return -ENODEV; \
- } \
-} while (0)
-
static inline int obd_check_dev_active(struct obd_device *obd)
{
int rc;
--
2.1.0
--
Kind Regards,
Aya Saif El-yazal Mahfouz
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev
2015-04-02 15:18 ` [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev Aya Mahfouz
@ 2015-04-02 16:49 ` Greg KH
2015-04-02 18:09 ` Aya Mahfouz
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2015-04-02 16:49 UTC (permalink / raw)
To: Aya Mahfouz; +Cc: outreachy-kernel
On Thu, Apr 02, 2015 at 05:18:10PM +0200, Aya Mahfouz wrote:
> Static inline functions are preferred over macros. Hence, the function
> obd_check_dev was introduced. obd_check_dev replaces the macro
> OBD_CHECK_DEV. All functions that call obd_check_dev store the return
> values and return them if they represent an error code.
>
> Some of the changes were carried out manually and others were
> done using coccinelle.
>
> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> ---
> v2: removed unneeded parenthesis around the variable obd.
> v3: v2 was sent as v1. Fixed that mistake.
>
> drivers/staging/lustre/lustre/include/obd_class.h | 33 ++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
> index 5314f6f..76f55d3 100644
> --- a/drivers/staging/lustre/lustre/include/obd_class.h
> +++ b/drivers/staging/lustre/lustre/include/obd_class.h
> @@ -341,6 +341,15 @@ do { \
> } \
> } while (0)
>
> +static inline int obd_check_dev(struct obd_device *obd)
> +{
> + if (!obd) {
> + CERROR("NULL device\n");
> + return -ENODEV;
> + }
> + return 0;
> +}
> +
> /* ensure obd_setup and !obd_stopping */
> #define OBD_CHECK_DEV_ACTIVE(obd) \
> do { \
> @@ -594,7 +603,9 @@ static inline int obd_precleanup(struct obd_device *obd,
> int rc;
> DECLARE_LU_VARS(ldt, d);
>
> - OBD_CHECK_DEV(obd);
> + rc = obd_check_dev(obd);
> + if (rc)
> + return rc;
> ldt = obd->obd_type->typ_lu;
> d = obd->obd_lu_dev;
> if (ldt != NULL && d != NULL) {
> @@ -620,7 +631,9 @@ static inline int obd_cleanup(struct obd_device *obd)
> int rc;
> DECLARE_LU_VARS(ldt, d);
>
> - OBD_CHECK_DEV(obd);
> + rc = obd_check_dev(obd);
> + if (rc)
> + return rc;
>
> ldt = obd->obd_type->typ_lu;
> d = obd->obd_lu_dev;
> @@ -668,7 +681,9 @@ obd_process_config(struct obd_device *obd, int datalen, void *data)
> int rc;
> DECLARE_LU_VARS(ldt, d);
>
> - OBD_CHECK_DEV(obd);
> + rc = obd_check_dev(obd);
> + if (rc)
> + return rc;
>
> obd->obd_process_conf = 1;
> ldt = obd->obd_type->typ_lu;
> @@ -1280,7 +1295,9 @@ static inline int obd_notify(struct obd_device *obd,
> {
> int rc;
>
> - OBD_CHECK_DEV(obd);
> + rc = obd_check_dev(obd);
> + if (rc)
> + return rc;
>
> /* the check for async_recov is a complete hack - I'm hereby
> overloading the meaning to also mean "this was called from
> @@ -1381,7 +1398,11 @@ static inline int obd_health_check(const struct lu_env *env,
> static inline int obd_register_observer(struct obd_device *obd,
> struct obd_device *observer)
> {
> - OBD_CHECK_DEV(obd);
> + int rc;
> +
> + rc = obd_check_dev(obd);
> + if (rc)
> + return rc;
> down_write(&obd->obd_observer_link_sem);
> if (obd->obd_observer && observer) {
> up_write(&obd->obd_observer_link_sem);
> @@ -1389,7 +1410,7 @@ static inline int obd_register_observer(struct obd_device *obd,
> }
> obd->obd_observer = observer;
> up_write(&obd->obd_observer_link_sem);
> - return 0;
> + return rc;
Again, as I stated before, this change is not needed.
sorry,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev
2015-04-02 16:49 ` [Outreachy kernel] " Greg KH
@ 2015-04-02 18:09 ` Aya Mahfouz
0 siblings, 0 replies; 6+ messages in thread
From: Aya Mahfouz @ 2015-04-02 18:09 UTC (permalink / raw)
To: Greg KH; +Cc: outreachy-kernel
On Thu, Apr 02, 2015 at 06:49:26PM +0200, Greg KH wrote:
> On Thu, Apr 02, 2015 at 05:18:10PM +0200, Aya Mahfouz wrote:
> > Static inline functions are preferred over macros. Hence, the function
> > obd_check_dev was introduced. obd_check_dev replaces the macro
> > OBD_CHECK_DEV. All functions that call obd_check_dev store the return
> > values and return them if they represent an error code.
> >
> > Some of the changes were carried out manually and others were
> > done using coccinelle.
> >
> > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > ---
> > v2: removed unneeded parenthesis around the variable obd.
> > v3: v2 was sent as v1. Fixed that mistake.
> >
> > drivers/staging/lustre/lustre/include/obd_class.h | 33 ++++++++++++++++++-----
> > 1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/include/obd_class.h b/drivers/staging/lustre/lustre/include/obd_class.h
> > index 5314f6f..76f55d3 100644
> > --- a/drivers/staging/lustre/lustre/include/obd_class.h
> > +++ b/drivers/staging/lustre/lustre/include/obd_class.h
> > @@ -341,6 +341,15 @@ do { \
> > } \
> > } while (0)
> >
> > +static inline int obd_check_dev(struct obd_device *obd)
> > +{
> > + if (!obd) {
> > + CERROR("NULL device\n");
> > + return -ENODEV;
> > + }
> > + return 0;
> > +}
> > +
> > /* ensure obd_setup and !obd_stopping */
> > #define OBD_CHECK_DEV_ACTIVE(obd) \
> > do { \
> > @@ -594,7 +603,9 @@ static inline int obd_precleanup(struct obd_device *obd,
> > int rc;
> > DECLARE_LU_VARS(ldt, d);
> >
> > - OBD_CHECK_DEV(obd);
> > + rc = obd_check_dev(obd);
> > + if (rc)
> > + return rc;
> > ldt = obd->obd_type->typ_lu;
> > d = obd->obd_lu_dev;
> > if (ldt != NULL && d != NULL) {
> > @@ -620,7 +631,9 @@ static inline int obd_cleanup(struct obd_device *obd)
> > int rc;
> > DECLARE_LU_VARS(ldt, d);
> >
> > - OBD_CHECK_DEV(obd);
> > + rc = obd_check_dev(obd);
> > + if (rc)
> > + return rc;
> >
> > ldt = obd->obd_type->typ_lu;
> > d = obd->obd_lu_dev;
> > @@ -668,7 +681,9 @@ obd_process_config(struct obd_device *obd, int datalen, void *data)
> > int rc;
> > DECLARE_LU_VARS(ldt, d);
> >
> > - OBD_CHECK_DEV(obd);
> > + rc = obd_check_dev(obd);
> > + if (rc)
> > + return rc;
> >
> > obd->obd_process_conf = 1;
> > ldt = obd->obd_type->typ_lu;
> > @@ -1280,7 +1295,9 @@ static inline int obd_notify(struct obd_device *obd,
> > {
> > int rc;
> >
> > - OBD_CHECK_DEV(obd);
> > + rc = obd_check_dev(obd);
> > + if (rc)
> > + return rc;
> >
> > /* the check for async_recov is a complete hack - I'm hereby
> > overloading the meaning to also mean "this was called from
> > @@ -1381,7 +1398,11 @@ static inline int obd_health_check(const struct lu_env *env,
> > static inline int obd_register_observer(struct obd_device *obd,
> > struct obd_device *observer)
> > {
> > - OBD_CHECK_DEV(obd);
> > + int rc;
> > +
> > + rc = obd_check_dev(obd);
> > + if (rc)
> > + return rc;
> > down_write(&obd->obd_observer_link_sem);
> > if (obd->obd_observer && observer) {
> > up_write(&obd->obd_observer_link_sem);
> > @@ -1389,7 +1410,7 @@ static inline int obd_register_observer(struct obd_device *obd,
> > }
> > obd->obd_observer = observer;
> > up_write(&obd->obd_observer_link_sem);
> > - return 0;
> > + return rc;
>
> Again, as I stated before, this change is not needed.
>
> sorry,
>
No, problem. Done in the fourth version.
> greg k-h
--
Kind Regards,
Aya Saif El-yazal Mahfouz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-02 18:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-02 15:16 [PATCH v3 0/3] staging: lustre: include: replace macros by functions Aya Mahfouz
2015-04-02 15:18 ` [PATCH v3 1/3] staging: lustre: include: replace OBD_CHECK_DEV by obd_check_dev Aya Mahfouz
2015-04-02 16:49 ` [Outreachy kernel] " Greg KH
2015-04-02 18:09 ` Aya Mahfouz
2015-04-02 15:20 ` [PATCH v3 2/3] staging: lustre: include: replace OBD_CHECK_DEV_ACTIVE by obd_check_dev_active Aya Mahfouz
2015-04-02 15:21 ` [PATCH v3 3/3] staging: lustre: include: remove unused macros Aya Mahfouz
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.