Re: [PATCH 1/3] raid5-cache: add trim support for log

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

 



Shaohua Li <shli@xxxxxx> writes:

> Since superblock is updated infrequently, we do a simple trim of log
> disk (a synchronous trim)
>
> Signed-off-by: Shaohua Li <shli@xxxxxx>
> ---
>  drivers/md/raid5-cache.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index a02f9ce..afc3b6b 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -654,6 +654,48 @@ static void r5l_kick_io_unit(struct r5l_log *log)
>  }
>  
>  static void r5l_write_super(struct r5l_log *log, sector_t cp);
> +static void r5l_write_super_and_discard_space(struct r5l_log *log,
> +	sector_t end)
> +{
> +	struct block_device *bdev = log->rdev->bdev;
> +	struct mddev *mddev;
> +
> +	r5l_write_super(log, end);
> +
> +	if (!blk_queue_discard(bdev_get_queue(bdev)))
> +		return;
> +
> +	/* discard destroy old data in log, so force a super update */
> +	mddev = log->rdev->mddev;
> +	/*
> +	 * mddev->thread could be shut down already in raid array stop. At that
> +	 * time, we should already lock reconfig_mutex
> +	 * */
> +	if (!mddev->thread) {
> +		WARN_ON(!mddev_is_locked(mddev));
> +		md_update_sb(mddev, 1);
> +	} else {
> +		set_bit(MD_CHANGE_PENDING, &mddev->flags);
> +		md_wakeup_thread(mddev->thread);
> +		wait_event(mddev->sb_wait,
> +			   !test_bit(MD_CHANGE_PENDING, &mddev->flags));
> +	}

I didn't like this.  It looks clumsy to me.

So I went to have a look at the code to understand why it was needed.
I found that r5l_exit_log() was being called from free_conf().
I didn't notice that before.

->free() is only supposed to free, not write anything.  It could be too
late to write anything.

You need to get the raid5_quiesce(1) call to stablise the array.  It can
do the final r5l_do_reclaim().

So it is OK for free_conf to call r5l_exit_log as long as it only
deregisters the thread and frees the data structures.
The "r5l_do_reclaim" needs to be moved out.

I wonder where the md_update_sb() should go...

We currently calls "stop_writes" and then "mddev_detach".
So we shouldn't be writing anything by the time we get to mddev_detach,
but in there we wait for writes to complete.  That looks wrong.

I might move some of that stuff from mddev_detach to __md_stop_writes.
Then md_stop_writes can call ->quiesce and then md_update_sb().  That is
enough for raid5-cache to just call r5l_do_reclaim in raid5_quiesce, and
have the md_update_sb() called at the right time.

I'll see if i can make that work.

Thanks,
NeilBrown

Attachment: signature.asc
Description: PGP signature


[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