From: Taesoo Kim <taesoo@gatech.edu>
Subject: Re: [PATCH 1/1] fs/affs/file.c: unlock/release page on error
Date: Sat, 21 Mar 2015 16:05:04 -0400
Message-ID: <20150321200504.GB5170@taesoo.org>
References: <1426959063-9329-1-git-send-email-tsgatesv@gmail.com>
 <2067582645.204975.1426967515410.open-xchange@webmail.nmp.proximus.be>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: akpm@linux-foundation.org, Taesoo Kim <tsgatesv@gmail.com>,
	linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk,
	jack@suse.cz, geert@linux-m68k.org, linux-fsdevel@vger.kernel.org,
	csong84@gatech.edu, blee@gatech.edu, sanidhya@gatech.edu,
	changwoo@gatech.edu
To: Fabian Frederick <fabf@skynet.be>
Return-path: <linux-kernel-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <2067582645.204975.1426967515410.open-xchange@webmail.nmp.proximus.be>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org

Thank you for letting us know. We will definitely do it for
the next time. Thanks again for checking this out.

Taesoo

On 03/21/15 at 08:51pm, Fabian Frederick wrote:
>=20
>=20
> > On 21 March 2015 at 18:31 Taesoo Kim <tsgatesv@gmail.com> wrote:
> >
> >
> > When affs_bread_ino() fails, correctly unlock the page and
> > release the page cache with proper error value. All write_end()
> > should unlock/release the page that was locked by write_beg().
> >
> > Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
> > ---
> >=A0 fs/affs/file.c | 19 ++++++++++++-------
> >=A0 1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/affs/file.c b/fs/affs/file.c
> > index d2468bf..a91795e 100644
> > --- a/fs/affs/file.c
> > +++ b/fs/affs/file.c
> > @@ -699,8 +699,10 @@ static int affs_write_end_ofs(struct file *fil=
e, struct
> > address_space *mapping,
> >=A0 =A0 =A0 =A0boff =3D tmp % bsize;
> >=A0 =A0 =A0 =A0if (boff) {
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bh =3D affs_bread_ino(inode, bidx, 0)=
;
> > -=A0 =A0 =A0 =A0 =A0 =A0 =A0if (IS_ERR(bh))
> > -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return PTR_ERR(bh);
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0if (IS_ERR(bh)) {
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0written =3D PTR_ERR(bh)=
;
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_first_bh;
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0}
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tmp =3D min(bsize - boff, to - from);
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BUG_ON(boff + tmp > bsize || tmp > bs=
ize);
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0memcpy(AFFS_DATA(bh) + boff, data + f=
rom, tmp);
> > @@ -712,14 +714,16 @@ static int affs_write_end_ofs(struct file *fi=
le, struct
> > address_space *mapping,
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bidx++;
> >=A0 =A0 =A0 =A0} else if (bidx) {
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bh =3D affs_bread_ino(inode, bidx - 1=
, 0);
> > -=A0 =A0 =A0 =A0 =A0 =A0 =A0if (IS_ERR(bh))
> > -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return PTR_ERR(bh);
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0if (IS_ERR(bh)) {
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0written =3D PTR_ERR(bh)=
;
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_first_bh;
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0}
> >=A0 =A0 =A0 =A0}
> >=A0 =A0 =A0 =A0while (from + bsize <=3D to) {
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0prev_bh =3D bh;
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bh =3D affs_getemptyblk_ino(inode, bi=
dx);
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (IS_ERR(bh))
> > -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out;
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_bh;
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0memcpy(AFFS_DATA(bh), data + from, bs=
ize);
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (buffer_new(bh)) {
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0AFFS_DATA_HEAD(bh)->p=
type =3D cpu_to_be32(T_DATA);
> > @@ -751,7 +755,7 @@ static int affs_write_end_ofs(struct file *file=
, struct
> > address_space *mapping,
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0prev_bh =3D bh;
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0bh =3D affs_bread_ino(inode, bidx, 1)=
;
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (IS_ERR(bh))
> > -=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out;
> > +=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_bh;
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tmp =3D min(bsize, to - from);
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BUG_ON(tmp > bsize);
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0memcpy(AFFS_DATA(bh), data + from, tm=
p);
> > @@ -790,12 +794,13 @@ done:
> >=A0 =A0 =A0 =A0if (tmp > inode->i_size)
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0inode->i_size =3D AFFS_I(inode)->mmu_=
private =3D tmp;
> >=A0
> > +err_first_bh:
> >=A0 =A0 =A0 =A0unlock_page(page);
> >=A0 =A0 =A0 =A0page_cache_release(page);
> >=A0
> >=A0 =A0 =A0 =A0return written;
> >=A0
> > -out:
> > +err_bh:
> >=A0 =A0 =A0 =A0bh =3D prev_bh;
> >=A0 =A0 =A0 =A0if (!written)
> >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0written =3D PTR_ERR(bh);
> > --
> > 2.3.3
> >
>=20
> Hi Taesoo,
>=20
> =A0 =A0 =A0 =A0 Your patch is ok for me but we generally send a new m=
essage
> for an update with the version in title and explain updates
> done to it after ---. Here's an example of an AFFS V2 patch:
> =A0
> http://marc.info/?l=3Dlinux-kernel&m=3D142316316001752&w=3D2
>=20
> Regards,
> Fabian