From: Tao Ma Subject: Re: [PATCH] VFS: call synchronize_rcu after kill_sb. Date: Wed, 09 Feb 2011 09:49:15 +0800 Message-ID: <4D51F29B.8060304@tao.ma> References: <1296896481-3650-1-git-send-email-tm@tao.ma> <4D4FF040.9050707@panasas.com> <87hbceqxax.fsf@linux.vnet.ibm.com> <4D517C98.7060000@panasas.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "Aneesh Kumar K. V" , Nick Piggin , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, Al Viro , Chris Mason To: Boaz Harrosh Return-path: In-Reply-To: <4D517C98.7060000@panasas.com> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-ext4.vger.kernel.org On 02/09/2011 01:25 AM, Boaz Harrosh wrote: > 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 > --- > 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 > It works now in my ext4 test box. Thanks for your work. Tested-by: Tao Ma > --- > git diff --stat -p -M fs/super.c > fs/super.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > 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 { > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >