[lkml]   [2008]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] Export shmem_file_setup and shmem_getpage for DRM-GEM
    On Mon, 4 Aug 2008, Keith Packard wrote:
    > On Mon, 2008-08-04 at 18:09 +0100, Hugh Dickins wrote:
    > > Whether such usage conforms to VFS API I'm not so sure: as I understand
    > > it, it's really for internal use by a filesystem
    > Sure, but presumably it could even be used by a layered file system?

    Could, yes, but should? I wouldn't presume to answer with any authority.

    > > - if it's going to be
    > > used beyond that, we ought to add a check that the filesystem it's used
    > > upon really has a ->readpage method (and I'd rather we add such a check
    > > than you do it at your end, in case we change the implementation later
    > > to use something other than a ->readpage method - Nick, you'll be
    > > nauseated to hear I was looking to see if ->fault with a pseudo-vma
    > > could do it). But if the layering police are happy with this, I am.
    > It seems like I should put a check into my code that is kernel version
    > dependent so that I can't oops if someone tries to use a filesystem that
    > doesn't have ->readpage.

    Well, I guess put the check on ->readpage into your code for now, and
    by the time GEM gets into Linus's tree, we should have -EINVAL checks
    on NULL filler() in __read_cache_page() and read_cache_page_async(),
    so remove check at your end before final submission.

    (You could leave it there, and strictly we ought to update GEM if we
    make any change to our implementation; but it is the kind of detail
    that gets overlooked - witness the way I failed to grasp the readahead
    side-effects of adding ->readpage into tmpfs until recently. I'm just
    afraid we'd break you unwittingly: better not, though easily fixed.)

    I'm not sending the patch right now, waiting to see if this direction
    wins general favour.

    > So, when I release the pages from the page cache, I'm currently calling
    > mark_page_accessed for all pages, and set_page_dirty for pages which may
    > have been written by the GPU. Are those calls still needed?

    I think you should drop the mark_page_accessed(): that's done in
    read_cache_page_async() as part of the initial read_mapping_page().
    But do it again when releasing if you think there's a good chance
    that object will be wanted again shortly. set_page_dirty() if
    modified by GPU, yes, that would still be needed.

    For how long are these objects' pages pinned in memory like this?
    I ask because Rik & Lee have patches in -mm, trying to avoid long
    scans of LRUs cluttered with unevictable pages. I've no idea
    whether you're adding a lot or a little to that problem.


     \ /
      Last update: 2008-08-04 20:43    [W:0.021 / U:84.832 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site