The Samba-Bugzilla – Attachment 11101 Details for
Bug 11295
Excessive cli_resolve_path() usage can slow down transmission
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for master.
0001-s3-libsmbclient-Re-resolving-targetcli-on-every-read.patch (text/plain), 10.82 KB, created by
Jeremy Allison
on 2015-05-28 18:10:38 UTC
(
hide
)
Description:
git-am fix for master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2015-05-28 18:10:38 UTC
Size:
10.82 KB
patch
obsolete
>From b96e95fd59a475d29429d50f8a8f9572a011350a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 28 May 2015 11:07:41 -0700 >Subject: [PATCH] s3: libsmbclient: Re-resolving targetcli on every > read/write/lseek/ftruncate/close is both incorrect and slow. > >Cache targetcli on file open in the SMBCFILE struct. > >Bug 11295 - Excessive cli_resolve_path() usage can slow down transmission. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=11295 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/include/libsmb_internal.h | 3 + > source3/libsmb/libsmb_file.c | 202 ++++---------------------------------- > 2 files changed, 20 insertions(+), 185 deletions(-) > >diff --git a/source3/include/libsmb_internal.h b/source3/include/libsmb_internal.h >index c9c83eb..73a7072 100644 >--- a/source3/include/libsmb_internal.h >+++ b/source3/include/libsmb_internal.h >@@ -100,6 +100,9 @@ struct smbc_dir_list { > */ > struct _SMBCFILE { > int cli_fd; >+ /* cache of cli_state we opened cli_fd on. Due to DFS can be a >+ subsidiary connection to srv->cli */ >+ struct cli_state *targetcli; > char *fname; > off_t offset; > struct _SMBCSRV *srv; >diff --git a/source3/libsmb/libsmb_file.c b/source3/libsmb/libsmb_file.c >index 11d6d82..9163649 100644 >--- a/source3/libsmb/libsmb_file.c >+++ b/source3/libsmb/libsmb_file.c >@@ -144,6 +144,14 @@ SMBC_open_ctx(SMBCCTX *context, > file->srv = srv; > file->offset = 0; > file->file = True; >+ /* >+ * targetcli is either equal to srv->cli or >+ * is a subsidiary DFS connection. Either way >+ * file->cli_fd belongs to it so we must cache >+ * it for read/write/close, not re-resolve each time. >+ * Re-resolving is both slow and incorrect. >+ */ >+ file->targetcli = targetcli; > > DLIST_ADD(context->internal->files, file); > >@@ -228,11 +236,6 @@ SMBC_read_ctx(SMBCCTX *context, > size_t count) > { > size_t ret; >- char *server = NULL, *share = NULL, *user = NULL, *password = NULL; >- char *path = NULL; >- char *targetpath = NULL; >- struct cli_state *targetcli = NULL; >- uint16_t port = 0; > TALLOC_CTX *frame = talloc_stackframe(); > NTSTATUS status; > >@@ -271,39 +274,10 @@ SMBC_read_ctx(SMBCCTX *context, > return -1; > } > >- /*d_printf(">>>read: parsing %s\n", file->fname);*/ >- if (SMBC_parse_path(frame, >- context, >- file->fname, >- NULL, >- &server, >- &port, >- &share, >- &path, >- &user, >- &password, >- NULL)) { >- errno = EINVAL; >- TALLOC_FREE(frame); >- return -1; >- } >- >- /*d_printf(">>>read: resolving %s\n", path);*/ >- status = cli_resolve_path(frame, "", context->internal->auth_info, >- file->srv->cli, path, >- &targetcli, &targetpath); >- if (!NT_STATUS_IS_OK(status)) { >- d_printf("Could not resolve %s\n", path); >- errno = ENOENT; >- TALLOC_FREE(frame); >- return -1; >- } >- /*d_printf(">>>fstat: resolved path as %s\n", targetpath);*/ >- >- status = cli_read(targetcli, file->cli_fd, (char *)buf, offset, >+ status = cli_read(file->targetcli, file->cli_fd, (char *)buf, offset, > count, &ret); > if (!NT_STATUS_IS_OK(status)) { >- errno = SMBC_errno(context, targetcli); >+ errno = SMBC_errno(context, file->targetcli); > TALLOC_FREE(frame); > return -1; > } >@@ -327,11 +301,6 @@ SMBC_write_ctx(SMBCCTX *context, > size_t count) > { > off_t offset; >- char *server = NULL, *share = NULL, *user = NULL, *password = NULL; >- char *path = NULL; >- char *targetpath = NULL; >- struct cli_state *targetcli = NULL; >- uint16_t port = 0; > TALLOC_CTX *frame = talloc_stackframe(); > NTSTATUS status; > >@@ -359,36 +328,7 @@ SMBC_write_ctx(SMBCCTX *context, > > offset = file->offset; /* See "offset" comment in SMBC_read_ctx() */ > >- /*d_printf(">>>write: parsing %s\n", file->fname);*/ >- if (SMBC_parse_path(frame, >- context, >- file->fname, >- NULL, >- &server, >- &port, >- &share, >- &path, >- &user, >- &password, >- NULL)) { >- errno = EINVAL; >- TALLOC_FREE(frame); >- return -1; >- } >- >- /*d_printf(">>>write: resolving %s\n", path);*/ >- status = cli_resolve_path(frame, "", context->internal->auth_info, >- file->srv->cli, path, >- &targetcli, &targetpath); >- if (!NT_STATUS_IS_OK(status)) { >- d_printf("Could not resolve %s\n", path); >- errno = ENOENT; >- TALLOC_FREE(frame); >- return -1; >- } >- /*d_printf(">>>write: resolved path as %s\n", targetpath);*/ >- >- status = cli_writeall(targetcli, file->cli_fd, >+ status = cli_writeall(file->targetcli, file->cli_fd, > 0, (const uint8_t *)buf, offset, count, NULL); > if (!NT_STATUS_IS_OK(status)) { > errno = map_errno_from_nt_status(status); >@@ -410,14 +350,7 @@ int > SMBC_close_ctx(SMBCCTX *context, > SMBCFILE *file) > { >- SMBCSRV *srv; >- char *server = NULL, *share = NULL, *user = NULL, *password = NULL; >- char *path = NULL; >- char *targetpath = NULL; >- uint16_t port = 0; >- struct cli_state *targetcli = NULL; > TALLOC_CTX *frame = talloc_stackframe(); >- NTSTATUS status; > > if (!context || !context->internal->initialized) { > errno = EINVAL; >@@ -437,41 +370,13 @@ SMBC_close_ctx(SMBCCTX *context, > return smbc_getFunctionClosedir(context)(context, file); > } > >- /*d_printf(">>>close: parsing %s\n", file->fname);*/ >- if (SMBC_parse_path(frame, >- context, >- file->fname, >- NULL, >- &server, >- &port, >- &share, >- &path, >- &user, >- &password, >- NULL)) { >- errno = EINVAL; >- TALLOC_FREE(frame); >- return -1; >- } >- >- /*d_printf(">>>close: resolving %s\n", path);*/ >- status = cli_resolve_path(frame, "", context->internal->auth_info, >- file->srv->cli, path, >- &targetcli, &targetpath); >- if (!NT_STATUS_IS_OK(status)) { >- d_printf("Could not resolve %s\n", path); >- errno = ENOENT; >- TALLOC_FREE(frame); >- return -1; >- } >- /*d_printf(">>>close: resolved path as %s\n", targetpath);*/ >- >- if (!NT_STATUS_IS_OK(cli_close(targetcli, file->cli_fd))) { >+ if (!NT_STATUS_IS_OK(cli_close(file->targetcli, file->cli_fd))) { >+ SMBCSRV *srv; > DEBUG(3, ("cli_close failed on %s. purging server.\n", > file->fname)); > /* Deallocate slot and remove the server > * from the server cache if unused */ >- errno = SMBC_errno(context, targetcli); >+ errno = SMBC_errno(context, file->targetcli); > srv = file->srv; > DLIST_REMOVE(context->internal->files, file); > SAFE_FREE(file->fname); >@@ -707,13 +612,7 @@ SMBC_lseek_ctx(SMBCCTX *context, > int whence) > { > off_t size; >- char *server = NULL, *share = NULL, *user = NULL, *password = NULL; >- char *path = NULL; >- char *targetpath = NULL; >- struct cli_state *targetcli = NULL; >- uint16_t port = 0; > TALLOC_CTX *frame = talloc_stackframe(); >- NTSTATUS status; > > if (!context || !context->internal->initialized) { > errno = EINVAL; >@@ -741,41 +640,12 @@ SMBC_lseek_ctx(SMBCCTX *context, > file->offset += offset; > break; > case SEEK_END: >- /*d_printf(">>>lseek: parsing %s\n", file->fname);*/ >- if (SMBC_parse_path(frame, >- context, >- file->fname, >- NULL, >- &server, >- &port, >- &share, >- &path, >- &user, >- &password, >- NULL)) { >- errno = EINVAL; >- TALLOC_FREE(frame); >- return -1; >- } >- >- /*d_printf(">>>lseek: resolving %s\n", path);*/ >- status = cli_resolve_path( >- frame, "", context->internal->auth_info, >- file->srv->cli, path, &targetcli, &targetpath); >- if (!NT_STATUS_IS_OK(status)) { >- d_printf("Could not resolve %s\n", path); >- errno = ENOENT; >- TALLOC_FREE(frame); >- return -1; >- } >- >- /*d_printf(">>>lseek: resolved path as %s\n", targetpath);*/ > if (!NT_STATUS_IS_OK(cli_qfileinfo_basic( >- targetcli, file->cli_fd, NULL, >+ file->targetcli, file->cli_fd, NULL, > &size, NULL, NULL, NULL, NULL, > NULL))) { > off_t b_size = size; >- if (!NT_STATUS_IS_OK(cli_getattrE(targetcli, file->cli_fd, >+ if (!NT_STATUS_IS_OK(cli_getattrE(file->targetcli, file->cli_fd, > NULL, &b_size, NULL, NULL, NULL))) { > errno = EINVAL; > TALLOC_FREE(frame); >@@ -805,16 +675,7 @@ SMBC_ftruncate_ctx(SMBCCTX *context, > off_t length) > { > off_t size = length; >- char *server = NULL; >- char *share = NULL; >- char *user = NULL; >- char *password = NULL; >- char *path = NULL; >- char *targetpath = NULL; >- uint16_t port = 0; >- struct cli_state *targetcli = NULL; > TALLOC_CTX *frame = talloc_stackframe(); >- NTSTATUS status; > > if (!context || !context->internal->initialized) { > errno = EINVAL; >@@ -834,36 +695,7 @@ SMBC_ftruncate_ctx(SMBCCTX *context, > return -1; > } > >- /*d_printf(">>>fstat: parsing %s\n", file->fname);*/ >- if (SMBC_parse_path(frame, >- context, >- file->fname, >- NULL, >- &server, >- &port, >- &share, >- &path, >- &user, >- &password, >- NULL)) { >- errno = EINVAL; >- TALLOC_FREE(frame); >- return -1; >- } >- >- /*d_printf(">>>fstat: resolving %s\n", path);*/ >- status = cli_resolve_path(frame, "", context->internal->auth_info, >- file->srv->cli, path, >- &targetcli, &targetpath); >- if (!NT_STATUS_IS_OK(status)) { >- d_printf("Could not resolve %s\n", path); >- errno = ENOENT; >- TALLOC_FREE(frame); >- return -1; >- } >- /*d_printf(">>>fstat: resolved path as %s\n", targetpath);*/ >- >- if (!NT_STATUS_IS_OK(cli_ftruncate(targetcli, file->cli_fd, (uint64_t)size))) { >+ if (!NT_STATUS_IS_OK(cli_ftruncate(file->targetcli, file->cli_fd, (uint64_t)size))) { > errno = EINVAL; > TALLOC_FREE(frame); > return -1; >-- >2.2.0.rc0.207.ga3a616c >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=11101">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 11295
:
11101
|
11124