Re: [patch]raid5: make release_stripe lockless

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Mar 17, 2013 at 9:31 PM, Shaohua Li <shli@xxxxxxxxxx> wrote:
> release_stripe still has big lock contention. We just add the stripe to a llist
> without taking device_lock. We let the raid5d thread to do the real stripe
> release, which must hold device_lock anyway. In this way, release_stripe
> doesn't hold any locks.
>
> The side effect is the released stripes order is changed. But sounds not a big
> deal, stripes are never handled in order. And I thought block layer can already
> do nice request merge, which means order isn't that important.
>
> I kept the unplug release batch, which is unnecessary with this patch from lock
> contention avoid point of view, and actually if we delete it, the stripe_head
> release_list and lru can share storage. But the unplug release batch is also
> helpful for request merge. We probably can delay wakeup raid5d till unplug, but
> I'm still afraid of the case which raid5d is running.
>
> Signed-off-by: Shaohua Li <shli@xxxxxxxxxxxx>
> ---
>  drivers/md/raid5.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  drivers/md/raid5.h |    3 +++
>  2 files changed, 51 insertions(+), 5 deletions(-)
>
> Index: linux/drivers/md/raid5.c
> ===================================================================
> --- linux.orig/drivers/md/raid5.c       2013-03-18 08:49:43.276628437 +0800
> +++ linux/drivers/md/raid5.c    2013-03-18 10:33:58.561988946 +0800
> @@ -262,12 +262,45 @@ static void __release_stripe(struct r5co
>                 do_release_stripe(conf, sh);
>  }
>
> +/* should hold conf->device_lock already */
> +static int release_stripe_list(struct r5conf *conf)
> +{
> +       struct stripe_head *sh;
> +       struct llist_node *node;
> +       int count = 0;
> +
> +       while (1) {
> +               node = llist_del_first(&conf->released_stripes);
> +               if (!node)
> +                       break;
> +               sh = llist_entry(node, struct stripe_head, release_list);

Do we need a smp_mb__before_clear_bit() here to pair with the
test_and_set_bit() to preclude re-adding the stripe_head before it is
fully deleted?
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux