[PATCH] vfs_gpfs: move btime stuff from stat to get_dos_attrs
Ralph Böhme
slow at samba.org
Thu Dec 15 17:21:17 UTC 2016
Hi!
Attached is a patch that addresses a performance problem in vfs_gpfs:
perf report found that in a kernel copy workload smbd was spending
considerable CPU time in vfs_gpfs_(f|l)stat -> gpfs_get_winattrs.
Most of the time the VFS stat caller is not interested in the btime. The
SMB frontend processing around btime is designed to fetch btime together
with DOS attributes via dos_mode() in all places that need these
attributes. That's the way it is implemented in the default VFS module
and that's what vfs_gpfs now does as well for performance reasons.
Please review and push if ok. Thanks!
Cheerio!
-slow
-------------- next part --------------
From 8cdef4977ab2f85d3dddb3974e34fd0cc9e391c8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Dec 2016 07:09:58 +0100
Subject: [PATCH 1/3] vfs_gpfs: update btime in vfs_gpfs_(f)get_dos_attributes
This paves the way for removing btime updates from the stat VFS
functions.
This way we behave like the default VFS module where DOS attributes and
btime are fetched from the same backing store and the frontend is
designed around using dos_mode() -> SMB_VFS_GET_ATTRIBUTES to update
both attributes as necessary in the SMB processing.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_gpfs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index f49ef0d..da7ec7a 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -1596,6 +1596,9 @@ static NTSTATUS vfs_gpfs_get_dos_attributes(struct vfs_handle_struct *handle,
}
*dosmode |= vfs_gpfs_winattrs_to_dosmode(attrs.winAttrs);
+ smb_fname->st.st_ex_calculated_birthtime = false;
+ smb_fname->st.st_ex_btime.tv_sec = attrs.creationTime.tv_sec;
+ smb_fname->st.st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec;
return NT_STATUS_OK;
}
@@ -1628,6 +1631,9 @@ static NTSTATUS vfs_gpfs_fget_dos_attributes(struct vfs_handle_struct *handle,
}
*dosmode |= vfs_gpfs_winattrs_to_dosmode(attrs.winAttrs);
+ fsp->fsp_name->st.st_ex_calculated_birthtime = false;
+ fsp->fsp_name->st.st_ex_btime.tv_sec = attrs.creationTime.tv_sec;
+ fsp->fsp_name->st.st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec;
return NT_STATUS_OK;
}
--
2.7.4
From ac7d5f1d39d0a73590765e872f50e29215004c5a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 28 Nov 2016 12:22:04 +0100
Subject: [PATCH 2/3] vfs_gpfs: remove updating btime from stat VFS calls
This is now handled by the vfs_gpfs_(f)get_dos_attributes. Getting rid
of this in the stat VFS functions is a huge performance saver. perf
report found that in a kernel copy workload smbd was spending
considerable CPU time in vfs_gpfs_(f|l)stat -> gpfs_get_winattrs.
Most of the time the VFS stat caller is not interested in the btime. The
SMB frontend processing around btime is designed to fetch btime together
with DOS attributes via dos_mode() in all places that need these
attributes. That's the way it is implemented in the default VFS module
and that's what vfs_gpfs now does as well for performance reasons.
This makes vfs_gpfs_fstat a null op and I'm therefor removing it.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_gpfs.c | 82 ++--------------------------------------------
1 file changed, 2 insertions(+), 80 deletions(-)
diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index da7ec7a..fedb5dd 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -1749,9 +1749,6 @@ static int stat_with_capability(struct vfs_handle_struct *handle,
static int vfs_gpfs_stat(struct vfs_handle_struct *handle,
struct smb_filename *smb_fname)
{
- struct gpfs_winattr attrs;
- char *fname = NULL;
- NTSTATUS status;
int ret;
struct gpfs_config_data *config;
@@ -1767,66 +1764,12 @@ static int vfs_gpfs_stat(struct vfs_handle_struct *handle,
ret = stat_with_capability(handle, smb_fname, 0);
}
#endif
- if (ret == -1) {
- return -1;
- }
-
- if (!config->winattr) {
- return 0;
- }
-
- status = get_full_smb_filename(talloc_tos(), smb_fname, &fname);
- if (!NT_STATUS_IS_OK(status)) {
- errno = map_errno_from_nt_status(status);
- return -1;
- }
- ret = gpfswrap_get_winattrs_path(discard_const_p(char, fname), &attrs);
- TALLOC_FREE(fname);
- if (ret == 0) {
- smb_fname->st.st_ex_calculated_birthtime = false;
- smb_fname->st.st_ex_btime.tv_sec = attrs.creationTime.tv_sec;
- smb_fname->st.st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec;
- }
- return 0;
-}
-
-static int vfs_gpfs_fstat(struct vfs_handle_struct *handle,
- struct files_struct *fsp, SMB_STRUCT_STAT *sbuf)
-{
- struct gpfs_winattr attrs;
- int ret;
- struct gpfs_config_data *config;
-
- SMB_VFS_HANDLE_GET_DATA(handle, config,
- struct gpfs_config_data,
- return -1);
-
- ret = SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf);
- if (ret == -1) {
- return -1;
- }
- if ((fsp->fh == NULL) || (fsp->fh->fd == -1)) {
- return 0;
- }
- if (!config->winattr) {
- return 0;
- }
-
- ret = gpfswrap_get_winattrs(fsp->fh->fd, &attrs);
- if (ret == 0) {
- sbuf->st_ex_calculated_birthtime = false;
- sbuf->st_ex_btime.tv_sec = attrs.creationTime.tv_sec;
- sbuf->st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec;
- }
- return 0;
+ return ret;
}
static int vfs_gpfs_lstat(struct vfs_handle_struct *handle,
struct smb_filename *smb_fname)
{
- struct gpfs_winattr attrs;
- char *path = NULL;
- NTSTATUS status;
int ret;
struct gpfs_config_data *config;
@@ -1843,27 +1786,7 @@ static int vfs_gpfs_lstat(struct vfs_handle_struct *handle,
AT_SYMLINK_NOFOLLOW);
}
#endif
-
- if (ret == -1) {
- return -1;
- }
- if (!config->winattr) {
- return 0;
- }
-
- status = get_full_smb_filename(talloc_tos(), smb_fname, &path);
- if (!NT_STATUS_IS_OK(status)) {
- errno = map_errno_from_nt_status(status);
- return -1;
- }
- ret = gpfswrap_get_winattrs_path(discard_const_p(char, path), &attrs);
- TALLOC_FREE(path);
- if (ret == 0) {
- smb_fname->st.st_ex_calculated_birthtime = false;
- smb_fname->st.st_ex_btime.tv_sec = attrs.creationTime.tv_sec;
- smb_fname->st.st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec;
- }
- return 0;
+ return ret;
}
static void timespec_to_gpfs_time(struct timespec ts, gpfs_timestruc_t *gt,
@@ -2611,7 +2534,6 @@ static struct vfs_fn_pointers vfs_gpfs_fns = {
.fchmod_fn = vfs_gpfs_fchmod,
.close_fn = vfs_gpfs_close,
.stat_fn = vfs_gpfs_stat,
- .fstat_fn = vfs_gpfs_fstat,
.lstat_fn = vfs_gpfs_lstat,
.ntimes_fn = vfs_gpfs_ntimes,
.aio_force_fn = vfs_gpfs_aio_force,
--
2.7.4
From 8902d885db3e7ce83e41cb129037e1bc1eecb0ca Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 15 Dec 2016 18:10:22 +0100
Subject: [PATCH 3/3] vfs_gpfs: simplify stat_with_capability() ifdef
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_gpfs.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index fedb5dd..768bf63 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -1706,10 +1706,10 @@ static NTSTATUS vfs_gpfs_fset_dos_attributes(struct vfs_handle_struct *handle,
return NT_STATUS_OK;
}
-#if defined(HAVE_FSTATAT)
static int stat_with_capability(struct vfs_handle_struct *handle,
struct smb_filename *smb_fname, int flag)
{
+#if defined(HAVE_FSTATAT)
int fd = -1;
bool b;
char *dir_name;
@@ -1743,8 +1743,10 @@ static int stat_with_capability(struct vfs_handle_struct *handle,
}
return ret;
-}
+#else
+ return -1;
#endif
+}
static int vfs_gpfs_stat(struct vfs_handle_struct *handle,
struct smb_filename *smb_fname)
@@ -1757,13 +1759,11 @@ static int vfs_gpfs_stat(struct vfs_handle_struct *handle,
return -1);
ret = SMB_VFS_NEXT_STAT(handle, smb_fname);
-#if defined(HAVE_FSTATAT)
if (ret == -1 && errno == EACCES) {
DEBUG(10, ("Trying stat with capability for %s\n",
smb_fname->base_name));
ret = stat_with_capability(handle, smb_fname, 0);
}
-#endif
return ret;
}
@@ -1778,14 +1778,12 @@ static int vfs_gpfs_lstat(struct vfs_handle_struct *handle,
return -1);
ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname);
-#if defined(HAVE_FSTATAT)
if (ret == -1 && errno == EACCES) {
DEBUG(10, ("Trying lstat with capability for %s\n",
smb_fname->base_name));
ret = stat_with_capability(handle, smb_fname,
AT_SYMLINK_NOFOLLOW);
}
-#endif
return ret;
}
--
2.7.4
More information about the samba-technical
mailing list