From patchwork Tue Feb 8 17:25:44 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boaz Harrosh X-Patchwork-Id: 82359 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 81E94B7127 for ; Wed, 9 Feb 2011 04:25:51 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755091Ab1BHRZt (ORCPT ); Tue, 8 Feb 2011 12:25:49 -0500 Received: from daytona.panasas.com ([67.152.220.89]:54027 "EHLO daytona.panasas.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754512Ab1BHRZt (ORCPT ); Tue, 8 Feb 2011 12:25:49 -0500 Received: from fs2.bhalevy.com ([172.17.33.147]) by daytona.panasas.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 8 Feb 2011 12:25:47 -0500 Message-ID: <4D517C98.7060000@panasas.com> Date: Tue, 08 Feb 2011 19:25:44 +0200 From: Boaz Harrosh User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10 MIME-Version: 1.0 To: "Aneesh Kumar K. V" CC: Tao Ma , Nick Piggin , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, Al Viro , Chris Mason Subject: Re: [PATCH] VFS: call synchronize_rcu after kill_sb. References: <1296896481-3650-1-git-send-email-tm@tao.ma> <4D4FF040.9050707@panasas.com> <87hbceqxax.fsf@linux.vnet.ibm.com> In-Reply-To: <87hbceqxax.fsf@linux.vnet.ibm.com> X-OriginalArrivalTime: 08 Feb 2011 17:25:47.0504 (UTC) FILETIME=[39548700:01CBC7B5] Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On 02/08/2011 06:57 PM, Aneesh Kumar K. V wrote: > On Mon, 07 Feb 2011 15:14:40 +0200, Boaz Harrosh wrote: >> On 02/05/2011 11:01 AM, Tao Ma wrote: >>> From: Tao Ma >>> >>> In fa0d7e3, we use rcu free inode instead of freeing the inode >>> directly. It causes a problem when we rmmod immediately after >>> we umount the volume[1]. >>> >>> So we need to call synchronize_rcu after we kill_sb so that >>> the inode is freed before we do rmmod. The idea is inspired >>> by Chris Mason[2]. I tested with ext4 by umount+rmmod and it >>> doesn't show any error by now. >>> >>> 1. http://marc.info/?l=linux-fsdevel&m=129680863330185&w=2 >>> 2. http://marc.info/?l=linux-fsdevel&m=129684698713709&w=2 >>> >>> Cc: Nick Piggin >>> Cc: Al Viro >>> Cc: Chris Mason >>> Cc: Boaz Harrosh >>> Signed-off-by: Tao Ma >>> --- >>> fs/super.c | 7 +++++++ >>> 1 files changed, 7 insertions(+), 0 deletions(-) >>> >>> diff --git a/fs/super.c b/fs/super.c >>> index 74e149e..315bce9 100644 >>> --- a/fs/super.c >>> +++ b/fs/super.c >>> @@ -177,6 +177,13 @@ void deactivate_locked_super(struct super_block *s) >>> struct file_system_type *fs = s->s_type; >>> if (atomic_dec_and_test(&s->s_active)) { >>> fs->kill_sb(s); >>> + /* >>> + * We need to synchronize rcu here so that >>> + * the delayed rcu inode free can be executed >>> + * before we put_super. >>> + * https://bugzilla.kernel.org/show_bug.cgi?id=27652 >>> + */ >>> + synchronize_rcu(); >>> put_filesystem(fs); >>> put_super(s); >>> } else { >> >> <> > > http://lwn.net/Articles/217484/ explains how to wait for rcu callback to finish > > -aneesh Yes thanks Aneesh, rcu_barrier does the trick Tested-by: Tao Ma --- From: Boaz Harrosh In fa0d7e3, we use rcu free inode instead of freeing the inode directly. It causes a problem when we rmmod immediately after we umount the volume[1]. So we need to call rcu_barrier after we kill_sb so that the inode is freed before we do rmmod. The idea is inspired by Aneesh Kumar. rcu_barrier will wait for all callbacks to end before preceding. The original patch was done by Tao Ma, but synchronize_rcu() is not enough here. 1. http://marc.info/?l=linux-fsdevel&m=129680863330185&w=2 2. http://marc.info/?l=linux-fsdevel&m=129684698713709&w=2 Cc: Nick Piggin Cc: Al Viro Cc: Chris Mason Cc: Tao Ma Signed-off-by: Boaz Harrosh --- git diff --stat -p -M fs/super.c fs/super.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) -- 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 diff --git a/fs/super.c b/fs/super.c index 74e149e..5fd4ec9 100644 --- a/fs/super.c +++ b/fs/super.c @@ -177,6 +177,13 @@ void deactivate_locked_super(struct super_block *s) struct file_system_type *fs = s->s_type; if (atomic_dec_and_test(&s->s_active)) { fs->kill_sb(s); + /* + * We need to synchronize rcu here so that + * the delayed rcu inode free can be executed + * before we put_super. + * https://bugzilla.kernel.org/show_bug.cgi?id=27652 + */ + rcu_barrier(); put_filesystem(fs); put_super(s); } else {