From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [PATCH 4/4] f2fs: add blk plugging support in f2fs Date: Tue, 15 Jan 2013 17:02:23 +0900 Message-ID: <1358236943.8234.56.camel@kjgkr> References: <1357969323-9396-1-git-send-email-linkinjeon@gmail.com> <1358128249.23942.6.camel@kjgkr> <1358217615.8234.35.camel@kjgkr> Reply-To: jaegeuk.kim@samsung.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-d1X//NyzqymR402E8j5y" Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Namjae Jeon , Amit Sahrawat To: Namjae Jeon Return-path: In-reply-to: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org --=-d1X//NyzqymR402E8j5y Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > Yes, You're right. I agree block plug is not needed in f2fs. So plz > ignore this patch. > note =3D> Regardless of the intent in the patch, it has already been > used in writepages (f2fs uses generic_writepages). > So to make the overall code consistent, either we should remove blk > plug from entire F2FS write part or change f2fs_write_data_pages to > include blk plug properly - like the code change for this part we > share in the patch. Agreed. :) How about this patch? =46rom 09e11ac9734cd085f9a92c46098fb083091f5429 Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Tue, 15 Jan 2013 16:45:24 +0900 Subject: [PATCH] f2fs: remove the blk_plug usage in f2fs_write_data_pages From: Namjae Jeon Let's consider the usage of blk_plug in f2fs_write_data_pages(). We can come up with the two issues: lock contention and task awareness. 1. Merging bios prior to grabing "queue lock" The f2fs merges consecutive IOs in the file system level before submitting any bios, which is similar with the back merge by the plugging mechanism in attempt_plug_merge(). Both of them need to acquire no queue lock. 2. Merging policy with respect to tasks The f2fs merges IOs as much as possible regardless of tasks, while blk-plugging is conducted on a basis of tasks. As we can understand there are trade-offs, f2fs tries to maximize the write performance with well-merged bios. As a result, if f2fs produces many consecutive but separated bios in writepages(), it would be good to use blk-plugging since f2fs would be able to avoid queue lock contention in the block layer by merging them. But, f2fs merges IOs and submit one bio, which means that there are not much chances to merge bios by attempt_plug_merge(). However, f2fs has already been used blk_plug by triggering generic_writepages() in f2fs_write_data_pages(). So to make the overall code consistency, I'd like to remove blk_plug there. Signed-off-by: Namjae Jeon Signed-off-by: Amit Sahrawat Signed-off-by: Jaegeuk Kim --- fs/f2fs/data.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 3aa5ce7..b1347fc 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -547,6 +547,15 @@ redirty_out: =20 #define MAX_DESIRED_PAGES_WP 4096 =20 +static int __f2fs_writepage(struct page *page, struct writeback_control *wbc, + void *data) +{ + struct address_space *mapping =3D data; + int ret =3D mapping->a_ops->writepage(page, wbc); + mapping_set_error(mapping, ret); + return ret; +} + static int f2fs_write_data_pages(struct address_space *mapping, struct writeback_control *wbc) { @@ -563,7 +572,7 @@ static int f2fs_write_data_pages(struct address_space *mapping, =20 if (!S_ISDIR(inode->i_mode)) mutex_lock(&sbi->writepages); - ret =3D generic_writepages(mapping, wbc); + ret =3D write_cache_pages(mapping, wbc, __f2fs_writepage, mapping); if (!S_ISDIR(inode->i_mode)) mutex_unlock(&sbi->writepages); f2fs_submit_bio(sbi, DATA, (wbc->sync_mode =3D=3D WB_SYNC_ALL)); --=20 1.8.0.1.250.gb7973fb --=20 Jaegeuk Kim Samsung --=-d1X//NyzqymR402E8j5y 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) iQIcBAABAgAGBQJQ9Q0PAAoJEEAUqH6CSFDSqigP/3t0uT+Lx6nXcYav2BQj44/x y9ip/+6Up78bQLDBw2Rd7DG15w1PncSwNxwk/T9soL/P9gB0bRyLE+zWnDfWQIpY eJ6srMtOLowOwS+9v3VccG3JDjymTQ2Z7qhtA0JG738a4FA5YyOYFL0I/01brAvV A321MWHT6ZHWYAk7LYG+8YcFxmrRSxhK6XJIsv0tzZ48OqomVYayf8TElvj1OjKz p6fPzIPQTwa+eFisSPFDKqXXnn47RN8sa2dP97hPA90jGGog2mJkKZUsi7jHSfNr wd/ViL5Wz6g4sp7qvIuOomWzctcfHJmf6mc+hhDqvwmZ40UBD6ATVLCHnQ1hkYuM d3YpQB8NKSu19TD3HTuCH1Sw+FHGTwEnbAkdEq8rFldwswvzS7N0ntiYXpksy3mY qi0PnlYPMPGC4N16aIcy4UDQAlX5DZC8a2ctGlADNTh2v2aIlVpZBF39ALhumnaS WFTFFL5R92jl53PKL1XeP3APItISeZNKOmk6GK67ZGMVX1LpfE0rOcDxnR46WDuj 9S52WIWMz3WrZnlGE5LRxfvuOaxryJZwpkrP0xOyGUYsQItbwbmzdIC4PQ5oSkx3 rQAHwwXnJzauxHr9sQbHdtWyKPeZsM0yQlM2LFE2fpR5PRKD2tpgRmcN4s5c+ksi 2AYBZr2BB4ZLiIGNcSHJ =EV8M -----END PGP SIGNATURE----- --=-d1X//NyzqymR402E8j5y--