lkml.org 
[lkml]   [2008]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/2] lld busy status exporting interface
    From
    Hi Andrew,

    On Fri, 19 Sep 2008 14:33:44 -0700, Andrew Morton wrote:
    > On Fri, 19 Sep 2008 10:48:54 -0400 (EDT)
    > Kiyoshi Ueda <k-ueda@ct.jp.nec.com> wrote:
    >
    > > This patch adds an interface to check lld's busy status
    > > from the block layer.
    > > This resolves a performance problem on request stacking devices below.
    > >
    > >
    > > Some drivers like scsi mid layer stop dispatching request when
    > > they detect busy state on its low-level device like host/bus/device.
    > > It allows other requests to stay in the I/O scheduler's queue
    > > for a chance of merging.
    > >
    > > Request stacking drivers like request-based dm should follow
    > > the same logic.
    > > However, there is no generic interface for the stacked device
    > > to check if the underlying device(s) are busy.
    > > If the request stacking driver dispatches and submits requests to
    > > the busy underlying device, the requests will stay in
    > > the underlying device's queue without a chance of merging.
    > > This causes performance problem on burst I/O load.
    > >
    > > With this patch, busy state of the underlying device is exported
    > > via the state flag of queue's backing_dev_info. So the request
    > > stacking driver can check it and stop dispatching requests if busy.
    > >
    > > The underlying device driver must set/clear the flag appropriately:
    > > ON: when the device driver can't process requests immediately.
    > > OFF: when the device driver can process requests immediately,
    > > including abnormal situations where the device driver needs
    > > to kill all requests.
    > >
    > >
    > > Signed-off-by: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
    > > Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
    > > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
    > > ---
    > > include/linux/backing-dev.h | 8 ++++++++
    > > mm/backing-dev.c | 12 ++++++++++++
    > > 2 files changed, 20 insertions(+)
    > >
    > > Index: scsi-misc-2.6/include/linux/backing-dev.h
    > > ===================================================================
    > > --- scsi-misc-2.6.orig/include/linux/backing-dev.h
    > > +++ scsi-misc-2.6/include/linux/backing-dev.h
    > > @@ -26,6 +26,7 @@ enum bdi_state {
    > > BDI_pdflush, /* A pdflush thread is working this device */
    > > BDI_write_congested, /* The write queue is getting full */
    > > BDI_read_congested, /* The read queue is getting full */
    > > + BDI_lld_congested, /* The device/host is busy */
    > > BDI_unused, /* Available bits start here */
    > > };
    > >
    > > @@ -226,8 +227,15 @@ static inline int bdi_rw_congested(struc
    > > (1 << BDI_write_congested));
    > > }
    > >
    > > +static inline int bdi_lld_congested(struct backing_dev_info *bdi)
    > > +{
    > > + return bdi_congested(bdi, 1 << BDI_lld_congested);
    > > +}
    > > +
    > > void clear_bdi_congested(struct backing_dev_info *bdi, int rw);
    > > void set_bdi_congested(struct backing_dev_info *bdi, int rw);
    > > +void clear_bdi_lld_congested(struct backing_dev_info *bdi);
    > > +void set_bdi_lld_congested(struct backing_dev_info *bdi);
    > > long congestion_wait(int rw, long timeout);
    > >
    > >
    > > Index: scsi-misc-2.6/mm/backing-dev.c
    > > ===================================================================
    > > --- scsi-misc-2.6.orig/mm/backing-dev.c
    > > +++ scsi-misc-2.6/mm/backing-dev.c
    > > @@ -279,6 +279,18 @@ void set_bdi_congested(struct backing_de
    > > }
    > > EXPORT_SYMBOL(set_bdi_congested);
    > >
    > > +void clear_bdi_lld_congested(struct backing_dev_info *bdi)
    > > +{
    > > + clear_bit(BDI_lld_congested, &bdi->state);
    > > +}
    > > +EXPORT_SYMBOL_GPL(clear_bdi_lld_congested);
    > > +
    > > +void set_bdi_lld_congested(struct backing_dev_info *bdi)
    > > +{
    > > + set_bit(BDI_lld_congested, &bdi->state);
    > > +}
    > > +EXPORT_SYMBOL_GPL(set_bdi_lld_congested);
    > > +
    > > /**
    > > * congestion_wait - wait for a backing_dev to become uncongested
    > > * @rw: READ or WRITE
    >
    > Is this really the right way to do it?

    I think so, but I may not understand what you mean correctly.
    So please elaborate your concern if my explanation below doesn't
    satisfy what you want to know.


    > Back in the days when we first did the backing_dev_info.congested_fn()
    > logic it was decided that there basically was no single place in which
    > the congested state could be stored.
    >
    > So we ended up deciding that whenever a caller wants to know a
    > backing_dev's congested status, it has to call in to the
    > ->congested_fn() and that congested_fn would then call down into all
    > the constituent low-level drivers/queues/etc asking each one if it is
    > congested.

    bdi_lld_congested() also does that using bdi_congested(), which calls
    ->congested_fn().
    And only real device drivers (e.g. scsi, ide) set/clear the flag.
    Stacking drivers like request-based dm don't.
    So stacking drivers always check the BDI_lld_congested flag of
    the bottom device of the device stack.

    BDI_[write|read]_congested flags have been using for queue's
    congestion, so that I/O queueing/merging can be proceeded even if
    the lld is congested. So I added a new flag.


    > I mean, as a simple example which is probably wrong - what happens if a
    > single backing_dev is implemented via two different disks and
    > controllers, and they both become congested and then one of them comes
    > uncongested. Is there no way in which the above implemention can
    > incorrectly flag the backing_dev as being uncongested?

    Do you mean that "a single backing_dev via two disks/controllers" is
    a dm device (e.g. a dm-multipath device having 2 paths, a dm-mirror
    device having 2 disks)?

    If so, dm doesn't set/clear the flag, and the decision, whether
    the dm device itself is congested or not, depends on dm's target driver.

    For example of dm-multipath,
    o call bdi_lld_congested() for each path.
    o if one of the paths are uncongested, dm-multipath will decide
    the dm device is uncongested and dispatch incoming I/Os to
    the uncongested path.

    For example of dm-mirror,
    o call bdi_lld_congested() for each disk.
    o if the incoming I/O is READ, same logic as dm-multipath above.
    if the incoming I/O is WRITE, dm-mirror will decide the dm device
    is uncongested only when all disks are uncongested.

    Thanks,
    Kiyoshi Ueda


    \
     
     \ /
      Last update: 2008-09-20 01:33    [W:0.030 / U:0.580 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site