From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [PATCH 4/4] f2fs: optimize build_free_nids() Date: Wed, 08 May 2013 18:50:04 +0900 Message-ID: <1368006604.16581.64.camel@kjgkr> References: <1367853344-28938-1-git-send-email-haicheng.li@linux.intel.com> <1367853344-28938-5-git-send-email-haicheng.li@linux.intel.com> <1367922839.16581.42.camel@kjgkr> <20130508062455.GB8705@hli22-desktop> Reply-To: jaegeuk.kim@samsung.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-FxnJ8iluf837Q/IpcjFx" Cc: linux-kernel@vger.kernel.org, Haicheng Li , linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net To: Haicheng Li Return-path: Received: from mailout4.samsung.com ([203.254.224.34]:22975 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753143Ab3EHJvI (ORCPT ); Wed, 8 May 2013 05:51:08 -0400 In-reply-to: <20130508062455.GB8705@hli22-desktop> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --=-FxnJ8iluf837Q/IpcjFx Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 2013-05-08 (=EC=88=98), 14:24 +0800, Haicheng Li: > On Tue, May 07, 2013 at 07:33:59PM +0900, Jaegeuk Kim wrote: > > 2013-05-06 (=EC=9B=94), 23:15 +0800, Haicheng Li: > > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > > > index 1fe3fe2..3136224 100644 > > > --- a/fs/f2fs/node.c > > > +++ b/fs/f2fs/node.c > > > @@ -1342,6 +1342,8 @@ static void build_free_nids(struct f2fs_sb_info= *sbi) > > > if (nid >=3D nm_i->max_nid) > > > nid =3D 0; > > > =20 > > > + if (nm_i->fcnt > 2 * MAX_FREE_NIDS) > > > + break; > >=20 > > Could you explain when this can happen? >=20 > I'm thinking of this possible scenario: >=20 > as we don't hold any spinlock to protect the context, add_free_nid() coul= d be=20 > called by other thread anytime, e.g. by the gc_thread_func() in backgroun= d. The gc_thread_func() is not a proper example here though, the buid_free_nids() is covered by nm_i->build_lock, so build_free_nids is entered only one at a time. In addtion, build_free_nids starts with checking if (nm_i->fcnt > NAT_ENTRY_PER_BLOCK) in order not to be conducted repeatedely. >=20 > then nm_i->fcnt could be increased as 2 * MAX_FREE_NIDS while i < FREE_NI= D_PAGES. > Anything I misconsidered? Apart from the correctness of this behavior, I'm not sure why we should strictly manage this threshold value. Should we really need to do this? >=20 > > IMO, this is an unnecessary condition check, since the below condition > > that includes FREE_NID_PAGES already limits the number of free nids. > > Thanks, >=20 > hmm, the pros is that this check may possibly avoid some (< 4) unnecessar= y while-loop, > the cons is that too many checks of (nm_i->fcnt > 2 * MAX_FREE_NIDS) > would make the code looking messy and fragmentary... > =20 > > > if (i++ =3D=3D FREE_NID_PAGES) > > > break; > > > } > >=20 > > --=20 > > Jaegeuk Kim > > Samsung >=20 >=20 > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" i= n > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ --=20 Jaegeuk Kim Samsung --=-FxnJ8iluf837Q/IpcjFx 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) iQIcBAABAgAGBQJRih/MAAoJEEAUqH6CSFDSvU8P/0EJC99JpN+D0IKuwjSmd4ap uQPIX1z+nb5V7WjNc/RT8vB94Fa4OakjD98TIcE/N2BiqNolM40uoUQ4Pkze3gd9 vHxs6rT6U5E93gI1ceQE8hzcUwzZ8vcwfPndJUd/5cQGUADpMrhRG0KUS+yKzkYe MAaPYOnIPFK39DqffxdzZa8A6SWyWN96UH9p+NcvDkiDEnDGRazJMJihPPbbnfgf xHUFBERKnlDYzfwgQZJGx4J6/NFz+03ToqZtCpDfR4MkrtHXy+lNcCmVp6+ndZag 9BkrvNBdK+Q1H5MQWeiKrC4p+Pv6M5WM1fHl/Am+EfsWKAUg7D5x57TFzomBuoES nKrd17J5l9OWXuCSPPMhW/Nq2velEcH9ScMIINy0qVtSh83FMRuq1+zCyliqdJkU E8pTTU6oY476tGs/nGOxg1wmoNVqDcLwlj6h+UqVvy5fX/5JNzfsmVfwr8Xcg+1O t1Uc6mzuo9kkYtSGA1YnngNHFH5EAYIl7XYJ+rcIVaPlnx5ctSmEJZiqpBYmua+B gu6oSNR2+f3KVpw/BOmLDBjwViq23Tu+cV5ujr8NJVq97yinatz7xfLE8ejeXnOB xsIriRyX+Ie0O89c7dK0mkO9YsdAA0U2uEhcqCzgOyw9j2X92XYbZDM0jxyE1Dni ZY74Lqh1+Ade0uX3NtHx =5iEr -----END PGP SIGNATURE----- --=-FxnJ8iluf837Q/IpcjFx--