[PATCH] Add sysfs support for fbdefio delay

This patch adds support for examining and modifying the fbdefio delay
parameter through sysfs. It also adds two driver definable minimum
and maximum bounds.

The default behavior is to not permit modifications if delay_max is 0,
thus preventing modification of the delay if the driver does not
explicitly permit modification.

Signed-off-by: Rick L. Vinyard, Jr <redacted>
---
 .../ABI/testing/sysfs-class-graphics-defio         |   35 ++++++++
 drivers/video/fbsysfs.c                            |   87 ++++++++++++++++++++
 include/linux/fb.h                                 |    9 ++-
 3 files changed, 130 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-graphics-defio

diff --git a/Documentation/ABI/testing/sysfs-class-graphics-defio b/Documentation/ABI/testing/sysfs-class-graphics-defio
new file mode 100644
index 0000000..e0ef924
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-graphics-defio

@@ -0,0 +1,35 @@
+What:		/sys/class/graphics/<fb>/defio_delay
+Date:		February 2010
+KernelVersion:	2.6.34
+Contact:	Rick L Vinyard Jr <rvinyard@cs.nmsu.edu>
+Description:
+		Set the deferred I/O delay of the framebuffer in ms.
+		This value can be used to throttle deferred I/O updates.
+		Most framebuffer devices do not have or need support for
+		deferred I/O. Accessing a framebuffer without deferred I/O
+		support will return -ENODEV. Can be read but not modified if
+		/sys/class/graphics/<fb>/defio_delay_max is 0. When modifying,
+		the value must be greater than or equal to
+		/sys/class/graphics/<fb>/defio_delay_min and less than or equal
+		to /sys/class/graphics/<fb>/defio_delay_max.
+
+What:		/sys/class/graphics/<fb>/defio_delay_min
+Date:		February 2010
+KernelVersion:	2.6.34
+Contact:	Rick L Vinyard Jr <rvinyard@cs.nmsu.edu>
+Description:
+		Minimum deferred I/O value in ms for this framebuffer.
+		This value is specified by the driver and cannot be modified
+		from sysfs. Default is 0.
+
+What:		/sys/class/graphics/<fb>/defio_delay_min
+Date:		February 2010
+KernelVersion:	2.6.34
+Contact:	Rick L Vinyard Jr <rvinyard@cs.nmsu.edu>
+Description:
+		Maximum deferred I/O value in ms for this framebuffer.
+		This value is specified by the driver and cannot be modified
+		from sysfs. Default is 0.
+		If this value is 0 /sys/class/graphics/<fb>/defio_delay cannot
+		be modified, but can be read.
+

diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
index d4a2c11..d00ea2d 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c

@@ -484,6 +484,87 @@ static ssize_t show_bl_curve(struct device *device,
 }
 #endif
 
+#ifdef CONFIG_FB_DEFERRED_IO
+static ssize_t store_defio_delay(struct device *device,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct fb_info *fb_info = dev_get_drvdata(device);
+	unsigned long delay_ms = 0;
+	unsigned long delay;
+	int error;
+	char *last = NULL;
+
+	/* Check to see whether this is a deferred I/O driver */
+	if (!fb_info || !fb_info->fbdefio)
+		return -ENODEV;
+
+	/* Check whether delay_max permits setting of delay */
+	if (fb_info->fbdefio->delay_max = 0)
+		return -EPERM;
+
+	error = strict_strtoul(buf, 10, &delay_ms);
+	if (error < 0)
+		return error;
+
+	delay = delay_ms * HZ / 1000;
+
+	if (delay < fb_info->fbdefio->delay_min ||
+	    delay > fb_info->fbdefio->delay_max)
+		return -EINVAL;
+
+	fb_info->fbdefio->delay = delay;
+
+	return count;
+}
+
+static ssize_t show_defio_delay(struct device *device,
+				struct device_attribute *attr, char *buf)
+{
+	struct fb_info *fb_info = dev_get_drvdata(device);
+	unsigned long delay_ms;
+
+	/* Check to see whether this is a deferred I/O driver */
+	if (!fb_info || !fb_info->fbdefio)
+		return -ENODEV;
+
+	delay_ms = fb_info->fbdefio->delay * 1000 / HZ;
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n", delay_ms);
+}
+
+static ssize_t show_defio_delay_min(struct device *device,
+				    struct device_attribute *attr, char *buf)
+{
+	struct fb_info *fb_info = dev_get_drvdata(device);
+	unsigned long delay_ms;
+
+	/* Check to see whether this is a deferred I/O driver */
+	if (!fb_info || !fb_info->fbdefio)
+		return -ENODEV;
+
+	delay_ms = fb_info->fbdefio->delay_min * 1000 / HZ;
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n", delay_ms);
+}
+
+static ssize_t show_defio_delay_max(struct device *device,
+				    struct device_attribute *attr, char *buf)
+{
+	struct fb_info *fb_info = dev_get_drvdata(device);
+	unsigned long delay_ms;
+
+	/* Check to see whether this is a deferred I/O driver */
+	if (!fb_info || !fb_info->fbdefio)
+		return -ENODEV;
+
+	delay_ms = fb_info->fbdefio->delay_max * 1000 / HZ;
+
+	return snprintf(buf, PAGE_SIZE, "%lu\n", delay_ms);
+}
+
+#endif
+
 /* When cmap is added back in it should be a binary attribute
  * not a text one. Consideration should also be given to converting
  * fbdev to use configfs instead of sysfs */

@@ -503,6 +584,12 @@ static struct device_attribute device_attrs[] = {
 #ifdef CONFIG_FB_BACKLIGHT
 	__ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
 #endif
+#ifdef CONFIG_FB_DEFERRED_IO
+	__ATTR(defio_delay, S_IRUGO|S_IWUSR,
+	       show_defio_delay, store_defio_delay),
+	__ATTR(defio_delay_min, S_IRUGO, show_defio_delay_min, NULL),
+	__ATTR(defio_delay_max, S_IRUGO, show_defio_delay_max, NULL),
+#endif
 };
 
 int fb_init_device(struct fb_info *fb_info)

diff --git a/include/linux/fb.h b/include/linux/fb.h
index 369767b..76f35fd 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h

@@ -591,8 +591,15 @@ struct fb_pixmap {
 
 #ifdef CONFIG_FB_DEFERRED_IO
 struct fb_deferred_io {
-	/* delay between mkwrite and deferred handler */
+	/* delay in jiffies between mkwrite and deferred handler */
 	unsigned long delay;
+	/* The minimum delay in jiffies that may be set through sysfs */
+	unsigned long delay_min;
+	/*
+	 * The maximum delay in jiffies that may be set through sysfs.
+	 * If delay_max is 0, delay cannot be set through sysfs.
+	 */
+	unsigned long delay_max;
 	struct mutex lock; /* mutex that protects the page list */
 	struct list_head pagelist; /* list of touched pages */
 	/* callback */

-- 
1.6.6.1

On Thu, 25 February 2010 "Rick L. Vinyard Jr." [off-list ref] wrote:

This patch adds support for examining and modifying the fbdefio delay
parameter through sysfs. It also adds two driver definable minimum
and maximum bounds.

The default behavior is to not permit modifications if delay_max is 0,
thus preventing modification of the delay if the driver does not
explicitly permit modification.

...

Would it be reasonable to add these attributes in
fb_deferred_io_init() and remove them in fb_deferred_io_cleanup()?
This would also make it possible to add write permission to delay
attribute only when it effectively can be modified.

IMHO having only attributes pertinent to the features supported by the
framebuffer is better than having all the possible ones and those of
unsupported features returning -ENODEV.

Bruno

Bruno Prémont wrote:

On Thu, 25 February 2010 "Rick L. Vinyard Jr." [off-list ref]
wrote:

quoted

This patch adds support for examining and modifying the fbdefio delay
parameter through sysfs. It also adds two driver definable minimum
and maximum bounds.

The default behavior is to not permit modifications if delay_max is 0,
thus preventing modification of the delay if the driver does not
explicitly permit modification.

...

quoted

@@ -503,6 +584,12 @@ static struct device_attribute device_attrs[] = {
 #ifdef CONFIG_FB_BACKLIGHT
 	__ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
 #endif
+#ifdef CONFIG_FB_DEFERRED_IO
+	__ATTR(defio_delay, S_IRUGO|S_IWUSR,
+	       show_defio_delay, store_defio_delay),
+	__ATTR(defio_delay_min, S_IRUGO, show_defio_delay_min, NULL),
+	__ATTR(defio_delay_max, S_IRUGO, show_defio_delay_max, NULL),
+#endif
 };

 int fb_init_device(struct fb_info *fb_info)

Would it be reasonable to add these attributes in
fb_deferred_io_init() and remove them in fb_deferred_io_cleanup()?
This would also make it possible to add write permission to delay
attribute only when it effectively can be modified.

IMHO having only attributes pertinent to the features supported by the
framebuffer is better than having all the possible ones and those of
unsupported features returning -ENODEV.

Bruno

I looked at that, but it seems like all the sysfs stuff has been
co-located in fbsysfs.c, so I tried to just follow the pattern of the
backlight code which takes the same approach.

-- 

Rick

Hi Rick,

On Fri, Feb 26, 2010 at 6:15 AM, Rick L. Vinyard Jr.
[off-list ref] wrote:

This patch adds support for examining and modifying the fbdefio delay
parameter through sysfs. It also adds two driver definable minimum
and maximum bounds.

Thanks for posting this. I've now read through this patch and your
past patches, but I couldn't get a good understanding of how this
userspace exposed defio delay would interact with most existing
systems.

+               Set the deferred I/O delay of the framebuffer in ms.
+               This value can be used to throttle deferred I/O updates.

Please help us understand how userspace should use the ability to set
this delay. Who in userspace is intended to use it? Would this be the
X server, cairo, or papyrus? A separate daemon? Human interaction?
When should said userspace entity throttle updates? How would this
entity know when throttling is needed? Could this throttling possibly
or should this possibly be done automatically by the driver itself?

+               Most framebuffer devices do not have or need support for
+               deferred I/O. Accessing a framebuffer without deferred I/O
+               support will return -ENODEV. Can be read but not modified if
+               /sys/class/graphics/<fb>/defio_delay_max is 0. When modifying,
+               the value must be greater than or equal to
+               /sys/class/graphics/<fb>/defio_delay_min and less than or equal
+               to /sys/class/graphics/<fb>/defio_delay_max.

Please help the reader of above understand the sequence of userspace
changing the defio delay, and how and when that would affect the
associated drivers. Will the delay behaviour be standard for all defio
client drivers? What happens in all the various timing scenarios, eg:
if a defio delay is changed in the middle of a display update or a
defio page clean or before? The sysfs parameter description mentioning
min/max above could use some elaboration as reading it doesn't make
clear if userspace can also affect min/max or if it is completely
owned by the driver.

Thanks,
jaya

ps: I only found your patches by accident when reading through the
fbdev mailing list. I'm very interested in defio (I happen to be the
author of it) and also drivers that use defio so I would appreciate
being CCed on such changes. Thanks a bunch.

Hi Jaya,

On Fri, 26 February 2010 Jaya Kumar [off-list ref] wrote:

On Fri, Feb 26, 2010 at 6:15 AM, Rick L. Vinyard Jr.
[off-list ref] wrote:

quoted

This patch adds support for examining and modifying the fbdefio
delay parameter through sysfs. It also adds two driver definable
minimum and maximum bounds.

Thanks for posting this. I've now read through this patch and your
past patches, but I couldn't get a good understanding of how this
userspace exposed defio delay would interact with most existing
systems.

In the case of my PicoLCD driver, the hardware can barely do 2 full
redraws of the display per second but in case only a single tile is
affected it can do more refreshes of such a single tile per second.

quoted

+               Set the deferred I/O delay of the framebuffer in ms.
+               This value can be used to throttle deferred I/O updates.

Please help us understand how userspace should use the ability to set
this delay. Who in userspace is intended to use it? Would this be the
X server, cairo, or papyrus? A separate daemon? Human interaction?
When should said userspace entity throttle updates? How would this
entity know when throttling is needed? Could this throttling possibly
or should this possibly be done automatically by the driver itself?

For me the driver would start with a default delay that matches the
full-redraw throughput of the device but userspace could reduce the
delay when it knows it will mostly just refresh small parts of the
display (one or two tiles) and would like those done at a higher rate.

A sample application would be displaying a media player interface
like the one of XMMS and clones where Umeter (the part displaying
volume per frequency range) could be refreshed ten times a second,
the current position once a second and all the rest only on song
change.

Knowing the size of the display, probability that it's being used
directly by X server is very small, it would rather be some application
using it as a sideport display.

quoted

+               Most framebuffer devices do not have or need support for
+               deferred I/O. Accessing a framebuffer without deferred I/O
+               support will return -ENODEV. Can be read but not modified if
+               /sys/class/graphics/<fb>/defio_delay_max is 0. When modifying,
+               the value must be greater than or equal to
+               /sys/class/graphics/<fb>/defio_delay_min and less than or equal
+               to /sys/class/graphics/<fb>/defio_delay_max.

Please help the reader of above understand the sequence of userspace
changing the defio delay, and how and when that would affect the
associated drivers. Will the delay behaviour be standard for all defio
client drivers? What happens in all the various timing scenarios, eg:
if a defio delay is changed in the middle of a display update or a
defio page clean or before? The sysfs parameter description mentioning
min/max above could use some elaboration as reading it doesn't make
clear if userspace can also affect min/max or if it is completely
owned by the driver.

Thanks,
Bruno

On Fri, Feb 26, 2010 at 6:53 PM, Bruno Prémont
[off-list ref] wrote:

For me the driver would start with a default delay that matches the
full-redraw throughput of the device but userspace could reduce the
delay when it knows it will mostly just refresh small parts of the
display (one or two tiles) and would like those done at a higher rate.

Who in userspace will know to reduce the delay? How will it know that
the delay should be reduced?

A sample application would be displaying a media player interface
like the one of XMMS and clones where Umeter (the part displaying
volume per frequency range) could be refreshed ten times a second,
the current position once a second and all the rest only on song
change.

xmms/umeter will talk to this sysfs entry?

Knowing the size of the display, probability that it's being used
directly by X server is very small, it would rather be some application
using it as a sideport display.

Yes, I'd like to know which applications these are.

Thanks,
jaya

On Fri, 26 February 2010 Jaya Kumar [off-list ref] wrote:

On Fri, Feb 26, 2010 at 6:53 PM, Bruno Prémont wrote:

quoted

For me the driver would start with a default delay that matches the
full-redraw throughput of the device but userspace could reduce the
delay when it knows it will mostly just refresh small parts of the
display (one or two tiles) and would like those done at a higher
rate.

Who in userspace will know to reduce the delay? How will it know that
the delay should be reduced?

The default (maybe even the largest) period would indicate what the
hardware can do in worst case and the smallest period what the hardware
can do in best/special cases (like single-tile update for PicoLCD)

Any application that wants to output information might want to know
how often the information can be displayed (what's the need to
calculate 1000 frames per second if only 2 of them will ever be seen
on display?). When possible and they know their display needs they
might benefit from ability to tune the result.

quoted

A sample application would be displaying a media player interface
like the one of XMMS and clones where Umeter (the part displaying
volume per frequency range) could be refreshed ten times a second,
the current position once a second and all the rest only on song
change.

xmms/umeter will talk to this sysfs entry?

Probably not XMMS itself but either a plugin for it or a human
interface to player that takes input from a remote control and
displays status to LCD (instead of/in addition to OSD)

quoted

Knowing the size of the display, probability that it's being used
directly by X server is very small, it would rather be some
application using it as a sideport display.

Yes, I'd like to know which applications these are.

I don't have an example at hand. I would rather see plugins for media
players (like those that use LCD4Linux or LCDproc) benefit from such
a feature.

Thanks,
Bruno

Jaya Kumar wrote:

Hi Rick,

On Fri, Feb 26, 2010 at 6:15 AM, Rick L. Vinyard Jr.
[off-list ref] wrote:

quoted

This patch adds support for examining and modifying the fbdefio delay
parameter through sysfs. It also adds two driver definable minimum
and maximum bounds.

Thanks for posting this. I've now read through this patch and your
past patches, but I couldn't get a good understanding of how this
userspace exposed defio delay would interact with most existing
systems.

quoted

+               Set the deferred I/O delay of the framebuffer in ms.
+               This value can be used to throttle deferred I/O updates.

Please help us understand how userspace should use the ability to set
this delay.

I think that there isn't one specific answer since, just like the approach
to priority of processes there isn't one specific answer. The real answer
is: it depends.

Bruno has already provided some feedback on his driver so I'll add a few
comments on what motivated it for the G13.

One of the factors I was concerned about was having two or more G13
devices on the same bus with other traffic. Each frame is sent as an
interrupt transfer out, and I was concerned that the framebuffer updates
might be detrimental to other traffic.

I know that a pair of G13 devices running at 30 FPS would only consume
about 5% of the USB 1.1 bandwidth (~500 Mbit/s) for framebuffer transfers
(sans headers and other control messages), but I was concerned that it
could introduce enough latency to make other devices laggy.

There is a bit of translation that has to go on for each framebuffer as
well. The G13 doesn't use the traditional bitwise layout where 0,0 is the
high bit of the first byte and 7,0 is the low bit of the first byte. The
format the G13 expects is 0,0 in the high bit and 0,7 in the low bit.

As a result of this translation, I was concerned with the driver consuming
too much CPU time when doing multiple translations for multiple devices...
or even one translation on a limited or saturated CPU at a high frame
rate.

Extending this beyond the simple case of the G13, I think it could be
useful for any deferred framebuffer that consumes a limited critical
resource, whether it be USB bandwidth, CPU usage, or some other resource.

One driver in particular that I noticed is the xen-fbfront. The update
rate is set at 20FPS, but what if someone wanted 30 FPS?

Who in userspace is intended to use it? Would this be the
X server, cairo, or papyrus? A separate daemon? Human interaction?

Again, that has the same issue as who sets priority on processes... It
depends.

As for the G13 I was planning, for the current time, on putting a slider
in the configuration window that allowed the user to modify the update
rate. The slider would populate off the sysfs values.

I think it would be an interesting possibility to develop a daemon that
watched traffic on buses with USB devices that could be throttled and
throttle them according to bus saturation.

However, at this point I wasn't planning on going there. But, having the
ability to throttle would be a key point for developing such daemons.

When should said userspace entity throttle updates? How would this
entity know when throttling is needed? Could this throttling possibly
or should this possibly be done automatically by the driver itself?

I don't think it should be incorporated into the driver because I believe
most circumstances will involve factors outside the purview of the driver,
such as bandwidth utilization. One factor could be rather subjective, like
'laggy'. What is 'laggy' to one user may not be to another.

quoted

+               Most framebuffer devices do not have or need support for
+               deferred I/O. Accessing a framebuffer without deferred
I/O
+               support will return -ENODEV. Can be read but not
modified if
+               /sys/class/graphics/<fb>/defio_delay_max is 0. When
modifying,
+               the value must be greater than or equal to
+               /sys/class/graphics/<fb>/defio_delay_min and less than
or equal
+               to /sys/class/graphics/<fb>/defio_delay_max.

Please help the reader of above understand the sequence of userspace
changing the defio delay, and how and when that would affect the
associated drivers. Will the delay behaviour be standard for all defio
client drivers? What happens in all the various timing scenarios, eg:
if a defio delay is changed in the middle of a display update or a
defio page clean or before?

I think this could be taken care of by explaining that the parameter will
only effect the next queued framebuffer update. But, it should definitely
be in there.

The sysfs parameter description mentioning
min/max above could use some elaboration as reading it doesn't make
clear if userspace can also affect min/max or if it is completely
owned by the driver.

I can add that as well.

Thanks,
jaya

ps: I only found your patches by accident when reading through the
fbdev mailing list. I'm very interested in defio (I happen to be the
author of it) and also drivers that use defio so I would appreciate
being CCed on such changes. Thanks a bunch.

No problem. I didn't mean to leave you out.

-- 

Rick

On Tue, Mar 2, 2010 at 12:10 AM, Rick L. Vinyard, Jr.
[off-list ref] wrote:

Jaya Kumar wrote:

quoted

Hi Rick,

On Fri, Feb 26, 2010 at 6:15 AM, Rick L. Vinyard Jr.
[off-list ref] wrote:

quoted

+               Set the deferred I/O delay of the framebuffer in ms.
+               This value can be used to throttle deferred I/O updates.

Please help us understand how userspace should use the ability to set
this delay.

I think that there isn't one specific answer since, just like the approach
to priority of processes there isn't one specific answer. The real answer
is: it depends.

Rick,

Thanks for your reply.

One of the factors I was concerned about was having two or more G13
devices on the same bus with other traffic. Each frame is sent as an
interrupt transfer out, and I was concerned that the framebuffer updates
might be detrimental to other traffic.

I know that a pair of G13 devices running at 30 FPS would only consume
about 5% of the USB 1.1 bandwidth (~500 Mbit/s) for framebuffer transfers
(sans headers and other control messages), but I was concerned that it
could introduce enough latency to make other devices laggy.

You mentioned in your responses and above that you're concerned about
saturating the USB bus and lagginess, and I think it is good to have
those concerns. Defio delay may be an easy parameter to expose, and in
at least a specific set of drivers, changing it would have a
meaningful effect on bus utilization, but are you convinced that
exposing it generally (remember your patch makes it apply to all fb
drivers) to userspace is a right way or even a good way to address bus
saturation concerns?

As a result of this translation, I was concerned with the driver consuming
too much CPU time when doing multiple translations for multiple devices...
or even one translation on a limited or saturated CPU at a high frame
rate.

Being concerned about CPU utilization is a good thing. But say for
example, your USB ethernet driver or USB audio driver is taking a lot
of cpu time packetizing traffic, then would I be correct that your
desire is to expose an inter-packetization driver specific sleep time
controllable by userspace via sysfs for all ethernet, audio, etc
drivers? (by driver specific, I mean the sysfs parameter would be
presented by all drivers but the value would be specific to each one,
which is the way your current patch would behave for all fb drivers).

I think it would be an interesting possibility to develop a daemon that
watched traffic on buses with USB devices that could be throttled and
throttle them according to bus saturation.

However, at this point I wasn't planning on going there. But, having the
ability to throttle would be a key point for developing such daemons.

Just curious, do such sysfs throttling parameters exist for any other
subsystems? If so, are userspace apps/services using them, and are
they happy with it?

Thanks,
jaya

Jaya Kumar wrote:

On Tue, Mar 2, 2010 at 12:10 AM, Rick L. Vinyard, Jr.
[off-list ref] wrote:

quoted

Jaya Kumar wrote:

quoted

Hi Rick,

On Fri, Feb 26, 2010 at 6:15 AM, Rick L. Vinyard Jr.
[off-list ref] wrote:

quoted

+               Set the deferred I/O delay of the framebuffer in ms.
+               This value can be used to throttle deferred I/O
updates.

Please help us understand how userspace should use the ability to set
this delay.

I think that there isn't one specific answer since, just like the
approach
to priority of processes there isn't one specific answer. The real
answer
is: it depends.

Rick,

Thanks for your reply.

quoted

One of the factors I was concerned about was having two or more G13
devices on the same bus with other traffic. Each frame is sent as an
interrupt transfer out, and I was concerned that the framebuffer updates
might be detrimental to other traffic.

I know that a pair of G13 devices running at 30 FPS would only consume
about 5% of the USB 1.1 bandwidth (~500 Mbit/s) for framebuffer
transfers
(sans headers and other control messages), but I was concerned that it
could introduce enough latency to make other devices laggy.

You mentioned in your responses and above that you're concerned about
saturating the USB bus and lagginess, and I think it is good to have
those concerns. Defio delay may be an easy parameter to expose, and in
at least a specific set of drivers, changing it would have a
meaningful effect on bus utilization, but are you convinced that
exposing it generally (remember your patch makes it apply to all fb
drivers) to userspace is a right way or even a good way to address bus
saturation concerns?

quoted

As a result of this translation, I was concerned with the driver
consuming
too much CPU time when doing multiple translations for multiple
devices...
or even one translation on a limited or saturated CPU at a high frame
rate.

Being concerned about CPU utilization is a good thing. But say for
example, your USB ethernet driver or USB audio driver is taking a lot
of cpu time packetizing traffic, then would I be correct that your
desire is to expose an inter-packetization driver specific sleep time
controllable by userspace via sysfs for all ethernet, audio, etc
drivers? (by driver specific, I mean the sysfs parameter would be
presented by all drivers but the value would be specific to each one,
which is the way your current patch would behave for all fb drivers).

I think the answer is yes, whether this were a fb, network, audio, etc. If
there is a clearly defined parameter at which resource utilization can be
adjusted in a standard way.

Although the value is specific to each one, the min/max parameters provide
the bounds and standardized units, allowing userspace to modify the delay
parameter in a standardized way.

I also tried to make it where the driver had to explicitly modify the
min/max parameters so that unless a driver sets min/max values the delay
parameter can be examined from userspace but not modified.

I agree that the motivation for throttling varies by device, and even as
in this case there could be different motivations for the same device (USB
bus bandwidth and/or CPU utilization).

I think that what also distinguishes the fbdefio case is the fact that
there is a single defined point at which resource utilization would be
throttled and that's the fb_deferred_io structure's deferred_io hook.

quoted

I think it would be an interesting possibility to develop a daemon that
watched traffic on buses with USB devices that could be throttled and
throttle them according to bus saturation.

However, at this point I wasn't planning on going there. But, having the
ability to throttle would be a key point for developing such daemons.

Just curious, do such sysfs throttling parameters exist for any other
subsystems? If so, are userspace apps/services using them, and are
they happy with it?

An example of something like this is the ACPI CPU thermal class. It
provides two parameters, cur_state and max_state. The minimum is presumed
to be 0.

In this case though, I think fbdefio needs to provide both a min and a max
as a driver may want to set the minimum refresh rate to be non-zero.

Another example would be the CPU frequency subsystem, and in particular
the userspace governor. There are similar sysfs attributes for obtaining
the current frequency, setting the frequency, and obtaining the min/max
frequencies.

In particular they used:
     scaling_cur_freq (ro)
     scaling_min_freq (ro)
     scaling_max_freq (ro)
     scaling_setspeed (wo)

I used a single read/write attribute for read/write on the delay instead
of the two parameter approach used for frequencies. I used:
     defio_delay     (rw)
     defio_delay_min (ro)
     defio_delay_max (ro)

But, it wouldn't be hard to use an analogous set like:
     defio_cur_delay (ro)
     defio_min_delay (ro)
     defio_max_delay (ro)
     defio_set_delay (wo)

-- 

Rick

On Tue, Mar 2, 2010 at 11:36 PM, Rick L. Vinyard, Jr.
[off-list ref] wrote:

Jaya Kumar wrote:

quoted

On Tue, Mar 2, 2010 at 12:10 AM, Rick L. Vinyard, Jr.
[off-list ref] wrote:

quoted

Jaya Kumar wrote:

Being concerned about CPU utilization is a good thing. But say for
example, your USB ethernet driver or USB audio driver is taking a lot
of cpu time packetizing traffic, then would I be correct that your
desire is to expose an inter-packetization driver specific sleep time
controllable by userspace via sysfs for all ethernet, audio, etc
drivers? (by driver specific, I mean the sysfs parameter would be
presented by all drivers but the value would be specific to each one,
which is the way your current patch would behave for all fb drivers).

I think the answer is yes, whether this were a fb, network, audio, etc. If
there is a clearly defined parameter at which resource utilization can be
adjusted in a standard way.

Hi Rick,

Sure, we can find these driver resource utilization choke points, and
maybe even make it sort of standard, but that does not mean that we
should expose all of this to userspace. Adding more userspace tunables
for each driver in order to effect fb, network, audio bus utilization,
is not appealing. So I think we disagree about the fundamentals.

My conclusion is: I'm opposed this patch, because it exposes the defio
delay parameter for _all_ fb drivers, _each_ through a
/sys/class/graphics/fb_driver_name/defio_delay sysfs entry and also
adds a min/max issue. The behaviour and system impact seen by
userspace when changing the parameter is not standard across the
typical range of systems and devices. More importantly, exposing each
driver's defio delay to userspace is not a good way to address the 2
underlying functional goals that were raised during this discussion:
1) being able to control a driver's bus/cpu utilization, and 2)
allowing certain plugins/etc to be able to increase display update
frequency for subregions of the display.

Just to be verbose, please don't let my rejection of this specific
patch affect your other patches as I see those as being very useful
and close to being suitable for merging.

Thanks,
jaya

Jaya Kumar wrote:

On Tue, Mar 2, 2010 at 11:36 PM, Rick L. Vinyard, Jr.
[off-list ref] wrote:

quoted

Jaya Kumar wrote:

quoted

On Tue, Mar 2, 2010 at 12:10 AM, Rick L. Vinyard, Jr.
[off-list ref] wrote:

quoted

Jaya Kumar wrote:

Being concerned about CPU utilization is a good thing. But say for
example, your USB ethernet driver or USB audio driver is taking a lot
of cpu time packetizing traffic, then would I be correct that your
desire is to expose an inter-packetization driver specific sleep time
controllable by userspace via sysfs for all ethernet, audio, etc
drivers? (by driver specific, I mean the sysfs parameter would be
presented by all drivers but the value would be specific to each one,
which is the way your current patch would behave for all fb drivers).

I think the answer is yes, whether this were a fb, network, audio, etc.
If
there is a clearly defined parameter at which resource utilization can
be
adjusted in a standard way.

Hi Rick,

Sure, we can find these driver resource utilization choke points, and
maybe even make it sort of standard, but that does not mean that we
should expose all of this to userspace. Adding more userspace tunables
for each driver in order to effect fb, network, audio bus utilization,
is not appealing. So I think we disagree about the fundamentals.

My conclusion is: I'm opposed this patch, because it exposes the defio
delay parameter for _all_ fb drivers, _each_ through a
/sys/class/graphics/fb_driver_name/defio_delay sysfs entry and also
adds a min/max issue. The behaviour and system impact seen by
userspace when changing the parameter is not standard across the
typical range of systems and devices. More importantly, exposing each
driver's defio delay to userspace is not a good way to address the 2
underlying functional goals that were raised during this discussion:
1) being able to control a driver's bus/cpu utilization, and 2)
allowing certain plugins/etc to be able to increase display update
frequency for subregions of the display.

Just to be verbose, please don't let my rejection of this specific
patch affect your other patches as I see those as being very useful
and close to being suitable for merging.

No problem. I understand the desire to be conservative.