- 29 Aug, 2023 40 commits
-
-
NeilBrown authored
When a sequence of numbers are needed for internal-use only, an enum is typically best. The sequence will inevitably need to be changed one day, and having an enum means the developer doesn't need to think about renumbering after insertion or deletion. Such patches will be easier to review. Signed-off-by:
NeilBrown <neilb@suse.de> Signed-off-by:
Chuck Lever <chuck.lever@oracle.com>
-
NeilBrown authored
When a sequence of numbers are needed for internal-use only, an enum is typically best. The sequence will inevitably need to be changed one day, and having an enum means the developer doesn't need to think about renumbering after insertion or deletion. Such patches will be easier to review. Signed-off-by:
-
NeilBrown authored
When a sequence of numbers are needed for internal-use only, an enum is typically best. The sequence will inevitably need to be changed one day, and having an enum means the developer doesn't need to think about renumbering after insertion or deletion. Such patches will be easier to review. Signed-off-by:
-
NeilBrown authored
Most svc threads have no interest in a timeout. nfsd sets it to 1 hour, but this is a wart of no significance. lockd uses the timeout so that it can call nlmsvc_retry_blocked(). It also sometimes calls svc_wake_up() to ensure this is called. So change lockd to be consistent and always use svc_wake_up() to trigger nlmsvc_retry_blocked() - using a timer instead of a timeout to svc_recv(). And change svc_recv() to not take a timeout arg. This makes the sp_threads_timedout counter always zero. Signed-off-by:
-
NeilBrown authored
svc_recv() currently returns a 0 on success or one of two errors: - -EAGAIN means no message was successfully received - -EINTR means the thread has been told to stop Previously nfsd would stop as the result of a signal as well as following kthread_stop(). In that case the difference was useful: EINTR means stop unconditionally. EAGAIN means stop if kthread_should_stop(), continue otherwise. Now threads only exit when kthread_should_stop() so we don't need the distinction. Signed-off-by:
-
NeilBrown authored
All callers of svc_recv() go on to call svc_process() on success. Simplify callers by having svc_recv() do that for them. This loses one call to validate_process_creds() in nfsd. That was debugging code added 14 years ago. I don't think we need to keep it. Signed-off-by:
Jeff Layton <jlayton@kernel.org> Signed-off-by:
-
NeilBrown authored
Now that the last nfsd thread is stopped by an explicit act of calling svc_set_num_threads() with a count of zero, we only have a limited number of places that can happen, and don't need to call nfsd_last_thread() in nfsd_put() So separate that out and call it at the two places where the number of threads is set to zero. Move the clearing of ->nfsd_serv and the call to svc_xprt_destroy_all() into nfsd_last_thread(), as they are really part of the same action. nfsd_put() is now a thin wrapper around svc_put(), so make it a static inline. nfsd_put() cannot be called after nfsd_last_thread(), so in a couple of places we have to use svc_put() instead. Signed-off-by:
-
NeilBrown authored
Previously a thread could exit asynchronously (due to a signal) so some care was needed to hold nfsd_mutex over the last svc_put() call. Now a thread can only exit when svc_set_num_threads() is called, and this is always called under nfsd_mutex. So no care is needed. Not only is the mutex held when a thread exits now, but the svc refcount is elevated, so the svc_put() in svc_exit_thread() will never be a final put, so the mutex isn't even needed at this point in the code. Signed-off-by:
-
NeilBrown authored
The original implementation of nfsd used signals to stop threads during shutdown. In Linux 2.3.46pre5 nfsd gained the ability to shutdown threads internally it if was asked to run "0" threads. After this user-space transitioned to using "rpc.nfsd 0" to stop nfsd and sending signals to threads was no longer an important part of the API. In commit 3ebdbe52 ("SUNRPC: discard svo_setup and rename svc_set_num_threads_sync()") (v5.17-rc1~75^2~41) we finally removed the use of signals for stopping threads, using kthread_stop() instead. This patch makes the "obvious" next step and removes the ability to signal nfsd threads - or any svc threads. nfsd stops allowing signals and we don't check for their delivery any more. This will allow for some simplification in later patches. A change worth noting is in nfsd4_ssc_setup_dul(). There was previously a signal_pending() check which would only succeed when the thread was being shut down. It should really have tested kthread_should_stop() as well. Now it just does the latter, not the former. Signed-off-by:
-
NeilBrown authored
lockd allows SIGKILL and responds by dropping all locks and restarting the grace period. This functionality has been present since 2.1.32 when lockd was added to Linux. This functionality is undocumented and most likely added as a useful debug aid. When there is a need to drop locks, the better approach is to use /proc/fs/nfsd/unlock_*. This patch removes SIGKILL handling as part of preparation for removing all signal handling from sunrpc service threads. Signed-off-by:
-
Su Hui authored
clang's static analysis warning: fs/lockd/mon.c: line 293, column 2: Null pointer passed as 2nd argument to memory copy function. Assuming 'hostname' is NULL and calling 'nsm_create_handle()', this will pass NULL as 2nd argument to memory copy function 'memcpy()'. So return NULL if 'hostname' is invalid. Fixes: 77a3ef33 ("NSM: More clean up of nsm_get_handle()") Signed-off-by:
Su Hui <suhui@nfschina.com> Reviewed-by:
Nick Desaulniers <ndesaulniers@google.com> Reviewed-by:
-
Zhu Wang authored
Remove kernel-doc warning in exportfs: fs/exportfs/expfs.c:395: warning: Function parameter or member 'parent' not described in 'exportfs_encode_inode_fh' Signed-off-by:
Zhu Wang <wangzhu9@huawei.com> Signed-off-by:
-
Chuck Lever authored
With large NFS WRITE requests on TCP, I measured 5-10 thread wake- ups to receive each request. This is because the socket layer calls ->sk_data_ready() frequently, and each call triggers a thread wake-up. Each recvmsg() seems to pull in less than 100KB. Have the socket layer hold ->sk_data_ready() calls until the full incoming message has arrived to reduce the wake-up rate. Signed-off-by: -
Chuck Lever authored
Flamegraph analysis showed that the cork/uncork calls consume nearly a third of the CPU time spent in svc_tcp_sendto(). The other two consumers are mutex lock/unlock and svc_tcp_sendmsg(). Now that svc_tcp_sendto() coalesces RPC messages properly, there is no need to introduce artificial delays to prevent sending partial messages. After applying this change, I measured a 1.2K read IOPS increase for 8KB random I/O (several percent) on 56Gb IP over IB. Reviewed-by:
David Howells <dhowells@redhat.com> Signed-off-by:
-
Chuck Lever authored
Commit da1661b9 ("SUNRPC: Teach server to use xprt_sock_sendmsg for socket sends") modified svc_udp_sendto() to use xprt_sock_sendmsg() because we originally believed xprt_sock_sendmsg() would be needed for TLS support. That does not actually appear to be the case. In addition, the linkage between the client and server send code has been a bit of a maintenance headache because of the distinct ways that the client and server handle memory allocation. Going forward, eventually the XDR layer will deal with its buffers in the form of bio_vec arrays, so convert this function accordingly. Signed-off-by:
-
Chuck Lever authored
There is now enough infrastructure in place to combine the stream record marker into the biovec array used to send each outgoing RPC message on TCP. The whole message can be more efficiently sent with a single call to sock_sendmsg() using a bio_vec iterator. Note that this also helps with RPC-with-TLS: the TLS implementation can now clearly see where the upper layer message boundaries are. Before, it would send each component of the xdr_buf (record marker, head, page payload, tail) in separate TLS records. Suggested-by:
-
Chuck Lever authored
Add a helper to convert a whole xdr_buf directly into an array of bio_vecs, then send this array instead of iterating piecemeal over the xdr_buf containing the outbound RPC message. Reviewed-by:
-
Jeff Layton authored
A well-formed NFSv4 ACL will always contain OWNER@/GROUP@/EVERYONE@ ACEs, but there is no requirement for inheritable entries for those entities. POSIX ACLs must always have owner/group/other entries, even for a default ACL. nfsd builds the default ACL from inheritable ACEs, but the current code just leaves any unspecified ACEs zeroed out. The result is that adding a default user or group ACE to an inode can leave it with unwanted deny entries. For instance, a newly created directory with no acl will look something like this: # NFSv4 translation by server A::OWNER@:rwaDxtTcCy A::GROUP@:rxtcy A::EVERYONE@:rxtcy # POSIX ACL of underlying file user::rwx group::r-x other::r-x ...if I then add new v4 ACE: nfs4_setfacl -a A:fd:1000:rwx /mnt/local/test ...I end up with a result like this today: user::rwx user:1000:rwx group::r-x mask::rwx other::r-x default:user::--- default:user:1000:rwx default:group::--- default:mask::rwx default:other::--- A::OWNER@:rwaDxtTcCy A::1000:rwaDxtcy A::GROUP@:rxtcy A::EVERYONE@:rxtcy D:fdi:OWNER@:rwaDx A:fdi:OWNER@:tTcCy A:fdi:1000:rwaDxtcy A:fdi:GROUP@:tcy A:fdi:EVERYONE@:tcy ...which is not at all expected. Adding a single inheritable allow ACE should not result in everyone else losing access. The setfacl command solves a silimar issue by copying owner/group/other entries from the effective ACL when none of them are set: "If a Default ACL entry is created, and the Default ACL contains no owner, owning group, or others entry, a copy of the ACL owner, owning group, or others entry is added to the Default ACL. Having nfsd do the same provides a more sane result (with no deny ACEs in the resulting set): user::rwx user:1000:rwx group::r-x mask::rwx other::r-x default:user::rwx default:user:1000:rwx default:group::r-x default:mask::rwx default:other::r-x A::OWNER@:rwaDxtTcCy A::1000:rwaDxtcy A::GROUP@:rxtcy A::EVERYONE@:rxtcy A:fdi:OWNER@:rwaDxtTcCy A:fdi:1000:rwaDxtcy A:fdi:GROUP@:rxtcy A:fdi:EVERYONE@:rxtcy Reported-by:Ondrej Valousek <ondrej.valousek@diasemi.com> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2136452Suggested-by:
Andreas Gruenbacher <agruenba@redhat.com> Signed-off-by:
-
YueHaibing authored
Since commit 49b28684 ("nfsd: Remove deprecated nfsctl system call and related code.") these declarations are unused, so can remove it. Signed-off-by:
YueHaibing <yuehaibing@huawei.com> Signed-off-by:
-
Alexander Aring authored
This patch fixes races when lockd accesses the global nlm_blocked list. It was mostly safe to access the list because everything was accessed from the lockd kernel thread context but there exist cases like nlmsvc_grant_deferred() that could manipulate the nlm_blocked list and it can be called from any context. Signed-off-by:
Alexander Aring <aahringo@redhat.com> Reviewed-by:
-
Jeff Layton authored
In the event that we can't fetch post_op_attr attributes, we still need to set a value for the after_change. The operation has already happened, so we're not able to return an error at that point, but we do want to ensure that the client knows that its cache should be invalidated. If we weren't able to fetch post-op attrs, then just set the after_change to before_change + 1. The atomic flag should already be clear in this case. Suggested-by:
-
Jeff Layton authored
At one time, nfsd would scrape inode information directly out of struct inode in order to populate the change_info4. At that time, the BUG_ON in set_change_info made some sense, since having it unset meant a coding error. More recently, it calls vfs_getattr to get this information, which can fail. If that fails, fh_pre_saved can end up not being set. While this situation is unfortunate, we don't need to crash the box. Move set_change_info to nfs4proc.c since all of the callers are there. Revise the condition for setting "atomic" to also check for fh_pre_saved. Drop the BUG_ON and just have it zero out both change_attr4s when this occurs. Reported-by:
Boyang Xue <bxue@redhat.com> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2223560Signed-off-by:
-
Jeff Layton authored
Collecting pre_op_attrs can fail, in which case it's probably best to fail the whole operation. Change fh_fill_pre_attrs and fh_fill_both_attrs to return __be32, and have the callers check the return code and abort the operation if it's not nfs_ok. Signed-off-by:
-
Jeff Layton authored
I got this today from modpost: WARNING: modpost: missing MODULE_DESCRIPTION() in fs/nfsd/nfsd.o Add a module description. Signed-off-by: -
Chuck Lever authored
The svc_ prefix is identified with the SunRPC layer. Although the duplicate reply cache caches RPC replies, it is only for the NFS protocol. Rename the struct to better reflect its purpose. Reviewed-by:
-
Chuck Lever authored
Over time I'd like to see NFS-specific fields moved out of struct svc_rqst, which is an RPC layer object. These fields are layering violations. Reviewed-by:
-
Chuck Lever authored
Avoid holding the bucket lock while freeing cache entries. This change also caps the number of entries that are freed when the shrinker calls to reduce the shrinker's impact on the cache's effectiveness. Reviewed-by:
-
Chuck Lever authored
Enable nfsd_prune_bucket() to drop the bucket lock while calling kfree(). Use the same pattern that Jeff recently introduced in the NFSD filecache. A few percpu operations are moved outside the lock since they temporarily disable local IRQs which is expensive and does not need to be done while the lock is held. Reviewed-by:
-
Chuck Lever authored
For readability, rename to match the other helpers. Reviewed-by:
-
Chuck Lever authored
To reduce contention on the bucket locks, we must avoid calling kfree() while each bucket lock is held. Start by refactoring nfsd_reply_cache_free_locked() into a helper that removes an entry from the bucket (and must therefore run under the lock) and a second helper that frees the entry (which does not need to hold the lock). For readability, rename the helpers nfsd_cacherep_<verb>. Reviewed-by:
-
Chuck Lever authored
These functions are no longer used. Reviewed-by:
-
Chuck Lever authored
All supported encryption types now use the same context import function. Reviewed-by:
-
Chuck Lever authored
This code is now always on, so the ifdef can be removed. Reviewed-by:
-
Chuck Lever authored
We no longer support importing v1 contexts. Reviewed-by:
-
Chuck Lever authored
This function is no longer used. Reviewed-by:
-
Chuck Lever authored
None of this code can be enabled any more. Reviewed-by:
-
Chuck Lever authored
These enctypes can no longer be enabled via CONFIG. Reviewed-by:
-
Chuck Lever authored
The DES3 encryption type is no longer implemented. Reviewed-by:
-
Chuck Lever authored
Make it impossible to enable support for the DES or DES3 Kerberos encryption types in SunRPC. These enctypes were deprecated by RFCs 6649 and 8429 because they are known to be insecure. Reviewed-by:
-
Dai Ngo authored
This patch grants write delegations for OPEN with NFS4_SHARE_ACCESS_WRITE if there is no conflict with other OPENs. Write delegation conflicts with another OPEN, REMOVE, RENAME and SETATTR are handled the same as read delegation using notify_change, try_break_deleg. The NFSv4.0 protocol does not enable a server to determine that a conflicting GETATTR originated from the client holding the delegation versus coming from some other client. With NFSv4.1 and later, the SEQUENCE operation that begins each COMPOUND contains a client ID, so delegation recall can be safely squelched in this case. With NFSv4.0, however, the server must recall or send a CB_GETATTR (per RFC 7530 Section 16.7.5) even when the GETATTR originates from the client holding that delegation. An NFSv4.0 client can trigger a pathological situation if it always sends a DELEGRETURN preceded by a conflicting GETATTR in the same COMPOUND. COMPOUND execution will always stop at the GETATTR and the DELEGRETURN will never get executed. The server eventually revokes the delegation, which can result in loss of open or lock state. Tracepoint added to track whether read or write delegation is granted. Signed-off-by:
Dai Ngo <dai.ngo@oracle.com> Signed-off-by:
-
