Re: [patch v3 2/5] raid5: fix stripe release order

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]

On Wed, 28 Aug 2013 14:29:05 +0800 Shaohua Li <shli@kernel.org> wrote:

> On Wed, Aug 28, 2013 at 01:41:50PM +1000, NeilBrown wrote:
> > 
> > > +struct llist_node *llist_reverse_order(struct llist_node *head)
> > > +{
> > > +	struct llist_node *second, *third;
> > > +
> > > +	if (head == NULL || head->next == NULL)
> > > +		return head;
> > > +	second = head->next;
> > > +	head->next = NULL;
> > > +
> > > +	do {
> > > +		third = second->next;
> > > +		second->next = head;
> > > +
> > > +		head = second;
> > > +		second = third;
> > > +	} while (second);
> > > +
> > > +	return head;
> > > +}
> > > +EXPORT_SYMBOL_GPL(llist_reverse_order);
> > 
> > This is somewhat longer that necessary.
> > 
> > struct llist_node *llist_reverse_order(struct llist_node *head)
> > {
> > 	struct llist_node *new_head = NULL;
> > 
> > 	while (head) {
> > 		struct llist_node *tmp = head;
> > 		head = head->next;
> > 		tmp->next = new_head;
> > 		new_head = tmp;
> > 	}
> > 
> > 	return new_head;
> > }
> > 
> > I think that is short enough to just open-code in the top of
> > release_stripe_list.
> > 
> > Are you OK with that?
> 
> It's ok. Other patches can still apply with hunks.
> 
> Subject: raid5: fix stripe release order
> 
> patch "make release_stripe lockless" changes the order stripes are released.
> Originally I thought block layer can take care of request merge, but it appears
> there are still some requests not merged. It's easy to fix the order.
> 
> Signed-off-by: Shaohua Li <shli@fusionio.com>
> ---
>  drivers/md/raid5.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> Index: linux/drivers/md/raid5.c
> ===================================================================
> --- linux.orig/drivers/md/raid5.c	2013-08-28 13:51:39.586914255 +0800
> +++ linux/drivers/md/raid5.c	2013-08-28 13:53:25.429582150 +0800
> @@ -239,6 +239,20 @@ static void __release_stripe(struct r5co
>  		do_release_stripe(conf, sh);
>  }
>  
> +static struct llist_node *llist_reverse_order(struct llist_node *head)
> +{
> +	struct llist_node *new_head = NULL;
> +
> +	while (head) {
> +		struct llist_node *tmp = head;
> +		head = head->next;
> +		tmp->next = new_head;
> +		new_head = tmp;
> +	}
> +
> +	return new_head;
> +}
> +
>  /* should hold conf->device_lock already */
>  static int release_stripe_list(struct r5conf *conf)
>  {
> @@ -247,6 +261,7 @@ static int release_stripe_list(struct r5
>  	struct llist_node *head;
>  
>  	head = llist_del_all(&conf->released_stripes);
> +	head = llist_reverse_order(head);
>  	while (head) {
>  		sh = llist_entry(head, struct stripe_head, release_list);
>  		head = llist_next(head);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Applied, thanks.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]