diff mbox

[3/6] gfs2: simplify current_tail() via list_last_entry_or_null()

Message ID 5285A3F4.4040000@oracle.com
State Not Applicable
Headers show

Commit Message

jeff.liu Nov. 15, 2013, 4:32 a.m. UTC 
From: Jie Liu <jeff.liu@oracle.com>

Simplify the code in current_tail() via list_last_entry_or_null().

Signed-off-by: Jie Liu <jeff.liu@oracle.com>
---
 fs/gfs2/log.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Oleg Nesterov Nov. 15, 2013, 4:39 p.m. UTC | #1 
On 11/15, Jeff Liu wrote:
>
> @@ -441,13 +441,9 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
>  
>  	spin_lock(&sdp->sd_ail_lock);
>  
> -	if (list_empty(&sdp->sd_ail1_list)) {
> -		tail = sdp->sd_log_head;
> -	} else {
> -		tr = list_entry(sdp->sd_ail1_list.prev, struct gfs2_trans,
> -				tr_list);
> -		tail = tr->tr_first;
> -	}
> +	tr = list_last_entry_or_null(&sdp->sd_ail1_list, struct gfs2_trans,
> +				     tr_list);
> +	tail = tr ? tr->tr_first : sdp->sd_log_head;
>  

Personally I agree with Steven. At least in this case
list_last_entry_or_null() doesn't really help to simplify the code.

But probably list_last_entry() makes sense in the "else" branch,
athough this is minor.


Off-topic. Not sure this really makes sense, but I was thinking about

	list_get_first(pos, head, member)	\
		((pos) = list_first_entry(head, typeof(*pos), member))

and list_get_first() last of course. The obvious advantage is that
compared to

	tr = list_last_entry(sdp->sd_ail1_list, struct gfs2_trans, tr_list);

above you do not need to type "struct gfs2_trans",

	list_get_last(tr, sdp->sd_ail1_list, tr_list);

looks a bit better.

Oleg.
diff mbox

Patch

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 610613f..555f767 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -441,13 +441,9 @@  static unsigned int current_tail(struct gfs2_sbd *sdp)
 
 	spin_lock(&sdp->sd_ail_lock);
 
-	if (list_empty(&sdp->sd_ail1_list)) {
-		tail = sdp->sd_log_head;
-	} else {
-		tr = list_entry(sdp->sd_ail1_list.prev, struct gfs2_trans,
-				tr_list);
-		tail = tr->tr_first;
-	}
+	tr = list_last_entry_or_null(&sdp->sd_ail1_list, struct gfs2_trans,
+				     tr_list);
+	tail = tr ? tr->tr_first : sdp->sd_log_head;
 
 	spin_unlock(&sdp->sd_ail_lock);