linux-kernel - Re: [PATCH] Export shmem_file_setup and shmem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1217613998.23437.246.camel@koto.keithp.com>
Date:	Fri, 01 Aug 2008 11:06:38 -0700
From:	Keith Packard <keithp@...thp.com>
To:	Hugh Dickins <hugh@...itas.com>
Cc:	keithp@...thp.com, Eric Anholt <eric@...olt.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Export shmem_file_setup and shmem_getpage for DRM-GEM

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@...el.com

Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)