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