Messages in this thread | | | Date | Fri, 15 Nov 2013 17:39:57 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 3/6] gfs2: simplify current_tail() via list_last_entry_or_null() |
| |
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.
| |