Signed-off-by: NTigran Mkrtchyan <kofemann@gmail.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NTigran Mkrtchyan <kofemann@gmail.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

This fixes an oops when a buggy client tries to use an initial seqid of
0 on a new slot, which we may misinterpret as a replay.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Combine two booleans into a single flag field, move the smaller fields
to the end.

(In practice this doesn't make the struct any smaller.  But we'll be
adding another flag here soon.)

Remove some debugging code that doesn't look useful, while we're in the
neighborhood.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

From RFC 5661 2.10.6.1: "If the previous sequence ID was 0xFFFFFFFF,
then the next request for the slot MUST have the sequence ID set to
zero."

While we're there, delete some redundant comments.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Otherwise the for loop could try to use a file recently removed from the
file_hashtbl list and oops.
Signed-off-by: NBryan Schumaker <bjschuma@netapp.com>
Tested-by: NCasey Bodley <cbodley@citi.umich.edu>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

unhash_delegation() will grab the recall lock before calling
list_del_init() in each of these places.  This patch removes the
redundant calls.
Signed-off-by: NBryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Stateid's with "other" ("opaque") field all zeros or all ones are
reserved.  We define all_ones separately on the off chance there will be
more such some day, though currently all the other special stateid's
have zero other field.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The below patch fixes some typos in various parts of the kernel, as well as fixes some comments.
Please let me know if I missed anything, and I will try to get it changed and resent.
Signed-off-by: NJustin P. Mattock <justinmattock@gmail.com>
Acked-by: NRandy Dunlap <rdunlap@xenotime.net>
Signed-off-by: NJiri Kosina <jkosina@suse.cz>

The semantic patch that makes this change is available
in scripts/coccinelle/api/memdup.cocci.
Signed-off-by: NThomas Meyer <thomas@m3y3r.de>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Address the possible performance regression mentioned in "nfsd4: hash
lockowners to simplify RELEASE_LOCKOWNER" by providing a separate
(lockowner, inode) hash.

Really, I doubt this matters much, but I think it's likely we'll change
these data structures here and I'd rather that the need for (owner,
inode) lookups be well-documented.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Reported-by: NStephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Now that they're used in the same way, it's a little simpler to put open
and lock owners in the same hash table, and I can't see a reason not to.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Hash lockowners on just the owner string rather than on (owner, inode).
This makes the owner-string lookup needed for RELEASE_LOCKOWNER simpler
(currently it's doing at a linear search through the entire hash
table!).  That may come at the expense of making (owner, inode) lookups
more expensive if a client reuses the same lockowner across multiple
files.  We might add a separate lookup for that.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

init_nfsd() was calling free_slabs() during cleanup code, but the call
to init_slabs() was hidden in nfsd4_state_init().  This could be
confusing to people unfamiliar with the code.
Signed-off-by: NBryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Fault injection on the NFS server makes it easier to test the client's
state manager and recovery threads.  Simulating errors on the server is
easier than finding the right conditions that cause them naturally.

This patch uses debugfs to add a simple framework for fault injection to
the server.  This framework is a config option, and can be enabled
through CONFIG_NFSD_FAULT_INJECTION.  Assuming you have debugfs mounted
to /sys/debug, a set of files will be created in /sys/debug/nfsd/.
Writing to any of these files will cause the corresponding action and
write a log entry to dmesg.
Signed-off-by: NBryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Instead of creating a new lockowner and stateid for every
open_to_lockowner call, reuse the existing lockowner if it exists.
Reported-by: NTrond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

I'd rather the "ignore clientid in sessions case" rule be enforced in
just one place.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Lockowners are looked up by file as well as by owner, but we were
forgetting to do a comparison on the file.  This could cause an
incorrect result from lockt.

(Note looking up the inode from the lockowner is pretty awkward here.
The data structures need fixing.)

Cc: stable@kernel.org
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

According to rfc5661 18.50, implement DESTROY_CLIENTID operation.
Signed-off-by: NMi Jinlong <mijinlong@cn.fujitsu.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Reported-by: NGopala Suryanarayana <gsuryanarayana@vmware.com>
Signed-off-by: NBenny Halevy <bhalevy@tonian.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NBenny Halevy <bhalevy@panasas.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

0c12eaff "nfsd: don't break lease on
CLAIM_DELEGATE_CUR" was a temporary workaround for a problem fixed
properly in the vfs layer by 778fc546
"locks: fix tracking of inprogress lease breaks", so we can revert that
change (but keeping some minor cleanup from that commit).
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

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: NJ. Bruce Fields <bfields@redhat.com>

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: NJ. Bruce Fields <bfields@redhat.com>

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: NJ. Bruce Fields <bfields@redhat.com>

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: NBryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

No change in behavior.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

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: NJ. Bruce Fields <bfields@redhat.com>

Mask out the WANT bits right at the start instead of on each use.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Again, these checks are better in the xdr code.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

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: NJ. Bruce Fields <bfields@redhat.com>

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

Cc: stable@kernel.org
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

In response to some review comments, get rid of the somewhat obscure
for-loop with bitops, and improve a comment.
Reported-by: NSteve Dickson <steved@redhat.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

In commit 5ec094c1 "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]  [<ffffffff8105fa4f>] warn_slowpath_common+0x7f/0xc0
[  142.152927]  [<ffffffff8105faaa>] warn_slowpath_null+0x1a/0x20
[  142.152927]  [<ffffffff810960ca>] debug_mutex_unlock+0xda/0xe0
[  142.152927]  [<ffffffff813e4200>] __mutex_unlock_slowpath+0x80/0x140
[  142.152927]  [<ffffffff813e42ce>] mutex_unlock+0xe/0x10
[  142.152927]  [<ffffffffa03bd3f5>] nfs4_lock_state+0x35/0x40 [nfsd]
[  142.152927]  [<ffffffffa03b0b71>] nfsd4_proc_compound+0x2a1/0x690
[nfsd]
[  142.152927]  [<ffffffffa039f9fb>] nfsd_dispatch+0xeb/0x230 [nfsd]
[  142.152927]  [<ffffffffa02b1055>] svc_process_common+0x345/0x690
[sunrpc]
[  142.152927]  [<ffffffff81058d10>] ? try_to_wake_up+0x280/0x280
[  142.152927]  [<ffffffffa02b16e2>] svc_process+0x102/0x150 [sunrpc]
[  142.152927]  [<ffffffffa039f0bd>] nfsd+0xbd/0x160 [nfsd]
[  142.152927]  [<ffffffffa039f000>] ? 0xffffffffa039efff
[  142.152927]  [<ffffffff8108230c>] kthread+0x8c/0xa0
[  142.152927]  [<ffffffff813e8694>] kernel_thread_helper+0x4/0x10
[  142.152927]  [<ffffffff81082280>] ? kthread_worker_fn+0x190/0x190
[  142.152927]  [<ffffffff813e8690>] ? gs_change+0x13/0x13
Reported-by: NBryan Schumaker <bjschuma@netapp.com>
Tested-by: NBryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Use a separate stateid idr per client, and lookup a stateid by first
finding the client, then looking up the stateid relative to that client.

Also some minor refactoring.

This allows us to improve error returns: we can return expired when the
clientid is not found and bad_stateid when the clientid is found but not
the stateid, as opposed to returning expired for both cases.

I hope this will also help to replace the state lock mostly by a
per-client lock, but that hasn't been done yet.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

Test_stateid is 4.1-only and only allowed after a sequence operation, so
this check is unnecessary.

Cc: Bryan Schumaker <bjschuma@netapp.com>
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

The idr system is designed exactly for generating id and looking up
integer id's.  Thanks to Trond for pointing it out.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>

This will be convenient.
Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>