xfs
[Top] [All Lists]

Re: [PATCH 3/6] gfs2: simplify current_tail() via list_last_entry_or_nul

To: Jeff Liu <jeff.liu@xxxxxxxxxx>
Subject: Re: [PATCH 3/6] gfs2: simplify current_tail() via list_last_entry_or_null()
From: Oleg Nesterov <oleg@xxxxxxxxxx>
Date: Fri, 15 Nov 2013 17:39:57 +0100
Cc: linux-kernel@xxxxxxxxxxxxxxx, akpm@xxxxxxxxxxxxxxxxxxxx, "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>, cluster-devel@xxxxxxxxxx, linux-mtd@xxxxxxxxxxxxxxxxxxx, jfs-discussion@xxxxxxxxxxxxxxxxxxxxx, jiri@xxxxxxxxxxx, gregkh@xxxxxxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5285A3F4.4040000@xxxxxxxxxx>
References: <5285A3F4.4040000@xxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
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.

<Prev in Thread] Current Thread [Next in Thread>