From: Frederic Weisbecker <fweisbec@gmail.com>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Christoph Hellwig <hch@lst.de>,
Andrew Morton <akpm@linux-foundation.org>,
reiserfs-devel@vger.kernel.org, linux-kernel@vger.kernel.org,
Chris Mason <chris.mason@oracle.com>,
Jeff Mahoney <jeffm@jeffreymahoney.com>
Subject: Re: reiserfs locking (v2)
Date: Fri, 2 Jul 2010 15:12:52 +0200 [thread overview]
Message-ID: <20100702131248.GA5324@nowhere> (raw)
In-Reply-To: <20100702093451.GA3973@swordfish.minsk.epam.com>
On Fri, Jul 02, 2010 at 12:34:51PM +0300, Sergey Senozhatsky wrote:
> Hello,
>
> I searched lkml and found the following report (titled reiserfs locking):
> http://lkml.org/lkml/2010/4/15/389 (Fri, 16 Apr 2010)
>
> I can see this backtrace while configuring emacs:
>
> [ 6136.579013]
> [ 6136.579015] =======================================================
> [ 6136.579021] [ INFO: possible circular locking dependency detected ]
> [ 6136.579025] 2.6.35-rc3-dbg-git3-00350-g94d119f #61
> [ 6136.579027] -------------------------------------------------------
> [ 6136.579031] conftest/13950 is trying to acquire lock:
> [ 6136.579034] (&sb->s_type->i_mutex_key#10){+.+.+.}, at: [<c10f1cf6>] reiserfs_file_release+0x11d/0x344
> [ 6136.579050]
> [ 6136.579051] but task is already holding lock:
> [ 6136.579054] (&mm->mmap_sem){++++++}, at: [<c1091b87>] sys_munmap+0x26/0x42
> [ 6136.579064]
> [ 6136.579065] which lock already depends on the new lock.
> [ 6136.579066]
> [ 6136.579069]
> [ 6136.579070] the existing dependency chain (in reverse order) is:
> [ 6136.579073]
> [ 6136.579074] -> #1 (&mm->mmap_sem){++++++}:
> [ 6136.579081] [<c104f53a>] lock_acquire+0x59/0x70
> [ 6136.579089] [<c108cf78>] might_fault+0x53/0x70
> [ 6136.579094] [<c11853b8>] copy_to_user+0x30/0x48
> [ 6136.579101] [<c10afaf9>] filldir64+0x95/0xc9
> [ 6136.579106] [<c10f2504>] reiserfs_readdir_dentry+0x35d/0x4d9
> [ 6136.579112] [<c10f2692>] reiserfs_readdir+0x12/0x17
> [ 6136.579117] [<c10afd17>] vfs_readdir+0x6d/0x92
> [ 6136.579122] [<c10afe91>] sys_getdents64+0x63/0xa2
> [ 6136.579127] [<c10027d3>] sysenter_do_call+0x12/0x32
> [ 6136.579134]
> [ 6136.579135] -> #0 (&sb->s_type->i_mutex_key#10){+.+.+.}:
> [ 6136.579142] [<c104ef30>] __lock_acquire+0x96d/0xbe1
> [ 6136.579148] [<c104f53a>] lock_acquire+0x59/0x70
> [ 6136.579153] [<c12c5614>] __mutex_lock_common+0x39/0x36b
> [ 6136.579160] [<c12c5980>] mutex_lock_nested+0x12/0x15
> [ 6136.579165] [<c10f1cf6>] reiserfs_file_release+0x11d/0x344
> [ 6136.579171] [<c10a580d>] fput+0xe0/0x16a
> [ 6136.579177] [<c1090ca6>] remove_vma+0x28/0x47
> [ 6136.579182] [<c1091a68>] do_munmap+0x1e8/0x200
> [ 6136.579188] [<c1091b93>] sys_munmap+0x32/0x42
> [ 6136.579193] [<c10027d3>] sysenter_do_call+0x12/0x32
> [ 6136.579198]
> [ 6136.579199] other info that might help us debug this:
> [ 6136.579200]
> [ 6136.579204] 1 lock held by conftest/13950:
> [ 6136.579207] #0: (&mm->mmap_sem){++++++}, at: [<c1091b87>] sys_munmap+0x26/0x42
> [ 6136.579216]
> [ 6136.579217] stack backtrace:
> [ 6136.579221] Pid: 13950, comm: conftest Not tainted 2.6.35-rc3-dbg-git3-00350-g94d119f #61
> [ 6136.579225] Call Trace:
> [ 6136.579230] [<c12c4893>] ? printk+0xf/0x11
> [ 6136.579235] [<c104dbdd>] print_circular_bug+0x8a/0x96
> [ 6136.579241] [<c104ef30>] __lock_acquire+0x96d/0xbe1
> [ 6136.579247] [<c104e436>] ? mark_lock+0x26/0x1b3
> [ 6136.579254] [<c104f53a>] lock_acquire+0x59/0x70
> [ 6136.579259] [<c10f1cf6>] ? reiserfs_file_release+0x11d/0x344
> [ 6136.579265] [<c12c5614>] __mutex_lock_common+0x39/0x36b
> [ 6136.579270] [<c10f1cf6>] ? reiserfs_file_release+0x11d/0x344
> [ 6136.579276] [<c1084375>] ? release_pages+0x55/0x116
> [ 6136.579282] [<c12c5980>] mutex_lock_nested+0x12/0x15
> [ 6136.579287] [<c10f1cf6>] ? reiserfs_file_release+0x11d/0x344
> [ 6136.579293] [<c10f1cf6>] reiserfs_file_release+0x11d/0x344
> [ 6136.579299] [<c10a57bd>] ? fput+0x90/0x16a
> [ 6136.579304] [<c10a580d>] fput+0xe0/0x16a
> [ 6136.579309] [<c1090ca6>] remove_vma+0x28/0x47
> [ 6136.579314] [<c1091819>] ? arch_unmap_area_topdown+0x0/0x18
> [ 6136.579319] [<c1091a68>] do_munmap+0x1e8/0x200
> [ 6136.579325] [<c1091b93>] sys_munmap+0x32/0x42
> [ 6136.579330] [<c10027d3>] sysenter_do_call+0x12/0x32
>
Right.
The problem is:
vfs_readdir() { do_munmap() {
mutex_lock(inode); read or write(don't know)_lock(mm->mmap_sem)
reiserfs_readdir() { reiserfs_file_release() {
read_lock(mm->mmap_sem) mutex_lock(inode);
} }
} }
I don't think the deadlock can really happen, as we can't release the directory while
we are reading it. Plus I guess we can't mmap a directory (someone correct me if
I'm wrong).
But still the warning is annoying. I suspect this was there for a while.
There are other known "inversions that can't happen" in reiserfs, one is
on the xattrs code (reiserfs/xattr.c:xattr_unlink()) which warning you
can trigger under testing pressure.
But this one looks easy to trigger, although I've never seen it, but I'll
try your testcase.
Anyway, we can't change the readdir path. mutex -> mm->mmap_sem is the correct
ordering, we need to call copy_to_user there anyway.
So the challenge is to look at this mutex_lock(&inode->i_mutex) in
reiserfs_file_release(). I really don't know what it is protecting.
Is there someone who could give me a hint here?
> As well as this:
>
> [ 202.300464] REISERFS error (device sda9): vs-2100 add_save_link:
> search_by_key ([-1 7812832 0x1 IND]) returned 1
> [ 202.300473] REISERFS (device sda9): Remounting filesystem read-only
> [ 202.301603] ------------[ cut here ]------------
> [ 202.301615] WARNING: at fs/reiserfs/journal.c:3436
> journal_end+0x5b/0xaf()
> [ 202.301619] Hardware name: F3JC
> [ 202.301622] Modules linked in: snd_seq_dummy snd_seq_oss
> snd_seq_midi_event snd_seq snd_seq_device snd_hda_codec_si3054
> snd_hda_codec_realtek snd_hwdep snd_pcm_oss snd_mixer_oss asus_laptop
> sparse_keymap sdhci_pci sdhci snd_hda_intel mmc_core led_class snd_hda_codec
> rng_core snd_pcm snd_timer snd_page_alloc snd i2c_i801 soundcore psmouse sg
> evdev serio_raw r8169 mii usbhid hid uhci_hcd ehci_hcd sr_mod usbcore cdrom
> sd_mod ata_piix
> [ 202.301689] Pid: 5055, comm: a.out Not tainted
> 2.6.35-rc3-dbg-git6-00502-g94feaba-dirty #65
> [ 202.301693] Call Trace:
> [ 202.301701] [<c102e3a2>] warn_slowpath_common+0x65/0x7a
> [ 202.301707] [<c1102d95>] ? journal_end+0x5b/0xaf
> [ 202.301712] [<c102e3c6>] warn_slowpath_null+0xf/0x13
> [ 202.301718] [<c1102d95>] journal_end+0x5b/0xaf
> [ 202.301725] [<c10eebaa>] reiserfs_truncate_file+0x19f/0x233
> [ 202.301733] [<c10f1ba1>] reiserfs_vfs_truncate_file+0xd/0xf
> [ 202.301738] [<c1084883>] vmtruncate+0x23/0x29
> [ 202.301745] [<c10b4ad4>] inode_setattr+0x47/0x68
> [ 202.301751] [<c10f1b3f>] reiserfs_setattr+0x242/0x297
> [ 202.301758] [<c12c5d05>] ? down_write+0x22/0x2a
> [ 202.301764] [<c10b4d6f>] notify_change+0x15c/0x26b
> [ 202.301770] [<c10a35c7>] do_truncate+0x64/0x7d
> [ 202.301776] [<c12c69b0>] ? _raw_spin_unlock+0x33/0x3f
> [ 202.301783] [<c10ad892>] do_last+0x450/0x459
> [ 202.301789] [<c10ada5b>] do_filp_open+0x1c0/0x41a
> [ 202.301798] [<c1029081>] ? get_parent_ip+0xb/0x31
> [ 202.301804] [<c1029123>] ? sub_preempt_count+0x7c/0x89
> [ 202.301810] [<c10b5746>] ? alloc_fd+0xb4/0xbf
> [ 202.301816] [<c10a3f46>] do_sys_open+0x48/0xdf
> [ 202.301821] [<c10a3ffb>] sys_open+0x1e/0x26
> [ 202.301827] [<c10027d3>] sysenter_do_call+0x12/0x32
> [ 202.301833] ---[ end trace c4e3312bdadd2dc5 ]---
Ah that's wicked. I'll try your testcase for that too.
Thanks a lot for providing these!
next prev parent reply other threads:[~2010-07-02 13:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-02 9:34 reiserfs locking (v2) Sergey Senozhatsky
2010-07-02 13:12 ` Frederic Weisbecker [this message]
2010-07-02 13:44 ` Peter Zijlstra
2010-07-02 14:34 ` Frederic Weisbecker
2010-07-02 14:38 ` Peter Zijlstra
2010-07-02 13:59 ` Edward Shishkin
2010-07-02 14:03 ` Sergey Senozhatsky
2010-07-03 9:24 ` Al Viro
2010-07-03 9:43 ` Al Viro
2010-07-04 9:15 ` Al Viro
2010-07-09 3:16 ` Frederic Weisbecker
2010-07-09 10:42 ` Sergey Senozhatsky
2010-07-10 13:57 ` Frederic Weisbecker
-- strict thread matches above, loose matches on Subject: below --
2010-07-02 9:49 Sergey Senozhatsky
2010-07-02 9:53 Sergey Senozhatsky
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=20100702131248.GA5324@nowhere \
--to=fweisbec@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=chris.mason@oracle.com \
--cc=hch@lst.de \
--cc=jack@suse.cz \
--cc=jeffm@jeffreymahoney.com \
--cc=linux-kernel@vger.kernel.org \
--cc=reiserfs-devel@vger.kernel.org \
--cc=sergey.senozhatsky@gmail.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;
as well as URLs for NNTP newsgroup(s).