Thanks for the review, answers inline.. > On 14 May 2020, at 13:04, Ritesh Harjani <riteshh@xxxxxxxxxxxxx> wrote: > Not needed in a commit msg. OK > >> cr=0 is supposed to be an optimization to save CPU cycles, but if buddy 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=0 mballoc doesn't store any avaibale chunk. cr=1 also skips groups using heuristic > /s/avaibale/available/ OK > >> based on avg. fragment size. it's more useful to skip such groups and switch to cr=2 where >> groups will be scanned for available chunks. >> using sparse image and dm-slow virtual device of 120TB was simulated. then the image was >> formatted and filled using debugfs to mark ~85% of available space as busy. mount process w/o >> the patch couldn't complete in half an hour (according to vmstat it would take ~10-11 hours). >> with the patch applied mount took ~20 seconds. > > 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=0/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 */ > > 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.
