From patchwork Wed Apr 3 14:41:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Czerner X-Patchwork-Id: 233495 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 331C62C00BB for ; Thu, 4 Apr 2013 01:41:12 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758945Ab3DCOlL (ORCPT ); Wed, 3 Apr 2013 10:41:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18621 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758547Ab3DCOlK (ORCPT ); Wed, 3 Apr 2013 10:41:10 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r33Ef75G013196 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 3 Apr 2013 10:41:07 -0400 Received: from vpn1-5-173.ams2.redhat.com (vpn1-5-173.ams2.redhat.com [10.36.5.173]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r33Ef2ov000993 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 3 Apr 2013 10:41:06 -0400 Date: Wed, 3 Apr 2013 16:41:01 +0200 (CEST) From: =?ISO-8859-15?Q?Luk=E1=A8_Czerner?= X-X-Sender: lukas@localhost To: "Theodore Ts'o" cc: linux-ext4@vger.kernel.org Subject: ext4 dev branch: mutex not locked in ext4_truncate() Message-ID: User-Agent: Alpine 2.00 (LFD 1167 2008-08-23) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Hi Ted, there is a problem with your patch: 32d90a241a44d22ebc5289d2a2561691fc2d1351 because there is one more case where we might call ext4_truncate() without i_mutex locked - from ext4_symlink(). Because we might be calling __page_symlink() and it will call ext4_write_begin(). In possible error case (ENOSPC for example) we might want to truncate everything which might have been instantiated past i_size, however at that point we're not holding i_mutex because there is no point in doing so - the inode can not be possibly held by anyone else. My proposal is to only check whether the mutex is locked if the inode is _not_ new or is _not_ being freed. There is a quick&dirty patch and it seems to be working well so far. Let me know if you prefer standalone patch, or if you'll include it into your commit. Another possibility is to drop the commit 32d90a241a44d22ebc5289d2a2561691fc2d1351 entirely. Thanks! -Lukas --- diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index a36ca99..70b5c18 100644 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -255,21 +255,8 @@ void ext4_evict_inode(struct inode *inode) "couldn't mark inode dirty (err %d)", err); goto stop_handle; } - if (inode->i_blocks) { - /* - * Since we are evicting the inode, it shouldn't be - * locked. We've added a warning which triggers if - * the mutex is not locked, so take the lock even - * though it's not strictly necessary. However, - * taking the lock using a simple mutex_lock() will - * trigger a (false positive) lockdep warning, so take - * it using a trylock. - */ - int locked = mutex_trylock(&inode->i_mutex); + if (inode->i_blocks) ext4_truncate(inode); - if (likely(locked)) - mutex_unlock(&inode->i_mutex); - } /* * ext4_ext_truncate() doesn't reserve any slop when it @@ -3690,7 +3677,13 @@ void ext4_truncate(struct inode *inode) handle_t *handle; struct address_space *mapping = inode->i_mapping; - WARN_ON(!mutex_is_locked(&inode->i_mutex)); + /* + * There is a possibility that we're either freeing the inode + * or it completely new indode. In those cases we might not + * have i_mutex locked because it's not necessary. + */ + if (!(inode->i_state & (I_NEW|I_FREEING))) + WARN_ON(!mutex_is_locked(&inode->i_mutex)); trace_ext4_truncate_enter(inode); if (!ext4_can_truncate(inode))