Re: [Linux-fbdev-devel] fix fb_info->lock and mm->mmap_sem problem again

[Ville S. <sy...@sc...>]

On Sun, May 03, 2009 at 09:19:38PM +0200, Krzysztof Helt wrote:
> Hi,
> 
> 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().

-- 
Ville Syrjälä
sy...@sc...
http://www.sci.fi/~syrjala/