* Re: [PATCH 2/8] nowait aio: Introduce RWF_NOWAIT
From: Goldwyn Rodrigues @ 2017-04-19 10:30 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-fsdevel, jack, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming
In-Reply-To: <20170419063930.GB20053@infradead.org>
On 04/19/2017 01:39 AM, Christoph Hellwig wrote:
>
>> @@ -1593,6 +1593,11 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
>> }
>>
>> req->common.ki_flags |= iocb_rw_flags(iocb->aio_rw_flags);
>> + if ((req->common.ki_flags & IOCB_NOWAIT) &&
>> + !(req->common.ki_flags & IOCB_DIRECT)) {
>> + ret = -EINVAL;
>> + goto out_put_req;
>> + }
>
> Wrong indentation. Also I think this should be EOPNOTSUPP here.
>
Do we plan to add support for nowait in buffered I/O in the future? It
is just too complicated. EINVAL suits best in this case.
--
Goldwyn
^ permalink raw reply
* Re: [PATCH] block: Make writeback throttling defaults consistent for SQ devices
From: Jan Kara @ 2017-04-19 9:41 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Jan Kara
In-Reply-To: <20170419093327.14986-1-jack@suse.cz>
On Wed 19-04-17 11:33:27, Jan Kara wrote:
> When CFQ is used as an elevator, it disables writeback throttling
> because they don't play well together. Later when a different elevator
> is chosen for the device, writeback throttling doesn't get enabled
> again as it should. Make sure CFQ enables writeback throttling (if it
> should be enabled by default) when we switch from it to another IO
> scheduler.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
Forgot to change header to v2 so I'm rather writing it here so that it's
clear.
Honza
> ---
> block/blk-sysfs.c | 19 +------------------
> block/blk-wbt.c | 19 +++++++++++++++++++
> block/blk-wbt.h | 4 ++++
> block/elevator.c | 3 +++
> 4 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index fc20489f0d2b..f85723332288 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -844,23 +844,6 @@ struct kobj_type blk_queue_ktype = {
> .release = blk_release_queue,
> };
>
> -static void blk_wb_init(struct request_queue *q)
> -{
> -#ifndef CONFIG_BLK_WBT_MQ
> - if (q->mq_ops)
> - return;
> -#endif
> -#ifndef CONFIG_BLK_WBT_SQ
> - if (q->request_fn)
> - return;
> -#endif
> -
> - /*
> - * If this fails, we don't get throttling
> - */
> - wbt_init(q);
> -}
> -
> int blk_register_queue(struct gendisk *disk)
> {
> int ret;
> @@ -908,7 +891,7 @@ int blk_register_queue(struct gendisk *disk)
>
> kobject_uevent(&q->kobj, KOBJ_ADD);
>
> - blk_wb_init(q);
> + wbt_enable_default(q);
>
> blk_throtl_register_queue(q);
>
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index b3b79149d3a0..26e1bb617877 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -665,6 +665,25 @@ void wbt_disable_default(struct request_queue *q)
> }
> EXPORT_SYMBOL_GPL(wbt_disable_default);
>
> +/*
> + * Enable wbt if defaults are configured that way
> + */
> +void wbt_enable_default(struct request_queue *q)
> +{
> + /* Throttling already enabled? */
> + if (q->rq_wb)
> + return;
> +
> + /* Queue not registered? Maybe shutting down... */
> + if (!test_bit(QUEUE_FLAG_REGISTERED, &q->queue_flags))
> + return;
> +
> + if ((q->mq_ops && IS_ENABLED(CONFIG_BLK_WBT_MQ)) ||
> + (q->request_fn && IS_ENABLED(CONFIG_BLK_WBT_SQ)))
> + wbt_init(q);
> +}
> +EXPORT_SYMBOL_GPL(wbt_enable_default);
> +
> u64 wbt_default_latency_nsec(struct request_queue *q)
> {
> /*
> diff --git a/block/blk-wbt.h b/block/blk-wbt.h
> index ad6c78507c3a..df6de50c5d59 100644
> --- a/block/blk-wbt.h
> +++ b/block/blk-wbt.h
> @@ -117,6 +117,7 @@ void wbt_update_limits(struct rq_wb *);
> void wbt_requeue(struct rq_wb *, struct blk_issue_stat *);
> void wbt_issue(struct rq_wb *, struct blk_issue_stat *);
> void wbt_disable_default(struct request_queue *);
> +void wbt_enable_default(struct request_queue *);
>
> void wbt_set_queue_depth(struct rq_wb *, unsigned int);
> void wbt_set_write_cache(struct rq_wb *, bool);
> @@ -155,6 +156,9 @@ static inline void wbt_issue(struct rq_wb *rwb, struct blk_issue_stat *stat)
> static inline void wbt_disable_default(struct request_queue *q)
> {
> }
> +static inline void wbt_enable_default(struct request_queue *q)
> +{
> +}
> static inline void wbt_set_queue_depth(struct rq_wb *rwb, unsigned int depth)
> {
> }
> diff --git a/block/elevator.c b/block/elevator.c
> index dbeecf7be719..fb50416b5aae 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -41,6 +41,7 @@
>
> #include "blk.h"
> #include "blk-mq-sched.h"
> +#include "blk-wbt.h"
>
> static DEFINE_SPINLOCK(elv_list_lock);
> static LIST_HEAD(elv_list);
> @@ -877,6 +878,8 @@ void elv_unregister_queue(struct request_queue *q)
> kobject_uevent(&e->kobj, KOBJ_REMOVE);
> kobject_del(&e->kobj);
> e->registered = 0;
> + /* Re-enable throttling in case elevator disabled it */
> + wbt_enable_default(q);
> }
> }
> EXPORT_SYMBOL(elv_unregister_queue);
> --
> 2.12.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [PATCH] block: Make writeback throttling defaults consistent for SQ devices
From: Jan Kara @ 2017-04-19 9:33 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Jan Kara
When CFQ is used as an elevator, it disables writeback throttling
because they don't play well together. Later when a different elevator
is chosen for the device, writeback throttling doesn't get enabled
again as it should. Make sure CFQ enables writeback throttling (if it
should be enabled by default) when we switch from it to another IO
scheduler.
Signed-off-by: Jan Kara <jack@suse.cz>
---
block/blk-sysfs.c | 19 +------------------
block/blk-wbt.c | 19 +++++++++++++++++++
block/blk-wbt.h | 4 ++++
block/elevator.c | 3 +++
4 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index fc20489f0d2b..f85723332288 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -844,23 +844,6 @@ struct kobj_type blk_queue_ktype = {
.release = blk_release_queue,
};
-static void blk_wb_init(struct request_queue *q)
-{
-#ifndef CONFIG_BLK_WBT_MQ
- if (q->mq_ops)
- return;
-#endif
-#ifndef CONFIG_BLK_WBT_SQ
- if (q->request_fn)
- return;
-#endif
-
- /*
- * If this fails, we don't get throttling
- */
- wbt_init(q);
-}
-
int blk_register_queue(struct gendisk *disk)
{
int ret;
@@ -908,7 +891,7 @@ int blk_register_queue(struct gendisk *disk)
kobject_uevent(&q->kobj, KOBJ_ADD);
- blk_wb_init(q);
+ wbt_enable_default(q);
blk_throtl_register_queue(q);
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index b3b79149d3a0..26e1bb617877 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -665,6 +665,25 @@ void wbt_disable_default(struct request_queue *q)
}
EXPORT_SYMBOL_GPL(wbt_disable_default);
+/*
+ * Enable wbt if defaults are configured that way
+ */
+void wbt_enable_default(struct request_queue *q)
+{
+ /* Throttling already enabled? */
+ if (q->rq_wb)
+ return;
+
+ /* Queue not registered? Maybe shutting down... */
+ if (!test_bit(QUEUE_FLAG_REGISTERED, &q->queue_flags))
+ return;
+
+ if ((q->mq_ops && IS_ENABLED(CONFIG_BLK_WBT_MQ)) ||
+ (q->request_fn && IS_ENABLED(CONFIG_BLK_WBT_SQ)))
+ wbt_init(q);
+}
+EXPORT_SYMBOL_GPL(wbt_enable_default);
+
u64 wbt_default_latency_nsec(struct request_queue *q)
{
/*
diff --git a/block/blk-wbt.h b/block/blk-wbt.h
index ad6c78507c3a..df6de50c5d59 100644
--- a/block/blk-wbt.h
+++ b/block/blk-wbt.h
@@ -117,6 +117,7 @@ void wbt_update_limits(struct rq_wb *);
void wbt_requeue(struct rq_wb *, struct blk_issue_stat *);
void wbt_issue(struct rq_wb *, struct blk_issue_stat *);
void wbt_disable_default(struct request_queue *);
+void wbt_enable_default(struct request_queue *);
void wbt_set_queue_depth(struct rq_wb *, unsigned int);
void wbt_set_write_cache(struct rq_wb *, bool);
@@ -155,6 +156,9 @@ static inline void wbt_issue(struct rq_wb *rwb, struct blk_issue_stat *stat)
static inline void wbt_disable_default(struct request_queue *q)
{
}
+static inline void wbt_enable_default(struct request_queue *q)
+{
+}
static inline void wbt_set_queue_depth(struct rq_wb *rwb, unsigned int depth)
{
}
diff --git a/block/elevator.c b/block/elevator.c
index dbeecf7be719..fb50416b5aae 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -41,6 +41,7 @@
#include "blk.h"
#include "blk-mq-sched.h"
+#include "blk-wbt.h"
static DEFINE_SPINLOCK(elv_list_lock);
static LIST_HEAD(elv_list);
@@ -877,6 +878,8 @@ void elv_unregister_queue(struct request_queue *q)
kobject_uevent(&e->kobj, KOBJ_REMOVE);
kobject_del(&e->kobj);
e->registered = 0;
+ /* Re-enable throttling in case elevator disabled it */
+ wbt_enable_default(q);
}
}
EXPORT_SYMBOL(elv_unregister_queue);
--
2.12.0
^ permalink raw reply related
* Re: [PATCH V4 00/16] Introduce the BFQ I/O scheduler
From: Paolo Valente @ 2017-04-19 9:23 UTC (permalink / raw)
To: Jens Axboe, Tejun Heo
Cc: Fabio Checconi, Arianna Avanzini, linux-block, Linux-Kernal,
Ulf Hansson, Linus Walleij, broonie
In-Reply-To: <20170412162322.11139-1-paolo.valente@linaro.org>
> Il giorno 12 apr 2017, alle ore 18:23, Paolo Valente =
<paolo.valente@linaro.org> ha scritto:
>=20
> Hi,
> new patch series, addressing (both) issues raised by Bart [1], and
> with block/Makefile fixed as suggested by Bart [2].
>=20
Hi Jens,
apparently no complain of any sort on this last series. Do you think
we could make it for 4.12, or shall we aim at 4.13?
Thanks,
Paolo
> Thanks,
> Paolo
>=20
> [1] https://lkml.org/lkml/2017/3/31/393
> [2] https://lkml.org/lkml/2017/4/12/502
>=20
> Arianna Avanzini (4):
> block, bfq: add full hierarchical scheduling and cgroups support
> block, bfq: add Early Queue Merge (EQM)
> block, bfq: reduce idling only in symmetric scenarios
> block, bfq: handle bursts of queue activations
>=20
> Paolo Valente (12):
> block, bfq: introduce the BFQ-v0 I/O scheduler as an extra scheduler
> block, bfq: improve throughput boosting
> block, bfq: modify the peak-rate estimator
> block, bfq: add more fairness with writes and slow processes
> block, bfq: improve responsiveness
> block, bfq: reduce I/O latency for soft real-time applications
> block, bfq: preserve a low latency also with NCQ-capable drives
> block, bfq: reduce latency during request-pool saturation
> block, bfq: boost the throughput on NCQ-capable flash-based devices
> block, bfq: boost the throughput with random I/O on NCQ-capable HDDs
> block, bfq: remove all get and put of I/O contexts
> block, bfq: split bfq-iosched.c into multiple source files
>=20
> Documentation/block/00-INDEX | 2 +
> Documentation/block/bfq-iosched.txt | 531 ++++
> block/Kconfig.iosched | 21 +
> block/Makefile | 2 +
> block/bfq-cgroup.c | 1139 ++++++++
> block/bfq-iosched.c | 5047 =
+++++++++++++++++++++++++++++++++++
> block/bfq-iosched.h | 942 +++++++
> block/bfq-wf2q.c | 1616 +++++++++++
> include/linux/blkdev.h | 2 +-
> 9 files changed, 9301 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/block/bfq-iosched.txt
> create mode 100644 block/bfq-cgroup.c
> create mode 100644 block/bfq-iosched.c
> create mode 100644 block/bfq-iosched.h
> create mode 100644 block/bfq-wf2q.c
>=20
> --
> 2.10.0
^ permalink raw reply
* Re: [PATCH 0/3] mtip32xx: make it working with mq scheduler
From: Ming Lei @ 2017-04-19 7:44 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, Omar Sandoval, Jozef Mikovic
In-Reply-To: <20170419062713.GA1372@infradead.org>
On Tue, Apr 18, 2017 at 11:27:13PM -0700, Christoph Hellwig wrote:
> Hi Ming,
>
> I don't think your patch goes int the right direction. The mtip32xx
> driver never submits the requests it allocates using blk_mq_alloc_request.
> So to fix this it should simply kmalloc the request, set a tag aside
> and not use a block layer request for it at all, similar to what nvme
> does for the AER command.
That is only about the internal command part(patch 1 and patch 2),
and the patch 3 is required regardless.
I choose to use blk_mq_get_driver_tag() for the 1st fix becasue it is the
simpleset way, also given blk_mq_alloc_request() has been working on that
for long time.
If We do that via kmalloc, the command has to be initialzed as the way in
.init_cmd(), the reserved tag space has to be accessed exclusively like what
block layer provides, and mtip_cmd_from_tag() need to be fixed too.
We still have to pass 'reserved_tags = 1' to blk-mq anyway.
That is why I suggest to use blk_mq_get_driver_tag() to fix the issue.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH 1/4] block: remove the osdblk driver
From: Boaz Harrosh @ 2017-04-19 7:21 UTC (permalink / raw)
To: Christoph Hellwig, martin.petersen, trond.myklebust, axboe
Cc: osd-dev, linux-nfs, linux-scsi, linux-block, linux-kernel
In-Reply-To: <20170412160109.10598-2-hch@lst.de>
On 04/12/2017 07:01 PM, Christoph Hellwig wrote:
> This was just a proof of concept user for the SCSI OSD library, and
> never had any real users.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Yes please remove this driver
ACK-by Boaz Harrosh <ooo@electrozaur.com>
> ---
> drivers/block/Kconfig | 16 --
> drivers/block/Makefile | 1 -
> drivers/block/osdblk.c | 693 -------------------------------------------------
> 3 files changed, 710 deletions(-)
> delete mode 100644 drivers/block/osdblk.c
>
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index a1c2e816128f..58e24c354933 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -312,22 +312,6 @@ config BLK_DEV_SKD
>
> Use device /dev/skd$N amd /dev/skd$Np$M.
>
> -config BLK_DEV_OSD
> - tristate "OSD object-as-blkdev support"
> - depends on SCSI_OSD_ULD
> - ---help---
> - Saying Y or M here will allow the exporting of a single SCSI
> - OSD (object-based storage) object as a Linux block device.
> -
> - For example, if you create a 2G object on an OSD device,
> - you can then use this module to present that 2G object as
> - a Linux block device.
> -
> - To compile this driver as a module, choose M here: the
> - module will be called osdblk.
> -
> - If unsure, say N.
> -
> config BLK_DEV_SX8
> tristate "Promise SATA SX8 support"
> depends on PCI
> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> index b12c772bbeb3..42e8cee1cbc7 100644
> --- a/drivers/block/Makefile
> +++ b/drivers/block/Makefile
> @@ -22,7 +22,6 @@ obj-$(CONFIG_CDROM_PKTCDVD) += pktcdvd.o
> obj-$(CONFIG_MG_DISK) += mg_disk.o
> obj-$(CONFIG_SUNVDC) += sunvdc.o
> obj-$(CONFIG_BLK_DEV_SKD) += skd.o
> -obj-$(CONFIG_BLK_DEV_OSD) += osdblk.o
>
> obj-$(CONFIG_BLK_DEV_UMEM) += umem.o
> obj-$(CONFIG_BLK_DEV_NBD) += nbd.o
> diff --git a/drivers/block/osdblk.c b/drivers/block/osdblk.c
> deleted file mode 100644
> index 8127b8201a01..000000000000
> --- a/drivers/block/osdblk.c
> +++ /dev/null
> @@ -1,693 +0,0 @@
> -
> -/*
> - osdblk.c -- Export a single SCSI OSD object as a Linux block device
> -
> -
> - Copyright 2009 Red Hat, Inc.
> -
> - This program is free software; you can redistribute it and/or modify
> - it under the terms of the GNU General Public License as published by
> - the Free Software Foundation.
> -
> - This program is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - GNU General Public License for more details.
> -
> - You should have received a copy of the GNU General Public License
> - along with this program; see the file COPYING. If not, write to
> - the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
> -
> -
> - Instructions for use
> - --------------------
> -
> - 1) Map a Linux block device to an existing OSD object.
> -
> - In this example, we will use partition id 1234, object id 5678,
> - OSD device /dev/osd1.
> -
> - $ echo "1234 5678 /dev/osd1" > /sys/class/osdblk/add
> -
> -
> - 2) List all active blkdev<->object mappings.
> -
> - In this example, we have performed step #1 twice, creating two blkdevs,
> - mapped to two separate OSD objects.
> -
> - $ cat /sys/class/osdblk/list
> - 0 174 1234 5678 /dev/osd1
> - 1 179 1994 897123 /dev/osd0
> -
> - The columns, in order, are:
> - - blkdev unique id
> - - blkdev assigned major
> - - OSD object partition id
> - - OSD object id
> - - OSD device
> -
> -
> - 3) Remove an active blkdev<->object mapping.
> -
> - In this example, we remove the mapping with blkdev unique id 1.
> -
> - $ echo 1 > /sys/class/osdblk/remove
> -
> -
> - NOTE: The actual creation and deletion of OSD objects is outside the scope
> - of this driver.
> -
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/device.h>
> -#include <linux/module.h>
> -#include <linux/fs.h>
> -#include <linux/slab.h>
> -#include <scsi/osd_initiator.h>
> -#include <scsi/osd_attributes.h>
> -#include <scsi/osd_sec.h>
> -#include <scsi/scsi_device.h>
> -
> -#define DRV_NAME "osdblk"
> -#define PFX DRV_NAME ": "
> -
> -/* #define _OSDBLK_DEBUG */
> -#ifdef _OSDBLK_DEBUG
> -#define OSDBLK_DEBUG(fmt, a...) \
> - printk(KERN_NOTICE "osdblk @%s:%d: " fmt, __func__, __LINE__, ##a)
> -#else
> -#define OSDBLK_DEBUG(fmt, a...) \
> - do { if (0) printk(fmt, ##a); } while (0)
> -#endif
> -
> -MODULE_AUTHOR("Jeff Garzik <jeff@garzik.org>");
> -MODULE_DESCRIPTION("block device inside an OSD object osdblk.ko");
> -MODULE_LICENSE("GPL");
> -
> -struct osdblk_device;
> -
> -enum {
> - OSDBLK_MINORS_PER_MAJOR = 256, /* max minors per blkdev */
> - OSDBLK_MAX_REQ = 32, /* max parallel requests */
> - OSDBLK_OP_TIMEOUT = 4 * 60, /* sync OSD req timeout */
> -};
> -
> -struct osdblk_request {
> - struct request *rq; /* blk layer request */
> - struct bio *bio; /* cloned bio */
> - struct osdblk_device *osdev; /* associated blkdev */
> -};
> -
> -struct osdblk_device {
> - int id; /* blkdev unique id */
> -
> - int major; /* blkdev assigned major */
> - struct gendisk *disk; /* blkdev's gendisk and rq */
> - struct request_queue *q;
> -
> - struct osd_dev *osd; /* associated OSD */
> -
> - char name[32]; /* blkdev name, e.g. osdblk34 */
> -
> - spinlock_t lock; /* queue lock */
> -
> - struct osd_obj_id obj; /* OSD partition, obj id */
> - uint8_t obj_cred[OSD_CAP_LEN]; /* OSD cred */
> -
> - struct osdblk_request req[OSDBLK_MAX_REQ]; /* request table */
> -
> - struct list_head node;
> -
> - char osd_path[0]; /* OSD device path */
> -};
> -
> -static struct class *class_osdblk; /* /sys/class/osdblk */
> -static DEFINE_MUTEX(ctl_mutex); /* Serialize open/close/setup/teardown */
> -static LIST_HEAD(osdblkdev_list);
> -
> -static const struct block_device_operations osdblk_bd_ops = {
> - .owner = THIS_MODULE,
> -};
> -
> -static const struct osd_attr g_attr_logical_length = ATTR_DEF(
> - OSD_APAGE_OBJECT_INFORMATION, OSD_ATTR_OI_LOGICAL_LENGTH, 8);
> -
> -static void osdblk_make_credential(u8 cred_a[OSD_CAP_LEN],
> - const struct osd_obj_id *obj)
> -{
> - osd_sec_init_nosec_doall_caps(cred_a, obj, false, true);
> -}
> -
> -/* copied from exofs; move to libosd? */
> -/*
> - * Perform a synchronous OSD operation. copied from exofs; move to libosd?
> - */
> -static int osd_sync_op(struct osd_request *or, int timeout, uint8_t *credential)
> -{
> - int ret;
> -
> - or->timeout = timeout;
> - ret = osd_finalize_request(or, 0, credential, NULL);
> - if (ret)
> - return ret;
> -
> - ret = osd_execute_request(or);
> -
> - /* osd_req_decode_sense(or, ret); */
> - return ret;
> -}
> -
> -/*
> - * Perform an asynchronous OSD operation. copied from exofs; move to libosd?
> - */
> -static int osd_async_op(struct osd_request *or, osd_req_done_fn *async_done,
> - void *caller_context, u8 *cred)
> -{
> - int ret;
> -
> - ret = osd_finalize_request(or, 0, cred, NULL);
> - if (ret)
> - return ret;
> -
> - ret = osd_execute_request_async(or, async_done, caller_context);
> -
> - return ret;
> -}
> -
> -/* copied from exofs; move to libosd? */
> -static int extract_attr_from_req(struct osd_request *or, struct osd_attr *attr)
> -{
> - struct osd_attr cur_attr = {.attr_page = 0}; /* start with zeros */
> - void *iter = NULL;
> - int nelem;
> -
> - do {
> - nelem = 1;
> - osd_req_decode_get_attr_list(or, &cur_attr, &nelem, &iter);
> - if ((cur_attr.attr_page == attr->attr_page) &&
> - (cur_attr.attr_id == attr->attr_id)) {
> - attr->len = cur_attr.len;
> - attr->val_ptr = cur_attr.val_ptr;
> - return 0;
> - }
> - } while (iter);
> -
> - return -EIO;
> -}
> -
> -static int osdblk_get_obj_size(struct osdblk_device *osdev, u64 *size_out)
> -{
> - struct osd_request *or;
> - struct osd_attr attr;
> - int ret;
> -
> - /* start request */
> - or = osd_start_request(osdev->osd, GFP_KERNEL);
> - if (!or)
> - return -ENOMEM;
> -
> - /* create a get-attributes(length) request */
> - osd_req_get_attributes(or, &osdev->obj);
> -
> - osd_req_add_get_attr_list(or, &g_attr_logical_length, 1);
> -
> - /* execute op synchronously */
> - ret = osd_sync_op(or, OSDBLK_OP_TIMEOUT, osdev->obj_cred);
> - if (ret)
> - goto out;
> -
> - /* extract length from returned attribute info */
> - attr = g_attr_logical_length;
> - ret = extract_attr_from_req(or, &attr);
> - if (ret)
> - goto out;
> -
> - *size_out = get_unaligned_be64(attr.val_ptr);
> -
> -out:
> - osd_end_request(or);
> - return ret;
> -
> -}
> -
> -static void osdblk_osd_complete(struct osd_request *or, void *private)
> -{
> - struct osdblk_request *orq = private;
> - struct osd_sense_info osi;
> - int ret = osd_req_decode_sense(or, &osi);
> -
> - if (ret) {
> - ret = -EIO;
> - OSDBLK_DEBUG("osdblk_osd_complete with err=%d\n", ret);
> - }
> -
> - /* complete OSD request */
> - osd_end_request(or);
> -
> - /* complete request passed to osdblk by block layer */
> - __blk_end_request_all(orq->rq, ret);
> -}
> -
> -static void bio_chain_put(struct bio *chain)
> -{
> - struct bio *tmp;
> -
> - while (chain) {
> - tmp = chain;
> - chain = chain->bi_next;
> -
> - bio_put(tmp);
> - }
> -}
> -
> -static struct bio *bio_chain_clone(struct bio *old_chain, gfp_t gfpmask)
> -{
> - struct bio *tmp, *new_chain = NULL, *tail = NULL;
> -
> - while (old_chain) {
> - tmp = bio_clone_kmalloc(old_chain, gfpmask);
> - if (!tmp)
> - goto err_out;
> -
> - tmp->bi_bdev = NULL;
> - gfpmask &= ~__GFP_DIRECT_RECLAIM;
> - tmp->bi_next = NULL;
> -
> - if (!new_chain)
> - new_chain = tail = tmp;
> - else {
> - tail->bi_next = tmp;
> - tail = tmp;
> - }
> -
> - old_chain = old_chain->bi_next;
> - }
> -
> - return new_chain;
> -
> -err_out:
> - OSDBLK_DEBUG("bio_chain_clone with err\n");
> - bio_chain_put(new_chain);
> - return NULL;
> -}
> -
> -static void osdblk_rq_fn(struct request_queue *q)
> -{
> - struct osdblk_device *osdev = q->queuedata;
> -
> - while (1) {
> - struct request *rq;
> - struct osdblk_request *orq;
> - struct osd_request *or;
> - struct bio *bio;
> - bool do_write, do_flush;
> -
> - /* peek at request from block layer */
> - rq = blk_fetch_request(q);
> - if (!rq)
> - break;
> -
> - /* deduce our operation (read, write, flush) */
> - /* I wish the block layer simplified cmd_type/cmd_flags/cmd[]
> - * into a clearly defined set of RPC commands:
> - * read, write, flush, scsi command, power mgmt req,
> - * driver-specific, etc.
> - */
> -
> - do_flush = (req_op(rq) == REQ_OP_FLUSH);
> - do_write = (rq_data_dir(rq) == WRITE);
> -
> - if (!do_flush) { /* osd_flush does not use a bio */
> - /* a bio clone to be passed down to OSD request */
> - bio = bio_chain_clone(rq->bio, GFP_ATOMIC);
> - if (!bio)
> - break;
> - } else
> - bio = NULL;
> -
> - /* alloc internal OSD request, for OSD command execution */
> - or = osd_start_request(osdev->osd, GFP_ATOMIC);
> - if (!or) {
> - bio_chain_put(bio);
> - OSDBLK_DEBUG("osd_start_request with err\n");
> - break;
> - }
> -
> - orq = &osdev->req[rq->tag];
> - orq->rq = rq;
> - orq->bio = bio;
> - orq->osdev = osdev;
> -
> - /* init OSD command: flush, write or read */
> - if (do_flush)
> - osd_req_flush_object(or, &osdev->obj,
> - OSD_CDB_FLUSH_ALL, 0, 0);
> - else if (do_write)
> - osd_req_write(or, &osdev->obj, blk_rq_pos(rq) * 512ULL,
> - bio, blk_rq_bytes(rq));
> - else
> - osd_req_read(or, &osdev->obj, blk_rq_pos(rq) * 512ULL,
> - bio, blk_rq_bytes(rq));
> -
> - OSDBLK_DEBUG("%s 0x%x bytes at 0x%llx\n",
> - do_flush ? "flush" : do_write ?
> - "write" : "read", blk_rq_bytes(rq),
> - blk_rq_pos(rq) * 512ULL);
> -
> - /* begin OSD command execution */
> - if (osd_async_op(or, osdblk_osd_complete, orq,
> - osdev->obj_cred)) {
> - osd_end_request(or);
> - blk_requeue_request(q, rq);
> - bio_chain_put(bio);
> - OSDBLK_DEBUG("osd_execute_request_async with err\n");
> - break;
> - }
> -
> - /* remove the special 'flush' marker, now that the command
> - * is executing
> - */
> - rq->special = NULL;
> - }
> -}
> -
> -static void osdblk_free_disk(struct osdblk_device *osdev)
> -{
> - struct gendisk *disk = osdev->disk;
> -
> - if (!disk)
> - return;
> -
> - if (disk->flags & GENHD_FL_UP)
> - del_gendisk(disk);
> - if (disk->queue)
> - blk_cleanup_queue(disk->queue);
> - put_disk(disk);
> -}
> -
> -static int osdblk_init_disk(struct osdblk_device *osdev)
> -{
> - struct gendisk *disk;
> - struct request_queue *q;
> - int rc;
> - u64 obj_size = 0;
> -
> - /* contact OSD, request size info about the object being mapped */
> - rc = osdblk_get_obj_size(osdev, &obj_size);
> - if (rc)
> - return rc;
> -
> - /* create gendisk info */
> - disk = alloc_disk(OSDBLK_MINORS_PER_MAJOR);
> - if (!disk)
> - return -ENOMEM;
> -
> - sprintf(disk->disk_name, DRV_NAME "%d", osdev->id);
> - disk->major = osdev->major;
> - disk->first_minor = 0;
> - disk->fops = &osdblk_bd_ops;
> - disk->private_data = osdev;
> -
> - /* init rq */
> - q = blk_init_queue(osdblk_rq_fn, &osdev->lock);
> - if (!q) {
> - put_disk(disk);
> - return -ENOMEM;
> - }
> -
> - /* switch queue to TCQ mode; allocate tag map */
> - rc = blk_queue_init_tags(q, OSDBLK_MAX_REQ, NULL, BLK_TAG_ALLOC_FIFO);
> - if (rc) {
> - blk_cleanup_queue(q);
> - put_disk(disk);
> - return rc;
> - }
> -
> - /* Set our limits to the lower device limits, because osdblk cannot
> - * sleep when allocating a lower-request and therefore cannot be
> - * bouncing.
> - */
> - blk_queue_stack_limits(q, osd_request_queue(osdev->osd));
> -
> - blk_queue_prep_rq(q, blk_queue_start_tag);
> - blk_queue_write_cache(q, true, false);
> -
> - disk->queue = q;
> -
> - q->queuedata = osdev;
> -
> - osdev->disk = disk;
> - osdev->q = q;
> -
> - /* finally, announce the disk to the world */
> - set_capacity(disk, obj_size / 512ULL);
> - add_disk(disk);
> -
> - printk(KERN_INFO "%s: Added of size 0x%llx\n",
> - disk->disk_name, (unsigned long long)obj_size);
> -
> - return 0;
> -}
> -
> -/********************************************************************
> - * /sys/class/osdblk/
> - * add map OSD object to blkdev
> - * remove unmap OSD object
> - * list show mappings
> - *******************************************************************/
> -
> -static void class_osdblk_release(struct class *cls)
> -{
> - kfree(cls);
> -}
> -
> -static ssize_t class_osdblk_list(struct class *c,
> - struct class_attribute *attr,
> - char *data)
> -{
> - int n = 0;
> - struct list_head *tmp;
> -
> - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> -
> - list_for_each(tmp, &osdblkdev_list) {
> - struct osdblk_device *osdev;
> -
> - osdev = list_entry(tmp, struct osdblk_device, node);
> -
> - n += sprintf(data+n, "%d %d %llu %llu %s\n",
> - osdev->id,
> - osdev->major,
> - osdev->obj.partition,
> - osdev->obj.id,
> - osdev->osd_path);
> - }
> -
> - mutex_unlock(&ctl_mutex);
> - return n;
> -}
> -
> -static ssize_t class_osdblk_add(struct class *c,
> - struct class_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct osdblk_device *osdev;
> - ssize_t rc;
> - int irc, new_id = 0;
> - struct list_head *tmp;
> -
> - if (!try_module_get(THIS_MODULE))
> - return -ENODEV;
> -
> - /* new osdblk_device object */
> - osdev = kzalloc(sizeof(*osdev) + strlen(buf) + 1, GFP_KERNEL);
> - if (!osdev) {
> - rc = -ENOMEM;
> - goto err_out_mod;
> - }
> -
> - /* static osdblk_device initialization */
> - spin_lock_init(&osdev->lock);
> - INIT_LIST_HEAD(&osdev->node);
> -
> - /* generate unique id: find highest unique id, add one */
> -
> - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> -
> - list_for_each(tmp, &osdblkdev_list) {
> - struct osdblk_device *osdev;
> -
> - osdev = list_entry(tmp, struct osdblk_device, node);
> - if (osdev->id > new_id)
> - new_id = osdev->id + 1;
> - }
> -
> - osdev->id = new_id;
> -
> - /* add to global list */
> - list_add_tail(&osdev->node, &osdblkdev_list);
> -
> - mutex_unlock(&ctl_mutex);
> -
> - /* parse add command */
> - if (sscanf(buf, "%llu %llu %s", &osdev->obj.partition, &osdev->obj.id,
> - osdev->osd_path) != 3) {
> - rc = -EINVAL;
> - goto err_out_slot;
> - }
> -
> - /* initialize rest of new object */
> - sprintf(osdev->name, DRV_NAME "%d", osdev->id);
> -
> - /* contact requested OSD */
> - osdev->osd = osduld_path_lookup(osdev->osd_path);
> - if (IS_ERR(osdev->osd)) {
> - rc = PTR_ERR(osdev->osd);
> - goto err_out_slot;
> - }
> -
> - /* build OSD credential */
> - osdblk_make_credential(osdev->obj_cred, &osdev->obj);
> -
> - /* register our block device */
> - irc = register_blkdev(0, osdev->name);
> - if (irc < 0) {
> - rc = irc;
> - goto err_out_osd;
> - }
> -
> - osdev->major = irc;
> -
> - /* set up and announce blkdev mapping */
> - rc = osdblk_init_disk(osdev);
> - if (rc)
> - goto err_out_blkdev;
> -
> - return count;
> -
> -err_out_blkdev:
> - unregister_blkdev(osdev->major, osdev->name);
> -err_out_osd:
> - osduld_put_device(osdev->osd);
> -err_out_slot:
> - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> - list_del_init(&osdev->node);
> - mutex_unlock(&ctl_mutex);
> -
> - kfree(osdev);
> -err_out_mod:
> - OSDBLK_DEBUG("Error adding device %s\n", buf);
> - module_put(THIS_MODULE);
> - return rc;
> -}
> -
> -static ssize_t class_osdblk_remove(struct class *c,
> - struct class_attribute *attr,
> - const char *buf,
> - size_t count)
> -{
> - struct osdblk_device *osdev = NULL;
> - int target_id, rc;
> - unsigned long ul;
> - struct list_head *tmp;
> -
> - rc = kstrtoul(buf, 10, &ul);
> - if (rc)
> - return rc;
> -
> - /* convert to int; abort if we lost anything in the conversion */
> - target_id = (int) ul;
> - if (target_id != ul)
> - return -EINVAL;
> -
> - /* remove object from list immediately */
> - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
> -
> - list_for_each(tmp, &osdblkdev_list) {
> - osdev = list_entry(tmp, struct osdblk_device, node);
> - if (osdev->id == target_id) {
> - list_del_init(&osdev->node);
> - break;
> - }
> - osdev = NULL;
> - }
> -
> - mutex_unlock(&ctl_mutex);
> -
> - if (!osdev)
> - return -ENOENT;
> -
> - /* clean up and free blkdev and associated OSD connection */
> - osdblk_free_disk(osdev);
> - unregister_blkdev(osdev->major, osdev->name);
> - osduld_put_device(osdev->osd);
> - kfree(osdev);
> -
> - /* release module ref */
> - module_put(THIS_MODULE);
> -
> - return count;
> -}
> -
> -static struct class_attribute class_osdblk_attrs[] = {
> - __ATTR(add, 0200, NULL, class_osdblk_add),
> - __ATTR(remove, 0200, NULL, class_osdblk_remove),
> - __ATTR(list, 0444, class_osdblk_list, NULL),
> - __ATTR_NULL
> -};
> -
> -static int osdblk_sysfs_init(void)
> -{
> - int ret = 0;
> -
> - /*
> - * create control files in sysfs
> - * /sys/class/osdblk/...
> - */
> - class_osdblk = kzalloc(sizeof(*class_osdblk), GFP_KERNEL);
> - if (!class_osdblk)
> - return -ENOMEM;
> -
> - class_osdblk->name = DRV_NAME;
> - class_osdblk->owner = THIS_MODULE;
> - class_osdblk->class_release = class_osdblk_release;
> - class_osdblk->class_attrs = class_osdblk_attrs;
> -
> - ret = class_register(class_osdblk);
> - if (ret) {
> - kfree(class_osdblk);
> - class_osdblk = NULL;
> - printk(PFX "failed to create class osdblk\n");
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -static void osdblk_sysfs_cleanup(void)
> -{
> - if (class_osdblk)
> - class_destroy(class_osdblk);
> - class_osdblk = NULL;
> -}
> -
> -static int __init osdblk_init(void)
> -{
> - int rc;
> -
> - rc = osdblk_sysfs_init();
> - if (rc)
> - return rc;
> -
> - return 0;
> -}
> -
> -static void __exit osdblk_exit(void)
> -{
> - osdblk_sysfs_cleanup();
> -}
> -
> -module_init(osdblk_init);
> -module_exit(osdblk_exit);
> -
>
^ permalink raw reply
* Re: [PATCH 4/8] nowait-aio: Introduce IOMAP_NOWAIT
From: Jan Kara @ 2017-04-19 7:12 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, jack, hch, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170414120257.8932-5-rgoldwyn@suse.de>
On Fri 14-04-17 07:02:53, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> IOCB_NOWAIT translates to IOMAP_NOWAIT for iomaps.
> This is used by XFS in the XFS patch.
Goldwyn, the patch is missing your Signed-off-by...
Honza
> ---
> fs/iomap.c | 2 ++
> include/linux/iomap.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 141c3cd55a8b..d1c81753d411 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -885,6 +885,8 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> } else {
> dio->flags |= IOMAP_DIO_WRITE;
> flags |= IOMAP_WRITE;
> + if (iocb->ki_flags & IOCB_NOWAIT)
> + flags |= IOMAP_NOWAIT;
> }
>
> if (mapping->nrpages) {
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 7291810067eb..53f6af89c625 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -51,6 +51,7 @@ struct iomap {
> #define IOMAP_REPORT (1 << 2) /* report extent status, e.g. FIEMAP */
> #define IOMAP_FAULT (1 << 3) /* mapping for page fault */
> #define IOMAP_DIRECT (1 << 4) /* direct I/O */
> +#define IOMAP_NOWAIT (1 << 5) /* Don't wait for writeback */
>
> struct iomap_ops {
> /*
> --
> 2.12.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH V3 02/16] block, bfq: add full hierarchical scheduling and cgroups support
From: Paolo Valente @ 2017-04-19 7:08 UTC (permalink / raw)
To: Tejun Heo
Cc: Jens Axboe, Fabio Checconi, Arianna Avanzini, linux-block,
Linux-Kernal, Ulf Hansson, Linus Walleij, broonie
In-Reply-To: <000F96F3-DF5D-4F3E-B1C5-C78795F5E0AA@linaro.org>
> Il giorno 19 apr 2017, alle ore 07:33, Paolo Valente =
<paolo.valente@linaro.org> ha scritto:
>=20
>>=20
>> Il giorno 18 apr 2017, alle ore 09:04, Tejun Heo <tj@kernel.org> ha =
scritto:
>>=20
>> Hello, Paolo.
>>=20
>> On Wed, Apr 12, 2017 at 07:22:03AM +0200, Paolo Valente wrote:
>>> could you elaborate a bit more on this? I mean, cgroups support has
>>> been in BFQ (and CFQ) for almost ten years, perfectly working as far
>>> as I know. Of course it is perfectly working in terms of I/O and =
not
>>> of CPU bandwidth distribution; and, for the moment, it is effective
>>> only for devices below 30-50KIOPS. What's the point in throwing
>>> (momentarily?) away such a fundamental feature? What am I missing?
>>=20
>> I've been trying to track down latency issues with the CPU controller
>> which basically takes the same approach and I'm not sure nesting
>> scheduler timelines is a good approach. It intuitively feels elegant
>> but seems to have some fundamental issues. IIUC, bfq isn't quite the
>> same in that it doesn't need load balancer across multiple queues and
>> it could be that bfq is close enough to the basic model that the
>> nested behavior maps to the correct scheduling behavior.
>>=20
>> However, for example, in the CPU controller, the nested timelines
>> break sleeper boost. The boost is implemented by considering the
>> thread to have woken up upto some duration prior to the current time;
>> however, it only affects the timeline inside the cgroup and there's =
no
>> good way to propagate it upwards. The final result is two threads in
>> a cgroup with the double weight can behave significantly worse in
>> terms of latency compared to two threads with the weight of 1 in the
>> root.
>>=20
>=20
> Hi Tejun,
> I don't know in detail the specific multiple-queue issues you report,
> but bfq implements the upward propagation you mention: if a process in
> a group is to be privileged, i.e., if the process has basically to be
> provided with a higher weight (in addition to other important forms of
> help), then this weight boost is propagated upward through the path
> from the process to the root node in the group hierarchy.
>=20
ERRATA CORRIGE: actually, this propagation is implemented in a simple
variant of bfq that I made for a virtualization company (to truly
guarantee a low latency to the processes in a guest OS, regardless of
the load in the host). The base version of bfq in these patches
contains all the mechanisms needed to get this propagation, but
doesn't modify group weights autonomously.
Paolo
>> Given that the nested scheduling ends up pretty expensive, I'm not
>> sure how good a model this nesting approach is. Especially if there
>> can be multiple queues, the weight distribution across cgroup
>> instances across multiple queues has to be coordinated globally
>> anyway,
>=20
> To get perfect global service guarantees, yes. But you can settle
> with tradeoffs that, according to my experience with storage and
> packet I/O, are so good to be probably indistinguishable from an
> ideal, but too costly solution. I mean, with a well-done approximated
> scheduling solution, the deviation with respect to an ideal service
> can be in the same order of the noise caused by unavoidable latencies
> of other sw and hw components than the scheduler.
>=20
>> so the weight / cost adjustment part can't happen
>> automatically anyway as in single queue case. If we're going there,
>> we might as well implement cgroup support by actively modulating the
>> combined weights, which will make individual scheduling operations
>> cheaper and it easier to think about and guarantee latency behaviors.
>>=20
>=20
> Yes. Anyway, I didn't quite understand what is or could be the
> alternative, w.r.t. hierarchical scheduling, for guaranteeing
> bandwidth distribution of shared resources in a complex setting. If
> you think I could be of any help on this, just put me somehow in the
> loop.
>=20
>> If you think that bfq will stay single queue and won't need timeline
>> modifying heuristics (for responsiveness or whatever), the current
>> approach could be fine, but I'm a bit awry about committing to the
>> current approach if we're gonna encounter the same problems.
>>=20
>=20
> As of now, bfq is targeted at not too fast devices (< 30-50KIOPS),
> which happen to be single queue. In particular, bfq is currently
> agnostic w.r.t. to the number of downstream queues.
>=20
> Thanks,
> Paolo
>=20
>> Thanks.
>>=20
>> --=20
>> tejun
^ permalink raw reply
* Re: bfq-mq performance comparison to cfq
From: Paolo Valente @ 2017-04-19 7:02 UTC (permalink / raw)
To: Bart Van Assche
Cc: aherrmann@suse.com, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, axboe@kernel.dk
In-Reply-To: <CY1PR0401MB15362FAB3D841CDF2C421A2981180@CY1PR0401MB1536.namprd04.prod.outlook.com>
> Il giorno 19 apr 2017, alle ore 07:01, Bart Van Assche =
<bart.vanassche@sandisk.com> ha scritto:
>=20
> On 04/11/17 00:29, Paolo Valente wrote:
>>=20
>>> Il giorno 10 apr 2017, alle ore 17:15, Bart Van Assche =
<bart.vanassche@sandisk.com> ha scritto:
>>>=20
>>> On Mon, 2017-04-10 at 11:55 +0200, Paolo Valente wrote:
>>>> That said, if you do always want maximum throughput, even at the
>>>> expense of latency, then just switch off low-latency heuristics, =
i.e.,
>>>> set low_latency to 0. Depending on the device, setting slice_ilde =
to
>>>> 0 may help a lot too (as well as with CFQ). If the throughput is
>>>> still low also after forcing BFQ to an only-throughput mode, then =
you
>>>> hit some bug, and I'll have a little more work to do ...
>>>=20
>>> Has it been considered to make applications tell the I/O scheduler
>>> whether to optimize for latency or for throughput? It shouldn't be =
that
>>> hard for window managers and shells to figure out whether or not a =
new
>>> application that is being started is interactive or not. This would
>>> require a mechanism that allows applications to provide such =
information
>>> to the I/O scheduler. Wouldn't that be a better approach than the =
I/O
>>> scheduler trying to guess whether or not an application is an =
interactive
>>> application?
>>=20
>> IMO that would be an (or maybe the) optimal solution, in terms of =
both
>> throughput and latency. We have even developed a prototype doing =
what
>> you propose, for Android. Unfortunately, I have not yet succeeded in
>> getting support, to turn it into candidate production code, or to =
make
>> a similar solution for lsb-compliant systems.
>=20
> Hello Paolo,
>=20
> What API was used by the Android application to tell the I/O scheduler=20=
> to optimize for latency? Do you think that it would be sufficient if =
the=20
> application uses the ioprio_set() system call to set the I/O priority =
to=20
> IOPRIO_CLASS_RT?
>=20
That's exactly the hack we are using in our prototype. However, it
can only be a temporary hack, because it mixes two slightly different
concepts: 1) the activation of weight raising and other mechanisms for
reducing latency for the target app, 2) the assignment of a different
priority class, which (cleanly) means just that processes in a lower
priority class will be served only when the processes of the target
app have no pending I/O request. Finding a clean boosting API would
be one of the main steps to turn our prototype into a usable solution.
Thanks,
Paolo
> Thanks,
>=20
> Bart.
^ permalink raw reply
* Re: [PATCH 7/8] nowait aio: xfs
From: Christoph Hellwig @ 2017-04-19 6:45 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, jack, hch, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170414120257.8932-8-rgoldwyn@suse.de>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 5/8] nowait aio: return on congested block device
From: Christoph Hellwig @ 2017-04-19 6:45 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, jack, hch, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170414120257.8932-6-rgoldwyn@suse.de>
On Fri, Apr 14, 2017 at 07:02:54AM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> A new bio operation flag REQ_NOWAIT is introduced to identify bio's
s/bio/block/
> @@ -1232,6 +1232,11 @@ static struct request *get_request(struct request_queue *q, unsigned int op,
> if (!IS_ERR(rq))
> return rq;
>
> + if (bio && (bio->bi_opf & REQ_NOWAIT)) {
> + blk_put_rl(rl);
> + return ERR_PTR(-EAGAIN);
> + }
Please check the op argument instead of touching bio.
> + if (bio->bi_opf & REQ_NOWAIT) {
> + if (!blk_queue_nowait(q)) {
> + err = -EOPNOTSUPP;
> + goto end_io;
> + }
> + if (!(bio->bi_opf & REQ_SYNC)) {
I don't understand this check at all..
> + if (unlikely(!blk_queue_dying(q) && (bio->bi_opf & REQ_NOWAIT)))
Please break lines after 80 characters.
> @@ -119,6 +119,9 @@ struct request *blk_mq_sched_get_request(struct request_queue *q,
> if (likely(!data->hctx))
> data->hctx = blk_mq_map_queue(q, data->ctx->cpu);
>
> + if (bio && (bio->bi_opf & REQ_NOWAIT))
> + data->flags |= BLK_MQ_REQ_NOWAIT;
Check the op flag again here.
> +++ b/block/blk-mq.c
> @@ -1538,6 +1538,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
> rq = blk_mq_sched_get_request(q, bio, bio->bi_opf, &data);
> if (unlikely(!rq)) {
> __wbt_done(q->rq_wb, wb_acct);
> + if (bio && (bio->bi_opf & REQ_NOWAIT))
> + bio_wouldblock_error(bio);
bio iѕ dereferences unconditionally above, so you can do the same.
> @@ -1662,6 +1664,8 @@ static blk_qc_t blk_sq_make_request(struct request_queue *q, struct bio *bio)
> rq = blk_mq_sched_get_request(q, bio, bio->bi_opf, &data);
> if (unlikely(!rq)) {
> __wbt_done(q->rq_wb, wb_acct);
> + if (bio && (bio->bi_opf & REQ_NOWAIT))
> + bio_wouldblock_error(bio);
Same here. Although blk_sq_make_request is gone anyway in the current
block tree..
> + /* Request queue supports BIO_NOWAIT */
> + queue_flag_set_unlocked(QUEUE_FLAG_NOWAIT, q);
BIO_NOWAIT is gone. And the comment would not be needed if the
flag had a more descriptive name, e.g. QUEUE_FLAG_NOWAIT_SUPPORT.
And I think all request based drivers should set the flag implicitly
as ->queuecommand can't sleep, and ->queue_rq only when it's always
offloaded to a workqueue when the BLK_MQ_F_BLOCKING flag is set.
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -480,8 +480,12 @@ static int dio_bio_complete(struct dio *dio, struct bio *bio)
> unsigned i;
> int err;
>
> - if (bio->bi_error)
> - dio->io_error = -EIO;
> + if (bio->bi_error) {
> + if (bio->bi_opf & REQ_NOWAIT)
> + dio->io_error = -EAGAIN;
> + else
> + dio->io_error = -EIO;
> + }
Huh? Once REQ_NOWAIT is set all errors are -EAGAIN?
^ permalink raw reply
* Re: [PATCH 4/8] nowait-aio: Introduce IOMAP_NOWAIT
From: Christoph Hellwig @ 2017-04-19 6:39 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, jack, hch, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170414120257.8932-5-rgoldwyn@suse.de>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 2/8] nowait aio: Introduce RWF_NOWAIT
From: Christoph Hellwig @ 2017-04-19 6:39 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, jack, hch, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170414120257.8932-3-rgoldwyn@suse.de>
> }
>
> -
> /* prevent overflows */
Weird whitespace change.
> @@ -1593,6 +1593,11 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> }
>
> req->common.ki_flags |= iocb_rw_flags(iocb->aio_rw_flags);
> + if ((req->common.ki_flags & IOCB_NOWAIT) &&
> + !(req->common.ki_flags & IOCB_DIRECT)) {
> + ret = -EINVAL;
> + goto out_put_req;
> + }
Wrong indentation. Also I think this should be EOPNOTSUPP here.
^ permalink raw reply
* Re: [PATCH 1/8] Use RWF_* flags for AIO operations
From: Christoph Hellwig @ 2017-04-19 6:37 UTC (permalink / raw)
To: Goldwyn Rodrigues
Cc: linux-fsdevel, jack, hch, linux-block, linux-btrfs, linux-ext4,
linux-xfs, sagi, avi, axboe, linux-api, willy, tom.leiming,
Goldwyn Rodrigues
In-Reply-To: <20170414120257.8932-2-rgoldwyn@suse.de>
On Fri, Apr 14, 2017 at 07:02:50AM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> RWF_* flags is used for preadv2/pwritev2 calls. Port to use
> it for aio operations as well. For this, aio_rw_flags is
> introduced in struct iocb (using aio_reserved1) which will
> carry these flags.
>
> This is a precursor to the nowait AIO calls.
>
> Note, the only place RWF_HIPRI comes in effect is dio_await_one().
> All the rest of the locations, aio code return -EIOCBQUEUED before the
> checks for RWF_HIPRI.
>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
> fs/aio.c | 10 +++++++++-
> fs/read_write.c | 7 +------
> include/linux/fs.h | 12 ++++++++++++
> include/uapi/linux/aio_abi.h | 2 +-
> 4 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index f52d925ee259..b8a33f5beef5 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1541,11 +1541,17 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> ssize_t ret;
>
> /* enforce forwards compatibility on users */
> - if (unlikely(iocb->aio_reserved1 || iocb->aio_reserved2)) {
> + if (unlikely(iocb->aio_reserved2)) {
> pr_debug("EINVAL: reserve field set\n");
> return -EINVAL;
> }
>
> + if (unlikely(iocb->aio_rw_flags & ~(RWF_HIPRI | RWF_DSYNC | RWF_SYNC))) {
> + pr_debug("EINVAL: aio_rw_flags set with incompatible flags\n");
> + return -EINVAL;
> + }
> +
> + req->common.ki_flags |= iocb_rw_flags(iocb->aio_rw_flags);
The flag validity checking also needs to go into what's currently
iocb_rw_flags (which will need a new name and a return value).
^ permalink raw reply
* Re: [PATCH 0/3] mtip32xx: make it working with mq scheduler
From: Christoph Hellwig @ 2017-04-19 6:27 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Christoph Hellwig, Omar Sandoval,
Jozef Mikovic
In-Reply-To: <20170419003142.9581-1-ming.lei@redhat.com>
Hi Ming,
I don't think your patch goes int the right direction. The mtip32xx
driver never submits the requests it allocates using blk_mq_alloc_request.
So to fix this it should simply kmalloc the request, set a tag aside
and not use a block layer request for it at all, similar to what nvme
does for the AER command.
^ permalink raw reply
* Re: [PATCH 5/5] block: Optimize ioprio_best()
From: Christoph Hellwig @ 2017-04-19 6:21 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Matias Bjørling
In-Reply-To: <20170418231037.3968-6-bart.vanassche@sandisk.com>
On Tue, Apr 18, 2017 at 04:10:37PM -0700, Bart Van Assche wrote:
> Since ioprio_best() translates IOPRIO_CLASS_NONE into IOPRIO_CLASS_BE
> and since lower numerical priority values represent a higher priority
> a simple numerical comparison is sufficient.
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 4/5] block: Inline blk_rq_set_prio()
From: Christoph Hellwig @ 2017-04-19 6:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Matias Bjørling
In-Reply-To: <20170418231037.3968-5-bart.vanassche@sandisk.com>
> + req->ioprio = ioprio_valid(bio_prio(bio)) ? bio_prio(bio) : ioc ?
> + ioc->ioprio : IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
I think this would be a tad cleaner with a traditional if / else if / else
chain, e.g.
if (ioprio_valid(bio_prio(bio)))
req->ioprio = bio_prio(bio);
else if (ioc)
req->ioprio = ioc->ioprio;
else
req->ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
But otherwise the patch looks good to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 3/5] lightnvm: Use blk_init_request_from_bio() instead of open-coding it
From: Christoph Hellwig @ 2017-04-19 6:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Matias Bjørling,
Adam Manzanares
In-Reply-To: <20170418231037.3968-4-bart.vanassche@sandisk.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 2/5] null_blk: Use blk_init_request_from_bio() instead of open-coding it
From: Christoph Hellwig @ 2017-04-19 6:18 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Matias Bjørling,
Adam Manzanares
In-Reply-To: <20170418231037.3968-3-bart.vanassche@sandisk.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
Although I wonder why the nvme lighnvm support cares about
a NULL-bio the null_blk one does not?
^ permalink raw reply
* Re: [PATCH 1/5] block: Export blk_init_request_from_bio()
From: Christoph Hellwig @ 2017-04-19 6:16 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Matias Bjørling,
Adam Manzanares
In-Reply-To: <20170418231037.3968-2-bart.vanassche@sandisk.com>
> +EXPORT_SYMBOL(blk_init_request_from_bio);
EXPORT_SYMBOL_GPL for block layer internals, please.
Otherwise this looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: block: add a error_count field to struct request
From: hch @ 2017-04-19 6:15 UTC (permalink / raw)
To: Bart Van Assche
Cc: hch@bombadil.infradead.org, axboe@kernel.dk,
linux-block@vger.kernel.org, hch@lst.de, konrad.wilk@oracle.com,
roger.pau@citrix.com, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, jbacik@fb.com,
james.smart@broadcom.com, dm-devel@redhat.com
In-Reply-To: <1492556229.2689.22.camel@sandisk.com>
On Tue, Apr 18, 2017 at 10:57:11PM +0000, Bart Van Assche wrote:
> Both blk-mq and the traditional block layer support a .cmd_size field to
> make the block layer core allocate driver-specific data at the end of struct
> request. Could that mechanism have been used for the error_count field?
It could, and that's what I did for the modern drivers. It would have
been a bit of a pain for these old floppy drivers, though.
^ permalink raw reply
* Re: scsi: introduce a new result field in struct scsi_request
From: hch @ 2017-04-19 6:14 UTC (permalink / raw)
To: Bart Van Assche
Cc: hch@bombadil.infradead.org, axboe@kernel.dk,
linux-block@vger.kernel.org, hch@lst.de, konrad.wilk@oracle.com,
roger.pau@citrix.com, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, jbacik@fb.com,
james.smart@broadcom.com, dm-devel@redhat.com
In-Reply-To: <1492554858.2689.17.camel@sandisk.com>
On Tue, Apr 18, 2017 at 10:34:20PM +0000, Bart Van Assche wrote:
> Did you perhaps intend "req->result" instead of "rq->result"?
Yes.
> Did you intend "war" or is that perhaps a typo?
I'll fix the comment.
> > trace_scsi_dispatch_cmd_done(cmd);
> > - blk_mq_complete_request(cmd->request, cmd->request->errors);
> > + blk_mq_complete_request(cmd->request, 0);
> > }
>
> Why has cmd->request->errors been changed into 0?
Because the argument is only used to set req->errors, which we won't
look at any more.
^ permalink raw reply
* Re: block: remove the blk_execute_rq return value
From: hch @ 2017-04-19 6:13 UTC (permalink / raw)
To: Bart Van Assche
Cc: hch@bombadil.infradead.org, axboe@kernel.dk,
linux-block@vger.kernel.org, hch@lst.de, konrad.wilk@oracle.com,
roger.pau@citrix.com, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, jbacik@fb.com,
james.smart@broadcom.com, dm-devel@redhat.com
In-Reply-To: <1492554253.2689.15.camel@sandisk.com>
On Tue, Apr 18, 2017 at 10:24:15PM +0000, Bart Van Assche wrote:
> On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> > diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
> > index b05e151c9b38..82c6d02193ae 100644
> > --- a/drivers/block/paride/pd.c
> > +++ b/drivers/block/paride/pd.c
> > @@ -747,7 +747,8 @@ static int pd_special_command(struct pd_unit *disk,
> >
> > rq->special = func;
> >
> > - err = blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
> > + blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
> > + err = req->errors ? -EIO : 0;
> >
> > blk_put_request(rq);
> > return err;
>
> Hello Christoph,
>
> Sorry that I hadn't noticed this before but shouldn't "req" be changed into
> "rq"? Otherwise this patch looks fine to me. If this comment gets addressed
> you can add:
It should. But it seems this no one caught this because the check gets
removed later in the series by "pd: remove bogus check for req->errors",
so I should just move that patch earlier in the series.
^ permalink raw reply
* Re: RDMA performance comparison: IBNBD, SCST, NVMEoF
From: Roman Penyaev @ 2017-04-19 6:02 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-block@vger.kernel.org, linux-rdma@vger.kernel.org,
mail@fholler.de, sagi@grimberg.me, jinpu.wang@profitbricks.com,
yun.wang@profitbricks.com, hch@lst.de, axboe@kernel.dk,
danil.kipnis@profitbricks.com, Milind.dumbare@gmail.com,
dledford@redhat.com
In-Reply-To: <1492539735.2689.7.camel@sandisk.com>
Hello Bart,
On Tue, Apr 18, 2017 at 8:22 PM, Bart Van Assche
<Bart.VanAssche@sandisk.com> wrote:
> On Tue, 2017-04-18 at 19:33 +0200, Roman Penyaev wrote:
>> By current email I would like to share some fresh RDMA performance
>> results of IBNBD, SCST and NVMEof, based on 4.10 kernel and variety
>> of configurations.
>
> Hello Roman,
>
> Thank you for having shared these results. But please do not expect me
> to have another look at IBNBD before the design bugs in the driver and
> also in the protocol get fixed.
I expected only that you might find results interesting, where I target
the following:
1) retest on latest kernel
2) compare against NVMEoF
3) retest using register_always=N
> The presentation during Vault 2017 made
> it clear that the driver does not scale if more than two CPUs submit I/O
> simultaneously at the initiator side.
On the iops graph, where I increase number of simultaneous fio jobs up to 128
(initiator has 64 CPUs), NVMEoF tends to repeat the same curve, staying always
below IBNBD. So even this is a scalability problem, it can be seen on NVMEoF
runs also. That's why I posted these results to draw someone's attention.
--
Roman
^ permalink raw reply
* Re: [PATCH V3 02/16] block, bfq: add full hierarchical scheduling and cgroups support
From: Paolo Valente @ 2017-04-19 5:33 UTC (permalink / raw)
To: Tejun Heo
Cc: Jens Axboe, Fabio Checconi, Arianna Avanzini, linux-block,
Linux-Kernal, Ulf Hansson, Linus Walleij, broonie
In-Reply-To: <20170418070435.GB3899@wtj.duckdns.org>
> Il giorno 18 apr 2017, alle ore 09:04, Tejun Heo <tj@kernel.org> ha =
scritto:
>=20
> Hello, Paolo.
>=20
> On Wed, Apr 12, 2017 at 07:22:03AM +0200, Paolo Valente wrote:
>> could you elaborate a bit more on this? I mean, cgroups support has
>> been in BFQ (and CFQ) for almost ten years, perfectly working as far
>> as I know. Of course it is perfectly working in terms of I/O and not
>> of CPU bandwidth distribution; and, for the moment, it is effective
>> only for devices below 30-50KIOPS. What's the point in throwing
>> (momentarily?) away such a fundamental feature? What am I missing?
>=20
> I've been trying to track down latency issues with the CPU controller
> which basically takes the same approach and I'm not sure nesting
> scheduler timelines is a good approach. It intuitively feels elegant
> but seems to have some fundamental issues. IIUC, bfq isn't quite the
> same in that it doesn't need load balancer across multiple queues and
> it could be that bfq is close enough to the basic model that the
> nested behavior maps to the correct scheduling behavior.
>=20
> However, for example, in the CPU controller, the nested timelines
> break sleeper boost. The boost is implemented by considering the
> thread to have woken up upto some duration prior to the current time;
> however, it only affects the timeline inside the cgroup and there's no
> good way to propagate it upwards. The final result is two threads in
> a cgroup with the double weight can behave significantly worse in
> terms of latency compared to two threads with the weight of 1 in the
> root.
>=20
Hi Tejun,
I don't know in detail the specific multiple-queue issues you report,
but bfq implements the upward propagation you mention: if a process in
a group is to be privileged, i.e., if the process has basically to be
provided with a higher weight (in addition to other important forms of
help), then this weight boost is propagated upward through the path
from the process to the root node in the group hierarchy.
> Given that the nested scheduling ends up pretty expensive, I'm not
> sure how good a model this nesting approach is. Especially if there
> can be multiple queues, the weight distribution across cgroup
> instances across multiple queues has to be coordinated globally
> anyway,
To get perfect global service guarantees, yes. But you can settle
with tradeoffs that, according to my experience with storage and
packet I/O, are so good to be probably indistinguishable from an
ideal, but too costly solution. I mean, with a well-done approximated
scheduling solution, the deviation with respect to an ideal service
can be in the same order of the noise caused by unavoidable latencies
of other sw and hw components than the scheduler.
> so the weight / cost adjustment part can't happen
> automatically anyway as in single queue case. If we're going there,
> we might as well implement cgroup support by actively modulating the
> combined weights, which will make individual scheduling operations
> cheaper and it easier to think about and guarantee latency behaviors.
>=20
Yes. Anyway, I didn't quite understand what is or could be the
alternative, w.r.t. hierarchical scheduling, for guaranteeing
bandwidth distribution of shared resources in a complex setting. If
you think I could be of any help on this, just put me somehow in the
loop.
> If you think that bfq will stay single queue and won't need timeline
> modifying heuristics (for responsiveness or whatever), the current
> approach could be fine, but I'm a bit awry about committing to the
> current approach if we're gonna encounter the same problems.
>=20
As of now, bfq is targeted at not too fast devices (< 30-50KIOPS),
which happen to be single queue. In particular, bfq is currently
agnostic w.r.t. to the number of downstream queues.
Thanks,
Paolo
> Thanks.
>=20
> --=20
> tejun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox