Re: fix fb_info->lock and mm->mmap_sem problem again

[Geert Uytterhoeven <geert@linux-m68k.org>]

On Mon, May 4, 2009 at 00:54, Ville Syrjälä <syrjala@sci.fi> wrote:
> On Sun, May 03, 2009 at 09:19:38PM +0200, Krzysztof Helt wrote:
>> I have run into the lockdep problem between the fb_info->lock and mm->mmap_sem again.
>> I run the Mplayer with fbdev output device. The latest git tree.
>>
>> I looked through the fb_info->lock usage and I have some questions.
>>
>> The main issue is that fb_mmap acquires the fb_info->lock. I wonder
>> what is protected there. The part of the fb_mmap which uses the lock
>> is below:
>>
>> ...
>>     if (fb->fb_mmap) {
>>         int res;
>>         mutex_lock(&info->lock);
>>         res = fb->fb_mmap(info, vma);
>>         mutex_unlock(&info->lock);
>>         return res;
>>     }
>>
>>     mutex_lock(&info->lock);
>>
>>     /* frame buffer memory */
>>     start = info->fix.smem_start;
>>     len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len);
>>     if (off >= len) {
>>         /* memory mapped io */
>>         off -= len;
>>         if (info->var.accel_flags) {
>>             mutex_unlock(&info->lock);
>>             return -EINVAL;
>>         }
>>         start = info->fix.mmio_start;
>>         len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len);
>>     }
>>     mutex_unlock(&info->lock);
>> ...
>>
>> The lock protects two things:
>> 1. Read of the info->fix.smem_start and info->fix.smem_len.
>> 2. The fbops->fb_mmap call.
>>
>> The first point seems redundant as I have checked and
>> the smem_start and smem_len are only set in the
>> probe and release functions. They are always set
>> before the register_framebuffer() is called and
>> after the unregister_framebuffer() call.
>
> matroxfb changes smem_start/len in set_par().

Exactly my comment: most drivers that support multiple heads need some memory
management scheme, which means the location and size of the memory allocated
to a specific head may change.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

------------------------------------------------------------------------------
Register Now & Save for Velocity, the Web Performance & Operations 
Conference from O'Reilly Media. Velocity features a full day of 
expert-led, hands-on workshops and two days of sessions from industry 
leaders in dedicated Performance & Operations tracks. Use code vel09scf 
and Save an extra 15% before 5/3. http://p.sf.net/sfu/velocityconf
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel