Vivek Goyal wrote:
...
> +ssize_t elv_fairness_store(struct request_queue *q, const char *name,
> + size_t count)
> +{
> + struct elv_fq_data *efqd;
> + unsigned int data;
> + unsigned long flags;
> +
> + char *p = (char *)name;
> +
> + data = simple_strtoul(p, &p, 10);
> +
> + if (data < 0)
> + data = 0;
> + else if (data > INT_MAX)
> + data = INT_MAX;
Hi Vivek,
data might overflow on 64 bit systems. In addition, since "fairness" is nothing
more than a switch, just let it be.
Signed-off-by: Gui Jianfeng <[email protected]>
---
block/elevator-fq.c | 10 +++++-----
block/elevator-fq.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/elevator-fq.c b/block/elevator-fq.c
index 655162b..42d4279 100644
--- a/block/elevator-fq.c
+++ b/block/elevator-fq.c
@@ -2605,7 +2605,7 @@ static inline int is_root_group_ioq(struct request_queue *q,
ssize_t elv_fairness_show(struct request_queue *q, char *name)
{
struct elv_fq_data *efqd;
- unsigned int data;
+ unsigned long data;
unsigned long flags;
spin_lock_irqsave(q->queue_lock, flags);
@@ -2619,17 +2619,17 @@ ssize_t elv_fairness_store(struct request_queue *q, const char *name,
size_t count)
{
struct elv_fq_data *efqd;
- unsigned int data;
+ unsigned long data;
unsigned long flags;
char *p = (char *)name;
data = simple_strtoul(p, &p, 10);
- if (data < 0)
+ if (!data)
data = 0;
- else if (data > INT_MAX)
- data = INT_MAX;
+ else
+ data = 1;
spin_lock_irqsave(q->queue_lock, flags);
efqd = &q->elevator->efqd;
diff --git a/block/elevator-fq.h b/block/elevator-fq.h
index b2bb11a..4fe843a 100644
--- a/block/elevator-fq.h
+++ b/block/elevator-fq.h
@@ -359,7 +359,7 @@ struct elv_fq_data {
* throughput and focus more on providing fairness for sync
* queues.
*/
- int fairness;
+ unsigned long fairness;
};
extern int elv_slice_idle;
--
1.5.4.rc3
On Tue, Jun 09, 2009 at 03:56:38PM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> ...
> > +ssize_t elv_fairness_store(struct request_queue *q, const char *name,
> > + size_t count)
> > +{
> > + struct elv_fq_data *efqd;
> > + unsigned int data;
> > + unsigned long flags;
> > +
> > + char *p = (char *)name;
> > +
> > + data = simple_strtoul(p, &p, 10);
> > +
> > + if (data < 0)
> > + data = 0;
> > + else if (data > INT_MAX)
> > + data = INT_MAX;
>
> Hi Vivek,
>
> data might overflow on 64 bit systems. In addition, since "fairness" is nothing
> more than a switch, just let it be.
>
> Signed-off-by: Gui Jianfeng <[email protected]>
> ---
Hi Gui,
How about following patch? Currently this should apply at the end of the
patch series. If it looks good, I will merge the changes in higher level
patches.
Thanks
Vivek
o Previously common layer elevator parameters were appearing as request
queue parameters in sysfs. But actually these are io scheduler parameters
in hiearchical mode. Fix it.
o Use macros to define multiple sysfs C functions doing the same thing. Code
borrowed from CFQ. Helps reduce the number of lines of by 140.
Signed-off-by: Vivek Goyal <[email protected]>
---
block/as-iosched.c | 5
block/blk-sysfs.c | 39 -------
block/cfq-iosched.c | 5
block/deadline-iosched.c | 5
block/elevator-fq.c | 245 +++++++++++------------------------------------
block/elevator-fq.h | 26 ++--
block/noop-iosched.c | 10 +
7 files changed, 97 insertions(+), 238 deletions(-)
Index: linux18/block/elevator-fq.h
===================================================================
--- linux18.orig/block/elevator-fq.h 2009-06-09 10:34:59.000000000 -0400
+++ linux18/block/elevator-fq.h 2009-06-09 13:35:03.000000000 -0400
@@ -27,6 +27,9 @@ struct io_queue;
#ifdef CONFIG_ELV_FAIR_QUEUING
+#define ELV_ATTR(name) \
+ __ATTR(name, S_IRUGO|S_IWUSR, elv_##name##_show, elv_##name##_store)
+
/**
* struct bfq_service_tree - per ioprio_class service tree.
* @active: tree for active entities (i.e., those backlogged).
@@ -364,7 +367,7 @@ struct elv_fq_data {
* throughput and focus more on providing fairness for sync
* queues.
*/
- int fairness;
+ unsigned int fairness;
int only_root_group;
};
@@ -650,23 +653,22 @@ static inline struct io_queue *elv_looku
#endif /* GROUP_IOSCHED */
-/* Functions used by blksysfs.c */
-extern ssize_t elv_slice_idle_show(struct request_queue *q, char *name);
-extern ssize_t elv_slice_idle_store(struct request_queue *q, const char *name,
+extern ssize_t elv_slice_idle_show(struct elevator_queue *e, char *name);
+extern ssize_t elv_slice_idle_store(struct elevator_queue *e, const char *name,
size_t count);
-extern ssize_t elv_slice_sync_show(struct request_queue *q, char *name);
-extern ssize_t elv_slice_sync_store(struct request_queue *q, const char *name,
+extern ssize_t elv_slice_sync_show(struct elevator_queue *e, char *name);
+extern ssize_t elv_slice_sync_store(struct elevator_queue *e, const char *name,
size_t count);
-extern ssize_t elv_async_slice_idle_show(struct request_queue *q, char *name);
-extern ssize_t elv_async_slice_idle_store(struct request_queue *q,
+extern ssize_t elv_async_slice_idle_show(struct elevator_queue *e, char *name);
+extern ssize_t elv_async_slice_idle_store(struct elevator_queue *e,
const char *name, size_t count);
-extern ssize_t elv_slice_async_show(struct request_queue *q, char *name);
-extern ssize_t elv_slice_async_store(struct request_queue *q, const char *name,
+extern ssize_t elv_slice_async_show(struct elevator_queue *e, char *name);
+extern ssize_t elv_slice_async_store(struct elevator_queue *e, const char *name,
size_t count);
-extern ssize_t elv_fairness_show(struct request_queue *q, char *name);
-extern ssize_t elv_fairness_store(struct request_queue *q, const char *name,
+extern ssize_t elv_fairness_show(struct elevator_queue *e, char *name);
+extern ssize_t elv_fairness_store(struct elevator_queue *e, const char *name,
size_t count);
/* Functions used by elevator.c */
Index: linux18/block/elevator-fq.c
===================================================================
--- linux18.orig/block/elevator-fq.c 2009-06-09 10:34:59.000000000 -0400
+++ linux18/block/elevator-fq.c 2009-06-09 13:39:48.000000000 -0400
@@ -2618,201 +2618,72 @@ static inline int is_root_group_ioq(stru
return (ioq->entity.sched_data == &efqd->root_group->sched_data);
}
-/* Functions to show and store fairness value through sysfs */
-ssize_t elv_fairness_show(struct request_queue *q, char *name)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- data = efqd->fairness;
- spin_unlock_irqrestore(q->queue_lock, flags);
- return sprintf(name, "%d\n", data);
-}
-
-ssize_t elv_fairness_store(struct request_queue *q, const char *name,
- size_t count)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- char *p = (char *)name;
-
- data = simple_strtoul(p, &p, 10);
-
- if (data < 0)
- data = 0;
- else if (data > INT_MAX)
- data = INT_MAX;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- efqd->fairness = data;
- spin_unlock_irqrestore(q->queue_lock, flags);
-
- return count;
-}
-
-/* Functions to show and store elv_idle_slice value through sysfs */
-ssize_t elv_slice_idle_show(struct request_queue *q, char *name)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- data = jiffies_to_msecs(efqd->elv_slice_idle);
- spin_unlock_irqrestore(q->queue_lock, flags);
- return sprintf(name, "%d\n", data);
-}
-
-ssize_t elv_slice_idle_store(struct request_queue *q, const char *name,
- size_t count)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- char *p = (char *)name;
-
- data = simple_strtoul(p, &p, 10);
-
- if (data < 0)
- data = 0;
- else if (data > INT_MAX)
- data = INT_MAX;
-
- data = msecs_to_jiffies(data);
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- efqd->elv_slice_idle = data;
- spin_unlock_irqrestore(q->queue_lock, flags);
-
- return count;
-}
-
-/* Functions to show and store elv_idle_slice value through sysfs */
-ssize_t elv_async_slice_idle_show(struct request_queue *q, char *name)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- data = jiffies_to_msecs(efqd->elv_async_slice_idle);
- spin_unlock_irqrestore(q->queue_lock, flags);
- return sprintf(name, "%d\n", data);
-}
-
-ssize_t elv_async_slice_idle_store(struct request_queue *q, const char *name,
- size_t count)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- char *p = (char *)name;
-
- data = simple_strtoul(p, &p, 10);
-
- if (data < 0)
- data = 0;
- else if (data > INT_MAX)
- data = INT_MAX;
-
- data = msecs_to_jiffies(data);
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- efqd->elv_async_slice_idle = data;
- spin_unlock_irqrestore(q->queue_lock, flags);
-
- return count;
-}
-
-/* Functions to show and store elv_slice_sync value through sysfs */
-ssize_t elv_slice_sync_show(struct request_queue *q, char *name)
+/*
+ * sysfs parts below -->
+ */
+static ssize_t
+elv_var_show(unsigned int var, char *page)
{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- data = efqd->elv_slice[1];
- spin_unlock_irqrestore(q->queue_lock, flags);
- return sprintf(name, "%d\n", data);
+ return sprintf(page, "%d\n", var);
}
-ssize_t elv_slice_sync_store(struct request_queue *q, const char *name,
- size_t count)
+static ssize_t
+elv_var_store(unsigned int *var, const char *page, size_t count)
{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- char *p = (char *)name;
-
- data = simple_strtoul(p, &p, 10);
-
- if (data < 0)
- data = 0;
- /* 100ms is the limit for now*/
- else if (data > 100)
- data = 100;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- efqd->elv_slice[1] = data;
- spin_unlock_irqrestore(q->queue_lock, flags);
+ char *p = (char *) page;
+ *var = simple_strtoul(p, &p, 10);
return count;
}
-/* Functions to show and store elv_slice_async value through sysfs */
-ssize_t elv_slice_async_show(struct request_queue *q, char *name)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- data = efqd->elv_slice[0];
- spin_unlock_irqrestore(q->queue_lock, flags);
- return sprintf(name, "%d\n", data);
-}
-
-ssize_t elv_slice_async_store(struct request_queue *q, const char *name,
- size_t count)
-{
- struct elv_fq_data *efqd;
- unsigned int data;
- unsigned long flags;
-
- char *p = (char *)name;
-
- data = simple_strtoul(p, &p, 10);
-
- if (data < 0)
- data = 0;
- /* 100ms is the limit for now*/
- else if (data > 100)
- data = 100;
-
- spin_lock_irqsave(q->queue_lock, flags);
- efqd = &q->elevator->efqd;
- efqd->elv_slice[0] = data;
- spin_unlock_irqrestore(q->queue_lock, flags);
-
- return count;
-}
+#define SHOW_FUNCTION(__FUNC, __VAR, __CONV) \
+ssize_t __FUNC(struct elevator_queue *e, char *page) \
+{ \
+ struct elv_fq_data *efqd = &e->efqd; \
+ unsigned int __data = __VAR; \
+ if (__CONV) \
+ __data = jiffies_to_msecs(__data); \
+ return elv_var_show(__data, (page)); \
+}
+SHOW_FUNCTION(elv_fairness_show, efqd->fairness, 0);
+EXPORT_SYMBOL(elv_fairness_show);
+SHOW_FUNCTION(elv_slice_idle_show, efqd->elv_slice_idle, 1);
+EXPORT_SYMBOL(elv_slice_idle_show);
+SHOW_FUNCTION(elv_async_slice_idle_show, efqd->elv_async_slice_idle, 1);
+EXPORT_SYMBOL(elv_async_slice_idle_show);
+SHOW_FUNCTION(elv_slice_sync_show, efqd->elv_slice[1], 1);
+EXPORT_SYMBOL(elv_slice_sync_show);
+SHOW_FUNCTION(elv_slice_async_show, efqd->elv_slice[0], 1);
+EXPORT_SYMBOL(elv_slice_async_show);
+#undef SHOW_FUNCTION
+
+#define STORE_FUNCTION(__FUNC, __PTR, MIN, MAX, __CONV) \
+ssize_t __FUNC(struct elevator_queue *e, const char *page, size_t count) \
+{ \
+ struct elv_fq_data *efqd = &e->efqd; \
+ unsigned int __data; \
+ int ret = elv_var_store(&__data, (page), count); \
+ if (__data < (MIN)) \
+ __data = (MIN); \
+ else if (__data > (MAX)) \
+ __data = (MAX); \
+ if (__CONV) \
+ *(__PTR) = msecs_to_jiffies(__data); \
+ else \
+ *(__PTR) = __data; \
+ return ret; \
+}
+STORE_FUNCTION(elv_fairness_store, &efqd->fairness, 0, 1, 0);
+EXPORT_SYMBOL(elv_fairness_store);
+STORE_FUNCTION(elv_slice_idle_store, &efqd->elv_slice_idle, 0, UINT_MAX, 1);
+EXPORT_SYMBOL(elv_slice_idle_store);
+STORE_FUNCTION(elv_async_slice_idle_store, &efqd->elv_async_slice_idle, 0, UINT_MAX, 1);
+EXPORT_SYMBOL(elv_async_slice_idle_store);
+STORE_FUNCTION(elv_slice_sync_store, &efqd->elv_slice[1], 1, UINT_MAX, 1);
+EXPORT_SYMBOL(elv_slice_sync_store);
+STORE_FUNCTION(elv_slice_async_store, &efqd->elv_slice[0], 1, UINT_MAX, 1);
+EXPORT_SYMBOL(elv_slice_async_store);
+#undef STORE_FUNCTION
void elv_schedule_dispatch(struct request_queue *q)
{
Index: linux18/block/blk-sysfs.c
===================================================================
--- linux18.orig/block/blk-sysfs.c 2009-06-09 10:34:59.000000000 -0400
+++ linux18/block/blk-sysfs.c 2009-06-09 13:24:42.000000000 -0400
@@ -307,38 +307,6 @@ static struct queue_sysfs_entry queue_io
.store = queue_iostats_store,
};
-#ifdef CONFIG_ELV_FAIR_QUEUING
-static struct queue_sysfs_entry queue_slice_idle_entry = {
- .attr = {.name = "slice_idle", .mode = S_IRUGO | S_IWUSR },
- .show = elv_slice_idle_show,
- .store = elv_slice_idle_store,
-};
-
-static struct queue_sysfs_entry queue_async_slice_idle_entry = {
- .attr = {.name = "async_slice_idle", .mode = S_IRUGO | S_IWUSR },
- .show = elv_async_slice_idle_show,
- .store = elv_async_slice_idle_store,
-};
-
-static struct queue_sysfs_entry queue_slice_sync_entry = {
- .attr = {.name = "slice_sync", .mode = S_IRUGO | S_IWUSR },
- .show = elv_slice_sync_show,
- .store = elv_slice_sync_store,
-};
-
-static struct queue_sysfs_entry queue_slice_async_entry = {
- .attr = {.name = "slice_async", .mode = S_IRUGO | S_IWUSR },
- .show = elv_slice_async_show,
- .store = elv_slice_async_store,
-};
-
-static struct queue_sysfs_entry queue_fairness_entry = {
- .attr = {.name = "fairness", .mode = S_IRUGO | S_IWUSR },
- .show = elv_fairness_show,
- .store = elv_fairness_store,
-};
-#endif
-
static struct attribute *default_attrs[] = {
&queue_requests_entry.attr,
#ifdef CONFIG_GROUP_IOSCHED
@@ -353,13 +321,6 @@ static struct attribute *default_attrs[]
&queue_nomerges_entry.attr,
&queue_rq_affinity_entry.attr,
&queue_iostats_entry.attr,
-#ifdef CONFIG_ELV_FAIR_QUEUING
- &queue_slice_idle_entry.attr,
- &queue_async_slice_idle_entry.attr,
- &queue_slice_sync_entry.attr,
- &queue_slice_async_entry.attr,
- &queue_fairness_entry.attr,
-#endif
NULL,
};
Index: linux18/block/cfq-iosched.c
===================================================================
--- linux18.orig/block/cfq-iosched.c 2009-06-09 10:34:55.000000000 -0400
+++ linux18/block/cfq-iosched.c 2009-06-09 13:25:42.000000000 -0400
@@ -2095,6 +2095,11 @@ static struct elv_fs_entry cfq_attrs[] =
CFQ_ATTR(back_seek_max),
CFQ_ATTR(back_seek_penalty),
CFQ_ATTR(slice_async_rq),
+ ELV_ATTR(fairness),
+ ELV_ATTR(slice_idle),
+ ELV_ATTR(async_slice_idle),
+ ELV_ATTR(slice_sync),
+ ELV_ATTR(slice_async),
__ATTR_NULL
};
Index: linux18/block/as-iosched.c
===================================================================
--- linux18.orig/block/as-iosched.c 2009-06-09 10:34:58.000000000 -0400
+++ linux18/block/as-iosched.c 2009-06-09 13:27:38.000000000 -0400
@@ -1766,6 +1766,11 @@ static struct elv_fs_entry as_attrs[] =
AS_ATTR(antic_expire),
AS_ATTR(read_batch_expire),
AS_ATTR(write_batch_expire),
+#ifdef CONFIG_IOSCHED_AS_HIER
+ ELV_ATTR(fairness),
+ ELV_ATTR(slice_idle),
+ ELV_ATTR(slice_sync),
+#endif
__ATTR_NULL
};
Index: linux18/block/deadline-iosched.c
===================================================================
--- linux18.orig/block/deadline-iosched.c 2009-06-09 10:34:55.000000000 -0400
+++ linux18/block/deadline-iosched.c 2009-06-09 13:28:51.000000000 -0400
@@ -460,6 +460,11 @@ static struct elv_fs_entry deadline_attr
DD_ATTR(writes_starved),
DD_ATTR(front_merges),
DD_ATTR(fifo_batch),
+#ifdef CONFIG_IOSCHED_DEADLINE_HIER
+ ELV_ATTR(fairness),
+ ELV_ATTR(slice_idle),
+ ELV_ATTR(slice_sync),
+#endif
__ATTR_NULL
};
Index: linux18/block/noop-iosched.c
===================================================================
--- linux18.orig/block/noop-iosched.c 2009-06-09 10:34:52.000000000 -0400
+++ linux18/block/noop-iosched.c 2009-06-09 13:31:48.000000000 -0400
@@ -82,6 +82,15 @@ static void noop_free_noop_queue(struct
kfree(nq);
}
+#ifdef CONFIG_IOSCHED_NOOP_HIER
+static struct elv_fs_entry noop_attrs[] = {
+ ELV_ATTR(fairness),
+ ELV_ATTR(slice_idle),
+ ELV_ATTR(slice_sync),
+ __ATTR_NULL
+};
+#endif
+
static struct elevator_type elevator_noop = {
.ops = {
.elevator_merge_req_fn = noop_merged_requests,
@@ -94,6 +103,7 @@ static struct elevator_type elevator_noo
},
#ifdef CONFIG_IOSCHED_NOOP_HIER
.elevator_features = ELV_IOSCHED_NEED_FQ | ELV_IOSCHED_SINGLE_IOQ,
+ .elevator_attrs = noop_attrs,
#endif
.elevator_name = "noop",
.elevator_owner = THIS_MODULE,
Vivek Goyal wrote:
> On Tue, Jun 09, 2009 at 03:56:38PM +0800, Gui Jianfeng wrote:
>> Vivek Goyal wrote:
>> ...
>>> +ssize_t elv_fairness_store(struct request_queue *q, const char *name,
>>> + size_t count)
>>> +{
>>> + struct elv_fq_data *efqd;
>>> + unsigned int data;
>>> + unsigned long flags;
>>> +
>>> + char *p = (char *)name;
>>> +
>>> + data = simple_strtoul(p, &p, 10);
>>> +
>>> + if (data < 0)
>>> + data = 0;
>>> + else if (data > INT_MAX)
>>> + data = INT_MAX;
>> Hi Vivek,
>>
>> data might overflow on 64 bit systems. In addition, since "fairness" is nothing
>> more than a switch, just let it be.
>>
>> Signed-off-by: Gui Jianfeng <[email protected]>
>> ---
>
> Hi Gui,
>
> How about following patch? Currently this should apply at the end of the
> patch series. If it looks good, I will merge the changes in higher level
> patches.
This patch seems good to me. Some trivial issues comment below.
>
> Thanks
> Vivek
>
> o Previously common layer elevator parameters were appearing as request
> queue parameters in sysfs. But actually these are io scheduler parameters
> in hiearchical mode. Fix it.
>
> o Use macros to define multiple sysfs C functions doing the same thing. Code
> borrowed from CFQ. Helps reduce the number of lines of by 140.
>
> Signed-off-by: Vivek Goyal <[email protected]>
... \
> +}
> +SHOW_FUNCTION(elv_fairness_show, efqd->fairness, 0);
> +EXPORT_SYMBOL(elv_fairness_show);
> +SHOW_FUNCTION(elv_slice_idle_show, efqd->elv_slice_idle, 1);
> +EXPORT_SYMBOL(elv_slice_idle_show);
> +SHOW_FUNCTION(elv_async_slice_idle_show, efqd->elv_async_slice_idle, 1);
> +EXPORT_SYMBOL(elv_async_slice_idle_show);
> +SHOW_FUNCTION(elv_slice_sync_show, efqd->elv_slice[1], 1);
> +EXPORT_SYMBOL(elv_slice_sync_show);
> +SHOW_FUNCTION(elv_slice_async_show, efqd->elv_slice[0], 1);
> +EXPORT_SYMBOL(elv_slice_async_show);
> +#undef SHOW_FUNCTION
> +
> +#define STORE_FUNCTION(__FUNC, __PTR, MIN, MAX, __CONV) \
> +ssize_t __FUNC(struct elevator_queue *e, const char *page, size_t count) \
> +{ \
> + struct elv_fq_data *efqd = &e->efqd; \
> + unsigned int __data; \
> + int ret = elv_var_store(&__data, (page), count); \
Since simple_strtoul returns unsigned long, it's better to make __data
be that type.
> + if (__data < (MIN)) \
> + __data = (MIN); \
> + else if (__data > (MAX)) \
> + __data = (MAX); \
> + if (__CONV) \
> + *(__PTR) = msecs_to_jiffies(__data); \
> + else \
> + *(__PTR) = __data; \
> + return ret; \
> +}
> +STORE_FUNCTION(elv_fairness_store, &efqd->fairness, 0, 1, 0);
> +EXPORT_SYMBOL(elv_fairness_store);
> +STORE_FUNCTION(elv_slice_idle_store, &efqd->elv_slice_idle, 0, UINT_MAX, 1);
Do we need to set an actual max limitation rather than UINT_MAX for these entries?
> +EXPORT_SYMBOL(elv_slice_idle_store);
> +STORE_FUNCTION(elv_async_slice_idle_store, &efqd->elv_async_slice_idle, 0, UINT_MAX, 1);
> +EXPORT_SYMBOL(elv_async_slice_idle_store);
> +STORE_FUNCTION(elv_slice_sync_store, &efqd->elv_slice[1], 1, UINT_MAX, 1);
> +EXPORT_SYMBOL(elv_slice_sync_store);
> +STORE_FUNCTION(elv_slice_async_store, &efqd->elv_slice[0], 1, UINT_MAX, 1);
> +EXPORT_SYMBOL(elv_slice_async_store);
> +#undef STORE_FUNCTION
>
> void elv_schedule_dispatch(struct request_queue *q)
> {
> Index: linux18/block/blk-sysfs.c
> ===================================================================
> --- linux18.orig/block/blk-sysfs.c 2009-06-09 10:34:59.000000000 -0400
> +++ linux18/block/blk-sysfs.c 2009-06-09 13:24:42.000000000 -0400
> @@ -307,38 +307,6 @@ static struct queue_sysfs_entry queue_io
> .store = queue_iostats_store,
> };
>
> -#ifdef CONFIG_ELV_FAIR_QUEUING
> -static struct queue_sysfs_entry queue_slice_idle_entry = {
> - .attr = {.name = "slice_idle", .mode = S_IRUGO | S_IWUSR },
> - .show = elv_slice_idle_show,
> - .store = elv_slice_idle_store,
> -};
> -
> -static struct queue_sysfs_entry queue_async_slice_idle_entry = {
> - .attr = {.name = "async_slice_idle", .mode = S_IRUGO | S_IWUSR },
> - .show = elv_async_slice_idle_show,
> - .store = elv_async_slice_idle_store,
> -};
> -
> -static struct queue_sysfs_entry queue_slice_sync_entry = {
> - .attr = {.name = "slice_sync", .mode = S_IRUGO | S_IWUSR },
> - .show = elv_slice_sync_show,
> - .store = elv_slice_sync_store,
> -};
> -
> -static struct queue_sysfs_entry queue_slice_async_entry = {
> - .attr = {.name = "slice_async", .mode = S_IRUGO | S_IWUSR },
> - .show = elv_slice_async_show,
> - .store = elv_slice_async_store,
> -};
> -
> -static struct queue_sysfs_entry queue_fairness_entry = {
> - .attr = {.name = "fairness", .mode = S_IRUGO | S_IWUSR },
> - .show = elv_fairness_show,
> - .store = elv_fairness_store,
> -};
> -#endif
> -
> static struct attribute *default_attrs[] = {
> &queue_requests_entry.attr,
> #ifdef CONFIG_GROUP_IOSCHED
> @@ -353,13 +321,6 @@ static struct attribute *default_attrs[]
> &queue_nomerges_entry.attr,
> &queue_rq_affinity_entry.attr,
> &queue_iostats_entry.attr,
> -#ifdef CONFIG_ELV_FAIR_QUEUING
> - &queue_slice_idle_entry.attr,
> - &queue_async_slice_idle_entry.attr,
> - &queue_slice_sync_entry.attr,
> - &queue_slice_async_entry.attr,
> - &queue_fairness_entry.attr,
> -#endif
> NULL,
> };
>
> Index: linux18/block/cfq-iosched.c
> ===================================================================
> --- linux18.orig/block/cfq-iosched.c 2009-06-09 10:34:55.000000000 -0400
> +++ linux18/block/cfq-iosched.c 2009-06-09 13:25:42.000000000 -0400
> @@ -2095,6 +2095,11 @@ static struct elv_fs_entry cfq_attrs[] =
> CFQ_ATTR(back_seek_max),
> CFQ_ATTR(back_seek_penalty),
> CFQ_ATTR(slice_async_rq),
> + ELV_ATTR(fairness),
> + ELV_ATTR(slice_idle),
> + ELV_ATTR(async_slice_idle),
> + ELV_ATTR(slice_sync),
> + ELV_ATTR(slice_async),
> __ATTR_NULL
> };
>
> Index: linux18/block/as-iosched.c
> ===================================================================
> --- linux18.orig/block/as-iosched.c 2009-06-09 10:34:58.000000000 -0400
> +++ linux18/block/as-iosched.c 2009-06-09 13:27:38.000000000 -0400
> @@ -1766,6 +1766,11 @@ static struct elv_fs_entry as_attrs[] =
> AS_ATTR(antic_expire),
> AS_ATTR(read_batch_expire),
> AS_ATTR(write_batch_expire),
> +#ifdef CONFIG_IOSCHED_AS_HIER
> + ELV_ATTR(fairness),
> + ELV_ATTR(slice_idle),
> + ELV_ATTR(slice_sync),
> +#endif
> __ATTR_NULL
> };
>
> Index: linux18/block/deadline-iosched.c
> ===================================================================
> --- linux18.orig/block/deadline-iosched.c 2009-06-09 10:34:55.000000000 -0400
> +++ linux18/block/deadline-iosched.c 2009-06-09 13:28:51.000000000 -0400
> @@ -460,6 +460,11 @@ static struct elv_fs_entry deadline_attr
> DD_ATTR(writes_starved),
> DD_ATTR(front_merges),
> DD_ATTR(fifo_batch),
> +#ifdef CONFIG_IOSCHED_DEADLINE_HIER
> + ELV_ATTR(fairness),
> + ELV_ATTR(slice_idle),
> + ELV_ATTR(slice_sync),
> +#endif
> __ATTR_NULL
> };
>
> Index: linux18/block/noop-iosched.c
> ===================================================================
> --- linux18.orig/block/noop-iosched.c 2009-06-09 10:34:52.000000000 -0400
> +++ linux18/block/noop-iosched.c 2009-06-09 13:31:48.000000000 -0400
> @@ -82,6 +82,15 @@ static void noop_free_noop_queue(struct
> kfree(nq);
> }
>
> +#ifdef CONFIG_IOSCHED_NOOP_HIER
> +static struct elv_fs_entry noop_attrs[] = {
> + ELV_ATTR(fairness),
> + ELV_ATTR(slice_idle),
> + ELV_ATTR(slice_sync),
> + __ATTR_NULL
> +};
> +#endif
> +
> static struct elevator_type elevator_noop = {
> .ops = {
> .elevator_merge_req_fn = noop_merged_requests,
> @@ -94,6 +103,7 @@ static struct elevator_type elevator_noo
> },
> #ifdef CONFIG_IOSCHED_NOOP_HIER
> .elevator_features = ELV_IOSCHED_NEED_FQ | ELV_IOSCHED_SINGLE_IOQ,
> + .elevator_attrs = noop_attrs,
> #endif
> .elevator_name = "noop",
> .elevator_owner = THIS_MODULE,
>
>
>
--
Regards
Gui Jianfeng
On Wed, Jun 10, 2009 at 09:30:38AM +0800, Gui Jianfeng wrote:
> Vivek Goyal wrote:
> > On Tue, Jun 09, 2009 at 03:56:38PM +0800, Gui Jianfeng wrote:
> >> Vivek Goyal wrote:
> >> ...
> >>> +ssize_t elv_fairness_store(struct request_queue *q, const char *name,
> >>> + size_t count)
> >>> +{
> >>> + struct elv_fq_data *efqd;
> >>> + unsigned int data;
> >>> + unsigned long flags;
> >>> +
> >>> + char *p = (char *)name;
> >>> +
> >>> + data = simple_strtoul(p, &p, 10);
> >>> +
> >>> + if (data < 0)
> >>> + data = 0;
> >>> + else if (data > INT_MAX)
> >>> + data = INT_MAX;
> >> Hi Vivek,
> >>
> >> data might overflow on 64 bit systems. In addition, since "fairness" is nothing
> >> more than a switch, just let it be.
> >>
> >> Signed-off-by: Gui Jianfeng <[email protected]>
> >> ---
> >
> > Hi Gui,
> >
> > How about following patch? Currently this should apply at the end of the
> > patch series. If it looks good, I will merge the changes in higher level
> > patches.
>
> This patch seems good to me. Some trivial issues comment below.
>
> >
> > Thanks
> > Vivek
> >
> > o Previously common layer elevator parameters were appearing as request
> > queue parameters in sysfs. But actually these are io scheduler parameters
> > in hiearchical mode. Fix it.
> >
> > o Use macros to define multiple sysfs C functions doing the same thing. Code
> > borrowed from CFQ. Helps reduce the number of lines of by 140.
> >
> > Signed-off-by: Vivek Goyal <[email protected]>
> ... \
> > +}
> > +SHOW_FUNCTION(elv_fairness_show, efqd->fairness, 0);
> > +EXPORT_SYMBOL(elv_fairness_show);
> > +SHOW_FUNCTION(elv_slice_idle_show, efqd->elv_slice_idle, 1);
> > +EXPORT_SYMBOL(elv_slice_idle_show);
> > +SHOW_FUNCTION(elv_async_slice_idle_show, efqd->elv_async_slice_idle, 1);
> > +EXPORT_SYMBOL(elv_async_slice_idle_show);
> > +SHOW_FUNCTION(elv_slice_sync_show, efqd->elv_slice[1], 1);
> > +EXPORT_SYMBOL(elv_slice_sync_show);
> > +SHOW_FUNCTION(elv_slice_async_show, efqd->elv_slice[0], 1);
> > +EXPORT_SYMBOL(elv_slice_async_show);
> > +#undef SHOW_FUNCTION
> > +
> > +#define STORE_FUNCTION(__FUNC, __PTR, MIN, MAX, __CONV) \
> > +ssize_t __FUNC(struct elevator_queue *e, const char *page, size_t count) \
> > +{ \
> > + struct elv_fq_data *efqd = &e->efqd; \
> > + unsigned int __data; \
> > + int ret = elv_var_store(&__data, (page), count); \
>
> Since simple_strtoul returns unsigned long, it's better to make __data
> be that type.
>
I just took it from CFQ. BTW, what's the harm here in truncating unsigned
long to int? Anyway for our variables we are not expecting any value
bigger than unsigned int and if it is, we expect to truncate it?
> > + if (__data < (MIN)) \
> > + __data = (MIN); \
> > + else if (__data > (MAX)) \
> > + __data = (MAX); \
> > + if (__CONV) \
> > + *(__PTR) = msecs_to_jiffies(__data); \
> > + else \
> > + *(__PTR) = __data; \
> > + return ret; \
> > +}
> > +STORE_FUNCTION(elv_fairness_store, &efqd->fairness, 0, 1, 0);
> > +EXPORT_SYMBOL(elv_fairness_store);
> > +STORE_FUNCTION(elv_slice_idle_store, &efqd->elv_slice_idle, 0, UINT_MAX, 1);
>
> Do we need to set an actual max limitation rather than UINT_MAX for these entries?
Again these are the same values CFQ was using. Do you have a better upper
limit in mind? Until and unless there is strong objection to UINT_MAX, we
can stick to what CFQ has been doing so far.
Thanks
Vivek
Vivek Goyal wrote:
> On Wed, Jun 10, 2009 at 09:30:38AM +0800, Gui Jianfeng wrote:
>> Vivek Goyal wrote:
>>> On Tue, Jun 09, 2009 at 03:56:38PM +0800, Gui Jianfeng wrote:
>>>> Vivek Goyal wrote:
>>>> ...
>>>>> +ssize_t elv_fairness_store(struct request_queue *q, const char *name,
>>>>> + size_t count)
>>>>> +{
>>>>> + struct elv_fq_data *efqd;
>>>>> + unsigned int data;
>>>>> + unsigned long flags;
>>>>> +
>>>>> + char *p = (char *)name;
>>>>> +
>>>>> + data = simple_strtoul(p, &p, 10);
>>>>> +
>>>>> + if (data < 0)
>>>>> + data = 0;
>>>>> + else if (data > INT_MAX)
>>>>> + data = INT_MAX;
>>>> Hi Vivek,
>>>>
>>>> data might overflow on 64 bit systems. In addition, since "fairness" is nothing
>>>> more than a switch, just let it be.
>>>>
>>>> Signed-off-by: Gui Jianfeng <[email protected]>
>>>> ---
>>> Hi Gui,
>>>
>>> How about following patch? Currently this should apply at the end of the
>>> patch series. If it looks good, I will merge the changes in higher level
>>> patches.
>> This patch seems good to me. Some trivial issues comment below.
>>
>>> Thanks
>>> Vivek
>>>
>>> o Previously common layer elevator parameters were appearing as request
>>> queue parameters in sysfs. But actually these are io scheduler parameters
>>> in hiearchical mode. Fix it.
>>>
>>> o Use macros to define multiple sysfs C functions doing the same thing. Code
>>> borrowed from CFQ. Helps reduce the number of lines of by 140.
>>>
>>> Signed-off-by: Vivek Goyal <[email protected]>
>> ... \
>>> +}
>>> +SHOW_FUNCTION(elv_fairness_show, efqd->fairness, 0);
>>> +EXPORT_SYMBOL(elv_fairness_show);
>>> +SHOW_FUNCTION(elv_slice_idle_show, efqd->elv_slice_idle, 1);
>>> +EXPORT_SYMBOL(elv_slice_idle_show);
>>> +SHOW_FUNCTION(elv_async_slice_idle_show, efqd->elv_async_slice_idle, 1);
>>> +EXPORT_SYMBOL(elv_async_slice_idle_show);
>>> +SHOW_FUNCTION(elv_slice_sync_show, efqd->elv_slice[1], 1);
>>> +EXPORT_SYMBOL(elv_slice_sync_show);
>>> +SHOW_FUNCTION(elv_slice_async_show, efqd->elv_slice[0], 1);
>>> +EXPORT_SYMBOL(elv_slice_async_show);
>>> +#undef SHOW_FUNCTION
>>> +
>>> +#define STORE_FUNCTION(__FUNC, __PTR, MIN, MAX, __CONV) \
>>> +ssize_t __FUNC(struct elevator_queue *e, const char *page, size_t count) \
>>> +{ \
>>> + struct elv_fq_data *efqd = &e->efqd; \
>>> + unsigned int __data; \
>>> + int ret = elv_var_store(&__data, (page), count); \
>> Since simple_strtoul returns unsigned long, it's better to make __data
>> be that type.
>>
>
> I just took it from CFQ. BTW, what's the harm here in truncating unsigned
> long to int? Anyway for our variables we are not expecting any value
> bigger than unsigned int and if it is, we expect to truncate it?
>
>>> + if (__data < (MIN)) \
>>> + __data = (MIN); \
>>> + else if (__data > (MAX)) \
>>> + __data = (MAX); \
>>> + if (__CONV) \
>>> + *(__PTR) = msecs_to_jiffies(__data); \
>>> + else \
>>> + *(__PTR) = __data; \
>>> + return ret; \
>>> +}
>>> +STORE_FUNCTION(elv_fairness_store, &efqd->fairness, 0, 1, 0);
>>> +EXPORT_SYMBOL(elv_fairness_store);
>>> +STORE_FUNCTION(elv_slice_idle_store, &efqd->elv_slice_idle, 0, UINT_MAX, 1);
>> Do we need to set an actual max limitation rather than UINT_MAX for these entries?
>
> Again these are the same values CFQ was using. Do you have a better upper
> limit in mind? Until and unless there is strong objection to UINT_MAX, we
> can stick to what CFQ has been doing so far.
Ok, I don't have strong opinion about the above things.
>
> Thanks
> Vivek
>
>
>
--
Regards
Gui Jianfeng