[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-raid
Subject:    Re: [PATCH 1/4] raid5: wakeup raid5d when R5_ALLOC_MORE is set
From:       Shaohua Li <shli () fb ! com>
Date:       2015-05-29 5:33:59
Message-ID: 20150529053340.GA2605536 () devbig257 ! prn2 ! facebook ! com
[Download RAW message or body]

On Fri, May 29, 2015 at 03:02:56PM +1000, NeilBrown wrote:
> On Thu, 28 May 2015 17:33:45 -0700 Shaohua Li <shli@fb.com> wrote:
> 
> > The run time stripe allocation is done at raid5d. When we set the
> > R5_ALLOC_MORE flag, we should notify raid5d to handle it
> > 
> > Signed-off-by: Shaohua Li <shli@fb.com>
> > ---
> >  drivers/md/raid5.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index 61e8e04..bfa2042 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -674,9 +674,11 @@ get_active_stripe(struct r5conf *conf, sector_t sector,
> >  			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))
> > +				    !test_bit(R5_DID_ALLOC, &conf->cache_state)) {
> >  					set_bit(R5_ALLOC_MORE,
> >  						&conf->cache_state);
> > +					md_wakeup_thread(conf->mddev->thread);
> > +				}
> >  			}
> >  			if (noblock && sh == NULL)
> >  				break;
> 
> Thanks for reviewing my code !!!
> 
> I'm not exactly against this patch, but I wonder if it is really needed.
> R5_ALLOC_MORE is really just a hint - "You can allocate another stripe if you
> like".  If the array is at all busy, raid5d will be called fairly often and
> the allocation will happen.  If the array is idle, it doesn't matter if
> memory is allocated for a while.  Does it?

So this is related to the usage in my raid5 cache patch. Say I need
allocate 100 stripes and dispatch them together. If there are only 99
free stripes, I want to allocate a new one, but the 99 stripes will not
be handled and freed, we can't steal one from the 99 stripes. In this
case, I hope the automatically stripe allocation is reliable. Handling
the stripes together is to reduce disk cache flush. I can workaround the
issue in other way (by increasing min stripe number), but thought making
the allocation reliable is better.

Thanks,
Shaohua
--
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
[prev in list] [next in list] [prev in thread] [next in thread]