On Wed, Mar 15 2006, Andreas Mohr wrote:
> Hello all,
>
> this is a small optimization to cfq_choose_req() in the CFQ I/O scheduler
> (this function is a semi-often invoked candidate in an oprofile log):
> by using a bit mask variable, we can use a simple switch() to check
> the various cases instead of having to query two variables for each check.
> Benefit: 251 vs. 285 bytes footprint of cfq_choose_req().
Not much saved, but every byte is worth while and the code is cleaner.
> Also, common case 0 (no request wrapping) is now checked first in code.
> During some heavy testing on a single-HDD UP P3/700 my instrumentation showed
> that case 0 was the *only* thing occurring, no request wrapping ever inside
> this function.
> Is that expected behaviour??
Hmm that does sound a little strange - care to do the same
instrumentation for 'as' to see if this is just a 'cfq' anomaly?
> --- linux-2.6.16-rc6-mm1/block/cfq-iosched.c.orig 2006-03-15 18:09:08.000000000 +0100
> +++ linux-2.6.16-rc6-mm1/block/cfq-iosched.c 2006-03-15 18:09:43.000000000 +0100
> @@ -96,6 +96,9 @@
> #define ASYNC (0)
> #define SYNC (1)
>
> +#define CFQ_RQ1_WRAP 0x01 /* request 1 wraps */
> +#define CFQ_RQ2_WRAP 0x02 /* request 2 wraps */
Please put these near where they are used, it's a little confusing to
have to go look for them. They only make sense in cfq_choose_req().
> +
> #define cfq_cfqq_dispatched(cfqq) \
> ((cfqq)->on_dispatch[ASYNC] + (cfqq)->on_dispatch[SYNC])
>
> @@ -361,15 +364,15 @@
>
> /*
> * Lifted from AS - choose which of crq1 and crq2 that is best served now.
> - * We choose the request that is closest to the head right now. Distance
> + * We choose the request that is closest to the head right now. Distances
> * behind the head are penalized and only allowed to a certain extent.
I sort-of prefer 'distance' here, you may want to change that to an 'is'
though :-)
> */
> static struct cfq_rq *
> cfq_choose_req(struct cfq_data *cfqd, struct cfq_rq *crq1, struct cfq_rq *crq2)
> {
> sector_t last, s1, s2, d1 = 0, d2 = 0;
> - int r1_wrap = 0, r2_wrap = 0; /* requests are behind the disk head */
> unsigned long back_max;
> + unsigned wrap = 0; /* bit mask: requests behind the disk head? */
>
> if (crq1 == NULL || crq1 == crq2)
> return crq2;
> @@ -401,38 +404,42 @@
> else if (s1 + back_max >= last)
> d1 = (last - s1) * cfqd->cfq_back_penalty;
> else
> - r1_wrap = 1;
> + wrap |= CFQ_RQ1_WRAP;
>
> if (s2 >= last)
> d2 = s2 - last;
> else if (s2 + back_max >= last)
> d2 = (last - s2) * cfqd->cfq_back_penalty;
> else
> - r2_wrap = 1;
> + wrap |= CFQ_RQ2_WRAP;
>
> /* Found required data */
> - if (!r1_wrap && r2_wrap)
> - return crq1;
> - else if (!r2_wrap && r1_wrap)
> - return crq2;
> - else if (r1_wrap && r2_wrap) {
> - /* both behind the head */
> - if (s1 <= s2)
> - return crq1;
> - else
> - return crq2;
> - }
>
> - /* Both requests in front of the head */
> - if (d1 < d2)
> - return crq1;
> - else if (d2 < d1)
> - return crq2;
> - else {
> - if (s1 >= s2)
> + /* by doing switch() on the bit mask "wrap" we avoid having to
> + * check two variables for all permutations: --> faster! */
Multiple line comments have /* and */ on a separate line.
> + switch (wrap) {
> + case 0: /* common case: crq1 and crq2 not wrapped */
> + if (d1 < d2)
> + return crq1;
> + else if (d2 < d1)
> + return crq2;
> + else {
> + if (s1 >= s2)
> + return crq1;
> + else
> + return crq2;
> + }
> +
> + case CFQ_RQ2_WRAP:
> return crq1;
> - else
> + case CFQ_RQ1_WRAP:
> return crq2;
> + case (CFQ_RQ1_WRAP|CFQ_RQ2_WRAP): /* both crqs wrapped */
> + default:
> + if (s1 <= s2)
> + return crq1;
> + else
> + return crq2;
> }
> }
Andrew will ask you to move that 'case' to be lined up with the
'switch'.
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
