Re: [Linux-fbdev-devel] Re: [PATCH] fbdev: colormap fixes

From: Antonino A. Daplas
Date: Thu Jul 28 2005 - 18:21:44 EST


Geert Uytterhoeven wrote:
On Thu, 28 Jul 2005, Antonino A. Daplas wrote:
Jon Smirl wrote:
On 7/28/05, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
On Wed, 27 Jul 2005, Linux Kernel Mailing List wrote:
There are a couple of ways to fix this. 1) Add a check to limit use of the sysfs attributes to 256 entries. If
you want more you have to use /dev/fb0 and the ioctl. More is an
uncommon case.
2) Switch this to a binary parameter. Now you have to use tools like
hexdump instead of cat to work with the data. It was nice to be able
to use cat to see the current map.

Does anyone have preferences for which way to fix it?
Or...

3) Add another file in sysfs which specifies at what index and how many
entries will be read or written from or to the cmap. With this additional
sysfs file, it should be able to handle any reasonable cmap length, but
it will take more than one reading of the color_map file. Another
advantage is that the entire color map need not be read or written if
only one field needs to be changed.

I've attached a test patch. Let me know what you think.

I like it! ... But, a disadvantages is that it needs to store state between two
non-atomic operations. E.g. imagine two processes doing this at the same time.

We can add a check that if the incoming buffer index start and length does
not match the current start and len (as set by cmap_range), then exit with
and empty buffer.

Conversely, users can always check if the output read from color_map matches
the value it entered in cmap_range.

As a side note:

The rest of the fbsysfs attributes suffer from the same thing, especially since the
value of each attribute depends on each other. What if one process reads the
virtual_size, while another one changes the bits_per_pixel? I'm pretty sure that
setting bits_per_pixel to a higher value will almost always change the virtual_size.

Currently, the only way to guarantee atomicity is to use the ioctls. The main
reason why we change the video mode, framebuffer format, color format, etc in one
go with fb_set_var() is precisely this.

Tony



-
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/