Re: [PATCH] Export shmem_file_setup and shmem_getpage for DRM-GEM

[Keith Packard <keithp@keithp.com>]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

On Fri, 2008-08-01 at 11:57 +0100, Hugh Dickins wrote:

> It's fine to be getting the GEM end working using this easiest way,
> and I don't think we'll have a problem with EXPORT_SYMBOL_GPL() on
> shmem_file_setup() or something very like it - it just happens that
> its current callers are not in loadable modules.

Cool. Using shmem saved a huge amount of code in DRM and allows the GEM
objects to be paged to disk if necessary. We haven't added the shrinker
API calls to finish this work, but that shouldn't take much additional
code.

> But shmem_getpage() is very much an internal helper function for
> mm/shmem.c, and I'm pretty sure we should keep it and its sgp_types
> that way: I'll take a look at how you're using it later on, and see
> what makes sense to offer instead.

We use it to get a struct page within a shmem object; we don't need the
sgp types exposed as we only ever use SGP_DIRTY. We could use a narrower
interface like:

int shmem_file_page(struct file *file, int idx, struct page **page)
{
        struct inode *inode = file->f_path.dentry->d_inode;

        return shmem_getpage(inode, idx, page, SGP_DIRTY, NULL);
}
EXPORT_SYMBOL(shmem_file_page);


> And I don't think you should assume that all GEM users must have
> CONFIG_SHMEM=y: we should aim for an interface that mm/tiny_shmem.c
> (redirecting to ramfs) can also support.

Seems like the simpler interface should be supportable from ramfs:

int shmem_file_page(struct file *file, int idx, struct page **page)
{
        struct inode *inode = file->f_path.dentry->d_inode;
        struct address_space *mapping = inode->i_mapping;
        struct page *filepage;
        
        filepage = find_lock_page(mapping, idx);
        if (!filepage)
                return -ENOSPC;
        *page = filepage;
        return 0;
}

(no, I didn't try this)

>   I can believe that GEM and
> TINY won't have much intersection, but let's not force that by using
> shmem_getpage().

I don't know; I'm running without swap on several netbooks these days,
and I can easily believe that TINY would be a reasonable option there.
Thanks for the suggestions.

-- 
keith.packard@intel.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]