02 Nov, 2011

1 commit


24 Oct, 2011

5 commits


19 Oct, 2011

2 commits


18 Oct, 2011

8 commits

  • If we create the object and then return failure to the client, we're
    left with an unexpected file in the filesystem.

    I'm trying to eliminate such cases but not 100% sure I have so an
    assertion might be helpful for now.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • As with the nfs4_file, we'd prefer to find out about any failure before
    creating a new file rather than after.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Move idr preallocation out of stateid initialization, into stateid
    allocation, so that we no longer have to handle any errors from the
    former.

    This is a little subtle due to the way the idr code manages these
    preallocated items--document that in comments.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Creating a new file is an irrevocable step--once it's visible in the
    filesystem, other processes may have seen it and done something with it,
    and unlinking it wouldn't simply undo the effects of the create.

    Therefore, in the case where OPEN creates a new file, we shouldn't do
    the create until we know that the rest of the OPEN processing will
    succeed.

    For example, we should preallocate a struct file in case we need it
    until waiting to allocate it till process_open2(), which is already too
    late.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • If process_open1() creates a new open owner, but the open later fails,
    the current code will leave the open owner around. It won't be on the
    close_lru list, and the client isn't expected to send a CLOSE, so it
    will hang around as long as the client does.

    Similarly, if process_open1() removes an existing open owner from the
    close lru, anticipating that an open owner that previously had no
    associated stateid's now will, but the open subsequently fails, then
    we'll again be left with the same leak.

    Fix both problems.

    Reported-by: Bryan Schumaker
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • No change in behavior.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • There doesn't seem to be any harm to renewing the client a bit earlier,
    when it is looked up. That saves us from having to sprinkle
    renew_client calls over quite so many places.

    Also remove a redundant comment and do a little cleanup.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     

17 Oct, 2011

1 commit


12 Oct, 2011

2 commits


11 Oct, 2011

5 commits

  • I'd rather put more of these sorts of checks into standardized xdr
    decoders for the various types rather than have them cluttering up the
    core logic in nfs4proc.c and nfs4state.c.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • We don't use WANT bits yet--and sending them can probably trigger a
    BUG() further down.

    Cc: stable@kernel.org
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • These comments are mostly out of date.

    Reported-by: Bryan Schumaker

    J. Bruce Fields
     
  • In response to some review comments, get rid of the somewhat obscure
    for-loop with bitops, and improve a comment.

    Reported-by: Steve Dickson
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • In commit 5ec094c1096ab3bb795651855d53f18daa26afde "nfsd4: extend state
    lock over seqid replay logic" I modified the exit logic of all the
    seqid-based procedures except nfsd4_locku(). Fix the oversight.

    The result of the bug was a double-unlock while handling the LOCKU
    procedure, and a warning like:

    [ 142.150014] WARNING: at kernel/mutex-debug.c:78 debug_mutex_unlock+0xda/0xe0()
    ...
    [ 142.152927] Pid: 742, comm: nfsd Not tainted 3.1.0-rc1-SLIM+ #9
    [ 142.152927] Call Trace:
    [ 142.152927] [] warn_slowpath_common+0x7f/0xc0
    [ 142.152927] [] warn_slowpath_null+0x1a/0x20
    [ 142.152927] [] debug_mutex_unlock+0xda/0xe0
    [ 142.152927] [] __mutex_unlock_slowpath+0x80/0x140
    [ 142.152927] [] mutex_unlock+0xe/0x10
    [ 142.152927] [] nfs4_lock_state+0x35/0x40 [nfsd]
    [ 142.152927] [] nfsd4_proc_compound+0x2a1/0x690
    [nfsd]
    [ 142.152927] [] nfsd_dispatch+0xeb/0x230 [nfsd]
    [ 142.152927] [] svc_process_common+0x345/0x690
    [sunrpc]
    [ 142.152927] [] ? try_to_wake_up+0x280/0x280
    [ 142.152927] [] svc_process+0x102/0x150 [sunrpc]
    [ 142.152927] [] nfsd+0xbd/0x160 [nfsd]
    [ 142.152927] [] ? 0xffffffffa039efff
    [ 142.152927] [] kthread+0x8c/0xa0
    [ 142.152927] [] kernel_thread_helper+0x4/0x10
    [ 142.152927] [] ? kthread_worker_fn+0x190/0x190
    [ 142.152927] [] ? gs_change+0x13/0x13

    Reported-by: Bryan Schumaker
    Tested-by: Bryan Schumaker
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     

27 Sep, 2011

4 commits


21 Sep, 2011

2 commits

  • I'm not sure why I used a new field for this originally.

    Also, the differences between some of these flags are a little subtle;
    add some comments to explain.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Yet another open-management regression:

    - nfs4_file_downgrade() doesn't remove the BOTH access bit on
    downgrade, so the server's idea of the stateid's access gets
    out of sync with the client's. If we want to keep an O_RDWR
    open in this case, we should do that in the file_put_access
    logic rather than here.
    - We forgot to convert v4 access to an open mode here.

    This logic has proven too hard to get right. In the future we may
    consider:
    - reexamining the lock/openowner relationship (locks probably
    don't really need to take their own references here).
    - adding open upgrade/downgrade support to the vfs.
    - removing the atomic operations. They're redundant as long as
    this is all under some other lock.

    Also, maybe some kind of additional static checking would help catch
    O_/NFS4_SHARE_ACCESS confusion.

    Cc: stable@kernel.org
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     

19 Sep, 2011

2 commits

  • Look up closed stateid's in the stateid hash like any other stateid
    rather than searching the close lru.

    This is simpler, and fixes a bug: currently we handle only the case of a
    close that is the last close for a given stateowner, but not the case of
    a close for a stateowner that still has active opens on other files.
    Thus in a case like:

    open(owner, file1)
    open(owner, file2)
    close(owner, file2)
    close(owner, file2)

    the final close won't be recognized as a retransmission.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Including the full clientid in the on-the-wire stateid allows more
    reliable detection of bad vs. expired stateid's, simplifies code, and
    ensures we won't reuse the opaque part of the stateid (as we currently
    do when the same openowner closes and reopens the same file).

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     

17 Sep, 2011

3 commits


16 Sep, 2011

1 commit

  • For checking the size of reply before calling a operation,
    we need try to get maxsize of the operation's reply.

    v3: using new method as Bruce said,

    "we could handle operations in two different ways:

    - For operations that actually change something (write, rename,
    open, close, ...), do it the way we're doing it now: be
    very careful to estimate the size of the response before even
    processing the operation.
    - For operations that don't change anything (read, getattr, ...)
    just go ahead and do the operation. If you realize after the
    fact that the response is too large, then return the error at
    that point.

    So we'd add another flag to op_flags: say, OP_MODIFIES_SOMETHING. And for
    operations with OP_MODIFIES_SOMETHING set, we'd do the first thing. For
    operations without it set, we'd do the second."

    Signed-off-by: Mi Jinlong
    [bfields@redhat.com: crash, don't attempt to handle, undefined op_rsize_bop]
    Signed-off-by: J. Bruce Fields

    Mi Jinlong
     

14 Sep, 2011

4 commits

  • For IPv6 local address, lockd can not callback to client for
    missing scope id when binding address at inet6_bind:

    324 if (addr_type & IPV6_ADDR_LINKLOCAL) {
    325 if (addr_len >= sizeof(struct sockaddr_in6) &&
    326 addr->sin6_scope_id) {
    327 /* Override any existing binding, if another one
    328 * is supplied by user.
    329 */
    330 sk->sk_bound_dev_if = addr->sin6_scope_id;
    331 }
    332
    333 /* Binding to link-local address requires an interface */
    334 if (!sk->sk_bound_dev_if) {
    335 err = -EINVAL;
    336 goto out_unlock;
    337 }

    Replacing svc_addr_u by sockaddr_storage, let rqstp->rq_daddr contains more info
    besides address.

    Reviewed-by: Jeff Layton
    Reviewed-by: Chuck Lever
    Signed-off-by: Mi Jinlong
    Signed-off-by: J. Bruce Fields

    Mi Jinlong
     
  • Signed-off-by: Trond Myklebust
    [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
    [ cel: implement S_ISVTX filter in bfields-normal form ]
    Signed-off-by: Chuck Lever
    Reviewed-by: Jeff Layton
    Signed-off-by: J. Bruce Fields

    Trond Myklebust
     
  • There are no more users...

    Signed-off-by: Trond Myklebust
    Reviewed-by: Jeff Layton
    Signed-off-by: J. Bruce Fields

    Trond Myklebust
     
  • The current code is sort of hackish in that it assumes a referral is always
    matched to an export. When we add support for junctions that may not be the
    case.
    We can replace nfsd4_path() with a function that encodes the components
    directly from the dentries. Since nfsd4_path is currently the only user of
    the 'ex_pathname' field in struct svc_export, this has the added benefit
    of allowing us to get rid of that.

    Signed-off-by: Trond Myklebust
    Reviewed-by: Jeff Layton
    Signed-off-by: J. Bruce Fields

    Trond Myklebust