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

On Mon, 2008-08-04 at 20:43 +1000, Nick Piggin wrote:

True. It would make it possible for the userspace program to pass in
anonymous pages, but maybe not a big deal if you're using files and
shmem based management.

We considered using anonymous pages, but as the user-mapping is not a
feature, it seemed like it wasn't the right model. Plus, many of these
objects need to be shared across multiple processes, so anonymous pages
would be a pain there.

pagecache_write_begin/pagecache_write_end should be reasonable, but you
have to be careful of the semantics of it. For example, you can't really
read anything from the page inside the calls because the filesystem may
not bring it up to date.

Ok, that's useful information which isn't clear from the docs.

read_mapping_page might help there.

That does look a lot more like what I want, as it returns an unlocked
page. And, makes my code look cleaner to boot:

inode = obj->filp->f_path.dentry->d_inode;
mapping = inode->i_mapping;
for (i = 0; i < page_count; i++) {
page = read_mapping_page(mapping, i, NULL);
if (IS_ERR(page)) {
ret = PTR_ERR(page);
DRM_ERROR("read_mapping_page failed: %d\n", ret);
i915_gem_object_free_page_list(obj);
return ret;
}
obj_priv->page_list[i] = page;
}

Does this look like it conforms to the vfs api? It appears to work when
using shmem at least.

Right, that's your specific implementation, but for some cases the
memory management can map or be implemented using generic primitives.
Using pagecache for your memory for example should work nicely. Making
it shmem specific and using internal APIs seems like a negative step
until you really have a good reason to.

Yup, I'm liking the general file mechanism, I used shmem only because
that seemed like the obvious file system you'd want underneath these
objects.

--
keith.packard@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part

Quantcast