Re: [PATCH 2/2] vfree: Update documentation

[Christoph Hellwig <hch@lst.de>]

On Tue, Sep 22, 2020 at 04:22:40PM +0100, Matthew Wilcox wrote:
> On Tue, Sep 22, 2020 at 05:09:10PM +0200, Christoph Hellwig wrote:
> > On Tue, Sep 22, 2020 at 04:06:03PM +0100, Matthew Wilcox wrote:
> > > I don't think it makes sense to list all vmalloc-style allocators here.
> > > It won't be updated by people who add new variations.  How about this?
> > > 
> > >  * Free the virtually continuous memory area starting at @addr, as
> > >  * obtained from one of the vmalloc() family of APIs.  This will
> > >  * usually also free the physical memory underlying the virtual
> > >  * allocation, but that memory is reference counted, so it will not
> > >  * be freed until the last user goes away.
> > >  *
> > >  * If @addr is NULL, no operation is performed.
> > > 
> > > I'm trying to strike a balance between being accurate and not requiring
> > > device driver authors to learn all about struct page.  I may be too
> > > close to the implementation to write good documentation for it.
> > 
> > I think the above is sensible, but not enough.  vmap really needs to
> > be treated special, as by default area->pages for vmap is NULL.  So
> > for vfree to be useful on a vmap mapping, the callers needs to
> > manually set it up by poking into the internals.  Actually, I think
> > we really want another API rather than vmap for that.  Let me respin
> > my series to include that.
> 
> I've been thinking about somethng like:
> 
> void *vmap_mapping(struct address_space *mapping, pgoff_t start,
> 		unsigned long len);
> 
> but it doesn't quite work for the shmem cases because they need to use
> shmem_read_mapping_page_gfp() instead of ->readpage.  I'd also want it
> to work for DAX, but I don't have a user for that yet so it's hard to
> justify adding it.

That seems a little too special cased to me.

FYI, if you are fine with it I'll pick your two patches with the
updates from this thread up for my series.  It has be now morphed into
a general vmalloc.c cleanup series.