mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@suse.de>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	mpm@selenic.com
Subject: [patch] mm: tiny-shmem fix lor, mmap_sem vs i_mutex
Date: Mon, 15 Sep 2008 00:12:31 +0200	[thread overview]
Message-ID: <20080914221231.GG27080@wotan.suse.de> (raw)
In-Reply-To: <20080914080631.GA10720@elte.hu>

On Sun, Sep 14, 2008 at 10:06:31AM +0200, Ingo Molnar wrote:
> 
> * Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > > [ 6460.634452] 
> > > [ 6460.634465] =======================================================
> > > [ 6460.634494] [ INFO: possible circular locking dependency detected ]
> > > [ 6460.634517] 2.6.27-rc6-tip-00290-g8e229c3-dirty #1
> > > [ 6460.634535] -------------------------------------------------------
> > > [ 6460.634555] gdm-simple-gree/4778 is trying to acquire lock:
> > > [ 6460.634574]  (&mm->mmap_sem){----}, at: [<c018fe33>] might_fault+0x36/0x73
> > > [ 6460.634639] 
> > > [ 6460.634645] but task is already holding lock:
> > > [ 6460.634662]  (&dev->ev_mutex){--..}, at: [<c01c7a76>] inotify_read+0xd8/0x16e
> > > [ 6460.634715] 
> > > [ 6460.634721] which lock already depends on the new lock.
> > 
> > Yes, there's a thread in my intray called "inotify_read's ev_mutex vs 
> > do_page_fault's mmap_sem...".  It's a bit flakey-looking, but there's 
> > a patch in there.
> 
> ah, thx. I picked up the patch into tip/out-of-tree. (see below for a 
> tided up changelog) Please queue it up as v2.6.27 material. (i'll report 
> it if anything breaks due to the patch)
> 
> 	Ingo
> 
> --------------->
> >From 1eb0a42e4eb3283521ee1de99adbf567874b622f Mon Sep 17 00:00:00 2001
> From: Nick Piggin <nickpiggin@yahoo.com.au>
> Date: Thu, 11 Sep 2008 06:12:51 +1000
> Subject: [PATCH] mm: fix locking, inotify_read's ev_mutex vs do_page_fault's mmap_sem...

I would call this "fs: fix inotify locking....", it's not really an mm bug
if another subsystem misuses mm's APIs. But that's a nitpick.

Here is the other patch I did too.


tiny-shmem calls do_truncate in shmem_file_setup. do_truncate takes i_mutex,
and shmem_file_setup is called with mmap_sem held. However i_mutex nests
outside mmap_sem.

Copy the code in shmem.c to avoid this problem.

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
Index: linux-2.6/mm/tiny-shmem.c
===================================================================
--- linux-2.6.orig/mm/tiny-shmem.c
+++ linux-2.6/mm/tiny-shmem.c
@@ -65,31 +65,25 @@ struct file *shmem_file_setup(char *name
 	if (!dentry)
 		goto put_memory;
 
+        error = -ENFILE;
+        file = get_empty_filp();
+        if (!file)
+                goto put_dentry;
+
 	error = -ENOSPC;
 	inode = ramfs_get_inode(root->d_sb, S_IFREG | S_IRWXUGO, 0);
 	if (!inode)
-		goto put_dentry;
-
-	d_instantiate(dentry, inode);
-	error = -ENFILE;
-	file = alloc_file(shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
-			&ramfs_file_operations);
-	if (!file)
-		goto put_dentry;
-
-	inode->i_nlink = 0;	/* It is unlinked */
-
-	/* notify everyone as to the change of file size */
-	error = do_truncate(dentry, size, 0, file);
-	if (error < 0)
 		goto close_file;
 
+        d_instantiate(dentry, inode);
+        inode->i_size = size;
+        inode->i_nlink = 0;     /* It is unlinked */
+        init_file(file, shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
+                        &ramfs_file_operations);
 	return file;
 
 close_file:
 	put_filp(file);
-	return ERR_PTR(error);
-
 put_dentry:
 	dput(dentry);
 put_memory:

  reply	other threads:[~2008-09-14 22:12 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-10 11:37 [patch] x86: some lock annotations for user copy paths Nick Piggin
2008-09-10 11:41 ` Peter Zijlstra
2008-09-10 11:47   ` Ingo Molnar
2008-09-10 11:50     ` Ingo Molnar
2008-09-10 12:12     ` Ingo Molnar
2008-09-10 12:32       ` Ingo Molnar
2008-09-10 15:12         ` Nick Piggin
2008-09-10 14:48       ` Nick Piggin
2008-09-10 15:01         ` Peter Zijlstra
2008-09-10 15:17           ` Nick Piggin
2008-09-10 15:26           ` Nick Piggin
2008-09-11  8:27             ` Ingo Molnar
2008-09-11 10:43               ` Nick Piggin
2008-09-12  9:24                 ` [PATCH] sysfs: fix deadlock Ingo Molnar
2008-09-14 22:02                   ` Nick Piggin
2008-09-15  9:15                     ` Peter Zijlstra
2008-09-14  7:39               ` [lockdep] possible circular locking, between &mm->mmap_sem and &dev->ev_mutex Ingo Molnar
2008-09-14  7:44                 ` Andrew Morton
2008-09-14  8:06                   ` [patch] mm: fix locking, inotify_read's ev_mutex vs do_page_fault's mmap_sem Ingo Molnar
2008-09-14 22:12                     ` Nick Piggin [this message]
2008-09-17 20:14                       ` [patch] mm: tiny-shmem fix lor, mmap_sem vs i_mutex Andrew Morton
2008-09-17 20:46                         ` Matt Mackall
2008-09-18 11:12                         ` Ingo Molnar
2008-09-18 19:29                           ` Jeremy Fitzhardinge
2008-09-18 21:11                             ` Matt Mackall
2008-09-20  2:18                               ` Dave Hansen
2008-09-20 16:12                                 ` Hugh Dickins
2008-09-22 15:14                                   ` Dave Hansen
2008-09-22 14:54                                 ` David Howells
2008-09-23  5:32                                   ` Nick Piggin
2008-09-24 19:29                                     ` Hugh Dickins
2008-09-24 19:47                                       ` Andrew Morton
2008-09-24 18:18                                   ` David Howells
2008-09-24 18:29                                     ` Matt Mackall
2008-09-24 19:41                                       ` Hugh Dickins
2008-09-24 19:59                                       ` David Howells
2008-09-24 23:43                                         ` Hugh Dickins
2008-09-24 18:56                                     ` David Howells
2008-09-24 19:11                                       ` Matt Mackall
2008-09-24 19:26                                       ` David Howells
2008-09-19  8:40                             ` Ingo Molnar
2008-09-14 21:36                   ` [lockdep] possible circular locking, between &mm->mmap_sem and &dev->ev_mutex Nick Piggin
2008-09-10 14:30     ` [patch] x86: some lock annotations for user copy paths Nick Piggin
2008-09-10 14:31 ` Andi Kleen

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=20080914221231.GG27080@wotan.suse.de \
    --to=npiggin@suse.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mpm@selenic.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®