Hi Martin, On Wed, Dec 13, 2023 at 11:38:10PM -0500, Martin K. Petersen wrote: > > Ming, On Thu, Dec 14, 2023 at 12:35 PM Martin K. Petersen <martin.petersen@xxxxxxxxxx> wrote: > > > Hi Ming! > > >> + lim->atomic_write_unit_min_sectors = 0; > >> + lim->atomic_write_unit_max_sectors = 0; > >> + lim->atomic_write_max_sectors = 0; > >> + lim->atomic_write_boundary_sectors = 0; > > > > Can we move the four into single structure and setup them in single > > API? Then cross-validation can be done in this API. > > Why would we put them in a separate struct? We don't do that for any of > the other queue_limits. All the four limits are for same purpose of supporting atomic-write, and there can many benefits to define single API to setup atomic parameters: 1) common logic can be put into single place, such as running cross-validation among them and setting up default value, and it is impossible to do that by the way in this patch 2) all limits are supposed to setup once by driver in same place, so doing them in single API actually simplifies driver and block layer, and API itself becomes less fragile 3) it is easier for trace or troubleshoot > > > Relying on driver to provide sound value is absolutely bad design from > > API viewpoint. > > All the other queue_limits are validated by the LLDs. It's challenging > to lift that validation to the block layer since the values reported are > heavily protocol-dependent and After atomic limits are put into block layer, it becomes not driver specific any more, scsi and nvme are going to support it first, sooner or later, other drivers(dm & md, loop or ublk, ...) may try to support it. Also in theory, they are not protocol-dependent, usually physical block size is the minimized atomic-write unit, now John's patches are trying to support bigger atomic-write unit as scsi/nvme's protocol supports it, and the concept is actually common in disk. Similar in implementation level too, such as, for NAND flash based storage, I guess atomic-write should be supported by atomic update of FTL's LBA/PBA mapping. > thus information is lost if we do it somewhere else. Block layer is only focusing on common logic, such as the four limits added in request queue, which are consumed by block layer and related with other generic limits(physical block size, max IO size), and it won't be same with driver's internal validation. Thanks, Ming
