The branch, master has been updated
via 6a1be2ff2af gpfs: Fetch GPFS DOS attributes asynchronously
via b75ce05c5c4 smbd: Add vfswrap_check_async_with_thread_creds() helper
via 66874f7726a smbd: Introduce struct vfs_pthread_pool_job_state
from 01fbe3eeb23 lib: Add a safeguard for misconfigured directory
permissions
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 6a1be2ff2afd76e4b2d65111f0ec5c807288fe24
Author: Pawan Sahu <[email protected]>
Date: Fri Jun 20 05:42:01 2025 +0200
gpfs: Fetch GPFS DOS attributes asynchronously
This change introduces asynchronous handling of GPFS DOS attributes
using the pthreadpool infrastructure. This is part of the overall
effort to improve SMB directory listing performance in environments
with GPFS as the backend filesystem.
Signed-off-by: Pawan Sahu <[email protected]>
Reviewed-by: Vinit Agnihotri <[email protected]>
Reviewed-by: Christof Schmitt <[email protected]>
Reviewed-by: Guenther Deschner <[email protected]>
Autobuild-User(master): Günther Deschner <[email protected]>
Autobuild-Date(master): Tue Sep 30 13:08:16 UTC 2025 on atb-devel-224
commit b75ce05c5c468ce96fc5477533f4a3ddb4fdbf94
Author: Pawan Sahu <[email protected]>
Date: Wed Jun 4 08:13:08 2025 +0200
smbd: Add vfswrap_check_async_with_thread_creds() helper
Move the logic that checks for sufficient threads and per-thread credential
support into a dedicated helper function:
vfswrap_check_async_with_thread_creds().
Signed-off-by: Pawan Sahu <[email protected]>
Reviewed-by: Vinit Agnihotri <[email protected]>
Reviewed-by: Christof Schmitt <[email protected]>
Reviewed-by: Guenther Deschner <[email protected]>
commit 66874f7726ae712b951c49fad06a53635a232436
Author: Pawan Sahu <[email protected]>
Date: Thu May 8 23:19:03 2025 -0700
smbd: Introduce struct vfs_pthread_pool_job_state
Refactor the vfswrap_getattrat_state structure by extracting the members
specific to pthreadpool job handling into a new struct
vfs_pthread_pool_job_state.
This improves code clarity and allows reuse of the job-related
state in other contexts.
Signed-off-by: Pawan Sahu <[email protected]>
Reviewed-by: Vinit Agnihotri <[email protected]>
Reviewed-by: Christof Schmitt <[email protected]>
Reviewed-by: Guenther Deschner <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
source3/include/vfs.h | 24 ++++
source3/modules/vfs_default.c | 130 ++++++++---------
source3/modules/vfs_gpfs.c | 328 ++++++++++++++++++++++++++++++++++++++++--
3 files changed, 410 insertions(+), 72 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index ac81b259f21..0acd7c86cad 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -32,6 +32,8 @@
#undef vfs_ops
#endif
+#include "smbprofile.h"
+
/*
* As we're now (thanks Andrew ! :-) using file_structs and connection
* structs in the vfs - then anyone writing a vfs must include includes.h...
@@ -412,6 +414,7 @@ struct ea_list;
struct smb_file_time;
struct smb_filename;
struct dfs_GetDFSReferral;
+struct pthreadpool_tevent;
typedef union unid_t {
uid_t uid;
@@ -923,6 +926,25 @@ struct vfs_rename_how {
#define VFS_PWRITE_APPEND_OFFSET -1
+struct vfs_pthreadpool_job_state {
+ struct tevent_context *ev;
+ struct vfs_handle_struct *handle;
+ files_struct *dir_fsp;
+ const struct smb_filename *smb_fname;
+
+ /*
+ * The following variables are talloced off "state" which is protected
+ * by a destructor and thus are guaranteed to be safe to be used in the
+ * job function in the worker thread.
+ */
+ char *name;
+ const struct security_unix_token *token;
+
+ struct vfs_aio_state vfs_aio_state;
+ SMBPROFILE_BYTES_ASYNC_STATE(profile_bytes);
+ SMBPROFILE_BYTES_ASYNC_STATE(profile_bytes_x);
+};
+
/*
Available VFS operations. These values must be in sync with vfs_ops struct
(struct vfs_fn_pointers and struct vfs_handle_pointers inside of struct
vfs_ops).
@@ -1854,6 +1876,8 @@ void *vfs_fetch_fsp_extension(vfs_handle_struct *handle,
const struct files_stru
void smb_vfs_assert_all_fns(const struct vfs_fn_pointers* fns,
const char *module);
+bool vfswrap_check_async_with_thread_creds(struct pthreadpool_tevent *pool);
+
/*
* Helper functions from source3/modules/vfs_not_implemented.c
*/
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 02057c7394c..918cea2855a 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -3575,25 +3575,11 @@ static ssize_t vfswrap_fgetxattr(struct
vfs_handle_struct *handle,
}
struct vfswrap_getxattrat_state {
- struct tevent_context *ev;
- struct vfs_handle_struct *handle;
- files_struct *dir_fsp;
- const struct smb_filename *smb_fname;
+ struct vfs_pthreadpool_job_state job_state;
- /*
- * The following variables are talloced off "state" which is protected
- * by a destructor and thus are guaranteed to be safe to be used in the
- * job function in the worker thread.
- */
- char *name;
const char *xattr_name;
uint8_t *xattr_value;
- struct security_unix_token *token;
-
ssize_t xattr_size;
- struct vfs_aio_state vfs_aio_state;
- SMBPROFILE_BYTES_ASYNC_STATE(profile_bytes);
- SMBPROFILE_BYTES_ASYNC_STATE(profile_bytes_x);
};
static int vfswrap_getxattrat_state_destructor(
@@ -3602,6 +3588,34 @@ static int vfswrap_getxattrat_state_destructor(
return -1;
}
+/* Check if there are enough threads available
+ * to run asynchronous jobs and if the current
+ * platform provides per-thread credentials.
+ */
+bool vfswrap_check_async_with_thread_creds(struct pthreadpool_tevent *pool)
+{
+ size_t max_threads = 0;
+ bool have_per_thread_cwd = false;
+ bool have_per_thread_creds = false;
+ bool do_async = false;
+
+ max_threads = pthreadpool_tevent_max_threads(pool);
+ if (max_threads >= 1) {
+ /*
+ * We need a non sync threadpool!
+ */
+ have_per_thread_cwd = per_thread_cwd_supported();
+ }
+#ifdef HAVE_LINUX_THREAD_CREDENTIALS
+ have_per_thread_creds = true;
+#endif
+ if (have_per_thread_cwd && have_per_thread_creds) {
+ do_async = true;
+ }
+
+ return do_async;
+}
+
static void vfswrap_getxattrat_do_sync(struct tevent_req *req);
static void vfswrap_getxattrat_do_async(void *private_data);
static void vfswrap_getxattrat_done(struct tevent_req *subreq);
@@ -3618,9 +3632,6 @@ static struct tevent_req *vfswrap_getxattrat_send(
struct tevent_req *req = NULL;
struct tevent_req *subreq = NULL;
struct vfswrap_getxattrat_state *state = NULL;
- size_t max_threads = 0;
- bool have_per_thread_cwd = false;
- bool have_per_thread_creds = false;
bool do_async = false;
SMB_ASSERT(!is_named_stream(smb_fname));
@@ -3631,30 +3642,19 @@ static struct tevent_req *vfswrap_getxattrat_send(
return NULL;
}
*state = (struct vfswrap_getxattrat_state) {
- .ev = ev,
- .handle = handle,
- .dir_fsp = dir_fsp,
- .smb_fname = smb_fname,
+ .job_state.ev = ev,
+ .job_state.handle = handle,
+ .job_state.dir_fsp = dir_fsp,
+ .job_state.smb_fname = smb_fname,
};
- max_threads =
pthreadpool_tevent_max_threads(dir_fsp->conn->sconn->pool);
- if (max_threads >= 1) {
- /*
- * We need a non sync threadpool!
- */
- have_per_thread_cwd = per_thread_cwd_supported();
- }
-#ifdef HAVE_LINUX_THREAD_CREDENTIALS
- have_per_thread_creds = true;
-#endif
- if (have_per_thread_cwd && have_per_thread_creds) {
- do_async = true;
- }
+ do_async = vfswrap_check_async_with_thread_creds(
+ dir_fsp->conn->sconn->pool);
SMBPROFILE_BYTES_ASYNC_START_X(SNUM(handle->conn),
syscall_asys_getxattrat,
- state->profile_bytes,
- state->profile_bytes_x,
+ state->job_state.profile_bytes,
+ state->job_state.profile_bytes_x,
0);
if (fsp_get_pathref_fd(dir_fsp) == -1) {
@@ -3684,8 +3684,8 @@ static struct tevent_req *vfswrap_getxattrat_send(
* must not be freed before all referencing threads terminate.
*/
- state->name = talloc_strdup(state, smb_fname->base_name);
- if (tevent_req_nomem(state->name, req)) {
+ state->job_state.name = talloc_strdup(state, smb_fname->base_name);
+ if (tevent_req_nomem(state->job_state.name, req)) {
return tevent_req_post(req, ev);
}
@@ -3702,18 +3702,19 @@ static struct tevent_req *vfswrap_getxattrat_send(
* without a malloc in most cases.
*/
if (geteuid() == sec_initial_uid()) {
- state->token = root_unix_token(state);
+ state->job_state.token = root_unix_token(state);
} else {
- state->token = copy_unix_token(
+ state->job_state.token = copy_unix_token(
state,
dir_fsp->conn->session_info->unix_token);
}
- if (tevent_req_nomem(state->token, req)) {
+
+ if (tevent_req_nomem(state->job_state.token, req)) {
return tevent_req_post(req, ev);
}
- SMBPROFILE_BYTES_ASYNC_SET_IDLE_X(state->profile_bytes,
- state->profile_bytes_x);
+ SMBPROFILE_BYTES_ASYNC_SET_IDLE_X(state->job_state.profile_bytes,
+ state->job_state.profile_bytes_x);
subreq = pthreadpool_tevent_job_send(
state,
@@ -3736,8 +3737,8 @@ static void vfswrap_getxattrat_do_sync(struct tevent_req
*req)
struct vfswrap_getxattrat_state *state = tevent_req_data(
req, struct vfswrap_getxattrat_state);
- state->xattr_size = vfswrap_fgetxattr(state->handle,
- state->smb_fname->fsp,
+ state->xattr_size = vfswrap_fgetxattr(state->job_state.handle,
+ state->job_state.smb_fname->fsp,
state->xattr_name,
state->xattr_value,
talloc_array_length(state->xattr_value));
@@ -3759,8 +3760,8 @@ static void vfswrap_getxattrat_do_async(void
*private_data)
int ret;
PROFILE_TIMESTAMP(&start_time);
- SMBPROFILE_BYTES_ASYNC_SET_BUSY_X(state->profile_bytes,
- state->profile_bytes_x);
+ SMBPROFILE_BYTES_ASYNC_SET_BUSY_X(state->job_state.profile_bytes,
+ state->job_state.profile_bytes_x);
/*
* Here we simulate a getxattrat()
@@ -3770,30 +3771,30 @@ static void vfswrap_getxattrat_do_async(void
*private_data)
per_thread_cwd_activate();
/* Become the correct credential on this thread. */
- ret = set_thread_credentials(state->token->uid,
- state->token->gid,
- (size_t)state->token->ngroups,
- state->token->groups);
+ ret = set_thread_credentials(state->job_state.token->uid,
+ state->job_state.token->gid,
+ (size_t)state->job_state.token->ngroups,
+ state->job_state.token->groups);
if (ret != 0) {
state->xattr_size = -1;
- state->vfs_aio_state.error = errno;
+ state->job_state.vfs_aio_state.error = errno;
goto end_profile;
}
- state->xattr_size = vfswrap_fgetxattr(state->handle,
- state->smb_fname->fsp,
+ state->xattr_size = vfswrap_fgetxattr(state->job_state.handle,
+ state->job_state.smb_fname->fsp,
state->xattr_name,
state->xattr_value,
talloc_array_length(state->xattr_value));
if (state->xattr_size == -1) {
- state->vfs_aio_state.error = errno;
+ state->job_state.vfs_aio_state.error = errno;
}
end_profile:
PROFILE_TIMESTAMP(&end_time);
- state->vfs_aio_state.duration = nsec_time_diff(&end_time, &start_time);
- SMBPROFILE_BYTES_ASYNC_SET_IDLE_X(state->profile_bytes,
- state->profile_bytes_x);
+ state->job_state.vfs_aio_state.duration = nsec_time_diff(&end_time,
&start_time);
+ SMBPROFILE_BYTES_ASYNC_SET_IDLE_X(state->job_state.profile_bytes,
+ state->job_state.profile_bytes_x);
}
static void vfswrap_getxattrat_done(struct tevent_req *subreq)
@@ -3808,13 +3809,14 @@ static void vfswrap_getxattrat_done(struct tevent_req
*subreq)
/*
* Make sure we run as the user again
*/
- ok = change_to_user_and_service_by_fsp(state->dir_fsp);
+ ok = change_to_user_and_service_by_fsp(state->job_state.dir_fsp);
SMB_ASSERT(ok);
ret = pthreadpool_tevent_job_recv(subreq);
TALLOC_FREE(subreq);
- SMBPROFILE_BYTES_ASYNC_END_X(state->profile_bytes,
- state->profile_bytes_x);
+
+ SMBPROFILE_BYTES_ASYNC_END_X(state->job_state.profile_bytes,
+ state->job_state.profile_bytes_x);
talloc_set_destructor(state, NULL);
if (ret != 0) {
if (ret != EAGAIN) {
@@ -3832,7 +3834,7 @@ static void vfswrap_getxattrat_done(struct tevent_req
*subreq)
}
if (state->xattr_size == -1) {
- tevent_req_error(req, state->vfs_aio_state.error);
+ tevent_req_error(req, state->job_state.vfs_aio_state.error);
return;
}
@@ -3870,7 +3872,7 @@ static ssize_t vfswrap_getxattrat_recv(struct tevent_req
*req,
return -1;
}
- *aio_state = state->vfs_aio_state;
+ *aio_state = state->job_state.vfs_aio_state;
xattr_size = state->xattr_size;
if (xattr_value != NULL) {
*xattr_value = talloc_move(mem_ctx, &state->xattr_value);
diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index 48f3bbad363..dffe5990253 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -22,6 +22,7 @@
#include "includes.h"
#include "smbd/smbd.h"
+#include "smbd/globals.h"
#include "include/smbprofile.h"
#include "modules/non_posix_acls.h"
#include "libcli/security/security.h"
@@ -29,6 +30,7 @@
#include "system/filesys.h"
#include "auth.h"
#include "lib/util/tevent_unix.h"
+#include "lib/pthreadpool/pthreadpool_tevent.h"
#include "lib/util/gpfswrap.h"
#include <gnutls/gnutls.h>
@@ -1433,6 +1435,322 @@ static unsigned int
vfs_gpfs_dosmode_to_winattrs(uint32_t dosmode)
return winattrs;
}
+struct vfs_gpfs_get_dos_attributes_state {
+ struct vfs_pthreadpool_job_state job_state;
+ struct gpfs_winattr attrs;
+ uint32_t dosmode;
+};
+
+static int vfs_gpfs_get_dos_attributes_state_destructor(
+ struct vfs_gpfs_get_dos_attributes_state *state)
+{
+ return -1;
+}
+
+static int vfs_gpfs_get_winattrs_helper(
+ struct vfs_gpfs_get_dos_attributes_state *state)
+{
+ int ret = 0;
+ files_struct* fd = state->job_state.smb_fname->fsp;
+
+ ret = gpfswrap_get_winattrs(fsp_get_pathref_fd(fd), &state->attrs);
+
+ if (ret == -1) {
+ state->job_state.vfs_aio_state.error = errno;
+ return ret;
+ }
+
+ if (ret == -1 && errno == EACCES) {
+ int saved_errno = 0;
+
+ /*
+ * According to MS-FSA 2.1.5.1.2.1 "Algorithm to Check Access to
+ * an Existing File" FILE_LIST_DIRECTORY on a directory implies
+ * FILE_READ_ATTRIBUTES for directory entries. Being able to
+ * open a file implies FILE_LIST_DIRECTORY.
+ */
+
+ set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+ ret = gpfswrap_get_winattrs(
+ fsp_get_pathref_fd(fd),
+ &state->attrs);
+ if (ret == -1) {
+ saved_errno = errno;
+ }
+
+ drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+ if (saved_errno != 0) {
+ state->job_state.vfs_aio_state.error = saved_errno;
+ ret = saved_errno;
+ }
+ }
+ return ret;
+}
+
+static void vfs_gpfs_get_winattrs_do_sync(struct tevent_req *req);
+static void vfs_gpfs_get_winattrs_do_async(void *private_data);
+static void vfs_gpfs_get_winattrs_done(struct tevent_req *subreq);
+
+static struct tevent_req *vfs_gpfs_get_dos_attributes_send(
+ TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct vfs_handle_struct
*handle,
+ files_struct *dir_fsp,
+ struct smb_filename *smb_fname)
+{
+ struct tevent_req *req = NULL;
+ struct tevent_req *subreq = NULL;
+ struct vfs_gpfs_get_dos_attributes_state *state = NULL;
+ bool do_async = false;
+ struct gpfs_config_data *config;
+
+ SMB_VFS_HANDLE_GET_DATA(handle, config,
+ struct gpfs_config_data,
+ return NULL);
+
+ if (!config->winattr) {
+ return SMB_VFS_GET_DOS_ATTRIBUTES_SEND(mem_ctx,
+ ev,
+ dir_fsp,
+ smb_fname);
+ }
+
+ SMB_ASSERT(!is_named_stream(smb_fname));
+
+ req = tevent_req_create(mem_ctx, &state,
+ struct vfs_gpfs_get_dos_attributes_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ *state = (struct vfs_gpfs_get_dos_attributes_state) {
+ .job_state.ev = ev,
+ .job_state.handle = handle,
+ .job_state.dir_fsp = dir_fsp,
+ .job_state.smb_fname = smb_fname,
+ };
+
+ do_async = vfswrap_check_async_with_thread_creds(
+ dir_fsp->conn->sconn->pool);
+
+ SMBPROFILE_BYTES_ASYNC_START_X(SNUM(handle->conn),
+ syscall_asys_getxattrat,
+ state->job_state.profile_bytes,
+
state->job_state.profile_bytes_x,
+ 0);
+
+
+ if (fsp_get_pathref_fd(dir_fsp) == -1) {
+ DBG_ERR("Need a valid directory fd for [%s]\n",
+ fsp_str_dbg(dir_fsp));
+
+ tevent_req_error(req, EINVAL);
+ return tevent_req_post(req, ev);
+ }
+
+ if (!do_async) {
+ vfs_gpfs_get_winattrs_do_sync(req);
+ return tevent_req_post(req, ev);
+ }
+
+ /*
+ * Now allocate all parameters from a memory context that won't go away
+ * no matter what. These parameters will get used in threads and we
+ * can't reliably cancel threads, so all buffers passed to the threads
+ * must not be freed before all referencing threads terminate.
+ */
+
+ state->job_state.name = talloc_strdup(state, smb_fname->base_name);
+ if (tevent_req_nomem(state->job_state.name, req)) {
+ return tevent_req_post(req, ev);
+ }
+
+ /*
+ * This is a hot codepath so at first glance one might think we should
+ * somehow optimize away the token allocation and do a
+ * talloc_reference() or similar black magic instead. But due to the
+ * talloc_stackframe pool per SMB2 request this should be a simple copy
+ * without a malloc in most cases.
+ */
+ if (geteuid() == sec_initial_uid()) {
+ state->job_state.token = root_unix_token(state);
+ } else {
+ state->job_state.token = copy_unix_token(
+ state,
+
dir_fsp->conn->session_info->unix_token);
+ }
+
+ if (tevent_req_nomem(state->job_state.token, req)) {
+ return tevent_req_post(req, ev);
+ }
+
+ SMBPROFILE_BYTES_ASYNC_SET_IDLE_X(state->job_state.profile_bytes,
+
state->job_state.profile_bytes_x);
+
+ subreq = pthreadpool_tevent_job_send(
+ state,
+ ev,
+ dir_fsp->conn->sconn->pool,
+ vfs_gpfs_get_winattrs_do_async,
+ state);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, vfs_gpfs_get_winattrs_done, req);
+
+ talloc_set_destructor(
+ state,
+ vfs_gpfs_get_dos_attributes_state_destructor);
+
+ return req;
--
Samba Shared Repository
