Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: ruF9ZyN4p8pineuZYEOOjhmCvWp9GTZ7JD3v9nv+PeDFb1vLIatdLRqUu6/Lh8M/dzw6pIUOyz
 Wmnfox78snSn/rVj4Au5dJWJrOpv2kDW/a3ZazXQgUX/AUwwfTKj7A0cJ4vkTGIeyNzhaDo5GU
 W+vwfOVDzJdVLJaiBZqmhzX4LfhDLLjJ9LuGj3sxH+FuGnubnlMK3YOQonK4hOcScralilG/Rf
 c8+GhQcnbBrED65Vd1PPsNBd72p2nBWaROzT7wFuiPdAZOx1kQMdtDH/fBK+IOM5ebr1nO0Thn
 QcE=
From:   Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
To:     "dsterba@suse.cz" <dsterba@suse.cz>
CC:     "lijian_8010a29@163.com" <lijian_8010a29@163.com>,
        "clm@fb.com" <clm@fb.com>,
        "josef@toxicpanda.com" <josef@toxicpanda.com>,
        "dsterba@suse.com" <dsterba@suse.com>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        lijian <lijian@yulong.com>
Subject: Re: [PATCH] fs: btrfs: extent_map: removed unneeded variable
Thread-Topic: [PATCH] fs: btrfs: extent_map: removed unneeded variable
Thread-Index: AQHXbMQAzBmALZ3G+kSY1PzHly/n7g==
Date:   Wed, 30 Jun 2021 11:17:30 +0000
Message-ID: <PH0PR04MB7416AEA4D23FAE000C86DF729B019@PH0PR04MB7416.namprd04.prod.outlook.com>
References: <20210629085025.98437-1-lijian_8010a29@163.com>
 <PH0PR04MB741666CF29DB96CF0DB0C26B9B029@PH0PR04MB7416.namprd04.prod.outlook.com>
 <20210630095923.GH2610@twin.jikos.cz>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?BBVoBYNjMT7qBSPXEevXHnMoCpUT/YZZ8Qws+T/n6G2ixsi1sCOQIfcK/jKx?=
 =?us-ascii?Q?6gkCTcIzp01VwXHJgw0XSgiRIfR7K6W69vg/9KjhLmkUMxYEsBTwSyG4LvTJ?=
 =?us-ascii?Q?7Zi7oakNvLS7oYLnzvi9ZnVSF4NvZWhOGyybtru9nhsdce2CKo/Kx0Y7DMoK?=
 =?us-ascii?Q?ptX3pJYYCxUlAOqvdTXpRB5bdCoQQ/rEHzTW+kw5k1K4LcvS6tgjiLxkuqrd?=
 =?us-ascii?Q?bQICNZErQhHnNBjLl4lcuYIjlQDH/wTHY8bfS3uiWSYTsqG8/1p4VLFOXbve?=
 =?us-ascii?Q?U9PYLB+ltKhWBtwAWhDdBi1k0gKNrVqqS4kuCv289gB0mt6jrkJCJ25YOyZx?=
 =?us-ascii?Q?0kshdgXPMqX7oe/bqsuzgal2r5YXxeFe1YRcE41X9ntM7SIWud6zLWUWXaFg?=
 =?us-ascii?Q?5RmWh2XwEamqsgGc0uP76FlEY8b+0H8LrHFYIbNA3b6ZVe0qFzyR58T4x/74?=
 =?us-ascii?Q?sI/ent3FxSC3nnRVabJc9KHgv6jlqesxhP6gMwOgi38ZyTokfGN6UOboRWqX?=
 =?us-ascii?Q?glsiRzILXieVuEC3SeWRs9Rw8PqWVcNS7HzFCw8G43Pwq3uaZl14D5Pculrj?=
 =?us-ascii?Q?9fMUbj0xqy1Zem9y4+Isd/zQ/1Z/10l6xN5ejcMGblWxKJ3w2mseNEOyWd6n?=
 =?us-ascii?Q?v/iPp31TiQK8O8l69aNSwLAlL7FL1wC7gDbl/b0RZ2NHPRa0ifI/aY5pzWx4?=
 =?us-ascii?Q?CvZWOyyDQrQ2M49/XZAe1MTlxIAxsscFRw7/crCeq+hiClCZRh0IFi3ZfG8O?=
 =?us-ascii?Q?Z9kMChZ73v7i+TM8XrPSggzwWtlIK8DyKzkcN/afEYktDsYL6CaWkGr2q0ug?=
 =?us-ascii?Q?broOZ+hR7XUt/xrEXuCLPtvJz+4r6VvN3bn5/JRjjZ330TorBI68bA4/4yyy?=
 =?us-ascii?Q?T1dmfaLcdrFj89UfgWdseQrHdR+mTAPM8yMwFf2CpRFbPVCmilTpN0EU0k3J?=
 =?us-ascii?Q?IZEWfFutruJEyX1c0GeNYt5QDhI13/eBmYWX9UbJlaW0LYVEQwyBQvR+NQkU?=
 =?us-ascii?Q?p/YtZyoO3Cub3TWlQRpqq3O94oUkTcggzR4HR1Y2p+mUh86z2+fyUTlyNFA4?=
 =?us-ascii?Q?wWFiuLCTqZJon5VOI7mycCfR482faqOomAnAR4M+c2MvyIde4EJigNJ9wsDq?=
 =?us-ascii?Q?tgql2at0EkXFa+vdBDifHmpczcEgkyDZXaS+/neZf3sRpAczEE2EHU5RkVa9?=
 =?us-ascii?Q?tYprOWb8Pv4nsBBZWojUjtklC9lUEcqhassl4kh+KRQRE0dp5ueoFpcrGdH2?=
 =?us-ascii?Q?fzsJJxe0wpqOWMuHGlBWmNBEPYDH8JUsVRFmLlPq0YkBWtN4ef8nEotNuNh9?=
 =?us-ascii?Q?y6VkoeMQ/ntnQenWiGSrfgm6VHBJu3nUDJLGxLkvoAvj8BJBD/tm9hPAgex/?=
 =?us-ascii?Q?bu0oWRHqgOIoKXsSMbzZq9rb8EQb/CAiYy0LI2CZikEkpY0Crw=3D=3D?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR04MB7416.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1473a590-205d-465b-06f0-08d93bb8a696
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Jun 2021 11:17:30.3232
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: bCMfe9UcMgQTuJb8r8iWL7dtAoLS/XyYUqok3PddOfQxWKptd9B+8/fRZfITuLniQbW3OMaCjc9XcftzgSHvphKQqgTynSEdMStiKxVOtXg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR04MB7416
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 30/06/2021 12:01, David Sterba wrote:=0A=
> On Tue, Jun 29, 2021 at 09:04:40AM +0000, Johannes Thumshirn wrote:=0A=
>> On 29/06/2021 10:51, lijian_8010a29@163.com wrote:=0A=
>>> From: lijian <lijian@yulong.com>=0A=
>>>=0A=
>>> removed unneeded variable 'ret'.=0A=
>>=0A=
>> Wouldn't it make more sense to return an error (-ENOENT??) in case=0A=
>> the em lookup fails and handle the error in btrfs_finish_ordered_io()=0A=
>> as this is the only caller of unpin_extent_cache()?=0A=
>>=0A=
>> I've actually tripped over this a couple of times already when =0A=
>> investigating extent map and ordered extent splitting problems=0A=
>> on zoned filesystems:=0A=
>>=0A=
>> 	em =3D lookup_extent_mapping(tree, start, len);=0A=
>> 	WARN_ON(!em || em->start !=3D start);=0A=
>>=0A=
>> Maybe even turn this WARN_ON() into an ASSERT() when introducing proper=
=0A=
>> error handling, as we shouldn't really get there unless we have a logica=
l=0A=
>> error.=0A=
> =0A=
> If you have real workloads hitting the warning then it really should be=
=0A=
> proper error handling, not even an assert.=0A=
> =0A=
=0A=
Up to now it's been coding errors from my side so I think it warrants an=0A=
ASSERT().=0A=
=0A=
But still we should handle the error in the caller.=0A=