[lkml]   [2012]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 5/6] vfs: reorganize do_lookup
    Al Viro <> writes:

    > On Tue, Apr 03, 2012 at 10:13:18AM +0200, Miklos Szeredi wrote:
    >> The do_last() reorganization in the atomic-open series needs to be split
    >> up, I realize. Do you have any other high level comments about that
    >> series?
    > Yes.
    > a) Please, pull removal of open-from-d_revalidate() as far in front
    > of the queue as possible, *along* *with* -EOPENSTALE stuff. Without the
    > latter the former is simply broken - we might have hit -ESTALE before and
    > LOOKUP_REVAL might have already been set, so just failing ->open() with
    > ESTALE may end up not repeating it.
    > TBH, had that thing been in front of the queue, I would've put
    > it into the last pull request; that particular idiocy (NFS4 doing open
    > from all methods involved, except for ->open()) had been a serious source
    > of annoyance for a long time. It really needs killing...

    Will post updated series in a minute.

    > b) opendata is simply bogus. You need to pass caller-allocated
    > struct file in any case, right? So why not use it to pass what you
    > need to pass? We want to be able to tell "it's a symlink, here's the
    > vfsmount/dentry, now sod off and handle it yourself"?

    My concern is about the visibility of a half cooked file pointer to the
    filesystem. The compiler won't notice anything wrong with the
    following, neither will casual testing, but it will go boom at
    inopportune times:

    foo_atomic_open(..., struct file *filp, ...)
    /* ... */

    if (unlikely error)
    goto cleanup;

    filp = finish_open(filp, dentry, NULL);

    /* ... */

    if (some other error)
    goto cleanup;

    return filp;

    return ERR_PTR(error);


     \ /
      Last update: 2012-04-05 16:47    [W:0.021 / U:130.440 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site