Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c4293742-06ba-8720-e2eb-d4d3bc4da044@suse.com>
Date:   Thu, 29 Sep 2022 11:08:10 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Content-Language: en-US
To:     "Flint.Wang" <hmsjwzb@zoho.com>
Cc:     stringbox8@zoho.com, 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: <20220929014402.2450-1-hmsjwzb@zoho.com>
From:   Qu Wenruo <wqu@suse.com>
Subject: Re: [PATCH] btrfs:remove redundant index_rbio_pages in
 raid56_rmw_stripe
In-Reply-To: <20220929014402.2450-1-hmsjwzb@zoho.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bzZFaDkrMmNLWjhpM1E1NnZRVnZteVRlQ3ZrTUc5QlEwNUNJQlQ2eTdQNXRp?=
 =?utf-8?B?MGpVcTNDNGlEV0xJcHE3SzdtdS91VVU5V29WZHJHL1RVYU5JeDB5aGFMMko5?=
 =?utf-8?B?L0JzQlhQOXIzSnE5RURVZ2xCd1dNUStUSDZvamxPZUdsMG9ES1RON0ViZzZ1?=
 =?utf-8?B?ZTlXOW9scDNFVGp5VHBPVmdmZW9HYmw0YWx2OHh2bGcyUEdqQUZEaG1tNmwx?=
 =?utf-8?B?ZlZ2KzhJdm5hdVdBMXRONHp2MGgzRGNJQ2RSdlVPWDJ4SEYzUGoxZTVhV3Zr?=
 =?utf-8?B?YXV2dnB6bzFBcTZWd2hFemNxb3RqV2hvalI4RTFNNWFDSnAzRDh6UnhqUzVQ?=
 =?utf-8?B?TWFVR0x4YXlwcnVwMDBnNnZBVUZhdmtTUHJ5R3FKL2FjZHZ6MU50N1FPdFdD?=
 =?utf-8?B?ZDJaYXJOZkVLU3pkNWtsd2ZXQTdDNVVDbC94SkU3dVdSY1RZalRjN1d6MEw1?=
 =?utf-8?B?YjYzcTRyRXNTdHJ6R0JrbXorTE9BYXBTL1Y2MnNUL1owUmRTZXA1U2w0K3B2?=
 =?utf-8?B?VTNLUitMbEI0b3lEcnJZeUtVckJWQllkNXZoT1J3aC9zVVNVd0Q1RTE4VnlN?=
 =?utf-8?B?VUFSVGlNS1gzYk43U0ROcS9jVlA4WGZKODdKNEVsVE5NR0FaRnU4OFFxcG03?=
 =?utf-8?B?cFU1MnFMMCtwa1Rxd2cyLzNBb05ZYm96NEZIT1IyZHovd1JTRDE3UHNXN1F0?=
 =?utf-8?B?Q0tES3B3OUM2clk0eEVsSzlsc1luYjFwTnZvRktvU2VjV1c4eEptaGdBczVF?=
 =?utf-8?B?UW9jZzF2UXhub2NvWFo0VFUvNDZ6TGFjMVlkcmJjbjVrVE1EODQrcWNwM1VK?=
 =?utf-8?B?QjdZNVVRWTBwU1UvWiszR1NORm9BMVRMR2hOOElERUphcFVZVGNNSDVETC9O?=
 =?utf-8?B?dVlacWFIUmcwSWRqZkMzVXBYM2NYSDBKZVB4Tndhc2xYaHhaSlBBbEhKN29V?=
 =?utf-8?B?elRoU3dnb3htOHpwRTN1c3cxN1dnQzlhZm9ZV3BuWFk4Yy9FSkkxOGlJaWhq?=
 =?utf-8?B?ZkcwaVZzTkg5YmgzZStpVzhBZm9Ob0REblN3TjFpT3lXQ2RnZkplSzNpWjY5?=
 =?utf-8?B?Y3YzSldqd3g0SGVlekdJM3ZuQmtjVXZIdk41RzZhTGk0WFJFaS9pYi9jVmJm?=
 =?utf-8?B?aGdTNUpvUVFDU0ZQM3Jscm1KcWxDZzBRajFGdEJTVTdqNHNLTDFjT0FQSEdl?=
 =?utf-8?B?YlY0bU5ubW1mbW45R296ZDRSOG5kMHdDRG5NYmJkTkszSnFHSVRSVEYyNHhS?=
 =?utf-8?B?SUR3eE1GaU5DY29HTFpmQjNTM2dZMGdLbnJ6TDVoMWQ4L09FRFJnN0RSRThQ?=
 =?utf-8?B?cXc4ZVJqaGF6M1A3eXVBTjNwK1Z4eExEWlRmRVBjbSt6S2lndzZJNERDaFA3?=
 =?utf-8?B?aWF3dWN3djNKTmxEY2J0amRETThTdUhIYXhYNnQrbWE2YTV2bnprajZoa2hy?=
 =?utf-8?B?L2xrdGhFRjRKRHlOVHo1N2ZLMjdRU3o5R3NSR21TbXhaOGdCVncyL0dSQVNh?=
 =?utf-8?B?RTZyVElETzU1TXI5TVh4OWwrbzlVRXJKWXpLZlY0MEpHckxkNzRyMEo2eVlz?=
 =?utf-8?B?UDBSREVLTEcybHU1QmJOdzZZYzJxVnNDcmFHSDA2bVhDMVZUUUFEMXV1K0xS?=
 =?utf-8?B?OE5xbm5HVGhCdzI2ZitBMW4xNjlWQXFUZlNFVXBwakw5T3U0bE90ZFNxMWJQ?=
 =?utf-8?B?R05LVVNKN0wzenZuL3F4Y29YUGtRbFhoOHU5UTY1UlE0RU9TSWozb0x1MTFP?=
 =?utf-8?B?YkhWUHNON2JXdzB6YW52bkl6WWt5SFBHMmpxamFWNmUrUStuZjdOSVZITWl4?=
 =?utf-8?B?c3VjNHZCWDY3VWtkWXdXZHZ0TjBMam5KeGtCWEhsQjB1ZEFHVFVGYU4rMzVa?=
 =?utf-8?B?Nk9MY2dxSjBRNExHcnl5U3UxdFlwSjdTeFNMM251a3FOSzR0RHB2alNXdG1p?=
 =?utf-8?B?Sm5kdFA0YnQ3Rkx2TTdYcmJlemYzWTFVUDZnWGtHU0tlVG9IL3RFUVkxNjZx?=
 =?utf-8?B?eDNtWWhmTVE5MWVNVEdWSDg1bEJtK0JtTS9rWmNEVXE4SkRqS1dSb0tiUmJ3?=
 =?utf-8?B?NEFrbnpibmlOVGxDOVgzTllMeSthc3p0d1FIaHc5eStxMERtOVh4aGtVRllX?=
 =?utf-8?B?Ymd4QkphVzl4UWczZms0cTFIRnVyMmxkS1p4TlZUL3M2N1cxdUQ2RUdhb1lO?=
 =?utf-8?Q?yoBvtYWy+pPuzr/krTQeLSg=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5e7d167e-96e9-491d-890d-08daa1c7dd05
X-MS-Exchange-CrossTenant-AuthSource: AS8PR04MB8465.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Sep 2022 03:08:20.7693
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6Uxan5Cba4HJ0jT70fHmyPAIhdW1ecuSvTWuarr4zWms57HfX0urBc/pZ2890dcI
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB7984
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2022/9/29 09:44, Flint.Wang wrote:
>    The index_rbio_pages in raid56_rmw_stripe is redundant.

index_rbio_pages() is to populate the rbio->bio_sectors array.

In raid56_rmw_stripe() we later calls sector_in_rbio(), which will check 
if a sector is belonging to bio_lists.

If not called, all sector will be returned using the sectors in 
rbio->bio_sectors, not using the sectors in bio lists.

Have you tried your patch with fstests runs?

IMHO it should fail a lot of very basic writes in RAID56.

Thanks,
Qu

>    It is invoked in finish_rmw anyway.
> 
> Signed-off-by: Flint.Wang <hmsjwzb@zoho.com>
> ---
>   fs/btrfs/raid56.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
> index f6395e8288d69..44266b2c5b86e 100644
> --- a/fs/btrfs/raid56.c
> +++ b/fs/btrfs/raid56.c
> @@ -1546,8 +1546,6 @@ static int raid56_rmw_stripe(struct btrfs_raid_bio *rbio)
>   	if (ret)
>   		goto cleanup;
>   
> -	index_rbio_pages(rbio);
> -
>   	atomic_set(&rbio->error, 0);
>   	/* Build a list of bios to read all the missing data sectors. */
>   	for (total_sector_nr = 0; total_sector_nr < nr_data_sectors;