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

Reply via email to