Prev: fs: kill blockdev_direct_IO_no_locking
Next: 2.6.35 merge window is over for USB
From: Christoph Hellwig on 21 May 2010 11:30
On Wed, May 19, 2010 at 04:24:51PM -0400, Josef Bacik wrote:
> Btrfs cannot handle having logically non-contiguous requests submitted. For
> example if you have
>
> Logical: [0-4095][HOLE][8192-12287]
> Physical: [0-4095] [4096-8191]
>
> Normally the DIO code would put these into the same BIO's. The problem is we
> need to know exactly what offset is associated with what BIO so we can do our
> checksumming and unlocking properly, so putting them in the same BIO doesn't
> work. So add another check where we submit the current BIO if the physical
> blocks are not contigous OR the logical blocks are not contiguous.

This gets us slightly less optimal I/O patters for other filesystems in
this case. But it's probably corner case enough to not care and make it
the default.

But please make the comment in the comment as verbose as the commit
message so that people understand why we're doing this when reading the
code in a few years.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Josef Bacik on 21 May 2010 11:40
On Fri, May 21, 2010 at 11:21:11AM -0400, Christoph Hellwig wrote:
> On Wed, May 19, 2010 at 04:24:51PM -0400, Josef Bacik wrote:
> > Btrfs cannot handle having logically non-contiguous requests submitted. For
> > example if you have
> >
> > Logical: [0-4095][HOLE][8192-12287]
> > Physical: [0-4095] [4096-8191]
> >
> > Normally the DIO code would put these into the same BIO's. The problem is we
> > need to know exactly what offset is associated with what BIO so we can do our
> > checksumming and unlocking properly, so putting them in the same BIO doesn't
> > work. So add another check where we submit the current BIO if the physical
> > blocks are not contigous OR the logical blocks are not contiguous.
>
> This gets us slightly less optimal I/O patters for other filesystems in
> this case. But it's probably corner case enough to not care and make it
> the default.
>
> But please make the comment in the comment as verbose as the commit
> message so that people understand why we're doing this when reading the
> code in a few years.
>

So after I sent this I thought that maybe I could make that test _only_ if we
provide submit_bio, that way it only affects btrfs and not everybody else, would
you prefer I do something like that? I will make the commit log a bit more
verbose. Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on 21 May 2010 11:40
On Fri, May 21, 2010 at 11:37:45AM -0400, Josef Bacik wrote:
> So after I sent this I thought that maybe I could make that test _only_ if we
> provide submit_bio, that way it only affects btrfs and not everybody else, would
> you prefer I do something like that? I will make the commit log a bit more
> verbose. Thanks,

Again, I think it's not really worth optimizing for. Just document very
clearly why it's there.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Josef Bacik on 22 May 2010 10:00
On Fri, May 21, 2010 at 06:47:36PM -0700, Mike Fedyk wrote:
> On Fri, May 21, 2010 at 10:03 AM, Josef Bacik <josef(a)redhat.com> wrote:
> > Btrfs cannot handle having logically non-contiguous requests submitted. �For
> > example if you have
> >
> > Logical: �[0-4095][HOLE][8192-12287]
> > Physical: [0-4095] � � �[4096-8191]
> >
> > Normally the DIO code would put these into the same BIO's. �The problem is we
> > need to know exactly what offset is associated with what BIO so we can do our
> > checksumming and unlocking properly, so putting them in the same BIO doesn't
> > work. �So add another check where we submit the current BIO if the physical
> > blocks are not contigous OR the logical blocks are not contiguous.
> >
> > Signed-off-by: Josef Bacik <josef(a)redhat.com>
> > ---
> >
> > V1->V2
> > -Be more verbose in the in-code comment
> >
> > �fs/direct-io.c | � 20 ++++++++++++++++++--
> > �1 files changed, 18 insertions(+), 2 deletions(-)
> >
>
> Btrfs has been pretty much self-contained (working well compiled
> against 2.6.32 for example). Is there a way that this wouldn't just
> start silently breaking for people compiling the latest btrfs with
> dkms against older kernels?

Nope, you have to have these generic patches for DIO to work, so building btrfs
like this will stop working with earlier kernels. Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: fs: kill blockdev_direct_IO_no_locking
Next: 2.6.35 merge window is over for USB