Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <78b4b92f-ae89-2e54-f96f-21d338ca042b@deltatee.com>
Date:   Wed, 27 Apr 2022 10:11:13 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Content-Language: en-US
To:     Guoqing Jiang <guoqing.jiang@linux.dev>,
        linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
        Song Liu <song@kernel.org>
Cc:     Christoph Hellwig <hch@infradead.org>,
        Stephen Bates <sbates@raithlin.com>,
        Martin Oliveira <Martin.Oliveira@eideticom.com>,
        David Sloan <David.Sloan@eideticom.com>
References: <20220420195425.34911-1-logang@deltatee.com>
 <20220420195425.34911-12-logang@deltatee.com>
 <7f61aacd-d378-092c-1291-f2aaf42a5537@linux.dev>
From:   Logan Gunthorpe <logang@deltatee.com>
In-Reply-To: <7f61aacd-d378-092c-1291-f2aaf42a5537@linux.dev>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PATCH v2 11/12] md/raid5: Check all disks in a stripe_head for
 reshape progress
Precedence: bulk


On 2022-04-26 19:53, Guoqing Jiang wrote:
>> +static bool stripe_ahead_of_reshape(struct mddev *mddev, struct
>> r5conf *conf,
>> +                    struct stripe_head *sh)
>> +{
>> +    sector_t max_sector = 0, min_sector = MaxSector;
>> +    bool ret = false;
>> +    int dd_idx;
>> +
>> +    for (dd_idx = 0; dd_idx < sh->disks; dd_idx++) {
>> +        if (dd_idx == sh->pd_idx)
>> +            continue;
>> +
>> +        min_sector = min(min_sector, sh->dev[dd_idx].sector);
>> +        max_sector = min(max_sector, sh->dev[dd_idx].sector);
>> +    }
>> +
>> +    spin_lock_irq(&conf->device_lock);
>> +
>> +    if (!range_ahead_of_reshape(mddev, min_sector, max_sector,
>> +                     conf->reshape_progress))
>> +        /* mismatch, need to try again */
>> +        ret = true;
> 
> I think we can just open code range_ahead_of_reshape.

I think this way is much easier to read. GCC is much

> And seems the above is not same as below original checking which compare
> logical_sector with reshape_progress. Is it intentional or am I miss
> something?

Yes, this was intentional, the change is necessary for the next patch to
ensure that all pages in a stripe are ahead of the reshape. This was not
intended as just a cleanup patch.

Logan