On Fri, Mar 15, 2024 at 06:12:09PM +0000, Matthew Wilcox (Oracle) wrote:
> static struct page *brd_lookup_page(struct brd_device *brd, sector_t sector)
> {
> - pgoff_t idx;
> - struct page *page;
> -
> - idx = sector >> PAGE_SECTORS_SHIFT; /* sector to page index */
> - page = xa_load(&brd->brd_pages, idx);
> -
> - BUG_ON(page && page->index != idx);
> -
> - return page;
> + return xa_load(&brd->brd_pages, sector >> PAGE_SECTORS_SHIFT);
> }
>
> /*
> @@ -67,8 +56,8 @@ static struct page *brd_lookup_page(struct brd_device *brd, sector_t sector)
> */
> static int brd_insert_page(struct brd_device *brd, sector_t sector, gfp_t gfp)
> {
> - pgoff_t idx;
> - struct page *page, *cur;
> + pgoff_t idx = sector >> PAGE_SECTORS_SHIFT;
> + struct page *page;
> int ret = 0;
>
> page = brd_lookup_page(brd, sector);
This looks good. Unnecessary suggestion, but since you're already
changing these, might as well replace "sector" with "idx" here and skip
the duplicated shift in brd_lookup_page().
Reviewed-by: Keith Busch <kbusch@xxxxxxxxxx>
