diff mbox series

[4/7] block: Introduce elevator features

Message ID 20190823001528.5673-5-damien.lemoal@wdc.com (mailing list archive)
State New, archived
Headers show
Series Elevator cleanups and improvements | expand

Commit Message

Damien Le Moal Aug. 23, 2019, 12:15 a.m. UTC
Introduce the elevator_features bitfield in the elevator_type structure
to allow an elevator to advertize special features that it supports.
Examples of such features are IO priorities, write hints, and zoned
block devices sequential write constraint support (aka zone write
locking).

The required_elevator_features field is added to the request_queue
structure to allow a device driver to specify features that an elevator
must support for the correct operation of the device. An example of a
typical required elevator feature is zone write locking for zoned block
devices. The helper function blk_queue_required_elevator_features() is
defined for setting this new new field.

With these two new information in place, the elevator functions
elevator_match() and elevator_find() are modified to allow a user to set
only an elevator with a set of features that satisfies the device
requirements. Elevators not matching the device requirements are not
listed in the device sysfs queue/scheduler file to prevent their use.

The "none" elevator can always be selected as before.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/blk-settings.c     | 15 ++++++++++++
 block/elevator.c         | 49 +++++++++++++++++++++++++++++++---------
 include/linux/blkdev.h   |  4 ++++
 include/linux/elevator.h |  1 +
 4 files changed, 58 insertions(+), 11 deletions(-)

Comments

Johannes Thumshirn Aug. 23, 2019, 11:19 a.m. UTC | #1
On 23/08/2019 02:15, Damien Le Moal wrote:
> -static struct elevator_type *elevator_find(const char *name)
> +static struct elevator_type *elevator_find(const char *name,
> +					   unsigned long required_features)

[...]
> +	const unsigned long elevator_features;


Wouldn't it make more sens to have a defined number of bits for the
features 'unsigned long' means 64 possible features on 64bit and 32 on
32 bit? Better make it explicit (u64 or u32 depending on how many
features you think you'll need).

Byte,
	Johannes
diff mbox series

Patch

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 2c1831207a8f..16471bf18810 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -832,6 +832,21 @@  void blk_queue_write_cache(struct request_queue *q, bool wc, bool fua)
 }
 EXPORT_SYMBOL_GPL(blk_queue_write_cache);
 
+/**
+ * blk_queue_required_elevator_features - Set queue's elevator required features
+ * @q:		the request queue for the device
+ * @features:	Required elevator features OR'ed together
+ *
+ * Tell the block layer that the device controlled through @q that the
+ * queue elevator must support all the features specified by @features.
+ */
+void blk_queue_required_elevator_features(struct request_queue *q,
+					  unsigned long features)
+{
+	q->required_elevator_features = features;
+}
+EXPORT_SYMBOL_GPL(blk_queue_required_elevator_features);
+
 static int __init blk_settings_init(void)
 {
 	blk_max_low_pfn = max_low_pfn - 1;
diff --git a/block/elevator.c b/block/elevator.c
index 6208ddc334ef..de11beb32893 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -83,8 +83,26 @@  bool elv_bio_merge_ok(struct request *rq, struct bio *bio)
 }
 EXPORT_SYMBOL(elv_bio_merge_ok);
 
-static bool elevator_match(const struct elevator_type *e, const char *name)
+static inline bool elv_support_features(unsigned long elv_features,
+					unsigned long required_features)
 {
+	return (required_features & elv_features) == required_features;
+}
+
+/**
+ * elevator_match - Test an elevator name and features
+ * @e: Scheduler to test
+ * @name: Elevator name to test
+ * @required_features: Features that the elevator must provide
+ *
+ * Return true is the elevator @e name matches @name and if @e provides all the
+ * the feratures spcified by @required_features.
+ */
+static bool elevator_match(const struct elevator_type *e, const char *name,
+			   unsigned long required_features)
+{
+	if (!elv_support_features(e->elevator_features, required_features))
+		return false;
 	if (!strcmp(e->elevator_name, name))
 		return true;
 	if (e->elevator_alias && !strcmp(e->elevator_alias, name))
@@ -93,15 +111,21 @@  static bool elevator_match(const struct elevator_type *e, const char *name)
 	return false;
 }
 
-/*
- * Return scheduler with name 'name'
+/**
+ * elevator_find - Find an elevator
+ * @name: Name of the elevator to find
+ * @required_features: Features that the elevator must provide
+ *
+ * Return the first registered scheduler with name @name and supporting the
+ * features @required_features and NULL otherwise.
  */
-static struct elevator_type *elevator_find(const char *name)
+static struct elevator_type *elevator_find(const char *name,
+					   unsigned long required_features)
 {
 	struct elevator_type *e;
 
 	list_for_each_entry(e, &elv_list, list) {
-		if (elevator_match(e, name))
+		if (elevator_match(e, name, required_features))
 			return e;
 	}
 
@@ -120,12 +144,12 @@  static struct elevator_type *elevator_get(struct request_queue *q,
 
 	spin_lock(&elv_list_lock);
 
-	e = elevator_find(name);
+	e = elevator_find(name, q->required_elevator_features);
 	if (!e && try_loading) {
 		spin_unlock(&elv_list_lock);
 		request_module("%s-iosched", name);
 		spin_lock(&elv_list_lock);
-		e = elevator_find(name);
+		e = elevator_find(name, q->required_elevator_features);
 	}
 
 	if (e && !try_module_get(e->elevator_owner))
@@ -526,7 +550,7 @@  int elv_register(struct elevator_type *e)
 
 	/* register, don't allow duplicate names */
 	spin_lock(&elv_list_lock);
-	if (elevator_find(e->elevator_name)) {
+	if (elevator_find(e->elevator_name, 0)) {
 		spin_unlock(&elv_list_lock);
 		kmem_cache_destroy(e->icq_cache);
 		return -EBUSY;
@@ -682,7 +706,8 @@  static int __elevator_change(struct request_queue *q, const char *name)
 	if (!e)
 		return -EINVAL;
 
-	if (q->elevator && elevator_match(q->elevator->type, elevator_name)) {
+	if (q->elevator &&
+	    elevator_match(q->elevator->type, elevator_name, 0)) {
 		elevator_put(e);
 		return 0;
 	}
@@ -722,11 +747,13 @@  ssize_t elv_iosched_show(struct request_queue *q, char *name)
 
 	spin_lock(&elv_list_lock);
 	list_for_each_entry(__e, &elv_list, list) {
-		if (elv && elevator_match(elv, __e->elevator_name)) {
+		if (elv && elevator_match(elv, __e->elevator_name, 0)) {
 			len += sprintf(name+len, "[%s] ", elv->elevator_name);
 			continue;
 		}
-		if (elv_support_iosched(q))
+		if (elv_support_iosched(q) &&
+		    elevator_match(__e, __e->elevator_name,
+				   q->required_elevator_features))
 			len += sprintf(name+len, "%s ", __e->elevator_name);
 	}
 	spin_unlock(&elv_list_lock);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4798bb25f1ee..1e173273511e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -492,6 +492,8 @@  struct request_queue {
 
 	struct queue_limits	limits;
 
+	unsigned long		required_elevator_features;
+
 #ifdef CONFIG_BLK_DEV_ZONED
 	/*
 	 * Zoned block device information for request dispatch control.
@@ -1084,6 +1086,8 @@  extern void blk_queue_dma_alignment(struct request_queue *, int);
 extern void blk_queue_update_dma_alignment(struct request_queue *, int);
 extern void blk_queue_rq_timeout(struct request_queue *, unsigned int);
 extern void blk_queue_write_cache(struct request_queue *q, bool enabled, bool fua);
+extern void blk_queue_required_elevator_features(struct request_queue *q,
+						 unsigned long features);
 
 /*
  * Number of physical segments as sent to the device.
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 1dd014c9c87b..a99ca9979d71 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -76,6 +76,7 @@  struct elevator_type
 	struct elv_fs_entry *elevator_attrs;
 	const char *elevator_name;
 	const char *elevator_alias;
+	const unsigned long elevator_features;
 	struct module *elevator_owner;
 #ifdef CONFIG_BLK_DEBUG_FS
 	const struct blk_mq_debugfs_attr *queue_debugfs_attrs;