Message-id: <1368006604.16581.64.camel@kjgkr>
Subject: Re: [PATCH 4/4] f2fs: optimize build_free_nids()
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Reply-to: jaegeuk.kim@samsung.com
To: Haicheng Li <haicheng.li@linux.intel.com>
Cc: linux-kernel@vger.kernel.org, Haicheng Li <haicheng.lee@gmail.com>,
        linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Date: Wed, 08 May 2013 18:50:04 +0900
In-reply-to: <20130508062455.GB8705@hli22-desktop>
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>
Organization: samsung
Content-type: multipart/signed; micalg=pgp-sha1;
 protocol="application/pgp-signature"; boundary="=-FxnJ8iluf837Q/IpcjFx"
MIME-version: 1.0
DLP-Filter: Pass
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3551
Lines: 104


--=-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--

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
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/