On 5/1/19 7:57 PM, Marcos Paulo de Souza wrote: > Changes from v2: > Rename size_to_sectors o bytes_to_sectors. (suggested by Martin K. Petersen) > > Changes from v1: > Reworked the documentation of size_to_sectors by removing a sentence that was > explaining the size -> sectors math, which wasn't necessary given the > description prior to the example. (suggested by Chaitanya Kulkarni) > > Let me know if you have more suggestions to this code. > > Here is the cover letter of the RFC sent prior to this patchset: > > While reading code of drivers/block, I was curious about the set_capacity > argument, always shifting the value by 9, and so I took me a while to realize > this is done on purpose: the capacity is the number of sectors of 512 bytes > related to the storage space. > > Rather the shifting by 9, there are other places where the value if shifted by > SECTOR_SHIFT, which is more readable. > This patch aims to reduce these differences by adding a new function called > bytes_to_sectors, adding a proper comment explaining why this is needed. > > null_blk was changed to use this new function. Maybe I'm alone in this, but I much prefer seeing this: sector = bytes >> SECTOR_SHIFT; to sector = bytes_to_sectors(bytes); The former tells me exactly what it does, the latter I'd need to look up. I do agree that any hard coding of 9 should be SECTOR_SHIFT, though. However, if we are going to do this, the functions would need a blk_ prefix. -- Jens Axboe
