[10/21] lustre: obdclass: use cl_object_for_each where appropriate
diff mbox series

Message ID 154949781310.10620.8883358407999258821.stgit@noble.brown
State New
Headers show
Series
  • lustre: Assorted cleanups for obdclass
Related show

Commit Message

NeilBrown Feb. 7, 2019, 12:03 a.m. UTC
There are various places which have a list_for_each_entry()
where cl_object_for_each (or .._reverse) is more appropriate.

Several of these re-use the 'obj' function parameter as a loop
iterator, which is a little confusing.

Change these to use cl_object_for_each{_reverse}, and where needed,
introduce a new iterator variable 'o'.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/lov/lov_page.c       |    3 -
 drivers/staging/lustre/lustre/obdclass/cl_lock.c   |    3 -
 drivers/staging/lustre/lustre/obdclass/cl_object.c |   82 +++++++++-----------
 drivers/staging/lustre/lustre/obdclass/cl_page.c   |   11 +--
 4 files changed, 44 insertions(+), 55 deletions(-)

Comments

Andreas Dilger Feb. 8, 2019, 1:10 a.m. UTC | #1
On Feb 6, 2019, at 17:03, NeilBrown <neilb@suse.com> wrote:
> 
> There are various places which have a list_for_each_entry()
> where cl_object_for_each (or .._reverse) is more appropriate.
> 
> Several of these re-use the 'obj' function parameter as a loop
> iterator, which is a little confusing.
> 
> Change these to use cl_object_for_each{_reverse}, and where needed,
> introduce a new iterator variable 'o'.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
> drivers/staging/lustre/lustre/lov/lov_page.c       |    3 -
> drivers/staging/lustre/lustre/obdclass/cl_lock.c   |    3 -
> drivers/staging/lustre/lustre/obdclass/cl_object.c |   82 +++++++++-----------
> drivers/staging/lustre/lustre/obdclass/cl_page.c   |   11 +--
> 4 files changed, 44 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index f724b2d62df1..d71a680660da 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -190,16 +190,15 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
> int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
> 		       struct cl_attr *attr)
> {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
> 	int result;
> 
> 	assert_spin_locked(cl_object_attr_guard(obj));
> 
> -	top = obj->co_lu.lo_header;
> 	result = 0;

May as well move the "result = 0" initialization to the declaration for
all of these?  Another 5 fewer lines of code under your belt.

Cheers, Andreas
---
Andreas Dilger
Principal Lustre Architect
Whamcloud
NeilBrown Feb. 11, 2019, 12:42 a.m. UTC | #2
On Fri, Feb 08 2019, Andreas Dilger wrote:

> On Feb 6, 2019, at 17:03, NeilBrown <neilb@suse.com> wrote:
>> 
>> There are various places which have a list_for_each_entry()
>> where cl_object_for_each (or .._reverse) is more appropriate.
>> 
>> Several of these re-use the 'obj' function parameter as a loop
>> iterator, which is a little confusing.
>> 
>> Change these to use cl_object_for_each{_reverse}, and where needed,
>> introduce a new iterator variable 'o'.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> drivers/staging/lustre/lustre/lov/lov_page.c       |    3 -
>> drivers/staging/lustre/lustre/obdclass/cl_lock.c   |    3 -
>> drivers/staging/lustre/lustre/obdclass/cl_object.c |   82 +++++++++-----------
>> drivers/staging/lustre/lustre/obdclass/cl_page.c   |   11 +--
>> 4 files changed, 44 insertions(+), 55 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
>> index f724b2d62df1..d71a680660da 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
>> @@ -190,16 +190,15 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
>> int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
>> 		       struct cl_attr *attr)
>> {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>> 	int result;
>> 
>> 	assert_spin_locked(cl_object_attr_guard(obj));
>> 
>> -	top = obj->co_lu.lo_header;
>> 	result = 0;
>
> May as well move the "result = 0" initialization to the declaration for
> all of these?  Another 5 fewer lines of code under your belt.

Yes, that seem good. New patch below.
Thanks,
NeilBrown


From: NeilBrown <neilb@suse.com>
Subject: [PATCH] lustre: obdclass: use cl_object_for_each where appropriate

There are various places which have a list_for_each_entry()
where cl_object_for_each (or .._reverse) is more appropriate.

Several of these re-use the 'obj' function parameter as a loop
iterator, which is a little confusing.

Change these to use cl_object_for_each{_reverse}, and where needed,
introduce a new iterator variable 'o'.

To further clean up these function, move initializtion of 'result'
to the variable declaration.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/staging/lustre/lustre/lov/lov_page.c       |  3 +-
 drivers/staging/lustre/lustre/obdclass/cl_lock.c   |  3 +-
 drivers/staging/lustre/lustre/obdclass/cl_object.c | 97 ++++++++++------------
 drivers/staging/lustre/lustre/obdclass/cl_page.c   | 11 ++-
 4 files changed, 49 insertions(+), 65 deletions(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_page.c b/drivers/staging/lustre/lustre/lov/lov_page.c
index 08485a95ec01..e64b350601d2 100644
--- a/drivers/staging/lustre/lustre/lov/lov_page.c
+++ b/drivers/staging/lustre/lustre/lov/lov_page.c
@@ -103,8 +103,7 @@ int lov_page_init_composite(const struct lu_env *env, struct cl_object *obj,
 		return PTR_ERR(sub);
 
 	subobj = lovsub2cl(r0->lo_sub[stripe]);
-	list_for_each_entry(o, &subobj->co_lu.lo_header->loh_layers,
-			    co_lu.lo_linkage) {
+	cl_object_for_each(o, subobj) {
 		if (o->co_ops->coo_page_init) {
 			rc = o->co_ops->coo_page_init(sub->sub_env, o, page,
 						      cl_index(subobj, suboff));
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
index 8133d992cc73..fc5976d8b37b 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
@@ -103,8 +103,7 @@ int cl_lock_init(const struct lu_env *env, struct cl_lock *lock,
 	LASSERT(obj);
 
 	INIT_LIST_HEAD(&lock->cll_layers);
-	list_for_each_entry(scan, &obj->co_lu.lo_header->loh_layers,
-			    co_lu.lo_linkage) {
+	cl_object_for_each(scan, obj) {
 		result = scan->co_ops->coo_lock_init(env, scan, lock, io);
 		if (result != 0) {
 			cl_lock_fini(env, lock);
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index f724b2d62df1..43bf1a422635 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -190,16 +190,14 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
 int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
 		       struct cl_attr *attr)
 {
-	struct lu_object_header *top;
-	int result;
+	struct cl_object *o;
+	int result = 0;
 
 	assert_spin_locked(cl_object_attr_guard(obj));
 
-	top = obj->co_lu.lo_header;
-	result = 0;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_attr_get) {
-			result = obj->co_ops->coo_attr_get(env, obj, attr);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_attr_get) {
+			result = o->co_ops->coo_attr_get(env, o, attr);
 			if (result != 0) {
 				if (result > 0)
 					result = 0;
@@ -221,17 +219,15 @@ EXPORT_SYMBOL(cl_object_attr_get);
 int cl_object_attr_update(const struct lu_env *env, struct cl_object *obj,
 			  const struct cl_attr *attr, unsigned int v)
 {
-	struct lu_object_header *top;
-	int result;
+	struct cl_object *o;
+	int result = 0;
 
 	assert_spin_locked(cl_object_attr_guard(obj));
 
-	top = obj->co_lu.lo_header;
-	result = 0;
-	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_attr_update) {
-			result = obj->co_ops->coo_attr_update(env, obj, attr,
-							      v);
+	cl_object_for_each_reverse(o, obj) {
+		if (o->co_ops->coo_attr_update) {
+			result = o->co_ops->coo_attr_update(env, o, attr,
+							    v);
 			if (result != 0) {
 				if (result > 0)
 					result = 0;
@@ -254,19 +250,17 @@ EXPORT_SYMBOL(cl_object_attr_update);
 int cl_object_glimpse(const struct lu_env *env, struct cl_object *obj,
 		      struct ost_lvb *lvb)
 {
-	struct lu_object_header *top;
-	int result;
+	struct cl_object *o;
+	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	result = 0;
-	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_glimpse) {
-			result = obj->co_ops->coo_glimpse(env, obj, lvb);
+	cl_object_for_each_reverse(o, obj) {
+		if (o->co_ops->coo_glimpse) {
+			result = o->co_ops->coo_glimpse(env, o, lvb);
 			if (result != 0)
 				break;
 		}
 	}
-	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(top),
+	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(obj->co_lu.lo_header),
 			 "size: %llu mtime: %llu atime: %llu ctime: %llu blocks: %llu\n",
 			 lvb->lvb_size, lvb->lvb_mtime, lvb->lvb_atime,
 			 lvb->lvb_ctime, lvb->lvb_blocks);
@@ -280,14 +274,12 @@ EXPORT_SYMBOL(cl_object_glimpse);
 int cl_conf_set(const struct lu_env *env, struct cl_object *obj,
 		const struct cl_object_conf *conf)
 {
-	struct lu_object_header *top;
-	int result;
+	struct cl_object *o;
+	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	result = 0;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_conf_set) {
-			result = obj->co_ops->coo_conf_set(env, obj, conf);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_conf_set) {
+			result = o->co_ops->coo_conf_set(env, o, conf);
 			if (result != 0)
 				break;
 		}
@@ -301,13 +293,10 @@ EXPORT_SYMBOL(cl_conf_set);
  */
 int cl_object_prune(const struct lu_env *env, struct cl_object *obj)
 {
-	struct lu_object_header *top;
 	struct cl_object *o;
-	int result;
+	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	result = 0;
-	list_for_each_entry(o, &top->loh_layers, co_lu.lo_linkage) {
+	cl_object_for_each(o, obj) {
 		if (o->co_ops->coo_prune) {
 			result = o->co_ops->coo_prune(env, o);
 			if (result != 0)
@@ -325,14 +314,13 @@ EXPORT_SYMBOL(cl_object_prune);
 int cl_object_getstripe(const struct lu_env *env, struct cl_object *obj,
 			struct lov_user_md __user *uarg, size_t size)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_getstripe) {
-			result = obj->co_ops->coo_getstripe(env, obj, uarg,
-							    size);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_getstripe) {
+			result = o->co_ops->coo_getstripe(env, o, uarg,
+							  size);
 			if (result)
 				break;
 		}
@@ -357,14 +345,13 @@ int cl_object_fiemap(const struct lu_env *env, struct cl_object *obj,
 		     struct ll_fiemap_info_key *key,
 		     struct fiemap *fiemap, size_t *buflen)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_fiemap) {
-			result = obj->co_ops->coo_fiemap(env, obj, key, fiemap,
-							 buflen);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_fiemap) {
+			result = o->co_ops->coo_fiemap(env, o, key, fiemap,
+						       buflen);
 			if (result)
 				break;
 		}
@@ -376,11 +363,11 @@ EXPORT_SYMBOL(cl_object_fiemap);
 int cl_object_layout_get(const struct lu_env *env, struct cl_object *obj,
 			 struct cl_layout *cl)
 {
-	struct lu_object_header *top = obj->co_lu.lo_header;
+	struct cl_object *o;
 
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_layout_get)
-			return obj->co_ops->coo_layout_get(env, obj, cl);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_layout_get)
+			return o->co_ops->coo_layout_get(env, o, cl);
 	}
 
 	return -EOPNOTSUPP;
@@ -389,12 +376,12 @@ EXPORT_SYMBOL(cl_object_layout_get);
 
 loff_t cl_object_maxbytes(struct cl_object *obj)
 {
-	struct lu_object_header *top = obj->co_lu.lo_header;
+	struct cl_object *o;
 	loff_t maxbytes = LLONG_MAX;
 
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_maxbytes)
-			maxbytes = min_t(loff_t, obj->co_ops->coo_maxbytes(obj),
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_maxbytes)
+			maxbytes = min_t(loff_t, o->co_ops->coo_maxbytes(o),
 					 maxbytes);
 	}
 
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
index 057318deaa4e..7d00a9233a3b 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
@@ -132,7 +132,7 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
 			      enum cl_page_type type)
 {
 	struct cl_page *page;
-	struct lu_object_header *head;
+	struct cl_object *o2;
 
 	page = kzalloc(cl_object_header(o)->coh_page_bufsize, GFP_NOFS);
 	if (page) {
@@ -149,11 +149,10 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
 		INIT_LIST_HEAD(&page->cp_layers);
 		INIT_LIST_HEAD(&page->cp_batch);
 		lu_ref_init(&page->cp_reference);
-		head = o->co_lu.lo_header;
-		list_for_each_entry(o, &head->loh_layers, co_lu.lo_linkage) {
-			if (o->co_ops->coo_page_init) {
-				result = o->co_ops->coo_page_init(env, o, page,
-								  ind);
+		cl_object_for_each(o2, o) {
+			if (o2->co_ops->coo_page_init) {
+				result = o2->co_ops->coo_page_init(env, o2, page,
+								   ind);
 				if (result != 0) {
 					__cl_page_delete(env, page);
 					cl_page_free(env, page);
James Simmons Feb. 11, 2019, 1:57 a.m. UTC | #3
> There are various places which have a list_for_each_entry()
> where cl_object_for_each (or .._reverse) is more appropriate.
> 
> Several of these re-use the 'obj' function parameter as a loop
> iterator, which is a little confusing.
> 
> Change these to use cl_object_for_each{_reverse}, and where needed,
> introduce a new iterator variable 'o'.

Surprised the reverse wasn't done in that the cl_* macro was
removed :-) 

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/lov/lov_page.c       |    3 -
>  drivers/staging/lustre/lustre/obdclass/cl_lock.c   |    3 -
>  drivers/staging/lustre/lustre/obdclass/cl_object.c |   82 +++++++++-----------
>  drivers/staging/lustre/lustre/obdclass/cl_page.c   |   11 +--
>  4 files changed, 44 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/lov/lov_page.c b/drivers/staging/lustre/lustre/lov/lov_page.c
> index 08485a95ec01..e64b350601d2 100644
> --- a/drivers/staging/lustre/lustre/lov/lov_page.c
> +++ b/drivers/staging/lustre/lustre/lov/lov_page.c
> @@ -103,8 +103,7 @@ int lov_page_init_composite(const struct lu_env *env, struct cl_object *obj,
>  		return PTR_ERR(sub);
>  
>  	subobj = lovsub2cl(r0->lo_sub[stripe]);
> -	list_for_each_entry(o, &subobj->co_lu.lo_header->loh_layers,
> -			    co_lu.lo_linkage) {
> +	cl_object_for_each(o, subobj) {
>  		if (o->co_ops->coo_page_init) {
>  			rc = o->co_ops->coo_page_init(sub->sub_env, o, page,
>  						      cl_index(subobj, suboff));
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> index 8133d992cc73..fc5976d8b37b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> @@ -103,8 +103,7 @@ int cl_lock_init(const struct lu_env *env, struct cl_lock *lock,
>  	LASSERT(obj);
>  
>  	INIT_LIST_HEAD(&lock->cll_layers);
> -	list_for_each_entry(scan, &obj->co_lu.lo_header->loh_layers,
> -			    co_lu.lo_linkage) {
> +	cl_object_for_each(scan, obj) {
>  		result = scan->co_ops->coo_lock_init(env, scan, lock, io);
>  		if (result != 0) {
>  			cl_lock_fini(env, lock);
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index f724b2d62df1..d71a680660da 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -190,16 +190,15 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
>  int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
>  		       struct cl_attr *attr)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result;
>  
>  	assert_spin_locked(cl_object_attr_guard(obj));
>  
> -	top = obj->co_lu.lo_header;
>  	result = 0;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_attr_get) {
> -			result = obj->co_ops->coo_attr_get(env, obj, attr);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_attr_get) {
> +			result = o->co_ops->coo_attr_get(env, o, attr);
>  			if (result != 0) {
>  				if (result > 0)
>  					result = 0;
> @@ -221,17 +220,16 @@ EXPORT_SYMBOL(cl_object_attr_get);
>  int cl_object_attr_update(const struct lu_env *env, struct cl_object *obj,
>  			  const struct cl_attr *attr, unsigned int v)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result;
>  
>  	assert_spin_locked(cl_object_attr_guard(obj));
>  
> -	top = obj->co_lu.lo_header;
>  	result = 0;
> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_attr_update) {
> -			result = obj->co_ops->coo_attr_update(env, obj, attr,
> -							      v);
> +	cl_object_for_each_reverse(o, obj) {
> +		if (o->co_ops->coo_attr_update) {
> +			result = o->co_ops->coo_attr_update(env, o, attr,
> +							    v);
>  			if (result != 0) {
>  				if (result > 0)
>  					result = 0;
> @@ -254,19 +252,18 @@ EXPORT_SYMBOL(cl_object_attr_update);
>  int cl_object_glimpse(const struct lu_env *env, struct cl_object *obj,
>  		      struct ost_lvb *lvb)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result;
>  
> -	top = obj->co_lu.lo_header;
>  	result = 0;
> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_glimpse) {
> -			result = obj->co_ops->coo_glimpse(env, obj, lvb);
> +	cl_object_for_each_reverse(o, obj) {
> +		if (o->co_ops->coo_glimpse) {
> +			result = o->co_ops->coo_glimpse(env, o, lvb);
>  			if (result != 0)
>  				break;
>  		}
>  	}
> -	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(top),
> +	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(obj->co_lu.lo_header),
>  			 "size: %llu mtime: %llu atime: %llu ctime: %llu blocks: %llu\n",
>  			 lvb->lvb_size, lvb->lvb_mtime, lvb->lvb_atime,
>  			 lvb->lvb_ctime, lvb->lvb_blocks);
> @@ -280,14 +277,13 @@ EXPORT_SYMBOL(cl_object_glimpse);
>  int cl_conf_set(const struct lu_env *env, struct cl_object *obj,
>  		const struct cl_object_conf *conf)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result;
>  
> -	top = obj->co_lu.lo_header;
>  	result = 0;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_conf_set) {
> -			result = obj->co_ops->coo_conf_set(env, obj, conf);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_conf_set) {
> +			result = o->co_ops->coo_conf_set(env, o, conf);
>  			if (result != 0)
>  				break;
>  		}
> @@ -301,13 +297,11 @@ EXPORT_SYMBOL(cl_conf_set);
>   */
>  int cl_object_prune(const struct lu_env *env, struct cl_object *obj)
>  {
> -	struct lu_object_header *top;
>  	struct cl_object *o;
>  	int result;
>  
> -	top = obj->co_lu.lo_header;
>  	result = 0;
> -	list_for_each_entry(o, &top->loh_layers, co_lu.lo_linkage) {
> +	cl_object_for_each(o, obj) {
>  		if (o->co_ops->coo_prune) {
>  			result = o->co_ops->coo_prune(env, o);
>  			if (result != 0)
> @@ -325,14 +319,13 @@ EXPORT_SYMBOL(cl_object_prune);
>  int cl_object_getstripe(const struct lu_env *env, struct cl_object *obj,
>  			struct lov_user_md __user *uarg, size_t size)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_getstripe) {
> -			result = obj->co_ops->coo_getstripe(env, obj, uarg,
> -							    size);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_getstripe) {
> +			result = o->co_ops->coo_getstripe(env, o, uarg,
> +							  size);
>  			if (result)
>  				break;
>  		}
> @@ -357,14 +350,13 @@ int cl_object_fiemap(const struct lu_env *env, struct cl_object *obj,
>  		     struct ll_fiemap_info_key *key,
>  		     struct fiemap *fiemap, size_t *buflen)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_fiemap) {
> -			result = obj->co_ops->coo_fiemap(env, obj, key, fiemap,
> -							 buflen);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_fiemap) {
> +			result = o->co_ops->coo_fiemap(env, o, key, fiemap,
> +						       buflen);
>  			if (result)
>  				break;
>  		}
> @@ -376,11 +368,11 @@ EXPORT_SYMBOL(cl_object_fiemap);
>  int cl_object_layout_get(const struct lu_env *env, struct cl_object *obj,
>  			 struct cl_layout *cl)
>  {
> -	struct lu_object_header *top = obj->co_lu.lo_header;
> +	struct cl_object *o;
>  
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_layout_get)
> -			return obj->co_ops->coo_layout_get(env, obj, cl);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_layout_get)
> +			return o->co_ops->coo_layout_get(env, o, cl);
>  	}
>  
>  	return -EOPNOTSUPP;
> @@ -389,12 +381,12 @@ EXPORT_SYMBOL(cl_object_layout_get);
>  
>  loff_t cl_object_maxbytes(struct cl_object *obj)
>  {
> -	struct lu_object_header *top = obj->co_lu.lo_header;
> +	struct cl_object *o;
>  	loff_t maxbytes = LLONG_MAX;
>  
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_maxbytes)
> -			maxbytes = min_t(loff_t, obj->co_ops->coo_maxbytes(obj),
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_maxbytes)
> +			maxbytes = min_t(loff_t, o->co_ops->coo_maxbytes(o),
>  					 maxbytes);
>  	}
>  
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> index 057318deaa4e..7d00a9233a3b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> @@ -132,7 +132,7 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
>  			      enum cl_page_type type)
>  {
>  	struct cl_page *page;
> -	struct lu_object_header *head;
> +	struct cl_object *o2;
>  
>  	page = kzalloc(cl_object_header(o)->coh_page_bufsize, GFP_NOFS);
>  	if (page) {
> @@ -149,11 +149,10 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
>  		INIT_LIST_HEAD(&page->cp_layers);
>  		INIT_LIST_HEAD(&page->cp_batch);
>  		lu_ref_init(&page->cp_reference);
> -		head = o->co_lu.lo_header;
> -		list_for_each_entry(o, &head->loh_layers, co_lu.lo_linkage) {
> -			if (o->co_ops->coo_page_init) {
> -				result = o->co_ops->coo_page_init(env, o, page,
> -								  ind);
> +		cl_object_for_each(o2, o) {
> +			if (o2->co_ops->coo_page_init) {
> +				result = o2->co_ops->coo_page_init(env, o2, page,
> +								   ind);
>  				if (result != 0) {
>  					__cl_page_delete(env, page);
>  					cl_page_free(env, page);
> 
> 
>
NeilBrown Feb. 11, 2019, 3:19 a.m. UTC | #4
On Mon, Feb 11 2019, James Simmons wrote:

>> There are various places which have a list_for_each_entry()
>> where cl_object_for_each (or .._reverse) is more appropriate.
>> 
>> Several of these re-use the 'obj' function parameter as a loop
>> iterator, which is a little confusing.
>> 
>> Change these to use cl_object_for_each{_reverse}, and where needed,
>> introduce a new iterator variable 'o'.
>
> Surprised the reverse wasn't done in that the cl_* macro was
> removed :-) 

Hmmm.... maybe .....
But
   ->co_lu.lo_header->loh_layers

is sufficiently scary looking that hiding it in a macro seems like a
good idea.

>
> Reviewed-by: James Simmons <jsimmons@infradead.org>

Thanks,
NeilBrown


>  
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>>  drivers/staging/lustre/lustre/lov/lov_page.c       |    3 -
>>  drivers/staging/lustre/lustre/obdclass/cl_lock.c   |    3 -
>>  drivers/staging/lustre/lustre/obdclass/cl_object.c |   82 +++++++++-----------
>>  drivers/staging/lustre/lustre/obdclass/cl_page.c   |   11 +--
>>  4 files changed, 44 insertions(+), 55 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/lov/lov_page.c b/drivers/staging/lustre/lustre/lov/lov_page.c
>> index 08485a95ec01..e64b350601d2 100644
>> --- a/drivers/staging/lustre/lustre/lov/lov_page.c
>> +++ b/drivers/staging/lustre/lustre/lov/lov_page.c
>> @@ -103,8 +103,7 @@ int lov_page_init_composite(const struct lu_env *env, struct cl_object *obj,
>>  		return PTR_ERR(sub);
>>  
>>  	subobj = lovsub2cl(r0->lo_sub[stripe]);
>> -	list_for_each_entry(o, &subobj->co_lu.lo_header->loh_layers,
>> -			    co_lu.lo_linkage) {
>> +	cl_object_for_each(o, subobj) {
>>  		if (o->co_ops->coo_page_init) {
>>  			rc = o->co_ops->coo_page_init(sub->sub_env, o, page,
>>  						      cl_index(subobj, suboff));
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> index 8133d992cc73..fc5976d8b37b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
>> @@ -103,8 +103,7 @@ int cl_lock_init(const struct lu_env *env, struct cl_lock *lock,
>>  	LASSERT(obj);
>>  
>>  	INIT_LIST_HEAD(&lock->cll_layers);
>> -	list_for_each_entry(scan, &obj->co_lu.lo_header->loh_layers,
>> -			    co_lu.lo_linkage) {
>> +	cl_object_for_each(scan, obj) {
>>  		result = scan->co_ops->coo_lock_init(env, scan, lock, io);
>>  		if (result != 0) {
>>  			cl_lock_fini(env, lock);
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
>> index f724b2d62df1..d71a680660da 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
>> @@ -190,16 +190,15 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
>>  int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
>>  		       struct cl_attr *attr)
>>  {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>>  	int result;
>>  
>>  	assert_spin_locked(cl_object_attr_guard(obj));
>>  
>> -	top = obj->co_lu.lo_header;
>>  	result = 0;
>> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_attr_get) {
>> -			result = obj->co_ops->coo_attr_get(env, obj, attr);
>> +	cl_object_for_each(o, obj) {
>> +		if (o->co_ops->coo_attr_get) {
>> +			result = o->co_ops->coo_attr_get(env, o, attr);
>>  			if (result != 0) {
>>  				if (result > 0)
>>  					result = 0;
>> @@ -221,17 +220,16 @@ EXPORT_SYMBOL(cl_object_attr_get);
>>  int cl_object_attr_update(const struct lu_env *env, struct cl_object *obj,
>>  			  const struct cl_attr *attr, unsigned int v)
>>  {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>>  	int result;
>>  
>>  	assert_spin_locked(cl_object_attr_guard(obj));
>>  
>> -	top = obj->co_lu.lo_header;
>>  	result = 0;
>> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_attr_update) {
>> -			result = obj->co_ops->coo_attr_update(env, obj, attr,
>> -							      v);
>> +	cl_object_for_each_reverse(o, obj) {
>> +		if (o->co_ops->coo_attr_update) {
>> +			result = o->co_ops->coo_attr_update(env, o, attr,
>> +							    v);
>>  			if (result != 0) {
>>  				if (result > 0)
>>  					result = 0;
>> @@ -254,19 +252,18 @@ EXPORT_SYMBOL(cl_object_attr_update);
>>  int cl_object_glimpse(const struct lu_env *env, struct cl_object *obj,
>>  		      struct ost_lvb *lvb)
>>  {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>>  	int result;
>>  
>> -	top = obj->co_lu.lo_header;
>>  	result = 0;
>> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_glimpse) {
>> -			result = obj->co_ops->coo_glimpse(env, obj, lvb);
>> +	cl_object_for_each_reverse(o, obj) {
>> +		if (o->co_ops->coo_glimpse) {
>> +			result = o->co_ops->coo_glimpse(env, o, lvb);
>>  			if (result != 0)
>>  				break;
>>  		}
>>  	}
>> -	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(top),
>> +	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(obj->co_lu.lo_header),
>>  			 "size: %llu mtime: %llu atime: %llu ctime: %llu blocks: %llu\n",
>>  			 lvb->lvb_size, lvb->lvb_mtime, lvb->lvb_atime,
>>  			 lvb->lvb_ctime, lvb->lvb_blocks);
>> @@ -280,14 +277,13 @@ EXPORT_SYMBOL(cl_object_glimpse);
>>  int cl_conf_set(const struct lu_env *env, struct cl_object *obj,
>>  		const struct cl_object_conf *conf)
>>  {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>>  	int result;
>>  
>> -	top = obj->co_lu.lo_header;
>>  	result = 0;
>> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_conf_set) {
>> -			result = obj->co_ops->coo_conf_set(env, obj, conf);
>> +	cl_object_for_each(o, obj) {
>> +		if (o->co_ops->coo_conf_set) {
>> +			result = o->co_ops->coo_conf_set(env, o, conf);
>>  			if (result != 0)
>>  				break;
>>  		}
>> @@ -301,13 +297,11 @@ EXPORT_SYMBOL(cl_conf_set);
>>   */
>>  int cl_object_prune(const struct lu_env *env, struct cl_object *obj)
>>  {
>> -	struct lu_object_header *top;
>>  	struct cl_object *o;
>>  	int result;
>>  
>> -	top = obj->co_lu.lo_header;
>>  	result = 0;
>> -	list_for_each_entry(o, &top->loh_layers, co_lu.lo_linkage) {
>> +	cl_object_for_each(o, obj) {
>>  		if (o->co_ops->coo_prune) {
>>  			result = o->co_ops->coo_prune(env, o);
>>  			if (result != 0)
>> @@ -325,14 +319,13 @@ EXPORT_SYMBOL(cl_object_prune);
>>  int cl_object_getstripe(const struct lu_env *env, struct cl_object *obj,
>>  			struct lov_user_md __user *uarg, size_t size)
>>  {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>>  	int result = 0;
>>  
>> -	top = obj->co_lu.lo_header;
>> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_getstripe) {
>> -			result = obj->co_ops->coo_getstripe(env, obj, uarg,
>> -							    size);
>> +	cl_object_for_each(o, obj) {
>> +		if (o->co_ops->coo_getstripe) {
>> +			result = o->co_ops->coo_getstripe(env, o, uarg,
>> +							  size);
>>  			if (result)
>>  				break;
>>  		}
>> @@ -357,14 +350,13 @@ int cl_object_fiemap(const struct lu_env *env, struct cl_object *obj,
>>  		     struct ll_fiemap_info_key *key,
>>  		     struct fiemap *fiemap, size_t *buflen)
>>  {
>> -	struct lu_object_header *top;
>> +	struct cl_object *o;
>>  	int result = 0;
>>  
>> -	top = obj->co_lu.lo_header;
>> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_fiemap) {
>> -			result = obj->co_ops->coo_fiemap(env, obj, key, fiemap,
>> -							 buflen);
>> +	cl_object_for_each(o, obj) {
>> +		if (o->co_ops->coo_fiemap) {
>> +			result = o->co_ops->coo_fiemap(env, o, key, fiemap,
>> +						       buflen);
>>  			if (result)
>>  				break;
>>  		}
>> @@ -376,11 +368,11 @@ EXPORT_SYMBOL(cl_object_fiemap);
>>  int cl_object_layout_get(const struct lu_env *env, struct cl_object *obj,
>>  			 struct cl_layout *cl)
>>  {
>> -	struct lu_object_header *top = obj->co_lu.lo_header;
>> +	struct cl_object *o;
>>  
>> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_layout_get)
>> -			return obj->co_ops->coo_layout_get(env, obj, cl);
>> +	cl_object_for_each(o, obj) {
>> +		if (o->co_ops->coo_layout_get)
>> +			return o->co_ops->coo_layout_get(env, o, cl);
>>  	}
>>  
>>  	return -EOPNOTSUPP;
>> @@ -389,12 +381,12 @@ EXPORT_SYMBOL(cl_object_layout_get);
>>  
>>  loff_t cl_object_maxbytes(struct cl_object *obj)
>>  {
>> -	struct lu_object_header *top = obj->co_lu.lo_header;
>> +	struct cl_object *o;
>>  	loff_t maxbytes = LLONG_MAX;
>>  
>> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
>> -		if (obj->co_ops->coo_maxbytes)
>> -			maxbytes = min_t(loff_t, obj->co_ops->coo_maxbytes(obj),
>> +	cl_object_for_each(o, obj) {
>> +		if (o->co_ops->coo_maxbytes)
>> +			maxbytes = min_t(loff_t, o->co_ops->coo_maxbytes(o),
>>  					 maxbytes);
>>  	}
>>  
>> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> index 057318deaa4e..7d00a9233a3b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
>> @@ -132,7 +132,7 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
>>  			      enum cl_page_type type)
>>  {
>>  	struct cl_page *page;
>> -	struct lu_object_header *head;
>> +	struct cl_object *o2;
>>  
>>  	page = kzalloc(cl_object_header(o)->coh_page_bufsize, GFP_NOFS);
>>  	if (page) {
>> @@ -149,11 +149,10 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
>>  		INIT_LIST_HEAD(&page->cp_layers);
>>  		INIT_LIST_HEAD(&page->cp_batch);
>>  		lu_ref_init(&page->cp_reference);
>> -		head = o->co_lu.lo_header;
>> -		list_for_each_entry(o, &head->loh_layers, co_lu.lo_linkage) {
>> -			if (o->co_ops->coo_page_init) {
>> -				result = o->co_ops->coo_page_init(env, o, page,
>> -								  ind);
>> +		cl_object_for_each(o2, o) {
>> +			if (o2->co_ops->coo_page_init) {
>> +				result = o2->co_ops->coo_page_init(env, o2, page,
>> +								   ind);
>>  				if (result != 0) {
>>  					__cl_page_delete(env, page);
>>  					cl_page_free(env, page);
>> 
>> 
>>
James Simmons Feb. 11, 2019, 4:19 a.m. UTC | #5
> On Fri, Feb 08 2019, Andreas Dilger wrote:
> 
> > On Feb 6, 2019, at 17:03, NeilBrown <neilb@suse.com> wrote:
> >> 
> >> There are various places which have a list_for_each_entry()
> >> where cl_object_for_each (or .._reverse) is more appropriate.
> >> 
> >> Several of these re-use the 'obj' function parameter as a loop
> >> iterator, which is a little confusing.
> >> 
> >> Change these to use cl_object_for_each{_reverse}, and where needed,
> >> introduce a new iterator variable 'o'.
> >> 
> >> Signed-off-by: NeilBrown <neilb@suse.com>
> >> ---
> >> drivers/staging/lustre/lustre/lov/lov_page.c       |    3 -
> >> drivers/staging/lustre/lustre/obdclass/cl_lock.c   |    3 -
> >> drivers/staging/lustre/lustre/obdclass/cl_object.c |   82 +++++++++-----------
> >> drivers/staging/lustre/lustre/obdclass/cl_page.c   |   11 +--
> >> 4 files changed, 44 insertions(+), 55 deletions(-)
> >> 
> >> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> >> index f724b2d62df1..d71a680660da 100644
> >> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> >> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> >> @@ -190,16 +190,15 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
> >> int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
> >> 		       struct cl_attr *attr)
> >> {
> >> -	struct lu_object_header *top;
> >> +	struct cl_object *o;
> >> 	int result;
> >> 
> >> 	assert_spin_locked(cl_object_attr_guard(obj));
> >> 
> >> -	top = obj->co_lu.lo_header;
> >> 	result = 0;
> >
> > May as well move the "result = 0" initialization to the declaration for
> > all of these?  Another 5 fewer lines of code under your belt.
> 
> Yes, that seem good. New patch below.
> Thanks,
> NeilBrown
> 
> 
> From: NeilBrown <neilb@suse.com>
> Subject: [PATCH] lustre: obdclass: use cl_object_for_each where appropriate
> 
> There are various places which have a list_for_each_entry()
> where cl_object_for_each (or .._reverse) is more appropriate.
> 
> Several of these re-use the 'obj' function parameter as a loop
> iterator, which is a little confusing.
> 
> Change these to use cl_object_for_each{_reverse}, and where needed,
> introduce a new iterator variable 'o'.
> 
> To further clean up these function, move initializtion of 'result'
> to the variable declaration.

Good with this version as well.

Reviewed-by: James Simmons <jsimmons@infradead.org>
 
> Signed-off-by: NeilBrown <neilb@suse.com>
> ---
>  drivers/staging/lustre/lustre/lov/lov_page.c       |  3 +-
>  drivers/staging/lustre/lustre/obdclass/cl_lock.c   |  3 +-
>  drivers/staging/lustre/lustre/obdclass/cl_object.c | 97 ++++++++++------------
>  drivers/staging/lustre/lustre/obdclass/cl_page.c   | 11 ++-
>  4 files changed, 49 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/lov/lov_page.c b/drivers/staging/lustre/lustre/lov/lov_page.c
> index 08485a95ec01..e64b350601d2 100644
> --- a/drivers/staging/lustre/lustre/lov/lov_page.c
> +++ b/drivers/staging/lustre/lustre/lov/lov_page.c
> @@ -103,8 +103,7 @@ int lov_page_init_composite(const struct lu_env *env, struct cl_object *obj,
>  		return PTR_ERR(sub);
>  
>  	subobj = lovsub2cl(r0->lo_sub[stripe]);
> -	list_for_each_entry(o, &subobj->co_lu.lo_header->loh_layers,
> -			    co_lu.lo_linkage) {
> +	cl_object_for_each(o, subobj) {
>  		if (o->co_ops->coo_page_init) {
>  			rc = o->co_ops->coo_page_init(sub->sub_env, o, page,
>  						      cl_index(subobj, suboff));
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> index 8133d992cc73..fc5976d8b37b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> @@ -103,8 +103,7 @@ int cl_lock_init(const struct lu_env *env, struct cl_lock *lock,
>  	LASSERT(obj);
>  
>  	INIT_LIST_HEAD(&lock->cll_layers);
> -	list_for_each_entry(scan, &obj->co_lu.lo_header->loh_layers,
> -			    co_lu.lo_linkage) {
> +	cl_object_for_each(scan, obj) {
>  		result = scan->co_ops->coo_lock_init(env, scan, lock, io);
>  		if (result != 0) {
>  			cl_lock_fini(env, lock);
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index f724b2d62df1..43bf1a422635 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -190,16 +190,14 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
>  int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
>  		       struct cl_attr *attr)
>  {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
>  
>  	assert_spin_locked(cl_object_attr_guard(obj));
>  
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_attr_get) {
> -			result = obj->co_ops->coo_attr_get(env, obj, attr);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_attr_get) {
> +			result = o->co_ops->coo_attr_get(env, o, attr);
>  			if (result != 0) {
>  				if (result > 0)
>  					result = 0;
> @@ -221,17 +219,15 @@ EXPORT_SYMBOL(cl_object_attr_get);
>  int cl_object_attr_update(const struct lu_env *env, struct cl_object *obj,
>  			  const struct cl_attr *attr, unsigned int v)
>  {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
>  
>  	assert_spin_locked(cl_object_attr_guard(obj));
>  
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_attr_update) {
> -			result = obj->co_ops->coo_attr_update(env, obj, attr,
> -							      v);
> +	cl_object_for_each_reverse(o, obj) {
> +		if (o->co_ops->coo_attr_update) {
> +			result = o->co_ops->coo_attr_update(env, o, attr,
> +							    v);
>  			if (result != 0) {
>  				if (result > 0)
>  					result = 0;
> @@ -254,19 +250,17 @@ EXPORT_SYMBOL(cl_object_attr_update);
>  int cl_object_glimpse(const struct lu_env *env, struct cl_object *obj,
>  		      struct ost_lvb *lvb)
>  {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_glimpse) {
> -			result = obj->co_ops->coo_glimpse(env, obj, lvb);
> +	cl_object_for_each_reverse(o, obj) {
> +		if (o->co_ops->coo_glimpse) {
> +			result = o->co_ops->coo_glimpse(env, o, lvb);
>  			if (result != 0)
>  				break;
>  		}
>  	}
> -	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(top),
> +	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(obj->co_lu.lo_header),
>  			 "size: %llu mtime: %llu atime: %llu ctime: %llu blocks: %llu\n",
>  			 lvb->lvb_size, lvb->lvb_mtime, lvb->lvb_atime,
>  			 lvb->lvb_ctime, lvb->lvb_blocks);
> @@ -280,14 +274,12 @@ EXPORT_SYMBOL(cl_object_glimpse);
>  int cl_conf_set(const struct lu_env *env, struct cl_object *obj,
>  		const struct cl_object_conf *conf)
>  {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_conf_set) {
> -			result = obj->co_ops->coo_conf_set(env, obj, conf);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_conf_set) {
> +			result = o->co_ops->coo_conf_set(env, o, conf);
>  			if (result != 0)
>  				break;
>  		}
> @@ -301,13 +293,10 @@ EXPORT_SYMBOL(cl_conf_set);
>   */
>  int cl_object_prune(const struct lu_env *env, struct cl_object *obj)
>  {
> -	struct lu_object_header *top;
>  	struct cl_object *o;
> -	int result;
> +	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry(o, &top->loh_layers, co_lu.lo_linkage) {
> +	cl_object_for_each(o, obj) {
>  		if (o->co_ops->coo_prune) {
>  			result = o->co_ops->coo_prune(env, o);
>  			if (result != 0)
> @@ -325,14 +314,13 @@ EXPORT_SYMBOL(cl_object_prune);
>  int cl_object_getstripe(const struct lu_env *env, struct cl_object *obj,
>  			struct lov_user_md __user *uarg, size_t size)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_getstripe) {
> -			result = obj->co_ops->coo_getstripe(env, obj, uarg,
> -							    size);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_getstripe) {
> +			result = o->co_ops->coo_getstripe(env, o, uarg,
> +							  size);
>  			if (result)
>  				break;
>  		}
> @@ -357,14 +345,13 @@ int cl_object_fiemap(const struct lu_env *env, struct cl_object *obj,
>  		     struct ll_fiemap_info_key *key,
>  		     struct fiemap *fiemap, size_t *buflen)
>  {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
>  	int result = 0;
>  
> -	top = obj->co_lu.lo_header;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_fiemap) {
> -			result = obj->co_ops->coo_fiemap(env, obj, key, fiemap,
> -							 buflen);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_fiemap) {
> +			result = o->co_ops->coo_fiemap(env, o, key, fiemap,
> +						       buflen);
>  			if (result)
>  				break;
>  		}
> @@ -376,11 +363,11 @@ EXPORT_SYMBOL(cl_object_fiemap);
>  int cl_object_layout_get(const struct lu_env *env, struct cl_object *obj,
>  			 struct cl_layout *cl)
>  {
> -	struct lu_object_header *top = obj->co_lu.lo_header;
> +	struct cl_object *o;
>  
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_layout_get)
> -			return obj->co_ops->coo_layout_get(env, obj, cl);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_layout_get)
> +			return o->co_ops->coo_layout_get(env, o, cl);
>  	}
>  
>  	return -EOPNOTSUPP;
> @@ -389,12 +376,12 @@ EXPORT_SYMBOL(cl_object_layout_get);
>  
>  loff_t cl_object_maxbytes(struct cl_object *obj)
>  {
> -	struct lu_object_header *top = obj->co_lu.lo_header;
> +	struct cl_object *o;
>  	loff_t maxbytes = LLONG_MAX;
>  
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_maxbytes)
> -			maxbytes = min_t(loff_t, obj->co_ops->coo_maxbytes(obj),
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_maxbytes)
> +			maxbytes = min_t(loff_t, o->co_ops->coo_maxbytes(o),
>  					 maxbytes);
>  	}
>  
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> index 057318deaa4e..7d00a9233a3b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> @@ -132,7 +132,7 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
>  			      enum cl_page_type type)
>  {
>  	struct cl_page *page;
> -	struct lu_object_header *head;
> +	struct cl_object *o2;
>  
>  	page = kzalloc(cl_object_header(o)->coh_page_bufsize, GFP_NOFS);
>  	if (page) {
> @@ -149,11 +149,10 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
>  		INIT_LIST_HEAD(&page->cp_layers);
>  		INIT_LIST_HEAD(&page->cp_batch);
>  		lu_ref_init(&page->cp_reference);
> -		head = o->co_lu.lo_header;
> -		list_for_each_entry(o, &head->loh_layers, co_lu.lo_linkage) {
> -			if (o->co_ops->coo_page_init) {
> -				result = o->co_ops->coo_page_init(env, o, page,
> -								  ind);
> +		cl_object_for_each(o2, o) {
> +			if (o2->co_ops->coo_page_init) {
> +				result = o2->co_ops->coo_page_init(env, o2, page,
> +								   ind);
>  				if (result != 0) {
>  					__cl_page_delete(env, page);
>  					cl_page_free(env, page);
> -- 
> 2.14.0.rc0.dirty
> 
>
Andreas Dilger Feb. 15, 2019, 6:15 p.m. UTC | #6
On Feb 10, 2019, at 17:42, NeilBrown <neilb@suse.com> wrote:
> From: NeilBrown <neilb@suse.com>
> Subject: [PATCH] lustre: obdclass: use cl_object_for_each where appropriate
> 
> There are various places which have a list_for_each_entry()
> where cl_object_for_each (or .._reverse) is more appropriate.
> 
> Several of these re-use the 'obj' function parameter as a loop
> iterator, which is a little confusing.
> 
> Change these to use cl_object_for_each{_reverse}, and where needed,
> introduce a new iterator variable 'o'.
> 
> To further clean up these function, move initializtion of 'result'
> to the variable declaration.
> 
> Signed-off-by: NeilBrown <neilb@suse.com>

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>

> ---
> drivers/staging/lustre/lustre/lov/lov_page.c       |  3 +-
> drivers/staging/lustre/lustre/obdclass/cl_lock.c   |  3 +-
> drivers/staging/lustre/lustre/obdclass/cl_object.c | 97 ++++++++++------------
> drivers/staging/lustre/lustre/obdclass/cl_page.c   | 11 ++-
> 4 files changed, 49 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/lov/lov_page.c b/drivers/staging/lustre/lustre/lov/lov_page.c
> index 08485a95ec01..e64b350601d2 100644
> --- a/drivers/staging/lustre/lustre/lov/lov_page.c
> +++ b/drivers/staging/lustre/lustre/lov/lov_page.c
> @@ -103,8 +103,7 @@ int lov_page_init_composite(const struct lu_env *env, struct cl_object *obj,
> 		return PTR_ERR(sub);
> 
> 	subobj = lovsub2cl(r0->lo_sub[stripe]);
> -	list_for_each_entry(o, &subobj->co_lu.lo_header->loh_layers,
> -			    co_lu.lo_linkage) {
> +	cl_object_for_each(o, subobj) {
> 		if (o->co_ops->coo_page_init) {
> 			rc = o->co_ops->coo_page_init(sub->sub_env, o, page,
> 						      cl_index(subobj, suboff));
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> index 8133d992cc73..fc5976d8b37b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
> @@ -103,8 +103,7 @@ int cl_lock_init(const struct lu_env *env, struct cl_lock *lock,
> 	LASSERT(obj);
> 
> 	INIT_LIST_HEAD(&lock->cll_layers);
> -	list_for_each_entry(scan, &obj->co_lu.lo_header->loh_layers,
> -			    co_lu.lo_linkage) {
> +	cl_object_for_each(scan, obj) {
> 		result = scan->co_ops->coo_lock_init(env, scan, lock, io);
> 		if (result != 0) {
> 			cl_lock_fini(env, lock);
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index f724b2d62df1..43bf1a422635 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -190,16 +190,14 @@ EXPORT_SYMBOL(cl_object_attr_unlock);
> int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
> 		       struct cl_attr *attr)
> {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
> 
> 	assert_spin_locked(cl_object_attr_guard(obj));
> 
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_attr_get) {
> -			result = obj->co_ops->coo_attr_get(env, obj, attr);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_attr_get) {
> +			result = o->co_ops->coo_attr_get(env, o, attr);
> 			if (result != 0) {
> 				if (result > 0)
> 					result = 0;
> @@ -221,17 +219,15 @@ EXPORT_SYMBOL(cl_object_attr_get);
> int cl_object_attr_update(const struct lu_env *env, struct cl_object *obj,
> 			  const struct cl_attr *attr, unsigned int v)
> {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
> 
> 	assert_spin_locked(cl_object_attr_guard(obj));
> 
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_attr_update) {
> -			result = obj->co_ops->coo_attr_update(env, obj, attr,
> -							      v);
> +	cl_object_for_each_reverse(o, obj) {
> +		if (o->co_ops->coo_attr_update) {
> +			result = o->co_ops->coo_attr_update(env, o, attr,
> +							    v);
> 			if (result != 0) {
> 				if (result > 0)
> 					result = 0;
> @@ -254,19 +250,17 @@ EXPORT_SYMBOL(cl_object_attr_update);
> int cl_object_glimpse(const struct lu_env *env, struct cl_object *obj,
> 		      struct ost_lvb *lvb)
> {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
> 
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_glimpse) {
> -			result = obj->co_ops->coo_glimpse(env, obj, lvb);
> +	cl_object_for_each_reverse(o, obj) {
> +		if (o->co_ops->coo_glimpse) {
> +			result = o->co_ops->coo_glimpse(env, o, lvb);
> 			if (result != 0)
> 				break;
> 		}
> 	}
> -	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(top),
> +	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(obj->co_lu.lo_header),
> 			 "size: %llu mtime: %llu atime: %llu ctime: %llu blocks: %llu\n",
> 			 lvb->lvb_size, lvb->lvb_mtime, lvb->lvb_atime,
> 			 lvb->lvb_ctime, lvb->lvb_blocks);
> @@ -280,14 +274,12 @@ EXPORT_SYMBOL(cl_object_glimpse);
> int cl_conf_set(const struct lu_env *env, struct cl_object *obj,
> 		const struct cl_object_conf *conf)
> {
> -	struct lu_object_header *top;
> -	int result;
> +	struct cl_object *o;
> +	int result = 0;
> 
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_conf_set) {
> -			result = obj->co_ops->coo_conf_set(env, obj, conf);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_conf_set) {
> +			result = o->co_ops->coo_conf_set(env, o, conf);
> 			if (result != 0)
> 				break;
> 		}
> @@ -301,13 +293,10 @@ EXPORT_SYMBOL(cl_conf_set);
>  */
> int cl_object_prune(const struct lu_env *env, struct cl_object *obj)
> {
> -	struct lu_object_header *top;
> 	struct cl_object *o;
> -	int result;
> +	int result = 0;
> 
> -	top = obj->co_lu.lo_header;
> -	result = 0;
> -	list_for_each_entry(o, &top->loh_layers, co_lu.lo_linkage) {
> +	cl_object_for_each(o, obj) {
> 		if (o->co_ops->coo_prune) {
> 			result = o->co_ops->coo_prune(env, o);
> 			if (result != 0)
> @@ -325,14 +314,13 @@ EXPORT_SYMBOL(cl_object_prune);
> int cl_object_getstripe(const struct lu_env *env, struct cl_object *obj,
> 			struct lov_user_md __user *uarg, size_t size)
> {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
> 	int result = 0;
> 
> -	top = obj->co_lu.lo_header;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_getstripe) {
> -			result = obj->co_ops->coo_getstripe(env, obj, uarg,
> -							    size);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_getstripe) {
> +			result = o->co_ops->coo_getstripe(env, o, uarg,
> +							  size);
> 			if (result)
> 				break;
> 		}
> @@ -357,14 +345,13 @@ int cl_object_fiemap(const struct lu_env *env, struct cl_object *obj,
> 		     struct ll_fiemap_info_key *key,
> 		     struct fiemap *fiemap, size_t *buflen)
> {
> -	struct lu_object_header *top;
> +	struct cl_object *o;
> 	int result = 0;
> 
> -	top = obj->co_lu.lo_header;
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_fiemap) {
> -			result = obj->co_ops->coo_fiemap(env, obj, key, fiemap,
> -							 buflen);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_fiemap) {
> +			result = o->co_ops->coo_fiemap(env, o, key, fiemap,
> +						       buflen);
> 			if (result)
> 				break;
> 		}
> @@ -376,11 +363,11 @@ EXPORT_SYMBOL(cl_object_fiemap);
> int cl_object_layout_get(const struct lu_env *env, struct cl_object *obj,
> 			 struct cl_layout *cl)
> {
> -	struct lu_object_header *top = obj->co_lu.lo_header;
> +	struct cl_object *o;
> 
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_layout_get)
> -			return obj->co_ops->coo_layout_get(env, obj, cl);
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_layout_get)
> +			return o->co_ops->coo_layout_get(env, o, cl);
> 	}
> 
> 	return -EOPNOTSUPP;
> @@ -389,12 +376,12 @@ EXPORT_SYMBOL(cl_object_layout_get);
> 
> loff_t cl_object_maxbytes(struct cl_object *obj)
> {
> -	struct lu_object_header *top = obj->co_lu.lo_header;
> +	struct cl_object *o;
> 	loff_t maxbytes = LLONG_MAX;
> 
> -	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
> -		if (obj->co_ops->coo_maxbytes)
> -			maxbytes = min_t(loff_t, obj->co_ops->coo_maxbytes(obj),
> +	cl_object_for_each(o, obj) {
> +		if (o->co_ops->coo_maxbytes)
> +			maxbytes = min_t(loff_t, o->co_ops->coo_maxbytes(o),
> 					 maxbytes);
> 	}
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> index 057318deaa4e..7d00a9233a3b 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
> @@ -132,7 +132,7 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
> 			      enum cl_page_type type)
> {
> 	struct cl_page *page;
> -	struct lu_object_header *head;
> +	struct cl_object *o2;
> 
> 	page = kzalloc(cl_object_header(o)->coh_page_bufsize, GFP_NOFS);
> 	if (page) {
> @@ -149,11 +149,10 @@ struct cl_page *cl_page_alloc(const struct lu_env *env,
> 		INIT_LIST_HEAD(&page->cp_layers);
> 		INIT_LIST_HEAD(&page->cp_batch);
> 		lu_ref_init(&page->cp_reference);
> -		head = o->co_lu.lo_header;
> -		list_for_each_entry(o, &head->loh_layers, co_lu.lo_linkage) {
> -			if (o->co_ops->coo_page_init) {
> -				result = o->co_ops->coo_page_init(env, o, page,
> -								  ind);
> +		cl_object_for_each(o2, o) {
> +			if (o2->co_ops->coo_page_init) {
> +				result = o2->co_ops->coo_page_init(env, o2, page,
> +								   ind);
> 				if (result != 0) {
> 					__cl_page_delete(env, page);
> 					cl_page_free(env, page);
> -- 
> 2.14.0.rc0.dirty

Cheers, Andreas
---
Andreas Dilger
CTO Whamcloud

Patch
diff mbox series

diff --git a/drivers/staging/lustre/lustre/lov/lov_page.c b/drivers/staging/lustre/lustre/lov/lov_page.c
index 08485a95ec01..e64b350601d2 100644
--- a/drivers/staging/lustre/lustre/lov/lov_page.c
+++ b/drivers/staging/lustre/lustre/lov/lov_page.c
@@ -103,8 +103,7 @@  int lov_page_init_composite(const struct lu_env *env, struct cl_object *obj,
 		return PTR_ERR(sub);
 
 	subobj = lovsub2cl(r0->lo_sub[stripe]);
-	list_for_each_entry(o, &subobj->co_lu.lo_header->loh_layers,
-			    co_lu.lo_linkage) {
+	cl_object_for_each(o, subobj) {
 		if (o->co_ops->coo_page_init) {
 			rc = o->co_ops->coo_page_init(sub->sub_env, o, page,
 						      cl_index(subobj, suboff));
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_lock.c b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
index 8133d992cc73..fc5976d8b37b 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_lock.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_lock.c
@@ -103,8 +103,7 @@  int cl_lock_init(const struct lu_env *env, struct cl_lock *lock,
 	LASSERT(obj);
 
 	INIT_LIST_HEAD(&lock->cll_layers);
-	list_for_each_entry(scan, &obj->co_lu.lo_header->loh_layers,
-			    co_lu.lo_linkage) {
+	cl_object_for_each(scan, obj) {
 		result = scan->co_ops->coo_lock_init(env, scan, lock, io);
 		if (result != 0) {
 			cl_lock_fini(env, lock);
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index f724b2d62df1..d71a680660da 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -190,16 +190,15 @@  EXPORT_SYMBOL(cl_object_attr_unlock);
 int cl_object_attr_get(const struct lu_env *env, struct cl_object *obj,
 		       struct cl_attr *attr)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result;
 
 	assert_spin_locked(cl_object_attr_guard(obj));
 
-	top = obj->co_lu.lo_header;
 	result = 0;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_attr_get) {
-			result = obj->co_ops->coo_attr_get(env, obj, attr);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_attr_get) {
+			result = o->co_ops->coo_attr_get(env, o, attr);
 			if (result != 0) {
 				if (result > 0)
 					result = 0;
@@ -221,17 +220,16 @@  EXPORT_SYMBOL(cl_object_attr_get);
 int cl_object_attr_update(const struct lu_env *env, struct cl_object *obj,
 			  const struct cl_attr *attr, unsigned int v)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result;
 
 	assert_spin_locked(cl_object_attr_guard(obj));
 
-	top = obj->co_lu.lo_header;
 	result = 0;
-	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_attr_update) {
-			result = obj->co_ops->coo_attr_update(env, obj, attr,
-							      v);
+	cl_object_for_each_reverse(o, obj) {
+		if (o->co_ops->coo_attr_update) {
+			result = o->co_ops->coo_attr_update(env, o, attr,
+							    v);
 			if (result != 0) {
 				if (result > 0)
 					result = 0;
@@ -254,19 +252,18 @@  EXPORT_SYMBOL(cl_object_attr_update);
 int cl_object_glimpse(const struct lu_env *env, struct cl_object *obj,
 		      struct ost_lvb *lvb)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result;
 
-	top = obj->co_lu.lo_header;
 	result = 0;
-	list_for_each_entry_reverse(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_glimpse) {
-			result = obj->co_ops->coo_glimpse(env, obj, lvb);
+	cl_object_for_each_reverse(o, obj) {
+		if (o->co_ops->coo_glimpse) {
+			result = o->co_ops->coo_glimpse(env, o, lvb);
 			if (result != 0)
 				break;
 		}
 	}
-	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(top),
+	LU_OBJECT_HEADER(D_DLMTRACE, env, lu_object_top(obj->co_lu.lo_header),
 			 "size: %llu mtime: %llu atime: %llu ctime: %llu blocks: %llu\n",
 			 lvb->lvb_size, lvb->lvb_mtime, lvb->lvb_atime,
 			 lvb->lvb_ctime, lvb->lvb_blocks);
@@ -280,14 +277,13 @@  EXPORT_SYMBOL(cl_object_glimpse);
 int cl_conf_set(const struct lu_env *env, struct cl_object *obj,
 		const struct cl_object_conf *conf)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result;
 
-	top = obj->co_lu.lo_header;
 	result = 0;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_conf_set) {
-			result = obj->co_ops->coo_conf_set(env, obj, conf);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_conf_set) {
+			result = o->co_ops->coo_conf_set(env, o, conf);
 			if (result != 0)
 				break;
 		}
@@ -301,13 +297,11 @@  EXPORT_SYMBOL(cl_conf_set);
  */
 int cl_object_prune(const struct lu_env *env, struct cl_object *obj)
 {
-	struct lu_object_header *top;
 	struct cl_object *o;
 	int result;
 
-	top = obj->co_lu.lo_header;
 	result = 0;
-	list_for_each_entry(o, &top->loh_layers, co_lu.lo_linkage) {
+	cl_object_for_each(o, obj) {
 		if (o->co_ops->coo_prune) {
 			result = o->co_ops->coo_prune(env, o);
 			if (result != 0)
@@ -325,14 +319,13 @@  EXPORT_SYMBOL(cl_object_prune);
 int cl_object_getstripe(const struct lu_env *env, struct cl_object *obj,
 			struct lov_user_md __user *uarg, size_t size)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_getstripe) {
-			result = obj->co_ops->coo_getstripe(env, obj, uarg,
-							    size);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_getstripe) {
+			result = o->co_ops->coo_getstripe(env, o, uarg,
+							  size);
 			if (result)
 				break;
 		}
@@ -357,14 +350,13 @@  int cl_object_fiemap(const struct lu_env *env, struct cl_object *obj,
 		     struct ll_fiemap_info_key *key,
 		     struct fiemap *fiemap, size_t *buflen)
 {
-	struct lu_object_header *top;
+	struct cl_object *o;
 	int result = 0;
 
-	top = obj->co_lu.lo_header;
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_fiemap) {
-			result = obj->co_ops->coo_fiemap(env, obj, key, fiemap,
-							 buflen);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_fiemap) {
+			result = o->co_ops->coo_fiemap(env, o, key, fiemap,
+						       buflen);
 			if (result)
 				break;
 		}
@@ -376,11 +368,11 @@  EXPORT_SYMBOL(cl_object_fiemap);
 int cl_object_layout_get(const struct lu_env *env, struct cl_object *obj,
 			 struct cl_layout *cl)
 {
-	struct lu_object_header *top = obj->co_lu.lo_header;
+	struct cl_object *o;
 
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_layout_get)
-			return obj->co_ops->coo_layout_get(env, obj, cl);
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_layout_get)
+			return o->co_ops->coo_layout_get(env, o, cl);
 	}
 
 	return -EOPNOTSUPP;
@@ -389,12 +381,12 @@  EXPORT_SYMBOL(cl_object_layout_get);
 
 loff_t cl_object_maxbytes(struct cl_object *obj)
 {
-	struct lu_object_header *top = obj->co_lu.lo_header;
+	struct cl_object *o;
 	loff_t maxbytes = LLONG_MAX;
 
-	list_for_each_entry(obj, &top->loh_layers, co_lu.lo_linkage) {
-		if (obj->co_ops->coo_maxbytes)
-			maxbytes = min_t(loff_t, obj->co_ops->coo_maxbytes(obj),
+	cl_object_for_each(o, obj) {
+		if (o->co_ops->coo_maxbytes)
+			maxbytes = min_t(loff_t, o->co_ops->coo_maxbytes(o),
 					 maxbytes);
 	}
 
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_page.c b/drivers/staging/lustre/lustre/obdclass/cl_page.c
index 057318deaa4e..7d00a9233a3b 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_page.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_page.c
@@ -132,7 +132,7 @@  struct cl_page *cl_page_alloc(const struct lu_env *env,
 			      enum cl_page_type type)
 {
 	struct cl_page *page;
-	struct lu_object_header *head;
+	struct cl_object *o2;
 
 	page = kzalloc(cl_object_header(o)->coh_page_bufsize, GFP_NOFS);
 	if (page) {
@@ -149,11 +149,10 @@  struct cl_page *cl_page_alloc(const struct lu_env *env,
 		INIT_LIST_HEAD(&page->cp_layers);
 		INIT_LIST_HEAD(&page->cp_batch);
 		lu_ref_init(&page->cp_reference);
-		head = o->co_lu.lo_header;
-		list_for_each_entry(o, &head->loh_layers, co_lu.lo_linkage) {
-			if (o->co_ops->coo_page_init) {
-				result = o->co_ops->coo_page_init(env, o, page,
-								  ind);
+		cl_object_for_each(o2, o) {
+			if (o2->co_ops->coo_page_init) {
+				result = o2->co_ops->coo_page_init(env, o2, page,
+								   ind);
 				if (result != 0) {
 					__cl_page_delete(env, page);
 					cl_page_free(env, page);