On Fri, Mar 15, 2024 at 12:23:23PM -0600, Keith Busch wrote:
> 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().
Sorry forget that, all the other existing callers just want the
sector_t.
