On Wed, 17 Dec 2014 10:49:33 -0500 Nicholas Krause <[email protected]>
wrote:
> Removes the no longer needed fix me comment related to not needing the agrument faster for the function,
> sync_request. Due to getting warnings when building without the agrument it seems that this fix me was
> wrong and we must keep this agrument in order to avoid build warnings when building without it.
>
> Signed-off-by: Nicholas Krause <[email protected]>
> ---
> drivers/md/raid5.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c1b0d52..c0fde2a 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4986,7 +4986,6 @@ ret:
> return reshape_sectors;
> }
>
> -/* FIXME go_faster isn't used */
> static inline sector_t sync_request(struct mddev *mddev, sector_t sector_nr, int *skipped, int go_faster)
> {
> struct r5conf *conf = mddev->private;
Why do you think the comment is no longer needed. Is "go_faster" being used
now? Is there a clear explanation somewhere of why "go_faster" should not be
used?
NeilBrown
On Thu, 18 Dec 2014 10:19:06 +1100, NeilBrown said:
> On Wed, 17 Dec 2014 10:49:33 -0500 Nicholas Krause <[email protected]> wrote:
>
> > Removes the no longer needed fix me comment related to not needing the agrument faster for the function,
> > sync_request. Due to getting warnings when building without the agrument it seems that this fix me was
> > wrong and we must keep this agrument in order to avoid build warnings when building without it.
Examining raid1.c, which *does* use the go_faster parameter, I suspect
that the *real* meaning of the FIXME is that the code in raid5.c *should*
possibly be doing this that's done in raid1.c's sync_request():
/*
* If there is non-resync activity waiting for a turn,
* and resync is going fast enough,
* then let it though before starting on this new sync request.
*/
if (!go_faster && conf->nr_waiting)
msleep_interruptible(1000);
(Which is one of the few flow differences for the sync_request() code
in raid1.c versus raid5.c. I'll however defer to actual RAID experts
on whether raid5 needs resync throttling, or if it's accomplished via
other means I'm not spotting. Finding a FIXME outside of drivers/staging
is usually a "Here be fearsome and nasty dragons" comment....