On 10/4/22 22:02, Damien Le Moal wrote: > On 10/5/22 12:16, Chaitanya Kulkarni wrote: >> Introduce and use two new macros for calculating the page index from >> given sector and index (offset) of the sector in the page. >> The newly added macros makes code easy to read with meaningful name and >> explanation comments attached to it. >> >> While at it adjust the code in the null_free_sector() to return early >> to get rid of the extra identation. >> >> Signed-off-by: Chaitanya Kulkarni <kch@xxxxxxxxxx> >> --- >> drivers/block/null_blk/main.c | 37 ++++++++++++++++++++--------------- >> 1 file changed, 21 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c >> index 2d592b4eb815..b82c2ffeb086 100644 >> --- a/drivers/block/null_blk/main.c >> +++ b/drivers/block/null_blk/main.c >> @@ -14,6 +14,11 @@ >> #undef pr_fmt >> #define pr_fmt(fmt) "null_blk: " fmt >> >> +/* Gives page index for which this sector belongs to. */ > > That is clear from the macro name. Not convinced this comment is useful. > >> +#define PAGE_IDX_FROM_SECT(sect) (sect >> PAGE_SECTORS_SHIFT) >> +/* Gives index (offset) of the sector within page. */ >> +#define SECT_IDX_IN_PAGE(sect) ((sect & SECTOR_MASK) << SECTOR_SHIFT) > > SECT_OFFSET_IN_PAGE() ? A "page" not being an array of sectors, using > index for a sector is a little strange I think. And you use this macro for > things like: > > offset = SECT_IDX_IN_PAGE(sect); > > So offset in the name makes more sense. > > okay I'll make it to SECT_OFFSET_IN_PAGE() -ck
