From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [f2fs-dev] [PATCH] f2fs:Add mutex_lock to protect f2fs_stat_list. Date: Tue, 15 Jan 2013 16:04:50 +0900 Message-ID: <1358233490.8234.47.camel@kjgkr> References: <201301142008133402501@gmail.com> Reply-To: jaegeuk.kim@samsung.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-9615nTF/OuDjkREV42Ke" Cc: majianpeng , linux-fsdevel , linux-f2fs-devel To: Huajun Li Return-path: Received: from mailout2.samsung.com ([203.254.224.25]:58365 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751795Ab3AOHEy (ORCPT ); Tue, 15 Jan 2013 02:04:54 -0500 Received: from epcpsbgm1.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MGN0040HNNJYEQ0@mailout2.samsung.com> for linux-fsdevel@vger.kernel.org; Tue, 15 Jan 2013 16:04:52 +0900 (KST) Received: from [12.52.126.105] by mmp2.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0MGN00ESTNO450A0@mmp2.samsung.com> for linux-fsdevel@vger.kernel.org; Tue, 15 Jan 2013 16:04:52 +0900 (KST) In-reply-to: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --=-9615nTF/OuDjkREV42Ke Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, 2013-01-15 (=ED=99=94), 10:57 +0800, Huajun Li: > On Mon, Jan 14, 2013 at 8:08 PM, majianpeng wrote: > > There is an race between umount f2fs and read f2fs/status. > > It will case oops. > > Fox example: > > Thread A = Thread B > > umount f2fs = cat f2fs/status > > f2fs_destroy_stats() { stat_sh= ow() { > > = list_for_each_entry_safe(&f2fs_stat_list= ) > > list_del(&si->stat_list); > > mutex_lock(&si->stat_lock); > > si->sbi =3D NULL; > > mutex_unlock(&si->stat_lock); > > kfree(sbi->stat_info); > > = mutex_lock(&si->stat_lock) > > Nice catch. Actually, &si->stat_lock was introduced to cope with this issue. So, we need to remove the existing &si->stat_lock, and add your global f2fs_stat_mutex. Then, we're able to remove "si->sbi =3D NULL"-related stuffs too. > > Signed-off-by: Jianpeng Ma > > --- > > fs/f2fs/debug.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c > > index 0e0380a..ec6d004 100644 > > --- a/fs/f2fs/debug.c > > +++ b/fs/f2fs/debug.c > > @@ -26,6 +26,7 @@ > > > > static LIST_HEAD(f2fs_stat_list); > > static struct dentry *debugfs_root; > > +static DEFINE_MUTEX(f2fs_stat_mutex); > > > > static void update_general_status(struct f2fs_sb_info *sbi) > > { > > @@ -180,6 +181,7 @@ static int stat_show(struct seq_file *s, void *v) > > int i =3D 0; > > int j; > > > > + mutex_lock(&f2fs_stat_mutex); > > list_for_each_entry_safe(si, next, &f2fs_stat_list, stat_list) = { > > > > mutex_lock(&si->stat_lock); > > @@ -288,6 +290,7 @@ static int stat_show(struct seq_file *s, void *v) > > si->base_mem >> 10, si->cache_mem >> 10= ); > > mutex_unlock(&si->stat_lock); > > } > > + mutex_unlock(&f2fs_stat_mutex); > > return 0; > > } > > > > @@ -314,7 +317,10 @@ static int init_stats(struct f2fs_sb_info *sbi) > > > > si =3D sbi->stat_info; > > mutex_init(&si->stat_lock); > > + > > + mutex_lock(&f2fs_stat_mutex); > > list_add_tail(&si->stat_list, &f2fs_stat_list); > > + mutex_unlock(&f2fs_stat_mutex); > > > > si->all_area_segs =3D le32_to_cpu(raw_super->segment_count); > > si->sit_area_segs =3D le32_to_cpu(raw_super->segment_count_sit)= ; > > @@ -347,7 +353,10 @@ void f2fs_destroy_stats(struct f2fs_sb_info *sbi) > > { > > struct f2fs_stat_info *si =3D sbi->stat_info; > > > > + mutex_lock(&f2fs_stat_mutex); > > list_del(&si->stat_list); > > + mutex_unlock(&f2fs_stat_mutex); > > + >=20 > Hi Jianpeng, > Is it possible to fix the issue by holding si->stat_lock while > executing list_del(&si->stat_list) ?=20 I think it cannot fix the problem. Still the above errorneous scenario is able to be occurred. Thank you, --=20 Jaegeuk Kim Samsung --=-9615nTF/OuDjkREV42Ke Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAABAgAGBQJQ9P+SAAoJEEAUqH6CSFDSoJEP+QFLMx85gMedMdqDOJmvRpGw l56nS03AJNb2y2LJWvAteDdI5YoZ61uAmepLpkUnLTGJL24AbF3d752qxnFUP3Z6 5aauvF5y0irxVpUTJ4jg0aHVmom2NWzunvqQdN9Rh6dOV6hwMyrkwF6VAlH6h1Q6 8wlltjta3cUzKARRGlv85+Ku45yYu3vbp//O7OvpZDv4xCnwSygj/AX3WxwP+yex 0YxAqBBW+lhXIdyvCP0oZ6hWL7qO55qopJat4VR4yJOxjnMbw5s0Hv/SqPWL7o1O BAL3Xwk3VBVLZD+A8EPv1JEz7TmvAwlZ69PcPzv08Axr+8ObTwTZXo68Mh/1+n6U yeHTW0GZlaTRYu1ousGz7KhCZoK6uk+HjBFcfavNp9DlYDvwWc77mLtb1P+h/G9E eZEPy2Tf8z17ucN7Eyz6OJneKObFbnr9bO0d6wyXdSVsnaRGAvIj8J+oeFuffIeT 2hAixYmrDSj0qLBExNt8VAYy9YpCyBl36pN0Ho8+ryei4ID6FUwLgKjZTZ3FSOSb Li0Aol59aZ6KxCreT94yBSTiDX54UPhecawBtAeGaV4pAWNV+VBv4ht6nHZAm1s6 4P8rHfP8QgRg5IonYG2RNnEjafQIAde9aA41lnRT65zdytwGxL5UCYI/g9DvvWFY DfxEJ0wsqbcbrzC2Yu6T =rx1W -----END PGP SIGNATURE----- --=-9615nTF/OuDjkREV42Ke--