From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaegeuk Kim Subject: Re: [PATCH] To add NULL pointer check Date: Thu, 04 Apr 2013 09:14:55 +0900 Message-ID: <1365034495.4353.14.camel@kjgkr> References: <1364958193.4353.4.camel@kjgkr> <1364975688.4353.7.camel@kjgkr> Reply-To: jaegeuk.kim@samsung.com Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-FuWwjiBCe5AaaWJ/qsNi" Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Petr Matousek To: P J P Return-path: In-reply-to: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org --=-FuWwjiBCe5AaaWJ/qsNi Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, 2013-04-03 (=EC=88=98), 14:43 +0530, P J P: > +-- On Wed, 3 Apr 2013, Jaegeuk Kim wrote --+ > | I'm confusing the question because f2fs doesn't use generic_writepages(= ),=20 > | since f2fs_write_data_pages() is linked to a_ops->writepages. In=20 > | do_writepages(), always f2fs_write_data_pages() is triggered instead of= =20 > | generic_writepages(). Isn't it? >=20 > Before commit fa9150a84c, when `generic_writepages' returned 0, it did no= t=20 > abort `f2fs_write_data_pages', as the proposed patch does. I was wonderin= g if=20 > that's intentional OR if the patch below does it right? >=20 > =3D=3D=3D > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 7bd22a2..7be750e 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -561,7 +561,7 @@ static int f2fs_write_data_pages(struct address_space= =20 > *mapping, > { > struct inode *inode =3D mapping->host; > struct f2fs_sb_info *sbi =3D F2FS_SB(inode->i_sb); > - int ret; > + int ret =3D 0; > long excess_nrtw =3D 0, desired_nrtw; > =20 > if (wbc->nr_to_write < MAX_DESIRED_PAGES_WP) { > @@ -572,7 +572,9 @@ static int f2fs_write_data_pages(struct address_space= *mapping, > =20 > if (!S_ISDIR(inode->i_mode)) > mutex_lock(&sbi->writepages); > - ret =3D write_cache_pages(mapping, wbc, __f2fs_writepage, mapping= ); > + /* deal with chardevs and other special file */ > + if (mapping->a_ops->writepage) > + ret =3D write_cache_pages(mapping, wbc, __f2fs_writepage,= mapping); Why should we take unnecessary locks and an f2fs_submit_bio call? Thanks, > if (!S_ISDIR(inode->i_mode)) > mutex_unlock(&sbi->writepages); > f2fs_submit_bio(sbi, DATA, (wbc->sync_mode =3D=3D WB_SYNC_ALL)); > =3D=3D=3D >=20 > Thank you. > -- > Prasad J Pandit / Red Hat Security Response Team > DB7A 84C5 D3F9 7CD1 B5EB C939 D048 7860 3655 602B > -- > 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 --=-FuWwjiBCe5AaaWJ/qsNi 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) iQIcBAABAgAGBQJRXMX/AAoJEEAUqH6CSFDSlCAP/jHaAzlUX3CnJQ81ZkNoDEW5 6X/R9xu6z/cPBeqqXZJmHit1kHuajS2nAmBYnuYIvNpcjgAumz0VzTff1nN/Oy2D 2tmS00E/rPo+I56wg2Eb9owXc39u1pYJDckFVPLIVRmT9j/3E2qHcGzealb65tyH ty05Y6I62U8rofKHDLJKMbrNnsV7fyzzgnKF2C16rI0cQVi85Ywlr0UfXepZXLwO Zj8h/dtL8jA2bzR6JUymQ8KdSw1e7RBLCNEexq3EApYAlSn0sfbFqyhgwiU2uxHo Iiwi1m0iLoa3cIKEOkeLSwTQjq6KWMwYPKfvMgn7EMTcKVyjCdoXNtC8YXnK9SKq dZZfFd3Bf8cC0bw1b7MedTuZeE91ze/RCgu63BpO9+tMDH5flzITu/4NF2zkJXQr ARvksnTViHxOjVjXS86jdA/PvVa8LOxggXP1sFS6HtIIBUGOBMnim8+L0GrXCOI8 gdgBU4AJBp+wp4Y5IjICuGEGFGan5oYnafjqYDMv7NEbR7B/mGnrumgIEI9KCstt i+ycY3PpyMurt0RIpNEerVJgh8qsPfd6FInus9V0XmSm5T8gDEhJsaBjH/P2ygC+ lycmOjGDA+Q46LqLXCKbNxxQtKq4mcxvN031PBDOIGHBlNls/Ej3do3Q74pHjdPB NQuoyxicEv2yhLwF3Z1Y =HLLI -----END PGP SIGNATURE----- --=-FuWwjiBCe5AaaWJ/qsNi--