Message-ID: <4C97BF40.6030708@gmx.de>
Date:	Mon, 20 Sep 2010 22:08:32 +0200
From:	Florian Tobias Schandinat <FlorianSchandinat@....de>
To:	Bruno Prémont <bonbons@...ux-vserver.org>
CC:	linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	Bernie Thompson <bernie@...gable.com>
Subject: Re: [Patch, RFC] Make struct fb_info ref-counted with kref

Bruno Prémont schrieb:
> On Mon, 20 September 2010 Florian Tobias Schandinat <FlorianSchandinat@....de> wrote:
>> Bruno Prémont schrieb:
>>> On Sun, 19 September 2010 Florian Tobias Schandinat <FlorianSchandinat@....de> wrote:
>>>> Bruno Prémont schrieb:
>>>>> If you have concerns regarding the API changes, please let me know.
>>>> Uhm, I'm not really happy with what we count. With the old method you mentioned 
>>>> we ref-counted framebuffer users, after your patch it's more counting users + 
>>>> uses. This might be okay as we usually are interested whether the ref_count is 0 
>>>> or not but it doesn't look right if we modify the refcount during nearly every 
>>>> framebuffer operation. Wouldn't it be sufficient to do the refcounting in 
>>>> fb_open & fb_release operation + in fbcon where open&release are done?
>>> Well I'm more for counting the uses, (especially as the aim is to not
>>> force the driver to look exactly when it can free the fb_info struct).
>>> If the driver needs to know about active users (e.g. for handling memory
>>> reorganization on mode change or the like) that would remain driver's job.
>> I don't see how your counting would influence the time fb_info is freed. It is 
>> freed when the last reference is gone but the last remaining reference is always 
>>   a user reference either from the framebuffer itself or from any user. But all 
>> users have to keep the framebuffer open to do anything with it therfore the last 
>> thing they do is releasing the framebuffer. So I do not really understand your 
>> reasoning, for me counting the users + uses is more error prone than just the 
>> users. But that's not important for me as I'm only interested in whether the 
>> count is 0, 1 or more (want to turn off the screen if there are no active [=1] 
>> users) which is the same regardless on what you count. So if you really want to 
>> stick to your way of counting, that's no problem for me.
> 
> In case of picoLCD driver (which uses a shadow framebuffer in system RAM) the
> last user can be a (userspace) process as on unplug driver unregisters that
> framebuffer and hands back it's own reference, the fb_destroy callback being
> in charge of freeing the shadow framebuffer when fb_info is being freed.

True. I think I understand the problem you want to solve.
My question is:
Do you keep a reference for each successful open operation until a release is done?
If I read your patch correctly, the answer is yes.
Than the operations/counting you do between such operations should be irrelevant 
to when the free is performed or?
So the free is done either when the framebuffer releases its handle or (in your 
case) when the process closes the file and therefore calls fb_release. Or do you 
have any way to perform framebuffer operations without an open framebuffer?

> This is quite different from built-in GPU with its video-ram.

I'm well aware of that. It is not always easy to understand but I got that your 
framebuffer can release its reference before the last user closes it. We do not 
disagree on that.


Thanks,

Florian Tobias Schandinat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists