Re: [PATCH] make blkfront/blktagp2 respect the elevator=xyz command line option

[Keir Fraser <keir.fraser@eu.citrix.com>]

Should we bother to call elevator_init() at all? The call has been in that
2.6 driver forever, and there's probably no great reason. Shall we just kill
it?

 -- Keir

On 09/10/2009 17:31, "Paolo Bonzini" <pbonzini@redhat.com> wrote:

> For some workloads, CFQ has better performance than the no-op scheduler
> that is forced by the blkfront driver.  The only way to set a different
> scheduler is the sysfs interface, because elevator_init is called
> unconditionally.  This patch allows one to use "elevator=cfq" as well.
> 
> While one could argue that the driver's behavior is expected (after all
> "elevator=cfq" is the default and should not have any effect), the
> do-what-I-mean behavior implemented by this patch is more logical.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/elevator.c             |    3 ++-
>  drivers/xen/blkfront/vbd.c   |   11 ++++++-----
>  drivers/xen/blktap2/device.c |   11 ++++++-----
>  3 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/block/elevator.c b/block/elevator.c
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -132,7 +132,8 @@
> eq->elevator_data = data;
>  }
>  
> -static char chosen_elevator[16];
> +char chosen_elevator[16];
> +EXPORT_SYMBOL_GPL(chosen_elevator);
>  
>  static int __init elevator_setup(char *str)
>  {
> diff --git a/drivers/xen/blkfront/vbd.c b/drivers/xen/blkfront/vbd.c
> --- a/drivers/xen/blkfront/vbd.c
> +++ b/drivers/xen/blkfront/vbd.c
> @@ -208,6 +208,8 @@
> /* XXX: release major if 0 */
>  }
>  
> +extern char chosen_elevator[];
> +
>  static int
>  xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size)
>  {
> @@ -217,11 +219,10 @@
> if (rq == NULL)
> return -1;
>  
> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,10)
> - elevator_init(rq, "noop");
> -#else
> - elevator_init(rq, &elevator_noop);
> -#endif
> + /* Always respect the user's explicitly chosen elevator, but otherwise
> +    pick a different default than CONFIG_DEFAULT_IOSCHED.  */
> + if (!*chosen_elevator)
> +  elevator_init(rq, "noop");
>  
> /* Hard sector size and max sectors impersonate the equiv. hardware. */
> blk_queue_hardsect_size(rq, sector_size);
> diff --git a/drivers/xen/blktap2/device.c b/drivers/xen/blktap2/device.c
> --- a/drivers/xen/blktap2/device.c
> +++ b/drivers/xen/blktap2/device.c
> @@ -1034,6 +1034,8 @@
> return 0;
>  }
>  
> +extern char chosen_elevator[];
> +
>  int
>  blktap_device_create(struct blktap *tap)
>  {
> @@ -1078,11 +1080,10 @@
> if (!rq)
> goto error;
>  
> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,10)
> - elevator_init(rq, "noop");
> -#else
> - elevator_init(rq, &elevator_noop);
> -#endif
> + /* Always respect the user's explicitly chosen elevator, but otherwise
> +    pick a different default than CONFIG_DEFAULT_IOSCHED.  */
> + if (!*chosen_elevator)
> +  elevator_init(rq, "noop");
>  
> gd->queue     = rq;
> rq->queuedata = dev;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel