Re: [PATCH 3/4] raid5: ignore released_stripes check

[NeilBrown <neilb@suse.de>]

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

On Thu, 28 May 2015 17:33:47 -0700 Shaohua Li <shli@fb.com> wrote:

> conf->released_stripes list ins't always related if there is free
> stripes pending. active stripes can be in the list too.
> 
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  drivers/md/raid5.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 0cceb71..67626f3 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -673,8 +673,8 @@ get_active_stripe(struct r5conf *conf, sector_t sector,
>  		if (!sh) {
>  			if (!test_bit(R5_INACTIVE_BLOCKED, &conf->cache_state)) {
>  				sh = get_free_stripe(conf, hash);
> -				if (!sh && llist_empty(&conf->released_stripes) &&
> -				    !test_bit(R5_DID_ALLOC, &conf->cache_state)) {
> +				if (!sh && !test_bit(R5_DID_ALLOC,
> +				    &conf->cache_state)) {
>  					set_bit(R5_ALLOC_MORE,
>  						&conf->cache_state);
>  					md_wakeup_thread(conf->mddev->thread);

Yes, I think I agree with this. 
I don't clearly remember why I put that test in, but it is certainly hard to
justify it.
Even if there are free stripes in ->released_stripes, they were in use very
recently, so counting them as still in use is easily justified.

BTW, is an open parenthesis (or bracket or brace) is not the last character
in the line, then everything from there until the close must be to the right
of the open.  So I indented the "&conf->cache_state)) {" line some more.

Thanks,
NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]