Re: [dm-devel] [PATCH] additional bio list helpers

[Kiyoshi Ueda <k-ueda ct jp nec com>]

Hi Bryn,

On Tue, 06 Feb 2007 11:00:15 +0000, "Bryn M. Reeves" <breeves redhat com> wrote:
> @@ -33,9 +55,6 @@ static inline void bio_list_add(struct bio_list *bl, struct bio *bio)
>  
>  static inline void bio_list_merge(struct bio_list *bl, struct bio_list *bl2)
>  {
> -	if (!bl2->head)
> -		return;
> -
>  	if (bl->tail)
>  		bl->tail->bi_next = bl2->head;
>  	else

This one breaks the bio_list if bl2 is a empty list.
Probably missed to add like below?
-----------------------------------
+	if (bio_list_empty(bl2))
+		return;
-----------------------------------

Other small comments are below:

> @@ -14,11 +14,33 @@ struct bio_list {
>  	struct bio *tail;
>  };
>  
> +static inline int  bio_list_empty(struct bio_list *bl)
> +{
> +	return bl->head == NULL && bl->tail == NULL;
> +}

I think that bl->tail is always NULL when bl->head is NULL,
unless the bio_list is corrupted.  So no need to check bl->tail.

> +#define bio_for_each(bio, bl) \
> +	for (bio = (bl)->head; bio; bio = bio->bi_next)
> +
> +static inline int bio_list_nr(struct bio_list *bl)
> +{
> +	int i=0;
> +	struct bio *bio;
> +
> +	if(bio_list_empty(bl))
> +		return i;

I think that this check in not needed because it is covered
by bio_for_each() below.

> +	bio_for_each(bio, bl)
> +		i++;
> +	return i;
> +}


> @@ -58,6 +77,40 @@ static inline void bio_list_merge_head(struct bio_list *bl,
>  	bl->head = bl2->head;
>  }
>  
> +static inline void bio_list_join(struct bio_list *bl, struct bio_list *bl2)
> +{
> +	if (bio_list_empty(bl2))
> +		return;
> +
> +	bl2->tail->bi_next = bl->head;
> +	bl->head = bl2->head;
> +
> +	if (!bl->tail)
> +		bl->tail = bl2->tail;
> +}
> +
> +static inline void bio_list_join_init(struct bio_list *bl, struct bio_list *bl2)
> +{
> +	bio_list_join(bl, bl2);
> +	bio_list_init(bl2);
> +}

I think that bio_list_join() is same as existing bio_list_merge_head().

Regards,
Kiyoshi Ueda