Subject: Re: Changing the I/O scheduler on-the-fly
To: Juan RP <juan@xtrarom.org>
From: Julio M. Merino Vidal <jmmv84@gmail.com>
List: tech-kern
Date: 09/07/2005 13:11:09
On 9/7/05, Juan RP <juan@xtrarom.org> wrote:
>=20
> Hi,
>=20
> I'm trying to implement this feature... anyway I'm in the point that
> I don't know how to fix the current problem:
>=20
> It won't free the remaining buffers before switching to the new
> scheduler (a big problem!).

Maybe you can look at how the file-systems flush all pending
activity in them in their unmount function.  It may not be of help
here (since it's specific to a file-system, not a drive), though.

> Please take a look at my unfinished patch:

+=09=09printf("%s: Scheduler fcfs (First Come First Serve)\n",
+=09=09=09=09dvname);

"Scheduler fcfs"[*] sounds wired.  I'd reverse the order, as in:
%s: fcfs scheduler.  Also, I don't think the list should include a
description of what fcfs is, because users wanting to switch
schedulers will probably know what it stands for (so it only
adds noise to the output).  However, this information could be
added to some manual page, where we'd eventually describe
all the algorithms in good depth (something that'd be certainly
nice).

[*] Extend to all other algorithms, not only fcfs.

+int disk_schedulers(int *bits)
+{

You have two function declarations like this one.  Please keep
the return type in its own line.

@@ -1439,7 +1448,6 @@
=20
 =09=09return (dkwedge_list(&wd->sc_dk, dkwl, p));
 =09    }
-
 =09default:
 =09=09return ENOTTY;
 =09}

This change shouldn't be there.

+=09=09bufq_alloc(&wd->sc_q, bits|BUFQ_SORT_RAWBLOCK);

Missing spaces around the | binary operator.  See style.

Cheers,

--=20
Julio M. Merino Vidal <jmmv84@gmail.com>
http://www.livejournal.com/users/jmmv/
The NetBSD Project - http://www.NetBSD.org/