Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] fs: Fix a missing check bug
To:     Dinghao Liu <dinghao.liu@zju.edu.cn>, kjlu@umn.edu
Cc:     pakki001@umn.edu, Chris Mason <clm@fb.com>,
        Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20191207144126.14320-1-dinghao.liu@zju.edu.cn>
From:   Qu Wenruo <quwenruo.btrfs@gmx.com>
Autocrypt: addr=quwenruo.btrfs@gmx.com; prefer-encrypt=mutual; keydata=
 mQENBFnVga8BCACyhFP3ExcTIuB73jDIBA/vSoYcTyysFQzPvez64TUSCv1SgXEByR7fju3o
 8RfaWuHCnkkea5luuTZMqfgTXrun2dqNVYDNOV6RIVrc4YuG20yhC1epnV55fJCThqij0MRL
 1NxPKXIlEdHvN0Kov3CtWA+R1iNN0RCeVun7rmOrrjBK573aWC5sgP7YsBOLK79H3tmUtz6b
 9Imuj0ZyEsa76Xg9PX9Hn2myKj1hfWGS+5og9Va4hrwQC8ipjXik6NKR5GDV+hOZkktU81G5
 gkQtGB9jOAYRs86QG/b7PtIlbd3+pppT0gaS+wvwMs8cuNG+Pu6KO1oC4jgdseFLu7NpABEB
 AAG0IlF1IFdlbnJ1byA8cXV3ZW5ydW8uYnRyZnNAZ214LmNvbT6JAU4EEwEIADgCGwMFCwkI
 BwIGFQgJCgsCBBYCAwECHgECF4AWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCXZw1oQAKCRDC
 PZHzoSX+qCY6CACd+mWu3okGwRKXju6bou+7VkqCaHTdyXwWFTsr+/0ly5nUdDtT3yEVggPJ
 3VP70wjlrxUjNjFb6iIvGYxiPOrop1NGwGYvQktgRhaIhALG6rPoSSAhGNjwGVRw0km0PlIN
 D29BTj/lYEk+jVM1YL0QLgAE1AI3krihg/lp/fQT53wLhR8YZIF8ETXbClQG1vJ0cllPuEEv
 efKxRyiTSjB+PsozSvYWhXsPeJ+KKjFen7ebE5reQTPFzSHctCdPnoR/4jSPlnTlnEvLeqcD
 ZTuKfQe1gWrPeevQzgCtgBF/WjIOeJs41klnYzC3DymuQlmFubss0jShLOW8eSOOWhLRuQEN
 BFnVga8BCACqU+th4Esy/c8BnvliFAjAfpzhI1wH76FD1MJPmAhA3DnX5JDORcgaCbPEwhLj
 1xlwTgpeT+QfDmGJ5B5BlrrQFZVE1fChEjiJvyiSAO4yQPkrPVYTI7Xj34FnscPj/IrRUUka
 68MlHxPtFnAHr25VIuOS41lmYKYNwPNLRz9Ik6DmeTG3WJO2BQRNvXA0pXrJH1fNGSsRb+pK
 EKHKtL1803x71zQxCwLh+zLP1iXHVM5j8gX9zqupigQR/Cel2XPS44zWcDW8r7B0q1eW4Jrv
 0x19p4P923voqn+joIAostyNTUjCeSrUdKth9jcdlam9X2DziA/DHDFfS5eq4fEvABEBAAGJ
 ATwEGAEIACYCGwwWIQQt33LlpaVbqJ2qQuHCPZHzoSX+qAUCXZw1rgUJCWpOfwAKCRDCPZHz
 oSX+qFcEB/95cs8cM1OQdE/GgOfCGxwgckMeWyzOR7bkAWW0lDVp2hpgJuxBW/gyfmtBnUai
 fnggx3EE3ev8HTysZU9q0h+TJwwJKGv6sUc8qcTGFDtavnnl+r6xDUY7A6GvXEsSoCEEynby
 72byGeSovfq/4AWGNPBG1L61Exl+gbqfvbECP3ziXnob009+z9I4qXodHSYINfAkZkA523JG
 ap12LndJeLk3gfWNZfXEWyGnuciRGbqESkhIRav8ootsCIops/SqXm0/k+Kcl4gGUO/iD/T5
 oagaDh0QtOd8RWSMwLxwn8uIhpH84Q4X1LadJ5NCgGa6xPP5qqRuiC+9gZqbq4Nj
Message-ID: <ada75c60-aefb-86ff-0f3a-33825fb4c958@gmx.com>
Date:   Sun, 8 Dec 2019 13:02:11 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.2
MIME-Version: 1.0
In-Reply-To: <20191207144126.14320-1-dinghao.liu@zju.edu.cn>
Content-Type: multipart/signed; micalg=pgp-sha256;
 protocol="application/pgp-signature";
 boundary="OljafmsOc4h5wEdWTbLiKily3BqmqVmqc"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--OljafmsOc4h5wEdWTbLiKily3BqmqVmqc
Content-Type: multipart/mixed; boundary="QOQIKyC0n9wuUj0H6ToJUWrQpQGROLGWR"

--QOQIKyC0n9wuUj0H6ToJUWrQpQGROLGWR
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable


On 2019/12/7 =E4=B8=8B=E5=8D=8810:41, Dinghao Liu wrote:
> The return value of link_free_space(ctl, info) is checked out-sync. Onl=
y one branch of an if statement checks this return value after WARN_ON(re=
t).
>=20
> Since this path pair is similar in semantic, there might be a missing c=
heck bug.
>=20
> Fix this by simply adding a check on ret.

The main failure mode for link_free_space() is -EEXIST, which means
there is already free space in the cache.

Here EEXIST may not be a big problem, and we may really want to continue
the iteration other than error out.


Would you explain in details about why you believe error out is the
correct way other than current continue behavior?

Thanks,
Qu

>=20
> Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
> ---
>  fs/btrfs/free-space-cache.c | 2 ++
>  1 file changed, 2 insertions(+)
>=20
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 3283da419200..acbb3a59d344 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2437,6 +2437,8 @@ int btrfs_remove_free_space(struct btrfs_block_gr=
oup *block_group,
>  			if (info->bytes) {
>  				ret =3D link_free_space(ctl, info);
>  				WARN_ON(ret);
> +				if (ret)
> +					goto out_lock;
>  			} else {
>  				kmem_cache_free(btrfs_free_space_cachep, info);
>  			}
>=20


--QOQIKyC0n9wuUj0H6ToJUWrQpQGROLGWR--

--OljafmsOc4h5wEdWTbLiKily3BqmqVmqc
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEELd9y5aWlW6idqkLhwj2R86El/qgFAl3sg9MACgkQwj2R86El
/qiaGgf/ZElzpjn9aMoFUlHerx4oosFcD0sEJ6GUbmz0hoM6h9ecafKg5C+KRRUH
Tj76Wr8lavpXz22SOfK2VP+XOc24kGu/Cyt80CKt0+h91H0JtW6ZAbf735TMkfQ6
roHgZpNAiP7hOJBGsVu2RSQClP+wNJVEcJ9nyw40Quz5WI3Dhbmqv11VZvw+nD+1
77qsZvD1o6rsgVwn3g+xXAzHLAKPRumWoQo93ujRiuWsGsXxQcHs55yoGnhxEcqW
P82Qoql3RTi+M9j689wX82FYWJeeN+xbP14cGsFXNdwS0FRyGQ5l5NeFcCDuyytD
R3YFNE40tPrBG5ixKIx+AMf5FbZ94g==
=Ldcx
-----END PGP SIGNATURE-----

--OljafmsOc4h5wEdWTbLiKily3BqmqVmqc--