On Wed, Jun 26, 2019 at 4:49 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
>
> On Wed 26-06-19 14:19:42, Alexander Potapenko wrote:
> [...]
> > diff --git a/mm/dmapool.c b/mm/dmapool.c
> > index 8c94c89a6f7e..fe5d33060415 100644
> > --- a/mm/dmapool.c
> > +++ b/mm/dmapool.c
> [...]
> > @@ -428,6 +428,8 @@ void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma)
> > }
> >
> > offset = vaddr - page->vaddr;
> > + if (want_init_on_free())
> > + memset(vaddr, 0, pool->size);
>
> any reason why this is not in DMAPOOL_DEBUG else branch? Why would you
> want to both zero on free and poison on free?
This makes sense, thanks.
> > #ifdef DMAPOOL_DEBUG
> > if ((dma - page->dma) != offset) {
> > spin_unlock_irqrestore(&pool->lock, flags);
>
> [...]
>
> > @@ -1142,6 +1200,8 @@ static __always_inline bool free_pages_prepare(struct page *page,
> > }
> > arch_free_page(page, order);
> > kernel_poison_pages(page, 1 << order, 0);
> > + if (want_init_on_free())
> > + kernel_init_free_pages(page, 1 << order);
>
> same here. If you don't want to make this exclusive then you have to
> zero before poisoning otherwise you are going to blow up on the poison
> check, right?
Note that we disable initialization if page poisoning is on.
As I mentioned on another thread we can eventually merge this code
with page poisoning, but right now it's better to make the user decide
which of the features they want instead of letting them guess how the
combination of the two is going to work.
> > if (debug_pagealloc_enabled())
> > kernel_map_pages(page, 1 << order, 0);
> >
> --
> Michal Hocko
> SUSE Labs
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
