linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Martijn Coenen <maco@android.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block <linux-block@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-team@android.com, Narayan Kamath <narayan@google.com>,
	Dario Freni <dariofreni@google.com>,
	Nikita Ioffe <ioffe@google.com>, Jiyong Park <jiyong@google.com>,
	Martijn Coenen <maco@google.com>,
	Greg KH <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] loop: change queue block size to match when using DIO.
Date: Mon, 2 Sep 2019 21:37:39 +0200	[thread overview]
Message-ID: <CAB0TPYELq72hjHvyFVmaAFZPCaSSxV-j6znM8peezqtep6i-1A@mail.gmail.com> (raw)
In-Reply-To: <20190830155024.GA23882@infradead.org>

On Fri, Aug 30, 2019 at 5:50 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Aug 28, 2019 at 12:32:29PM +0200, Martijn Coenen wrote:
> > The loop driver assumes that if the passed in fd is opened with
> > O_DIRECT, the caller wants to use direct I/O on the loop device.
> > However, if the underlying filesystem has a different block size than
> > the loop block queue, direct I/O can't be enabled. Instead of requiring
> > userspace to manually change the blocksize and re-enable direct I/O,
> > just change the queue block size to match.
>
> Why can't we enable the block device in that case?  All the usual
> block filesystems support 512 byte aligned direct I/O with a 4k
> file system block size (as long as the underlying block device
> sector size is also 512 bytes).

Sorry, I didn't word that correctly: it's not the logical block size
of the filesystem, but the logical block size of the underlying block
device that loop's queue must match (or exceed). With the current loop
code, if the backing file is opened with O_DIRECT and resides on a
block device with a 512 bytes logical block size, the loop device will
correctly use direct I/O. If instead the backing file happened to
reside on a block device with a 4k logical block size, the loop device
would silently fall back to cached mode. I think there's a benefit in
the behavior being consistent independent of the block size of the
backing device, and I don't see a good reason for not automatically
switching loop's logical/physical queue sizes to match the backing
device in this specific case.

Will send a v2.

Thanks,
Martijn

  reply	other threads:[~2019-09-02 19:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 10:32 [PATCH] loop: change queue block size to match when using DIO Martijn Coenen
2019-08-30 15:50 ` Christoph Hellwig
2019-09-02 19:37   ` Martijn Coenen [this message]
2019-09-04 19:49 ` [PATCH v2] " Martijn Coenen
2019-09-10 10:12   ` Martijn Coenen
2019-09-30  8:24   ` Christoph Hellwig
2019-10-01 10:03     ` Martijn Coenen
2019-10-01 14:10   ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAB0TPYELq72hjHvyFVmaAFZPCaSSxV-j6znM8peezqtep6i-1A@mail.gmail.com \
    --to=maco@android.com \
    --cc=axboe@kernel.dk \
    --cc=dariofreni@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=ioffe@google.com \
    --cc=jiyong@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maco@google.com \
    --cc=narayan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).