Re: [PATCH 14/14] skd: remove skd_bio code

[Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>]

Hi Ramprasad,

On Tuesday, October 01, 2013 06:46:00 PM Ramprasad C wrote:
> Hi Bartlomiej,
> 
> Thank for your contributions to skd driver.
> 
> The reason why skd driver has two code paths is:-
> 1. For some workloads, Bio code path (skd_bio=1) performs better as
> compared to default code, like random r/w, high queue depths, direct i/o
> etc. In Bio code path, driver bypasses linux block elevator + scheduler
> layers and 'struct bio' comes directly to driver through make_request_fn().

Have you tried using "noop" I/O scheduler to alleviate the issue?

You need the kernel compiled with CONFIG_IOSCHED_NOOP=y (it is enabled by
default) and then do i.e. "echo noop > /sys/block/skd0/queue/scheduler" to
enable "noop" I/O scheduler for the skd0 block device.

> 2. For some workloads like buffered mode (app) i/o, the default code path
> performs much better. Hence, both the code paths are kept with module
> parameter. We would prefer to have both code paths.

It would be very useful to see some numbers for skd_bio=1 and skd_bio=0
(with "noop" I/O scheduler enabled) for the affected workloads to see what
kind of improvement is offered by skd_bio=1.

If the block layer queuing code is a problem even with "noop" I/O scheduler
it would be much better try to identify and fix (if possible) the problem
parts in the generic block layer code. This would benefit all SSD hardware
used on Linux not only the one supported by skd driver.

Also it is very inflexible to require the driver re-load just to optimize
it for some workload. I believe that a better solution (runtime selection
of the mode of operation at the block layer level) can be provided if
really necessary.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Regards,
> Ramprasad
> 
> 
> 
> On Mon, Sep 30, 2013 at 6:55 PM, Bartlomiej Zolnierkiewicz <
> b.zolnierkie@samsung.com> wrote:
> 
> > skd_bio feature adds a possibility to use the internal bio list to process
> > requests instead of using the normal block layer queueing functionality.
> > Its potential advantages are unclear and if there are any it is better to
> > identify and fix the block layer code deficiences instead. Moreover it
> > introduces separate code-paths through the whole driver which are difficult
> > to test properly and maintain in the long-term. Since it is currently not
> > used unless explicitly enabled by module parameter just remove it.
> >
> > Cc: Akhil Bhansali <abhansali@stec-inc.com>
> > Cc: Jeff Moyer <jmoyer@redhat.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/block/skd_main.c | 605
> > ++++++-----------------------------------------
> >  1 file changed, 76 insertions(+), 529 deletions(-)