In nfs4_set_lease_period(), the passed 32-bit lease period in seconds is
multiplied by HZ -- that might overflow before being implicitly cast to
*unsigned long* (32/64-bit type), while initializing the lease variable.
Cap the lease period at MAX_LEASE_PERIOD (#define'd to 1 hour for now),
before multipying to avoid such overflow...
Found by Linux Verification Center (linuxtesting.org) with the Svace static
analysis tool.
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Suggested-by: Trond Myklebust <trondmy@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
There's no need to multiply the lease period by HZ at all the call sites of
nfs4_set_lease_period() -- it makes more sense to do that only once, inside
that function, by passing to it lease period as 32-bit # of seconds instead
of 32/64-bit *unsigned long* # of jiffies...
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
commit 243fea1346 ("NFSv4.2: fix listxattr to return selinux
security label") introduced a direct call to
security_inode_listsecurity() in nfs4_listxattr(). However,
nfs4_listxattr() already indirectly called
security_inode_listsecurity() via nfs4_listxattr_nfs4_label() if
CONFIG_NFS_V4_SECURITY_LABEL is enabled and the server has the
NFS_CAP_SECURITY_LABEL capability enabled. This duplication was fixed
by commit 9acb237def ("NFSv4.2: another fix for listxattr") by
making the second call conditional on NFS_CAP_SECURITY_LABEL not being
set by the server. However, the combination of the two changes
effectively makes one call to security_inode_listsecurity() in every
case - which is the desired behavior since getxattr() always returns a
security xattr even if it has to synthesize one. Further, the two
different calls produce different xattr name ordering between
security.* and user.* xattr names. Unify the two separate calls into a
single call and get rid of nfs4_listxattr_nfs4_label() altogether.
Link: https://lore.kernel.org/selinux/CAEjxPJ6e8z__=MP5NfdUxkOMQ=EnUFSjWFofP4YPwHqK=Ki5nw@mail.gmail.com/
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Commit 580f236737 ("NFS: Adjust the amount of readahead
performed by NFS readdir") reduces the amount of readahead names
caching done by the client.
The downside of this approach is READDIR now may suffer from
a slow-start issue, where initially it will fetch names that fit
in a single page, then in 2, 4, 8 until the maximum supported
transfer size (usually 1M).
This patch tries to take a balanced approach between mitigating
the slow-start issue still maintaining some efficiency gains.
Fixes: 580f236737 ("NFS: Adjust the amount of readahead performed by NFS readdir")
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
It is possible to have a task get stuck on waiting on the
NFS_LAYOUT_DRAIN in the following scenario
1. cpu a: waiter test NFS_LAYOUT_DRAIN (1) and plh_outstanding (1)
2. cpu b: atomic_dec_and_test() -> clear bit -> wake up
3. cpu c: sets NFS_LAYOUT_DRAIN again
4. cpu a: calls wait_on_bit() sleeps forever.
To expand on this we have say 2 outstanding pnfs write IO that get
ESTALE which causes both to call pnfs_destroy_layout() and set the
NFS_LAYOUT_DRAIN bit but the 1st one doesn't call the
pnfs_put_layout_hdr() yet (as that would prevent the 2nd ESTALE write
from trying to call pnfs_destroy_layout()). If the 1st ESTALE write
is the one that initially sets the NFS_LAYOUT_DRAIN so that new IO
on this file initiates new LAYOUTGET. Another new write would find
NFS_LAYOUT_DRAIN set and phl_outstanding>0 (step 1) and would
wait_on_bit(). LAYOUTGET completes doing step 2. Now, the 2nd of
ESTALE writes is calling pnfs_destory_layout() and set the
NFS_LAYOUT_DRAIN bit (step 3). Finally, the waiting write wakes up
to check the bit and goes back to sleep.
The problem revolves around the fact that if NFS_LAYOUT_INVALID_STID
was already set, it should not do the work of
pnfs_mark_layout_stateid_invalid(), thus NFS_LAYOUT_DRAIN will not
be set more than once for an invalid layout.
Suggested-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Fixes: 880265c77a ("pNFS: Avoid a live lock condition in pnfs_update_layout()")
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Rework this code that was totally busted at least as of my most
recent changes. Introduce a separate list for delayed delegations
so that they can't get lost and don't clutter up the returns list.
Add a missing spin_unlock in the helper marking it as a regular
pending return.
Fixes: 0ebe655bd0 ("NFS: add a separate delegation return list")
Reported-by: Chris Mason <clm@meta.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Drop the pointless delegation->lock held over setting multiple
atomic bits in different structures, and use separate labels
for the delay vs abort cases.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
This will allow to simplify the error handling flow.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Replace the integer used as boolean with a bool type, and tidy up
the prototype and top of function comment.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
The caller doesn't check the return value, so drop it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
None of the callers checks the return value, so drop it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Compiling the NFSv4 module without any minorversion support doesn't make
much sense, so this patch sets NFS v4.1 as the default, always enabled
NFS version allowing us to replace all the CONFIG_NFS_V4_1s scattered
throughout the code with CONFIG_NFS_V4.
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
I introduce NFS4_MIN_MINOR_VERSION as a parallel to
NFS4_MAX_MINOR_VERSION to check if NFS v4.0 has been compiled in and
return an appropriate error if not.
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
At the same time, I move the NFS v4.0 functions into nfs40proc.c to keep
v4.0 features together in their own files.
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
No functional change in this patch. This just makes the next patch where
I introduce "sequence slot operations" simpler.
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
They don't need to be visible outside of nfs40proc.c anymore now that
the minor version ops have been moved over.
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
This is the first step in extracting NFS v4.0 into its own set of files
that can be disabled through Kconfig.
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Both nfs_local_do_read and nfs_local_do_write only return 0 at the
end, so switch them to returning void.
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Handling -EAGAIN in nfs_local_doio() was introduced with commit
0978e5b85f (nfs_do_local_{read,write} were made to have negative
checks for correspoding iter method) but commit e43e9a3a3d
since eliminated the possibility for this -EAGAIN early return.
So remove nfs_local_doio()'s -EAGAIN handling that calls
nfs_localio_disable_client() -- while it should never happen from
nfs_do_local_{read,write} this particular -EAGAIN handling is now
"dead" and so it has become a liability.
Fixes: e43e9a3a3d ("nfs/localio: refactor iocb initialization")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
nfslocaliod_workqueue is a non-memreclaim workqueue (it isn't
initialized with WQ_MEM_RECLAIM), see commit b9f5dd57f4
("nfs/localio: use dedicated workqueues for filesystem read and
write").
Use nfslocaliod_workqueue for LOCALIO's SYNC work.
Also, set PF_LOCAL_THROTTLE | PF_MEMALLOC_NOIO in
nfs_local_fsync_work.
Fixes: b9f5dd57f4 ("nfs/localio: use dedicated workqueues for filesystem read and write")
Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
LOCALIO is an NFS loopback mount optimization that avoids using the
network for READ, WRITE and COMMIT if the NFS client and server are
determined to be on the same system. But because LOCALIO is still
fundamentally "just NFS loopback mount" it is susceptible to recursion
deadlock via direct reclaim, e.g.: NFS LOCALIO down to XFS and then
back into NFS via nfs_writepages.
Fix LOCALIO's potential for direct reclaim deadlock by ensuring that
all its page cache allocations are done from GFP_NOFS context.
Thanks to Ben Coddington for pointing out commit ad22c7a043 ("xfs:
prevent stack overflows from page cache allocation").
Reported-by: John Cagle <john.cagle@hammerspace.com>
Tested-by: Allen Lu <allen.lu@hammerspace.com>
Suggested-by: Benjamin Coddington <bcodding@hammerspace.com>
Fixes: 70ba381e1a ("nfs: add LOCALIO support")
Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
The current code for handling short writes in localio just truncates the
I/O and then sets an error. While that is close to how the ordinary NFS
code behaves, it does mean there is a chance the data that got written
is lost because it isn't persisted.
To fix this, change localio so that the upper layers can direct the
behaviour to persist any unstable data by rewriting it, and then
continuing writing until an ENOSPC is hit.
Fixes: 70ba381e1a ("nfs: add LOCALIO support")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Reviewed-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Currently nfs_mark_return_unreferenced_delegations marks all open but
not referenced delegations (i.e., those were found by a previous pass)
as return on close, which means that we'll return them on close without
a way out.
Replace this with only iterating delegations that are on the LRU list,
and avoid delegations that are in use by an open files to avoid this.
Delegations that were never referenced while open still are be prime
candidates for return from the LRU if the number of delegations is over
the watermark, or otherwise will be returned by the next
nfs_mark_return_unreferenced_delegations pass after they are closed.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Directly returning delegations on close when over the watermark is
rather suboptimal as these delegations are much more likely to be reused
than those that have been unused for a long time. Switch to returning
unused delegations from a new LRU list when we are above the threshold and
there are reclaimable delegations instead.
Pass over referenced delegations during the first pass to give delegations
that aren't in active used by frequently used for stat() or similar another
chance to not be instantly reclaimed. This scheme works the same as the
referenced flags in the VFS inode and dentry caches.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Searching for returnable delegations in the per-server delegations list
can be very expensive. While commit e04bbf6b1b ("NFS: Avoid quadratic
search when freeing delegations.") reduced the overhead a bit, the
fact that all the non-returnable delegations have to be searched limits
the amount of optimizations that can be done.
Fix this by introducing a separate list that only contains delegations
scheduled for return.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Remove a level of indentation for the main code path.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Nested RCU critical sections are fine and very cheap. Have a local one
in nfs_start_delegation_return so that the function is self-contained
and to prepare for simplifying the callers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Using the unconditional reference increment means we can take a
reference to a delegation already in the RCU grace period, which could
cause a use after free under very unlikely conditions. Switch to use
refcount_inc_not_zero instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
All callers now hold references to the delegation as part of the lookup,
removing the need for an extra reference for those that are actually
returned which is then dropped in nfs_end_delegation_return.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Currently most work on struct nfs_delegation happens directly under RCU
protection. This is generally fine, despite that long RCU sections are
not good for performance. But for operations later taking a reference
to the delegation to perform blocking work, refcount_inc is used, which
can be racy against dropping the last reference and thus lead to use
after frees in extremely rare cases.
Fix this by taking a reference in nfs4_get_valid_delegation using
refcount_inc_not_zero so that the callers have a stabilized reference
they can work with and can be moved outside the RCU critical section.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
When nfs_detach_delegation_locked detaches a delegation from an inode,
it clears both nfsi->delegation and delegation->inode. Use the later
in update_open_stateid to check for a detached inode, as that avoids
an extra local variable, and removes the need for a RCU derefernence
as we already hold the lock in the delegation. This prepares for
removing the surrounding RCU critical section.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
nfs_inode_set_delegation as the only direct caller of
nfs_detach_delegation_locked already check this under cl_lock, so
don't repeat it.
Replace the lockdep coverage for the lock that was implicitly provided by
the rcu_dereference_protected call that is removed with an explicit
lockdep assert to keep the coverage.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
nfs_detach_delegation always returns either the passed in delegation
or NULL, simplify this to a bool return.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Keep the delegation handling in a single place, and just return the
stateid in an optional argument.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Renewal only cares for active delegations and not revoked ones. Replace
the list empty check with reading the active delegation counter to
implement this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Open code nfs_free_delegation in the callers, because having a "free"
function that wraps a revoke and put operation is a bit confusing,
especially when the __free version does the actual freeing triggered by
the last put.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
There is only a single caller, and the function can be condensed into a
single if statement, making it more clear what is being tested there.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
This essentially reverts commit 6f9449be53 ("NFS: Fix a soft lockup in
the delegation recovery code") because the code walking the per-server
delegation list has been fixed to just skip inodes for which
nfs_delegation_grab_inode fails, instead of having to restart the entire
series in commit f92214e4c3 ("NFS: Avoid unnecessary rescanning of the
per-server delegation list").
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
Now that nfs_start_delegation_return_locked is gone, and we have
RCU locking asserts, drop the extra postfix.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
And clean up the dereference of the delegation a bit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>
