On Wed, 19 Jul 2023, Christoph Hellwig wrote:
> > +static void brd_free_page_rcu(struct rcu_head *head)
> > +{
> > + struct page *page = container_of(head, struct page, rcu_head);
> > + __free_page(page);
>
> Nit: missing empty line here. Although I see no point in the local
> variable anyay.
>
> > +}
> > +
> > +static void brd_free_page(struct brd_device *brd, sector_t sector)
> > +{
> > + struct page *page;
> > + pgoff_t idx;
> > +
> > + idx = sector >> PAGE_SECTORS_SHIFT;
> > + page = xa_erase(&brd->brd_pages, idx);
> > +
> > + if (page) {
> > + BUG_ON(page->index != idx);
> > + call_rcu(&page->rcu_head, brd_free_page_rcu);
> > + }
>
> Doing one call_rcu per page is horribly inefficient. Please look into
> batching this up.
>
> > +static bool discard = false;
> > +module_param(discard, bool, 0444);
> > +MODULE_PARM_DESC(discard, "Support discard");
>
> Doing this as a global paramter that can't even be changed at run time
> does not feel very user friendly.
Hi
I addressed these issues and I'll send a new version.
Mikulas
