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

["Antonino A. Daplas" <adaplas@gmail.com>]

Geert Uytterhoeven wrote:
> On Thu, 28 Jul 2005, Antonino A. Daplas wrote:
>> Jon Smirl wrote:
>>> On 7/28/05, Geert Uytterhoeven <geert@linux-m68k.org> 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