lkml.org 
[lkml]   [2012]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/4] block: Optionally snapshot page contents to provide stable pages during write
    On Thu 13-12-12 00:08:11, Darrick J. Wong wrote:
    > Several complaints have been received regarding long file write latencies when
    > memory pages must be held stable during writeback. Since it might not be
    > acceptable to stall programs for the entire duration of a page write (which may
    > take many milliseconds even on good hardware), enable a second strategy wherein
    > pages are snapshotted as part of submit_bio; the snapshot can be held stable
    > while writes continue.
    >
    > This provides a band-aid to provide stable page writes on jbd without needing
    > to backport the fixed locking scheme in jbd2. A mount option is added to ext4
    > to allow administrators to enable it there.
    >
    > For those wondering about the ext3 bandage -- fixing the jbd locking (which was
    > done as part of ext4dev years ago) is a lot of surgery, and setting
    > PG_writeback on data pages when we actually hold the page lock dropped ext3
    > performance by nearly an order of magnitude. If we're going to migrate iscsi
    > and raid to use stable page writes, the complaints about high latency will
    > likely return. We might as well centralize their page snapshotting thing to
    > one place.
    Umm, I kind of like this solution for ext3...

    > diff --git a/mm/Kconfig b/mm/Kconfig
    > index a3f8ddd..78db0e1 100644
    > --- a/mm/Kconfig
    > +++ b/mm/Kconfig
    > @@ -224,6 +224,16 @@ config BOUNCE
    > def_bool y
    > depends on BLOCK && MMU && (ZONE_DMA || HIGHMEM)
    >
    > +# On the 'tile' arch, USB OHCI needs the bounce pool since tilegx will often
    > +# have more than 4GB of memory, but we don't currently use the IOTLB to present
    > +# a 32-bit address to OHCI. So we need to use a bounce pool instead.
    > +#
    > +# We also use the bounce pool to provide stable page writes for users that
    > +# don't (or can't) afford the wait latency.
    > +config NEED_BOUNCE_POOL
    > + bool
    > + default y if (TILE && USB_OHCI_HCD) || (BLK_DEV_INTEGRITY && (EXT3_FS || EXT4_FS))
    > +
    This means that NEED_BOUNCE_POOL is going to be enabled on pretty much
    any distro kernel...

    > config NR_QUICK
    > int
    > depends on QUICKLIST
    > diff --git a/mm/bounce.c b/mm/bounce.c
    > index 0420867..fa11935 100644
    > --- a/mm/bounce.c
    > +++ b/mm/bounce.c
    > @@ -178,6 +178,38 @@ static void bounce_end_io_read_isa(struct bio *bio, int err)
    > __bounce_end_io_read(bio, isa_page_pool, err);
    > }
    >
    > +#ifdef CONFIG_NEED_BOUNCE_POOL
    > +static int might_snapshot_stable_page_write(struct bio **bio_orig)
    > +{
    > + return bio_data_dir(*bio_orig) == WRITE;
    > +}
    ... so might might_snapshot_stable_page_write() will be true for each
    write. And thus blk_queue_bounce() becomes considerably more expensive?
    Also calling should_snapshot_stable_pages() for every page seems to be
    stupid since its result is going to be the same for all the pages in the
    bio (well, I could imagine setups where it won't be but do we want to
    support them?).

    So cannot we just make a function like should_snapshot_stable_pages() to
    test whether we really need the bouncing, use it in blk_queue_bounce() and
    then pass the information to __blk_queue_bounce() if needed?

    Honza

    > +static int should_snapshot_stable_pages(struct page *page, int rw)
    > +{
    > + struct backing_dev_info *bdi;
    > + struct address_space *mapping = page_mapping(page);
    > +
    > + if (!mapping)
    > + return 0;
    > + bdi = mapping->backing_dev_info;
    > + if (!bdi_cap_stable_pages_required(bdi))
    > + return 0;
    > +
    > + return mapping->host->i_sb->s_flags & MS_SNAP_STABLE &&
    > + rw == WRITE;
    > +}
    > +#else
    > +static int might_snapshot_stable_page_write(struct bio **bio_orig)
    > +{
    > + return 0;
    > +}
    > +
    > +static int should_snapshot_static_pages(struct page *page, int rw)
    > +{
    > + return 0;
    > +}
    > +#endif /* CONFIG_NEED_BOUNCE_POOL */
    > +
    > static void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig,
    > mempool_t *pool)
    > {
    > @@ -192,7 +224,8 @@ static void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig,
    > /*
    > * is destination page below bounce pfn?
    > */
    > - if (page_to_pfn(page) <= queue_bounce_pfn(q))
    > + if (page_to_pfn(page) <= queue_bounce_pfn(q) &&
    > + !should_snapshot_stable_pages(page, rw))
    > continue;
    >
    > /*
    > @@ -284,7 +317,8 @@ void blk_queue_bounce(struct request_queue *q, struct bio **bio_orig)
    > * don't waste time iterating over bio segments
    > */
    > if (!(q->bounce_gfp & GFP_DMA)) {
    > - if (queue_bounce_pfn(q) >= blk_max_pfn)
    > + if (queue_bounce_pfn(q) >= blk_max_pfn &&
    > + !might_snapshot_stable_page_write(bio_orig))
    > return;
    > pool = page_pool;
    > } else {
    > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
    > index 3e4a8cc..fbd8efb 100644
    > --- a/mm/page-writeback.c
    > +++ b/mm/page-writeback.c
    > @@ -2291,6 +2291,10 @@ void wait_for_stable_page(struct page *page)
    >
    > if (!bdi_cap_stable_pages_required(bdi))
    > return;
    > +#ifdef CONFIG_NEED_BOUNCE_POOL
    > + if (mapping->host->i_sb->s_flags & MS_SNAP_STABLE)
    > + return;
    > +#endif /* CONFIG_NEED_BOUNCE_POOL */
    >
    > wait_on_page_writeback(page);
    > }
    >
    --
    Jan Kara <jack@suse.cz>
    SUSE Labs, CR


    \
     
     \ /
      Last update: 2012-12-17 12:01    [W:4.149 / U:0.192 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site