All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Borislav Petkov <petkovbb@googlemail.com>,
	David Airlie <airlied@linux.ie>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg KH <greg@kroah.com>, Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: drm_vm.c:drm_mmap: possible circular locking dependency detected (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...)
Date: Wed, 30 Dec 2009 13:34:38 -0800	[thread overview]
Message-ID: <m11victac1.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <alpine.LFD.2.00.0912301259220.11961@localhost.localdomain> (Linus Torvalds's message of "Wed\, 30 Dec 2009 13\:10\:51 -0800 \(PST\)")

Linus Torvalds <torvalds@linux-foundation.org> writes:

> We've seen it several times (yes, mostly with drm, but it's been seen with 
> others too), and it's very annoying. It can be fixed by having very 
> careful readdir implementations, but I really would blame sysfs in 
> particular for having a very annoying lock reversal issue when used 
> reasonably.

Maybe.  The mnmap_sem has some interesting issues all of it's own.
What reasonable thing is the drm doing that is causing problems?

> So the optimal situation would be for sysfs to not have that annoying lock 
> dependency, and it would really have to be sysfs_readdir() that drops the 
> sysfs_mutex around the filldir call (and that obviously implies having to 
> re-validate and be really careful).
>
> Added Eric and Greg to the cc, in case the sysfs people want to solve it.

There are scalability reasons for dropping the sysfs_mutex in sysfs_readdir
and I have some tenative patches for that.  I will take a look after I
come back from the holidays, in a couple of days.  I don't understand
the issue as described.

> And yes, one option would be to just fix drm - by avoiding calling any 
> sysfs functions while holding the mmap_lock (either in the mmap callback 
> or the page fault paths). However, as mentioned, I really do think that 
> the blame can be laid on sysfs for trying to be a nice generic interface, 
> but having a damn inconvenient locking model.

Could be.  I have simplified the sysfs locking quite a bit this last
round.  I don't know if there is much more than corner cases left to
improve.

Eric


  reply	other threads:[~2009-12-30 21:34 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-24 22:00 Linux 2.6.33-rc2 - Merry Christmas Linus Torvalds
2009-12-25 10:27 ` -tip: origin tree boot crash Ingo Molnar
2009-12-25 19:49   ` Dmitry Torokhov
2009-12-26 20:19     ` Len Brown
2009-12-26 20:17   ` Len Brown
2009-12-27  4:20     ` Len Brown
2009-12-28  9:44       ` Ingo Molnar
2009-12-28 12:01         ` Ingo Molnar
2009-12-28 15:02           ` Paul Rolland
2009-12-28 16:15             ` Paul Rolland
2009-12-28 16:53             ` Paul Rolland
2009-12-28 20:17               ` Dmitry Torokhov
2009-12-30  6:14               ` Len Brown
2009-12-30  7:13                 ` Paul Rolland
2009-12-30  6:19               ` [PATCH] wmi: check find_guid() return value to prevent oops Len Brown
2009-12-30  6:21               ` [PATCH] dell-wmi: sys_init_module: 'dell_wmi'->init suspiciously returned 21, it should follow 0/-E convention Len Brown
2009-12-25 13:10 ` Linux 2.6.33-rc2 - Blank screen for Intel KMS Miguel Calleja
2009-12-29  9:50   ` Miguel Calleja
2009-12-29 14:01     ` Rafael J. Wysocki
2009-12-25 20:00 ` Linux 2.6.33-rc2 - Merry Christmas Borislav Petkov
2009-12-25 21:50   ` Borislav Petkov
2009-12-26  6:00     ` Jesse Barnes
2009-12-26  8:02       ` Borislav Petkov
2009-12-26  9:36 ` EHCI resume sysfs duplicates (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...) Borislav Petkov
2009-12-26  9:45 ` drm_vm.c:drm_mmap: possible circular locking dependency detected " Borislav Petkov
2009-12-28  0:40   ` KOSAKI Motohiro
2009-12-30 21:10     ` Linus Torvalds
2009-12-30 21:34       ` Eric W. Biederman [this message]
2009-12-30 22:03         ` Linus Torvalds
2009-12-31  8:40           ` Eric W. Biederman
2009-12-31 19:04             ` Linus Torvalds
2010-01-01 13:58               ` [PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability Eric W. Biederman
2010-01-01 15:33                 ` Borislav Petkov
2010-01-01 18:56                 ` Linus Torvalds
2010-01-01 22:43                   ` [PATCH] sysfs: Cache the last sysfs_dirent to improve readdir scalability v2 Eric W. Biederman
2010-01-01 23:10                     ` Linus Torvalds
2010-01-02  5:59                       ` Greg KH
2010-01-02 15:40                       ` Borislav Petkov
2010-01-01 15:16               ` drm_vm.c:drm_mmap: possible circular locking dependency detected (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...) Eric W. Biederman
2010-01-02  2:59                 ` drm_vm.c:drm_mmap: possible circular locking dependency detected Tejun Heo
2010-01-02 21:37                   ` [PATCH] sysfs: Add lockdep annotations for the sysfs active reference Eric W. Biederman
2010-01-03  0:02                     ` Tejun Heo
2010-01-17 16:26                     ` Ming Lei
2010-01-17 17:18                       ` Eric W. Biederman
2010-01-17 18:03                         ` Dominik Brodowski
2010-01-02 21:49                   ` drm_vm.c:drm_mmap: possible circular locking dependency detected Eric W. Biederman
2010-01-03  0:32                     ` Tejun Heo
2010-01-03  2:06                       ` Eric W. Biederman
2010-01-03  5:01                         ` Tejun Heo
2010-01-03  5:38                           ` Eric W. Biederman
2010-01-03  6:05                             ` Tejun Heo
2010-01-03  7:47                       ` Dmitry Torokhov
2010-01-03 10:57                         ` Eric W. Biederman
2010-01-03 11:14                           ` Eric W. Biederman
2010-01-04 19:16                             ` Dmitry Torokhov
2010-01-04 18:57                           ` Dmitry Torokhov
2010-01-04 19:43                             ` Eric W. Biederman
2010-01-04 21:12                               ` Dmitry Torokhov
2010-01-04 23:09                               ` Tejun Heo
2009-12-31  8:40           ` drm_vm.c:drm_mmap: possible circular locking dependency detected (was: Re: Linux 2.6.33-rc2 - Merry Christmas ...) Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m11victac1.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=airlied@linux.ie \
    --cc=greg@kroah.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=petkovbb@googlemail.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.