Re: [PATCH] Add sysfs support for fbdefio delay

[Jaya Kumar]

On Tue, Mar 2, 2010 at 11:36 PM, Rick L. Vinyard, Jr.
<rvinyard@xxxxxxxxxxx> wrote:
>
> Jaya Kumar wrote:
>> On Tue, Mar 2, 2010 at 12:10 AM, Rick L. Vinyard, Jr.
>> <rvinyard@xxxxxxxxxxx> wrote:
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/