Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1201585 > unrolled thread

[PATCH v9 0/6] block: loop: improve loop with AIO

Back to article view | Back to linux.kernel

Contents

  [PATCH v9 0/6] block: loop: improve loop with AIO Ming Lei <ming.lei@canonical.com> - 2015-08-06 10:50 +0200
    [PATCH v9 6/6] block: loop: support DIO & AIO Ming Lei <ming.lei@canonical.com> - 2015-08-06 10:50 +0200
      Re: [PATCH v9 6/6] block: loop: support DIO & AIO Christoph Hellwig <hch@infradead.org> - 2015-08-07 09:50 +0200
        Re: [PATCH v9 6/6] block: loop: support DIO & AIO Ming Lei <ming.lei@canonical.com> - 2015-08-07 10:30 +0200
          Re: [PATCH v9 6/6] block: loop: support DIO & AIO Christoph Hellwig <hch@infradead.org> - 2015-08-09 08:50 +0200
            Re: [PATCH v9 6/6] block: loop: support DIO & AIO Ming Lei <ming.lei@canonical.com> - 2015-08-09 15:10 +0200
              Re: [PATCH v9 6/6] block: loop: support DIO & AIO Christoph Hellwig <hch@infradead.org> - 2015-08-09 15:50 +0200
                Re: [PATCH v9 6/6] block: loop: support DIO & AIO Ming Lei <ming.lei@canonical.com> - 2015-08-09 16:30 +0200
                  Re: [PATCH v9 6/6] block: loop: support DIO & AIO Dave Chinner <david@fromorbit.com> - 2015-08-10 01:50 +0200
                    Re: [PATCH v9 6/6] block: loop: support DIO & AIO Ming Lei <ming.lei@canonical.com> - 2015-08-10 05:00 +0200
                      Re: [PATCH v9 6/6] block: loop: support DIO & AIO Christoph Hellwig <hch@infradead.org> - 2015-08-10 10:10 +0200
                        Re: [PATCH v9 6/6] block: loop: support DIO & AIO Ming Lei <ming.lei@canonical.com> - 2015-08-10 16:40 +0200

#1201585 — [PATCH v9 0/6] block: loop: improve loop with AIO

Hi Guys,

There are about 3 advantages to use direct I/O and AIO on
read/write loop's backing file:

1) double cache can be avoided, then memory usage gets
decreased a lot

2) not like user space direct I/O, there isn't cost of
pinning pages

3) avoid context switch for obtaining good throughput
- in buffered file read, random I/O throughput is often obtained
only if they are submitted concurrently from lots of tasks; but for
sequential I/O, most of times they can be hit from page cache, so
concurrent submissions often introduce unnecessary context switch
and can't improve throughput much. There was such discussion[1]
to use non-blocking I/O to improve the problem for application.
- with direct I/O and AIO, concurrent submissions can be
avoided and random read throughput can't be affected meantime

So this patchset trys to improve loop via AIO, and about 45% memory
usage can be decreased, see detailed data in commit log of patch4,
also IO throughput isn't affected too.

Following losetup changes for supporting loop dio/aio:
        - losetup git tree:
                http://kernel.ubuntu.com/git/ming/util-linux.git/log/?h=loop-dio-v7
        - how to set direct io
                losetup --direct-io 1 /dev/loopN
        - how to clear direct io
                losetup --direct-io 0 /dev/loopN
        - how to show if direct io is used for accessing backing file
                losetup -l

V9:
        - only 4/6 and 5/6 updated
        - update __loop_update_dio()
        - serialize direct I/O with buffered I/O for avoiding
        possible race

V8:
        - only 4/6 and 5/6 updated
        - support direct io when backing device's sector size is
          512, 1024, 2048 and 4096(only 512-byte sector was supported
          in V7)
V7:
        - only 4/6 and 5/6 updated
        - update direct io after lo->offset is changed(4/6)
        - fix updating LO_FLAGS_DIRECT_IO in __loop_update_dio()(4/6)
        - introduce ioctl cmd of LOOP_SET_DIRECT_IO(5/6) for case of
          'mount -o loop'

V6:
        - only patch 4 and patch 5 get updated
        - check lo->lo_offset to decide if direct IO can be supported(4/5)
        - introduce one flag for userspace(losetup) to keep updated
        if using direct I/O to access backing file(4/5)
        - implement patches for util-linux(losetup) so that losetup can
        enable direct I/O feature:(4/5)

           http://kernel.ubuntu.com/git/ming/util-linux.git/log/?h=losetup-dio

        - remove the direct IO control interface from sysfs(4/5)
        - handle partial read in case of direct read (5/5)
        - add more comments for direct IO (5/5)
V5:
        - don't introduce IOCB_DONT_DIRTY_PAGE and bypass dirtying
        for ITER_KVEC and ITER_BVEC direct IO(read), as required by
        Christoph

V4:
        - add detailed commit log for 'use kthread_work'
        - allow userspace(sysfs, losetup) to decide if dio/aio is
        used as suggested by Christoph and Dave Chinner
        - only use dio if the backing block device's min io size
        is 512 as pointed by Dave Chinner & Christoph
V3:
        - based on Al's iov_iter work and Christoph's kiocb changes
        - use kthread_work
        - introduce IOCB_DONT_DIRTY_PAGE flag
        - set QUEUE_FLAG_NOMERGES for loop's request queue
V2:
        - remove 'extra' parameter to aio_kernel_alloc()
        - try to avoid memory allcation inside queue req callback
        - introduce 'use_mq' sysfs file for enabling kernel aio or disabling it
V1:
        - link:
                http://marc.info/?t=140803157700004&r=1&w=2
        - improve failure path in aio_kernel_submit()

 drivers/block/loop.c      | 312 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 drivers/block/loop.h      |  17 ++--
 fs/direct-io.c            |   9 +-
 include/uapi/linux/loop.h |   2 +
 4 files changed, 274 insertions(+), 66 deletions(-)

Thanks,
Ming


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

[toc] | [next] | [standalone]

#1201587 — [PATCH v9 6/6] block: loop: support DIO & AIO

There are about 3 advantages to use direct I/O and AIO on
read/write loop's backing file:

1) double cache can be avoided, then memory usage gets
decreased a lot

2) not like user space direct I/O, there isn't cost of
pinning pages

3) avoid context switch for obtaining good throughput
- in buffered file read, random I/O top throughput is often obtained
only if they are submitted concurrently from lots of tasks; but for
sequential I/O, most of times they can be hit from page cache, so
concurrent submissions often introduce unnecessary context switch
and can't improve throughput much. There was such discussion[1]
to use non-blocking I/O to improve the problem for application.
- with direct I/O and AIO, concurrent submissions can be
avoided and random read throughput can't be affected meantime

Follows my fio test result:

1. 16 jobs fio test inside ext4 file system over loop block
1) How to run
	- linux kernel: 4.1.0-rc2-next-20150506 with the patchset
	- the loop block is over one image on HDD.
	- linux psync, 16 jobs, size 400M, ext4 over loop block
	- test result: IOPS from fio output

2) Throughput result:
        -------------------------------------------------------------
        test cases          |randread   |read   |randwrite  |write  |
        -------------------------------------------------------------
        base                |240        |8705   |3763       |20914
        -------------------------------------------------------------
        base+loop aio       |242        |9258   |4577       |21451
        -------------------------------------------------------------

3) context switch
        - context switch decreased by ~16% with loop aio for randread,
	and decreased by ~33% for read

4) memory usage
	- After these four tests with loop aio: ~10% memory becomes used
	- After these four tests without loop aio: more than 55% memory
	becomes used

2. single job fio test inside ext4 file system over loop block(for Maxim Patlasov)
1) How to run
	- linux kernel: 4.1.0-rc2-next-20150506 with the patchset
	- the loop block is over one image on HDD.
	- linux psync, 1 job, size 4000M, ext4 over loop block
	- test result: IOPS from fio output

2) Throughput result:
        -------------------------------------------------------------
        test cases          |randread   |read   |randwrite  |write  |
        -------------------------------------------------------------
        base                |109        |21180  |4192       |22782
        -------------------------------------------------------------
        base+loop aio       |114        |21018  |5404       |22670
        -------------------------------------------------------------

3) context switch
        - context switch decreased by ~10% with loop aio for randread,
	and decreased by ~50% for read

4) memory usage
	- After these four tests with loop aio: ~10% memory becomes used
	- After these four tests without loop aio: more than 55% memory
	becomes used

Both 'context switch' and 'memory usage' data are got from sar.

[1] https://lwn.net/Articles/612483/
[2] sar graph when running fio over loop without the patchset
http://kernel.ubuntu.com/~ming/block/loop-aio/v3/lo-nonaio.pdf

[3] sar graph when running fio over loop with the patchset
http://kernel.ubuntu.com/~ming/block/loop-aio/v3/lo-aio.pdf

[4] sar graph when running fio over loop without the patchset
http://kernel.ubuntu.com/~ming/block/loop-aio/v3/lo-nonaio-1job.pdf

[5] sar graph when running fio over loop with the patchset
http://kernel.ubuntu.com/~ming/block/loop-aio/v3/lo-aio-1job.pdf

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/block/loop.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/block/loop.h |   5 ++
 2 files changed, 149 insertions(+), 3 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 06a737b..edaa18a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -200,6 +200,8 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
 	blk_mq_freeze_queue(lo->lo_queue);
 	lo->use_dio = use_dio;
 	lo->dio_align = dio_align;
+	lo->last_use_dio = use_dio;
+	atomic_set(&lo->pending_dio, 0);
 	if (use_dio)
 		lo->lo_flags |= LO_FLAGS_DIRECT_IO;
 	else
@@ -436,6 +438,128 @@ static int lo_req_flush(struct loop_device *lo, struct request *rq)
 	return ret;
 }
 
+static inline void handle_partial_read(struct loop_cmd *cmd, long bytes)
+{
+	if (bytes < 0 || (cmd->rq->cmd_flags & REQ_WRITE))
+		return;
+
+	if (unlikely(bytes < blk_rq_bytes(cmd->rq))) {
+		struct bio *bio = cmd->rq->bio;
+
+		bio_advance(bio, bytes);
+		zero_fill_bio(bio);
+	}
+}
+
+static void lo_rw_aio_complete(struct kiocb *iocb, long ret, long ret2)
+{
+	struct loop_cmd *cmd = container_of(iocb, struct loop_cmd, iocb);
+	struct request *rq = cmd->rq;
+	struct loop_device *lo = cmd->rq->q->queuedata;
+
+	handle_partial_read(cmd, ret);
+
+	if (ret > 0)
+		ret = 0;
+	else if (ret < 0)
+		ret = -EIO;
+
+	rq->errors = ret;
+	blk_mq_complete_request(rq);
+
+	if (atomic_dec_and_test(&lo->pending_dio)) {
+		if (lo->waiter)
+			wake_up_process(lo->waiter);
+	}
+}
+
+static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
+		     loff_t pos, bool rw)
+{
+	struct iov_iter iter;
+	struct bio_vec *bvec;
+	struct bio *bio = cmd->rq->bio;
+	struct file *file = lo->lo_backing_file;
+	int ret;
+
+	/* nomerge for loop request queue */
+	WARN_ON(cmd->rq->bio != cmd->rq->biotail);
+
+	bvec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter);
+	iov_iter_bvec(&iter, ITER_BVEC | rw, bvec,
+		      bio_segments(bio), blk_rq_bytes(cmd->rq));
+
+	cmd->iocb.ki_pos = pos;
+	cmd->iocb.ki_filp = file;
+	cmd->iocb.ki_complete = lo_rw_aio_complete;
+	cmd->iocb.ki_flags = IOCB_DIRECT;
+
+	atomic_inc(&lo->pending_dio);
+
+	if (rw == WRITE)
+		ret = file->f_op->write_iter(&cmd->iocb, &iter);
+	else
+		ret = file->f_op->read_iter(&cmd->iocb, &iter);
+
+	if (ret != -EIOCBQUEUED)
+		cmd->iocb.ki_complete(&cmd->iocb, ret, 0);
+	return 0;
+}
+
+static inline void lo_drain_pending_dio(struct loop_device *lo)
+{
+	lo->waiter = current;
+
+	/* order between writing waiter and reading pending_dio */
+	smp_mb__before_atomic();
+
+	for (;;) {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		if (!atomic_read(&lo->pending_dio))
+			break;
+		schedule();
+	}
+	set_current_state(TASK_RUNNING);
+	lo->waiter = NULL;
+}
+
+static inline int lo_rw_simple(struct loop_device *lo,
+		struct request *rq, loff_t pos, bool rw)
+{
+	struct loop_cmd *cmd = blk_mq_rq_to_pdu(rq);
+
+	/*
+	 * When working at direct I/O, under very unusual cases,
+	 * such as unaligned direct I/O from application and
+	 * access to loop block device with 'unaligned' offset & size,
+	 * wait for completion of the pending dio/aio before swiching
+	 * to buffered I/O for avoiding probable race between dio and
+	 * buffered I/O.
+	 *
+	 * Page cache flush is handled inside VFS direct I/O path if
+	 * it is needed.
+	 */
+	if (unlikely(lo->use_dio && lo->last_use_dio && !cmd->use_aio))
+		lo_drain_pending_dio(lo);
+
+	if (cmd->use_aio)
+		return lo_rw_aio(lo, cmd, pos, rw);
+
+	/*
+	 * lo_write_simple and lo_read_simple should have been covered
+	 * by io submit style function like lo_rw_aio(), one blocker
+	 * is that lo_read_simple() need to call flush_dcache_page after
+	 * the page is written from kernel, and it isn't easy to handle
+	 * this in io submit style function which submits all segments
+	 * of the req at one time. And direct read IO doesn't need to
+	 * run flush_dcache_page().
+	 */
+	if (rw == WRITE)
+		return lo_write_simple(lo, rq, pos);
+	else
+		return lo_read_simple(lo, rq, pos);
+}
+
 static int do_req_filebacked(struct loop_device *lo, struct request *rq)
 {
 	loff_t pos;
@@ -451,13 +575,13 @@ static int do_req_filebacked(struct loop_device *lo, struct request *rq)
 		else if (lo->transfer)
 			ret = lo_write_transfer(lo, rq, pos);
 		else
-			ret = lo_write_simple(lo, rq, pos);
+			ret = lo_rw_simple(lo, rq, pos, WRITE);
 
 	} else {
 		if (lo->transfer)
 			ret = lo_read_transfer(lo, rq, pos);
 		else
-			ret = lo_read_simple(lo, rq, pos);
+			ret = lo_rw_simple(lo, rq, pos, READ);
 	}
 
 	return ret;
@@ -1545,6 +1669,13 @@ int loop_unregister_transfer(int number)
 EXPORT_SYMBOL(loop_register_transfer);
 EXPORT_SYMBOL(loop_unregister_transfer);
 
+static inline bool req_dio_aligned(struct loop_device *lo,
+		const struct request *rq)
+{
+	return !((blk_rq_pos(rq) << 9) & lo->dio_align) &&
+		!(blk_rq_bytes(rq) & lo->dio_align);
+}
+
 static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 		const struct blk_mq_queue_data *bd)
 {
@@ -1556,6 +1687,13 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 	if (lo->lo_state != Lo_bound)
 		return -EIO;
 
+	if (lo->use_dio && !lo->transfer &&
+			req_dio_aligned(lo, bd->rq) &&
+			!(cmd->rq->cmd_flags & (REQ_FLUSH | REQ_DISCARD)))
+		cmd->use_aio = true;
+	else
+		cmd->use_aio = false;
+
 	queue_kthread_work(&lo->worker, &cmd->work);
 
 	return BLK_MQ_RQ_QUEUE_OK;
@@ -1571,11 +1709,14 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
 		goto failed;
 
 	ret = do_req_filebacked(lo, cmd->rq);
+	lo->last_use_dio = cmd->use_aio;
 
  failed:
 	if (ret)
 		cmd->rq->errors = -EIO;
-	blk_mq_complete_request(cmd->rq);
+	/* complete non-aio request */
+	if (!cmd->use_aio || ret)
+		blk_mq_complete_request(cmd->rq);
 }
 
 static void loop_queue_work(struct kthread_work *work)
diff --git a/drivers/block/loop.h b/drivers/block/loop.h
index 63f8e14..439054b 100644
--- a/drivers/block/loop.h
+++ b/drivers/block/loop.h
@@ -60,6 +60,9 @@ struct loop_device {
 	struct task_struct	*worker_task;
 	unsigned		dio_align;
 	bool			use_dio;
+	bool			last_use_dio;
+	atomic_t		pending_dio;
+	struct task_struct	*waiter;
 
 	struct request_queue	*lo_queue;
 	struct blk_mq_tag_set	tag_set;
@@ -70,6 +73,8 @@ struct loop_cmd {
 	struct kthread_work work;
 	struct request *rq;
 	struct list_head list;
+	bool use_aio;           /* use AIO interface to handle I/O */
+	struct kiocb iocb;
 };
 
 /* Support for loadable transfer modules */
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1202392 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

I really disagree with the per-cmd use_dio tracking.

If we know at setup time that the loop device sector size is smaller
than the sector size of the underlying device we should never allow
dio, and othewise it should always work for data.

The ->transfer check also is one to be done at setup time, and there
is no need for draining or mode checking for an fsync - FLUSH is always
only guranteed to flush out I/O that has completed by the time it's
issued.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202446 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Fri, Aug 7, 2015 at 3:43 AM, Christoph Hellwig <hch@infradead.org> wrote:
> I really disagree with the per-cmd use_dio tracking.

Could you explain it in a bit?

>
> If we know at setup time that the loop device sector size is smaller
> than the sector size of the underlying device we should never allow
> dio, and othewise it should always work for data.

Yes, that is just what I did in v7, and we can only do dio in case
of 512 byte sector size of backing device(not considering the
following patches from Hannes).

When sector size of backing device isn't 512, most of transfer(buffered I/O
and normal dio) is still 4k aligned, that is why I suggest to use per-cmd
use_dio tracking.

The patch avoids the race between buffered io and dio, doesn't it?
The introduced cost is trivial and most of times it needn't to wait for
completion of pending dio.

>
> The ->transfer check also is one to be done at setup time, and there

OK.

> is no need for draining or mode checking for an fsync - FLUSH is always
> only guranteed to flush out I/O that has completed by the time it's
> issued.

Could you point it out in the patch?

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

[toc] | [prev] | [next] | [standalone]

#1203589 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Fri, Aug 07, 2015 at 04:25:52AM -0400, Ming Lei wrote:
> On Fri, Aug 7, 2015 at 3:43 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > I really disagree with the per-cmd use_dio tracking.
> 
> Could you explain it in a bit?
> 
> >
> > If we know at setup time that the loop device sector size is smaller
> > than the sector size of the underlying device we should never allow
> > dio, and othewise it should always work for data.
> 
> Yes, that is just what I did in v7, and we can only do dio in case
> of 512 byte sector size of backing device(not considering the
> following patches from Hannes).
> 
> When sector size of backing device isn't 512, most of transfer(buffered I/O
> and normal dio) is still 4k aligned, that is why I suggest to use per-cmd
> use_dio tracking.
> 
> The patch avoids the race between buffered io and dio, doesn't it?
> The introduced cost is trivial and most of times it needn't to wait for
> completion of pending dio.

All block filesystems can do direct I/O on a _sector size_, not
_block size_ boundary, e.g. for the typical setup of a 4k block size
xfs/btrfs/ext4 file system on a 512 byte sector device you can do 512
byte aligned direct I/O.

> > is no need for draining or mode checking for an fsync - FLUSH is always
> > only guranteed to flush out I/O that has completed by the time it's
> > issued.
> 
> Could you point it out in the patch?

Basically your lo_drain_pending_dio() functionality is not needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1203638 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Sun, Aug 9, 2015 at 2:44 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, Aug 07, 2015 at 04:25:52AM -0400, Ming Lei wrote:
>> On Fri, Aug 7, 2015 at 3:43 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> > I really disagree with the per-cmd use_dio tracking.
>>
>> Could you explain it in a bit?
>>
>> >
>> > If we know at setup time that the loop device sector size is smaller
>> > than the sector size of the underlying device we should never allow
>> > dio, and othewise it should always work for data.
>>
>> Yes, that is just what I did in v7, and we can only do dio in case
>> of 512 byte sector size of backing device(not considering the
>> following patches from Hannes).
>>
>> When sector size of backing device isn't 512, most of transfer(buffered I/O
>> and normal dio) is still 4k aligned, that is why I suggest to use per-cmd
>> use_dio tracking.
>>
>> The patch avoids the race between buffered io and dio, doesn't it?
>> The introduced cost is trivial and most of times it needn't to wait for
>> completion of pending dio.
>
> All block filesystems can do direct I/O on a _sector size_, not
> _block size_ boundary, e.g. for the typical setup of a 4k block size
> xfs/btrfs/ext4 file system on a 512 byte sector device you can do 512
> byte aligned direct I/O.

It isn't the problem which lo_drain_pending_dio() in this patch is
trying to avoid/fix.

Now the logical block size of loop is 512byte, but the sector size of
backing device may be 4K, so we can't do 512 byte algined direct
I/O to the filesystem in this situation.

With runtime switch to buffered I/O we can fix this problem, but
most of times it won't fall in this case.

>
>> > is no need for draining or mode checking for an fsync - FLUSH is always
>> > only guranteed to flush out I/O that has completed by the time it's
>> > issued.
>>
>> Could you point it out in the patch?
>
> Basically your lo_drain_pending_dio() functionality is not needed.

For non-512 byte sector size of backing device, it is needed as
I described above.

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

[toc] | [prev] | [next] | [standalone]

#1203642 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Sun, Aug 09, 2015 at 09:04:32AM -0400, Ming Lei wrote:
> Now the logical block size of loop is 512byte, but the sector size of
> backing device may be 4K, so we can't do 512 byte algined direct
> I/O to the filesystem in this situation.
> 
> With runtime switch to buffered I/O we can fix this problem, but
> most of times it won't fall in this case.

The right way to handle this case is to not allow direct I/O at all,
you'll be constantly switching between direct and buffered I/O
otherwise, which is a bad idea.

> For non-512 byte sector size of backing device, it is needed as
> I described above.

If the backing device sector size is smaller than that of the loop
device you should not allow direct I/O as you will be switching forth
and back ue to the IO patterns all the time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1203648 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Sun, Aug 9, 2015 at 9:44 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Sun, Aug 09, 2015 at 09:04:32AM -0400, Ming Lei wrote:
>> Now the logical block size of loop is 512byte, but the sector size of
>> backing device may be 4K, so we can't do 512 byte algined direct
>> I/O to the filesystem in this situation.
>>
>> With runtime switch to buffered I/O we can fix this problem, but
>> most of times it won't fall in this case.
>
> The right way to handle this case is to not allow direct I/O at all,
> you'll be constantly switching between direct and buffered I/O
> otherwise, which is a bad idea.

In theory we should not allow dio for this case, but in reality,
as Dave Chinner mentioned, all buffered I/O and normal dio
are page aligned, so we can make most of sane applications
benefited from this patch, and for few insane applications,
frequent swich between buffered I/O and direct I/O shouldn't
be very bad too since draining dio is just like batched sync I/O.

>
>> For non-512 byte sector size of backing device, it is needed as
>> I described above.
>
> If the backing device sector size is smaller than that of the loop

I guess you mean bigger? Suppose the backing device sector
size is 256(smaller), direct I/O should be used without problem.

> device you should not allow direct I/O as you will be switching forth
> and back ue to the IO patterns all the time.


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

[toc] | [prev] | [next] | [standalone]

#1203709 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Sun, Aug 09, 2015 at 10:27:14AM -0400, Ming Lei wrote:
> On Sun, Aug 9, 2015 at 9:44 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Sun, Aug 09, 2015 at 09:04:32AM -0400, Ming Lei wrote:
> >> Now the logical block size of loop is 512byte, but the sector size of
> >> backing device may be 4K, so we can't do 512 byte algined direct
> >> I/O to the filesystem in this situation.
> >>
> >> With runtime switch to buffered I/O we can fix this problem, but
> >> most of times it won't fall in this case.
> >
> > The right way to handle this case is to not allow direct I/O at all,
> > you'll be constantly switching between direct and buffered I/O
> > otherwise, which is a bad idea.
> 
> In theory we should not allow dio for this case, but in reality,
> as Dave Chinner mentioned, all buffered I/O and normal dio
> are page aligned, so we can make most of sane applications
> benefited from this patch, and for few insane applications,
> frequent swich between buffered I/O and direct I/O shouldn't
> be very bad too since draining dio is just like batched sync I/O.

If Christoph thinks we should not allow this case, then go with what
Christoph says.

Cheers,

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

[toc] | [prev] | [next] | [standalone]

#1203754 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Sun, Aug 9, 2015 at 7:41 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Sun, Aug 09, 2015 at 10:27:14AM -0400, Ming Lei wrote:
>> On Sun, Aug 9, 2015 at 9:44 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> > On Sun, Aug 09, 2015 at 09:04:32AM -0400, Ming Lei wrote:
>> >> Now the logical block size of loop is 512byte, but the sector size of
>> >> backing device may be 4K, so we can't do 512 byte algined direct
>> >> I/O to the filesystem in this situation.
>> >>
>> >> With runtime switch to buffered I/O we can fix this problem, but
>> >> most of times it won't fall in this case.
>> >
>> > The right way to handle this case is to not allow direct I/O at all,
>> > you'll be constantly switching between direct and buffered I/O
>> > otherwise, which is a bad idea.
>>
>> In theory we should not allow dio for this case, but in reality,
>> as Dave Chinner mentioned, all buffered I/O and normal dio
>> are page aligned, so we can make most of sane applications
>> benefited from this patch, and for few insane applications,
>> frequent swich between buffered I/O and direct I/O shouldn't
>> be very bad too since draining dio is just like batched sync I/O.
>
> If Christoph thinks we should not allow this case, then go with what
> Christoph says.

No problem, and we still may improve it in future.

Christoph, could you share your idea again about this approch?

If you still don't think it is good to switch between buffered I/O
and direct I/O, I will not do that in next post.

Thanks,
Ming

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

[toc] | [prev] | [next] | [standalone]

#1203879 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Sun, Aug 09, 2015 at 10:50:41PM -0400, Ming Lei wrote:
> Christoph, could you share your idea again about this approch?
> 
> If you still don't think it is good to switch between buffered I/O
> and direct I/O, I will not do that in next post.

If we epxose a 512 byte block size to the loop device people will use
it, for example mkfs.xfs might do smaller I/O, extN will default to
small blocksize for small devices, etc.

If you are using a loop devices on a 4k sector backing device and care
about performance (which current loop users generally don't..) just
make sure to set the loop block size to 4k, which Hannes' series will
allow us to do easily.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204332 — Re: [PATCH v9 6/6] block: loop: support DIO & AIO

On Mon, Aug 10, 2015 at 4:08 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Sun, Aug 09, 2015 at 10:50:41PM -0400, Ming Lei wrote:
>> Christoph, could you share your idea again about this approch?
>>
>> If you still don't think it is good to switch between buffered I/O
>> and direct I/O, I will not do that in next post.
>
> If we epxose a 512 byte block size to the loop device people will use
> it, for example mkfs.xfs might do smaller I/O, extN will default to
> small blocksize for small devices, etc.

Looks the default block size of mkfs.ext4/mkfs.xfs is 4096 for loop.

>
> If you are using a loop devices on a 4k sector backing device and care
> about performance (which current loop users generally don't..) just
> make sure to set the loop block size to 4k, which Hannes' series will
> allow us to do easily.

IMO, using 4k sector size may not improve performance for most of
applications(buffered I/O, normal direct I/O) because the request from
these applications is already 4K aligned even the sector size is 512 and
the transfer from loop block device to backing file isn't changed basically.

Also for old applications, userpsace can't use 4K sector size.

BTW, could you let me know if you still object the approch(runtime switch)
in this patch? I am fine to not do that if you aren't happy with the approach.


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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web