linux-kernel - Re: [PATCH 1/3] direct-io: add a hook for the fs to provide its own bio merging check function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1290012777-sup-9103@think>
Date:	Wed, 17 Nov 2010 11:55:28 -0500
From:	Chris Mason <chris.mason@...cle.com>
To:	Josef Bacik <josef@...hat.com>
Cc:	Miao Xie <miaox@...fujitsu.com>, viro <viro@...iv.linux.org.uk>,
	Linux Fsdevel <linux-fsdevel@...r.kernel.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux Btrfs <linux-btrfs@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ito <t-itoh@...fujitsu.com>
Subject: Re: [PATCH 1/3] direct-io: add a hook for the fs to provide its own bio merging check function

Excerpts from Josef Bacik's message of 2010-11-17 07:50:11 -0500:
> On Wed, Nov 17, 2010 at 06:11:03PM +0800, Miao Xie wrote:
> > Hi, Josef
> >
> > On wed, 17 Nov 2010 04:37:21 -0500, Josef Bacik wrote:
> >>> Heh so I was going to fix this after the hole punching stuff.  The fact is btrfs
> >>> maps everything that is ok to do in one IO via get_blocks().  So all we need to
> >>> do is add another DIO_ flag to tell us to treat each get_blocks() call as
> >>> discrete.  I wanted to use buffer_boundary for this, but I think it's too
> >>> drastic of a change for people who already use buffer_boundary();
> >>>
> >>> What happens today is that say we map 4k, we do submit_page_section, but if this
> >>> is our first bit of IO we just set dio->cur_page and such and then loop again.
> >>> Say there is 4k-hole-4k, we do the next mapping and set buffer_boundary again,
> >>> and come into submit_page_section and because cur_page is set, we do
> >>> dio_send_cur_page.  Because there is no dio->bio we setup a new bio, but when we
> >>> do that we clear dio->boundary, and leave the bio all setup.  So the next time
> >>> we loop around the tail 4k gets added to our previously setup bio and boom we
> >>> hit this problem with btrfs.
> >>>
> >>> If we can add a DIO_GET_BLOCKS_DISCRETE or some other such non-sense then we can
> >>> easily kill all the logical offset code I had and just make some simple changes
> >>> to make the DIO stuff work for us.  All we do is in get_more_blocks we do
> >>>
> >>> if ((dio->flags&  DIO_GET_BLOCKS_DISCRETE)&&  dio->bio)
> >>>     dio_submit_bio(dio);
> >>>
> >>
> >> Right after I went to bed I realized this should be
> >>
> >> if (dio->flags&  DIO_GET_BLOCKS_DISCRETE) {
> >>     if (dio->cur_page) {
> >>         dio_send_cur_page(dio);
> >>         page_cache_release(dio->cur_page);
> >>         dio->cur_page = NULL;
> >>     }
> >>
> >>     if (dio->bio)
> >>         dio_submit_bio(dio);
> >>   }
> >
> > As far as I know, get_block() can not make sure the IO doesn't span the chunks or
> > stripes. Maybe we can do this check in get_blocks(). In this way, we needn't change
> > vfs.
> >
> 
> Right thats the idea, if we can't span chunks/stripes we should be doing that
> limiting in our get_blocks call and that way we don't have to screw with the
> generic direct io stuff too much.  Thanks,

In this case we're adding complexity to the O_DIRECT mapping code, when
we really should be adding it to the btrfs submit bio hook.  It can
easily break up the bio into smaller units, which will leave us with a
smaller number of get_blocks calls overall.

I'm working that out now.

-chris


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/