linux-kernel - Re: [PATCH 05/10] fblog: add framebuffer helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANq1E4RZoxUFJ+Ofx_MVu8xBvRdsS2kmBusKWEitEjPBwukpkA@mail.gmail.com>
Date:	Mon, 18 Jun 2012 20:50:12 +0200
From:	David Herrmann <dh.herrmann@...glemail.com>
To:	Bruno Prémont <bonbons@...ux-vserver.org>
Cc:	linux-serial@...r.kernel.org,
	Florian Tobias Schandinat <FlorianSchandinat@....de>,
	linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 05/10] fblog: add framebuffer helpers

Hi Bruno

On Sun, Jun 17, 2012 at 12:33 AM, Bruno Prémont
<bonbons@...ux-vserver.org> wrote:
> On Sun, 17 June 2012 David Herrmann <dh.herrmann@...glemail.com> wrote:
>> These helpers scan the system for all available framebuffers and register
>> or unregister them. This is needed during startup and stopping fblog so we
>> are aware of all connected displays.
>>
>> The third helper handles mode changes by rescanning the mode and adjusting
>> the buffer size.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@...glemail.com>
>> ---
>>  drivers/video/console/fblog.c |   29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/video/console/fblog.c b/drivers/video/console/fblog.c
>> index e790971..7d4032e 100644
>> --- a/drivers/video/console/fblog.c
>> +++ b/drivers/video/console/fblog.c
>> @@ -399,6 +399,35 @@ static void fblog_unregister(struct fblog_fb *fb)
>>       kfree(fb);
>>  }
>>
>> +static void fblog_register_all(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < FB_MAX; ++i)
>> +             fblog_register(registered_fb[i]);
>
> You should take registration_lock mutex for accessing registered_fb[],
> even better would be to make use of get_fb_info() and put_fb_info()

Indeed, I will change it to use get_fb_info()/put_fb_info().

Btw., is it safe to call console_lock() during
FB_EVENT_FB_(UN)REGISTERED? I definitely need the console lock when
calling fblog_register() but I am not sure whether all fb-drivers lock
the console during "(un)register_framebuffer()". Otherwise, I would
have to avoid redrawing the console inside of fblog_register().

>> +}
>> +
>> +static void fblog_unregister_all(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < FB_MAX; ++i)
>> +             fblog_unregister(fblog_info2fb(registered_fb[i]));
>
> Same here.
>
> Though for unregistering I'm wondering why you still scan through
> registered_fb[], you should just scan your fblog_fbs[] array!

Oops, you're right. I will change it.

> But here again, make sure to have proper locking to not get races with
> registration of new framebuffers or removal of existing ones via
> notifications.
>
>> +}
>> +
>> +static void fblog_refresh(struct fblog_fb *fb)
>> +{
>> +     unsigned int width, height;
>> +
>> +     if (!fb || !fb->font)
>> +             return;
>> +
>> +     width = fb->info->var.xres / fb->font->width;
>> +     height = fb->info->var.yres / fb->font->height;
>> +     fblog_buf_resize(&fb->buf, width, height);
>> +     fblog_redraw(fb);
>> +}
>> +
>
> All these new functions are still unused, for easier following of your
> patch series it would be nice to have them connected when they are
> introduced as otherwise on has to search all following patches for
> finding possible users.

I have to admit the split was crap. I am sorry. I will recreate the
patchset with a proper split. Thanks!

>>  static int __init fblog_init(void)
>>  {
>>       return 0;
>
> Bruno

Thanks for reviewing, regards
David
--
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/