From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [f2fs-dev] [PATCH] f2fs: cleanup the f2fs_bio_alloc routine Date: Tue, 11 Dec 2012 12:00:24 +0900 Message-ID: <1355194824.7291.93.camel@kjgkr> References: <1355123083-14085-1-git-send-email-jaegeuk.kim@samsung.com> Reply-To: jaegeuk.kim@samsung.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-w2FM/kvgYJ6WcNbMxN2H" Cc: linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net To: Ezequiel Garcia Return-path: Received: from mailout4.samsung.com ([203.254.224.34]:15903 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751879Ab2LKDAc (ORCPT ); Mon, 10 Dec 2012 22:00:32 -0500 Received: from epcpsbgm2.samsung.com (epcpsbgm2 [203.254.230.27]) by mailout4.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MEU00JTKJ06NP90@mailout4.samsung.com> for linux-fsdevel@vger.kernel.org; Tue, 11 Dec 2012 12:00:30 +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 <0MEU000MZJ0TH140@mmp2.samsung.com> for linux-fsdevel@vger.kernel.org; Tue, 11 Dec 2012 12:00:29 +0900 (KST) In-reply-to: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --=-w2FM/kvgYJ6WcNbMxN2H Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 2012-12-10 (=EC=9B=94), 09:01 -0300, Ezequiel Garcia: > On Mon, Dec 10, 2012 at 4:04 AM, Jaegeuk Kim wr= ote: > > Do cleanup more for better code readability. > > > > - Change the parameter set of f2fs_bio_alloc() > > This function should allocate a bio only since it is not something li= ke > > f2fs_bio_init(). Instead, the caller should initialize the allocated = bio. > > > > - Introduce SECTOR_FROM_BLOCK > > This macro translates a block address to its sector address. > > > > Signed-off-by: Jaegeuk Kim > > --- > > fs/f2fs/data.c | 5 +++-- > > fs/f2fs/f2fs.h | 2 +- > > fs/f2fs/segment.c | 33 ++++++++++++++++++--------------- > > fs/f2fs/segment.h | 3 +++ > > 4 files changed, 25 insertions(+), 18 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index 444c2a6..655aeab 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -343,11 +343,12 @@ int f2fs_readpage(struct f2fs_sb_info *sbi, struc= t page *page, > > down_read(&sbi->bio_sem); > > > > /* Allocate a new bio */ > > - bio =3D f2fs_bio_alloc(bdev, blk_addr << (sbi->log_blocksize - = 9), > > - 1, GFP_NOFS | __GFP_HIGH); > > + bio =3D f2fs_bio_alloc(bdev, 1); > > > > /* Initialize the bio */ > > + bio->bi_sector =3D SECTOR_FROM_BLOCK(sbi, blk_addr); > > bio->bi_end_io =3D read_end_io; > > + > > if (bio_add_page(bio, page, PAGE_CACHE_SIZE, 0) < PAGE_CACHE_SI= ZE) { > > kfree(bio->bi_private); > > bio_put(bio); > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 8c3f1ef..2bce3a6 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -924,7 +924,7 @@ void clear_prefree_segments(struct f2fs_sb_info *); > > int npages_for_summary_flush(struct f2fs_sb_info *); > > void allocate_new_segments(struct f2fs_sb_info *); > > struct page *get_sum_page(struct f2fs_sb_info *, unsigned int); > > -struct bio *f2fs_bio_alloc(struct block_device *, sector_t, int, gfp_t= ); > > +struct bio *f2fs_bio_alloc(struct block_device *, int); > > void f2fs_submit_bio(struct f2fs_sb_info *, enum page_type, bool sync)= ; > > int write_meta_page(struct f2fs_sb_info *, struct page *, > > struct writeback_control *); > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > index 8894b39..1b26e4e 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -643,23 +643,21 @@ static void f2fs_end_io_write(struct bio *bio, in= t err) > > bio_put(bio); > > } > > > > -struct bio *f2fs_bio_alloc(struct block_device *bdev, sector_t first_s= ector, > > - int nr_vecs, gfp_t gfp_flags) > > +struct bio *f2fs_bio_alloc(struct block_device *bdev, int npages) > > { > > struct bio *bio; > > - > > - /* allocate new bio */ > > - bio =3D bio_alloc(gfp_flags, nr_vecs); > > - > > - bio->bi_bdev =3D bdev; > > - bio->bi_sector =3D first_sector; > > + struct bio_private *priv; > > retry: > > - bio->bi_private =3D kmalloc(sizeof(struct bio_private), > > - GFP_NOFS | __GFP_HIGH); > > - if (!bio->bi_private) { > > + priv =3D kmalloc(sizeof(struct bio_private), GFP_NOFS); >=20 > Suggestion: >=20 > Given you're writing a f2fs specific allocator wrapper for kmalloc, > you might want to use kmalloc_track_caller and friends. >=20 > This way, if you use ftrace kmem events you'll be able to track > the real caller, instead of f2fs_bio_alloc. Ok, I'll keep in mind. But, well, currently it seems there is no need to use that. Thanks, >=20 > Thanks, >=20 > Ezequiel > -- > 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 --=20 Jaegeuk Kim Samsung --=-w2FM/kvgYJ6WcNbMxN2H 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) iQIcBAABAgAGBQJQxqHJAAoJEEAUqH6CSFDS3GkP/1qf1FoQHNWQMba5r1pBmsFb JVOYe/xOGSSQ6nt3PKcnfrLbHrGnMokW8s7WKYCd9M2EI3e0npW8WLSZOQhw6lXr iYQgasSU1YzOxeKhmCksYgGgYvG1fJjQNlvEuht+jIKZw/5LoM6/GBgvyzmzc5Ui D9zcpEkbe0PcoXpWyQuaGPDeaJ0fcdGKs8kTceYY5RQ/Efe3cQv0dX7dda8MFsCV uYwJ8ytA9FMK7+JQlgJIRwqJ3F2dSOWQSmd2LcG1h0rNHIuT61iSP/gBZlryTQ9p LTJix3n/qj/molckPr7d++ymTujiloPiF7rexEEqJJhZO3h4uu2Vi6YJkFa9DctV 8+QT1rRXpxrpJealJ44V7q2/cgokareYT10DogkPuA9yKRQ6huYtfOBjImYc3TsH 1m6yl+9VV50sab9X+9+YtOIkPhrTJzY4UdKCKI12VJtB+4/N6t31gnbyerL1l/L6 Y+0TvuJKqFp5hBExHSRgbS1iSgSILdtCfgjW3RMa4qVqK2Ph9me3yp28u3Zb8vLx QXv0gT6KUlS6jB5i7sYwYu0QUPQAOzO8aHmdUwhjPg3fuUPFqgdbsX/oRrRE5ofp 2PBHcy1/if+z3IEG25M4oKhIOvwdKzP2rfY3RqLAsUc3spNml/tTzWwih2F6clO7 9FSBtsLMtX69KMazMTMH =T2BF -----END PGP SIGNATURE----- --=-w2FM/kvgYJ6WcNbMxN2H--