Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Alex Zhuravlev <azhuravlev@whamcloud.com>
To:     Ritesh Harjani <riteshh@linux.ibm.com>
CC:     Alex Zhuravlev <azhuravlev@whamcloud.com>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>
Subject: Re: [PATCH 2/2] ext4: skip non-loaded groups at cr=0/1
Thread-Topic: [PATCH 2/2] ext4: skip non-loaded groups at cr=0/1
Thread-Index: AQHWHEjMGVpmuPlRl0Kqrw5gTNRAkaindXQAgAF/eoA=
Date:   Fri, 15 May 2020 08:56:42 +0000
Message-ID: <914597DA-395A-47A5-A8D6-DFCE2D674289@whamcloud.com>
References: <0B6BF408-EDF7-4363-80CD-BDA0136BF62C@whamcloud.com>
 <20200514100411.D1A15A405C@b06wcsmtp001.portsmouth.uk.ibm.com>
In-Reply-To: <20200514100411.D1A15A405C@b06wcsmtp001.portsmouth.uk.ibm.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <AB2C887153CEB0499E8B32D176325088@namprd19.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 9500f853-b008-462a-2a7e-08d7f8ade357
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 May 2020 08:56:42.2346
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 753b6e26-6fd3-43e6-8248-3f1735d59bb4
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: pOuTno0gQcwyiQVjzMGdvZAQj4Sgm/QZKfg3pY+9Y8osv63xG0oPJeGVCRp7zxU8sF4pRdFxXvhHCOJ+P+AATw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR19MB4344
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

Thanks for the review, answers inline..


> On 14 May 2020, at 13:04, Ritesh Harjani <riteshh@linux.ibm.com> wrote:
> Not needed in a commit msg.

OK

>=20
>> cr=3D0 is supposed to be an optimization to save CPU cycles, but if budd=
y data (in memory)
>> is not initialized then all this makes no sense as we have to do sync IO=
 taking a lot of cycles.
>> also, at cr=3D0 mballoc doesn't store any avaibale chunk. cr=3D1 also sk=
ips groups using heuristic
> /s/avaibale/available/

OK

>=20
>> based on avg. fragment size. it's more useful to skip such groups and sw=
itch to cr=3D2 where
>> groups will be scanned for available chunks.
>> using sparse image and dm-slow virtual device of 120TB was simulated. th=
en the image was
>> formatted and filled using debugfs to mark ~85% of available space as bu=
sy. mount process w/o
>> the patch couldn't complete in half an hour (according to vmstat it woul=
d take ~10-11 hours).
>> with the patch applied mount took ~20 seconds.
>=20
> I guess what we should edit the commit msg to explain that it is not the
> mount process but the very first write whose performance is improved via
> this patch.

Correct

>> +		/* cr=3D0/1 is a very optimistic search to find large
>> +		 * good chunks almost for free. if buddy data is
>> +		 * not ready, then this optimization makes no sense */
>=20
> I guess it will be also helpful to mention a comment related to the
> discussion that we had on why this should be ok to skip those groups.
> Because this could result into we skipping the group which is closer to
> our inode. I somehow couldn't recollect it completely.

Please remind where the discussion took place? I must be missing that.