On Fri, Nov 22, 2019 at 10:18:38AM +1100, Dave Chinner wrote: > On Thu, Nov 21, 2019 at 10:44:44PM +0100, Pavel Reichl wrote: > > Signed-off-by: Pavel Reichl <preichl@xxxxxxxxxx> > > --- > > This is mixing an explanation about why the change is being made > and what was considered when making decisions about the change. > > e.g. my first questions on looking at the patch were: > > - why do we need to break up the discards into 2GB chunks? > - why 2GB? Yeah, I'm wondering that too. > - why not use libblkid to query the maximum discard size > and use that as the step size instead? FWIW my SATA SSDs the discard-max is 2G whereas on the NVME it's 2T. I guess firmwares have gotten 1000x better in the past few years, possibly because of the hundred or so 10x programmers that they've all been hiring. > - is there any performance impact from breaking up large > discards that might be optimised by the kernel into many > overlapping async operations into small, synchronous > discards? Also: What is the end goal that you have in mind? Is the progress reporting the ultimate goal? Or is it to break up the BLKDISCARD calls so that someone can ^C a mkfs operation and not have it just sit there continuing to run? --D > i.e. the reviewer can read what the patch does, but that deosn't > explain why the patch does this. Hence it's a good idea to explain > the problem being solved or the feature requirements that have lead > to the changes in the patch.... > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
