Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <f8e478c86bb80f37fbedc4f17b782918bc1c3607.camel@mailo.com>
Subject: Re: [PATCH] gfs2: Set error on error path
From:   drv <drv@mailo.com>
To:     Dan Carpenter <dan.carpenter@linaro.org>
Cc:     Bob Peterson <rpeterso@redhat.com>,
        Andreas Gruenbacher <agruenba@redhat.com>,
        gfs2@lists.linux.dev, linux-kernel@vger.kernel.org,
        linux-kernel-mentees@lists.linuxfoundation.org,
        Dan Carpenter <error27@gmail.com>
Date:   Fri, 29 Sep 2023 13:02:51 +0530
In-Reply-To: <72de9c2e-7c60-4930-acfb-556a1f0fa4c4@kadam.mountain>
References: <ZRZrUMzweaJeEJuD@runicha.com>
         <72de9c2e-7c60-4930-acfb-556a1f0fa4c4@kadam.mountain>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4-2 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-09-29 at 10:14 +0300, Dan Carpenter wrote:
> On Fri, Sep 29, 2023 at 11:44:40AM +0530, Deepak R Varma wrote:
> > Set the error variable inside the error path on failure. Saves
> > unnecessary variable assignment during normal execution.
> >=20
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> > =C2=A0fs/gfs2/bmap.c | 5 +++--
> > =C2=A01 file changed, 3 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> > index ef7017fb6951..93bd8ea34444 100644
> > --- a/fs/gfs2/bmap.c
> > +++ b/fs/gfs2/bmap.c
> > @@ -162,9 +162,10 @@ int gfs2_unstuff_dinode(struct gfs2_inode *ip)
> > =C2=A0
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0down_write(&ip->i_rw_mu=
tex);
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0page =3D grab_cache_pag=
e(inode->i_mapping, 0);
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0error =3D -ENOMEM;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!page)
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (!page) {
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0error =3D -ENOMEM;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0goto out;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
>=20
> It used to be that the other way was faster?=C2=A0 I'm pretty sure the
> compiler can figure it out these days.=C2=A0 Please don't randomly start
> changing people's prefered style unless it's drivers/staging/ code.
>=20
Hi Dan,
Thank you for the feedback. I did not know about the compiler
optimization resulting in original code being faster.
I will let the maintainers comment on breaking the style.

regards,
deepak.

> regards,
> dan carpenter
>=20