The Samba-Bugzilla – Attachment 17488 Details for
Bug 15125
Performance regression on contended path based operations
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Additional patches for v4-17-test
bfixes-tmp417.txt (text/plain), 26.14 KB, created by
Stefan Metzmacher
on 2022-08-22 06:09:24 UTC
(
hide
)
Description:
Additional patches for v4-17-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2022-08-22 06:09:24 UTC
Size:
26.14 KB
patch
obsolete
>From 0c44c8bba7f31625d1897ff3077c47569bfd16db Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 18 Aug 2022 16:06:02 +0200 >Subject: [PATCH 1/9] lib/util: add unlikely() to SMB_ASSERT() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit bf1dd1a188c096093bedc628a14bb037e3209630) >--- > lib/util/fault.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/lib/util/fault.h b/lib/util/fault.h >index f3b1666a1721..6aceaf6dabcc 100644 >--- a/lib/util/fault.h >+++ b/lib/util/fault.h >@@ -33,7 +33,7 @@ > #ifdef _SAMBA_DEBUG_H > #define SMB_ASSERT(b) \ > do { \ >- if (!(b)) { \ >+ if (unlikely(!(b))) { \ > DEBUG(0,("PANIC: assert failed at %s(%d): %s\n", \ > __FILE__, __LINE__, #b)); \ > smb_panic("assert failed: " #b); \ >-- >2.34.1 > > >From 6b08b3b643849251602e15d447306fc9925d4512 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 18 Aug 2022 17:52:33 +0200 >Subject: [PATCH 2/9] s3:g_lock: add some const to the shared array passed via > g_lock_dump*() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit bb3dddcdf11e6c2f5319d64bf2ef20636d0ed82f) >--- > source3/include/g_lock.h | 4 ++-- > source3/lib/g_lock.c | 6 +++--- > source3/locking/share_mode_lock.c | 16 ++++++++-------- > source3/smbd/server.c | 2 +- > source3/torture/test_g_lock.c | 12 ++++++------ > source3/utils/net_g_lock.c | 2 +- > 6 files changed, 21 insertions(+), 21 deletions(-) > >diff --git a/source3/include/g_lock.h b/source3/include/g_lock.h >index 3a94879d209f..749cb57ed458 100644 >--- a/source3/include/g_lock.h >+++ b/source3/include/g_lock.h >@@ -71,7 +71,7 @@ struct tevent_req *g_lock_dump_send( > TDB_DATA key, > void (*fn)(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data), >@@ -81,7 +81,7 @@ NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, > TDB_DATA key, > void (*fn)(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data), >diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c >index d683c4ddee6f..35279dddb76f 100644 >--- a/source3/lib/g_lock.c >+++ b/source3/lib/g_lock.c >@@ -1210,7 +1210,7 @@ struct g_lock_dump_state { > TDB_DATA key; > void (*fn)(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data); >@@ -1265,7 +1265,7 @@ static void g_lock_dump_fn(TDB_DATA key, TDB_DATA data, > NTSTATUS g_lock_dump(struct g_lock_ctx *ctx, TDB_DATA key, > void (*fn)(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data), >@@ -1300,7 +1300,7 @@ struct tevent_req *g_lock_dump_send( > TDB_DATA key, > void (*fn)(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data), >diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c >index dd19c53024b9..cf1447d889ef 100644 >--- a/source3/locking/share_mode_lock.c >+++ b/source3/locking/share_mode_lock.c >@@ -468,7 +468,7 @@ struct locking_tdb_data_fetch_state { > static void locking_tdb_data_fetch_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -768,7 +768,7 @@ struct get_static_share_mode_data_state { > static void get_static_share_mode_data_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -1017,7 +1017,7 @@ struct share_mode_do_locked_state { > static void share_mode_do_locked_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -1324,7 +1324,7 @@ struct fetch_share_mode_unlocked_state { > static void fetch_share_mode_unlocked_parser( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -1395,7 +1395,7 @@ struct fetch_share_mode_state { > static void fetch_share_mode_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data); >@@ -1460,7 +1460,7 @@ struct tevent_req *fetch_share_mode_send(TALLOC_CTX *mem_ctx, > static void fetch_share_mode_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -1564,7 +1564,7 @@ struct share_mode_forall_state { > static void share_mode_forall_dump_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -2108,7 +2108,7 @@ struct share_mode_count_entries_state { > static void share_mode_count_entries_fn( > struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >diff --git a/source3/smbd/server.c b/source3/smbd/server.c >index 7e2a10532f6d..25cb84ab9d06 100644 >--- a/source3/smbd/server.c >+++ b/source3/smbd/server.c >@@ -1436,7 +1436,7 @@ struct smbd_claim_version_state { > > static void smbd_claim_version_parser(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >diff --git a/source3/torture/test_g_lock.c b/source3/torture/test_g_lock.c >index 002e390af190..511333939609 100644 >--- a/source3/torture/test_g_lock.c >+++ b/source3/torture/test_g_lock.c >@@ -115,7 +115,7 @@ struct lock2_parser_state { > > static void lock2_parser(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -216,14 +216,14 @@ struct lock3_parser_state { > > static void lock3_parser(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) > { > struct lock3_parser_state *state = private_data; > size_t num_locks = num_shared + ((exclusive.pid != 0) ? 1 : 0); >- struct server_id *pid; >+ const struct server_id *pid; > > if (datalen != 0) { > fprintf(stderr, "datalen=%zu\n", datalen); >@@ -423,7 +423,7 @@ struct lock4_check_state { > > static void lock4_check(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -712,7 +712,7 @@ struct lock5_parser_state { > > static void lock5_parser(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >@@ -859,7 +859,7 @@ struct lock6_parser_state { > > static void lock6_parser(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >diff --git a/source3/utils/net_g_lock.c b/source3/utils/net_g_lock.c >index 90c0f15fe780..2a3b105b2760 100644 >--- a/source3/utils/net_g_lock.c >+++ b/source3/utils/net_g_lock.c >@@ -113,7 +113,7 @@ done: > > static void net_g_lock_dump_fn(struct server_id exclusive, > size_t num_shared, >- struct server_id *shared, >+ const struct server_id *shared, > const uint8_t *data, > size_t datalen, > void *private_data) >-- >2.34.1 > > >From c8584c0b8ddace9885c97f11dd45b8883efd2ea4 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 18 Aug 2022 17:32:43 +0200 >Subject: [PATCH 3/9] s3:g_lock: avoid useless talloc_array(0) in g_lock_dump() > >In the common case we don't have any shared lock holders, >so there's no need to allocate memory for the empty array. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit c75de325710c0fbbd50a0acd3af55404165440d6) >--- > source3/lib/g_lock.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > >diff --git a/source3/lib/g_lock.c b/source3/lib/g_lock.c >index 35279dddb76f..8a07949b3d92 100644 >--- a/source3/lib/g_lock.c >+++ b/source3/lib/g_lock.c >@@ -1238,12 +1238,14 @@ static void g_lock_dump_fn(TDB_DATA key, TDB_DATA data, > return; > } > >- shared = talloc_array( >- state->mem_ctx, struct server_id, lck.num_shared); >- if (shared == NULL) { >- DBG_DEBUG("talloc failed\n"); >- state->status = NT_STATUS_NO_MEMORY; >- return; >+ if (lck.num_shared > 0) { >+ shared = talloc_array( >+ state->mem_ctx, struct server_id, lck.num_shared); >+ if (shared == NULL) { >+ DBG_DEBUG("talloc failed\n"); >+ state->status = NT_STATUS_NO_MEMORY; >+ return; >+ } > } > > for (i=0; i<lck.num_shared; i++) { >-- >2.34.1 > > >From 547facbfe4940e67eaf97345960c3b1f165df404 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Fri, 19 Aug 2022 15:17:41 +0000 >Subject: [PATCH 4/9] s3:smbd: only run validate_oplock_types() with > smbd:validate_oplock_types = yes > >This is really expensive as share_mode_forall_entries() is currently >doing a talloc_memdup() of the whole record... > >This is mainly used to avoid regressions, so only >use smbd:validate_oplock_types = yes in make test, >but skip it for production. > >This improves the following test: > > time smbtorture //127.0.0.1/m -Uroot%test \ > smb2.create.bench-path-contention-shared \ > --option='torture:bench_path=file.dat' \ > --option="torture:timelimit=60" \ > --option="torture:nprocs=256" \ > --option="torture:qdepth=1" > >From: > > open[num/s=8852,avslat=0.014999,minlat=0.000042,maxlat=0.054600] > close[num/s=8850,avslat=0.014136,minlat=0.000025,maxlat=0.054537] > >to: > > open[num/s=11377,avslat=0.012075,minlat=0.000041,maxlat=0.054107] > close[num/s=11375,avslat=0.010594,minlat=0.000023,maxlat=0.053620] > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 0fbca175ae4763d82f8a414ee3d6354c95d5294e) >--- > selftest/target/Samba3.pm | 1 + > selftest/target/Samba4.pm | 1 + > source3/smbd/open.c | 13 +++++++++++++ > 3 files changed, 15 insertions(+) > >diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm >index b12011a71fae..a5392fbf26cc 100755 >--- a/selftest/target/Samba3.pm >+++ b/selftest/target/Samba3.pm >@@ -2772,6 +2772,7 @@ sub provision($$) > panic action = cd $self->{srcdir} && $self->{srcdir}/selftest/gdb_backtrace %d %\$(MAKE_TEST_BINARY) > smbd:suicide mode = yes > smbd:FSCTL_SMBTORTURE = yes >+ smbd:validate_oplock_types = yes > > client min protocol = SMB2_02 > server min protocol = SMB2_02 >diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm >index 1762ae4ae79b..e5ae57cb7fcc 100755 >--- a/selftest/target/Samba4.pm >+++ b/selftest/target/Samba4.pm >@@ -778,6 +778,7 @@ sub provision_raw_step1($$) > panic action = $RealBin/gdb_backtrace \%d > smbd:suicide mode = yes > smbd:FSCTL_SMBTORTURE = yes >+ smbd:validate_oplock_types = yes > wins support = yes > server role = $ctx->{server_role} > server services = +echo $services >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 992656563bf9..ae49a7f57280 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -2203,8 +2203,21 @@ static bool validate_oplock_types_fn( > static bool validate_oplock_types(struct share_mode_lock *lck) > { > struct validate_oplock_types_state state = { .valid = true }; >+ static bool skip_validation; >+ bool validate; > bool ok; > >+ if (skip_validation) { >+ return true; >+ } >+ >+ validate = lp_parm_bool(-1, "smbd", "validate_oplock_types", false); >+ if (!validate) { >+ DBG_DEBUG("smbd:validate_oplock_types not set to yes\n"); >+ skip_validation = true; >+ return true; >+ } >+ > ok = share_mode_forall_entries(lck, validate_oplock_types_fn, &state); > if (!ok) { > DBG_DEBUG("share_mode_forall_entries failed\n"); >-- >2.34.1 > > >From 30227959adfdecc281af70340d90ea649d347acf Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 10 Aug 2022 08:27:15 +0000 >Subject: [PATCH 5/9] s3:locking: pass lease_key explicitly to set_share_mode() > >We should avoid accessing fsp->lease if possible. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 8b3b316680221487f84a7cfe14f52e8ffd64ba85) >--- > source3/locking/share_mode_lock.c | 3 ++- > source3/locking/share_mode_lock.h | 2 ++ > source3/smbd/open.c | 7 +++++++ > 3 files changed, 11 insertions(+), 1 deletion(-) > >diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c >index cf1447d889ef..b3f30c0f5128 100644 >--- a/source3/locking/share_mode_lock.c >+++ b/source3/locking/share_mode_lock.c >@@ -1782,6 +1782,7 @@ bool set_share_mode(struct share_mode_lock *lck, > uid_t uid, > uint64_t mid, > uint16_t op_type, >+ const struct smb2_lease_key *lease_key, > uint32_t share_access, > uint32_t access_mask) > { >@@ -1839,7 +1840,7 @@ bool set_share_mode(struct share_mode_lock *lck, > if (op_type == LEASE_OPLOCK) { > const struct GUID *client_guid = fsp_client_guid(fsp); > e.client_guid = *client_guid; >- e.lease_key = fsp->lease->lease.lease_key; >+ e.lease_key = *lease_key; > } > > ok = share_mode_entry_put(&e, &e_buf); >diff --git a/source3/locking/share_mode_lock.h b/source3/locking/share_mode_lock.h >index bd4e98ccecde..f8693b14b547 100644 >--- a/source3/locking/share_mode_lock.h >+++ b/source3/locking/share_mode_lock.h >@@ -26,6 +26,7 @@ struct share_mode_lock; > struct share_mode_entry; > struct smb_filename; > struct files_struct; >+struct smb2_lease_key; > > bool locking_init(void); > bool locking_init_readonly(void); >@@ -52,6 +53,7 @@ bool set_share_mode( > uid_t uid, > uint64_t mid, > uint16_t op_type, >+ const struct smb2_lease_key *lease_key, > uint32_t share_access, > uint32_t access_mask); > bool reset_share_mode_entry( >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index ae49a7f57280..d95bab6d9a49 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -3541,6 +3541,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > uint32_t existing_dos_attributes = 0; > struct share_mode_lock *lck = NULL; > uint32_t open_access_mask = access_mask; >+ const struct smb2_lease_key *lease_key = NULL; > NTSTATUS status; > SMB_STRUCT_STAT saved_stat = smb_fname->st; > struct timespec old_write_time; >@@ -4056,6 +4057,10 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > return status; > } > >+ if (fsp->oplock_type == LEASE_OPLOCK) { >+ lease_key = &lease->lease_key; >+ } >+ > share_mode_flags_restrict(lck, access_mask, share_access, 0); > > ok = set_share_mode( >@@ -4064,6 +4069,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, > get_current_uid(fsp->conn), > req ? req->mid : 0, > fsp->oplock_type, >+ lease_key, > share_access, > access_mask); > if (!ok) { >@@ -4744,6 +4750,7 @@ static NTSTATUS open_directory(connection_struct *conn, > get_current_uid(conn), > req ? req->mid : 0, > NO_OPLOCK, >+ NULL, > share_access, > fsp->access_mask); > if (!ok) { >-- >2.34.1 > > >From 0c09b6311fc645aeeb33a55c2d8de0ab066e14a3 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 18 Aug 2022 14:14:20 +0200 >Subject: [PATCH 6/9] s3:locking: move get_existing_share_mode_lock() to > share_mode_lock.[ch] > >This should be where get_share_mode_lock() is located. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit bf8f2258497f7d2a5a5f8d1cacf1a30899ed455c) >--- > source3/locking/locking.c | 11 ----------- > source3/locking/proto.h | 2 -- > source3/locking/share_mode_lock.c | 11 +++++++++++ > source3/locking/share_mode_lock.h | 2 ++ > source3/smbd/smbd.h | 1 + > 5 files changed, 14 insertions(+), 13 deletions(-) > >diff --git a/source3/locking/locking.c b/source3/locking/locking.c >index b002e80fee4b..befdc10f369b 100644 >--- a/source3/locking/locking.c >+++ b/source3/locking/locking.c >@@ -468,17 +468,6 @@ char *share_mode_str(TALLOC_CTX *ctx, int num, > (unsigned int)e->name_hash); > } > >-/******************************************************************* >- Fetch a share mode where we know one MUST exist. This call reference >- counts it internally to allow for nested lock fetches. >-********************************************************************/ >- >-struct share_mode_lock *get_existing_share_mode_lock(TALLOC_CTX *mem_ctx, >- const struct file_id id) >-{ >- return get_share_mode_lock(mem_ctx, id, NULL, NULL, NULL); >-} >- > struct rename_share_filename_state { > struct share_mode_lock *lck; > struct messaging_context *msg_ctx; >diff --git a/source3/locking/proto.h b/source3/locking/proto.h >index d6e7a54a7665..52452e56a7b7 100644 >--- a/source3/locking/proto.h >+++ b/source3/locking/proto.h >@@ -128,8 +128,6 @@ void locking_close_file(files_struct *fsp, > char *share_mode_str(TALLOC_CTX *ctx, int num, > const struct file_id *id, > const struct share_mode_entry *e); >-struct share_mode_lock *get_existing_share_mode_lock(TALLOC_CTX *mem_ctx, >- struct file_id id); > > bool rename_share_filename(struct messaging_context *msg_ctx, > struct share_mode_lock *lck, >diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c >index b3f30c0f5128..75912ec5dc86 100644 >--- a/source3/locking/share_mode_lock.c >+++ b/source3/locking/share_mode_lock.c >@@ -1005,6 +1005,17 @@ static int share_mode_lock_destructor(struct share_mode_lock *lck) > return 0; > } > >+/******************************************************************* >+ Fetch a share mode where we know one MUST exist. This call reference >+ counts it internally to allow for nested lock fetches. >+********************************************************************/ >+ >+struct share_mode_lock *get_existing_share_mode_lock(TALLOC_CTX *mem_ctx, >+ const struct file_id id) >+{ >+ return get_share_mode_lock(mem_ctx, id, NULL, NULL, NULL); >+} >+ > struct share_mode_do_locked_state { > TDB_DATA key; > void (*fn)(const uint8_t *buf, >diff --git a/source3/locking/share_mode_lock.h b/source3/locking/share_mode_lock.h >index f8693b14b547..4f47b6fbcbe5 100644 >--- a/source3/locking/share_mode_lock.h >+++ b/source3/locking/share_mode_lock.h >@@ -38,6 +38,8 @@ struct share_mode_lock *get_share_mode_lock( > const char *servicepath, > const struct smb_filename *smb_fname, > const struct timespec *old_write_time); >+struct share_mode_lock *get_existing_share_mode_lock(TALLOC_CTX *mem_ctx, >+ struct file_id id); > > bool del_share_mode(struct share_mode_lock *lck, > struct files_struct *fsp); >diff --git a/source3/smbd/smbd.h b/source3/smbd/smbd.h >index f2c3abcda2d0..6cf2b64a5a15 100644 >--- a/source3/smbd/smbd.h >+++ b/source3/smbd/smbd.h >@@ -25,6 +25,7 @@ struct dptr_struct; > #include "vfs.h" > #include "smbd/proto.h" > #include "locking/proto.h" >+#include "locking/share_mode_lock.h" > #include "smbd/fd_handle.h" > #if defined(WITH_SMB1SERVER) > #include "smbd/smb1_message.h" >-- >2.34.1 > > >From d5fe4637db08bff4f15d15098d978bb7c75ef898 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Thu, 18 Aug 2022 12:52:54 +0200 >Subject: [PATCH 7/9] s3:smbd: inline fsp_lease_type_is_exclusive() logic into > contend_level2_oplocks_begin_default > >SMB2_LEASE_WRITE is the indication for an exclusive lease, >the fact that a SMB2_LEASE_WRITE can't exists without >SMB2_LEASE_READ is not important here. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 76da56aa65bb9fe7f2f8c4a2e30e278a61db1ff5) >--- > source3/locking/leases_util.c | 17 ----------------- > source3/locking/proto.h | 1 - > source3/smbd/smb2_oplock.c | 3 ++- > 3 files changed, 2 insertions(+), 19 deletions(-) > >diff --git a/source3/locking/leases_util.c b/source3/locking/leases_util.c >index cb62bffbd7d6..9ae4081cd7bb 100644 >--- a/source3/locking/leases_util.c >+++ b/source3/locking/leases_util.c >@@ -71,23 +71,6 @@ uint32_t fsp_lease_type(struct files_struct *fsp) > return fsp->lease_type; > } > >-static uint32_t lease_type_is_exclusive(uint32_t lease_type) >-{ >- if ((lease_type & (SMB2_LEASE_READ | SMB2_LEASE_WRITE)) == >- (SMB2_LEASE_READ | SMB2_LEASE_WRITE)) { >- return true; >- } >- >- return false; >-} >- >-bool fsp_lease_type_is_exclusive(struct files_struct *fsp) >-{ >- uint32_t lease_type = fsp_lease_type(fsp); >- >- return lease_type_is_exclusive(lease_type); >-} >- > const struct GUID *fsp_client_guid(const files_struct *fsp) > { > return &fsp->conn->sconn->client->global->client_guid; >diff --git a/source3/locking/proto.h b/source3/locking/proto.h >index 52452e56a7b7..7fc177d7aa6d 100644 >--- a/source3/locking/proto.h >+++ b/source3/locking/proto.h >@@ -210,7 +210,6 @@ bool release_posix_lock_posix_flavour(files_struct *fsp, > /* The following definitions come from locking/leases_util.c */ > uint32_t map_oplock_to_lease_type(uint16_t op_type); > uint32_t fsp_lease_type(struct files_struct *fsp); >-bool fsp_lease_type_is_exclusive(struct files_struct *fsp); > const struct GUID *fsp_client_guid(const files_struct *fsp); > > #endif /* _LOCKING_PROTO_H_ */ >diff --git a/source3/smbd/smb2_oplock.c b/source3/smbd/smb2_oplock.c >index 1f143840b341..404746d6f6aa 100644 >--- a/source3/smbd/smb2_oplock.c >+++ b/source3/smbd/smb2_oplock.c >@@ -1248,6 +1248,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp, > .sconn = fsp->conn->sconn, .id = fsp->file_id, > }; > struct share_mode_lock *lck = NULL; >+ uint32_t fsp_lease = fsp_lease_type(fsp); > bool ok, has_read_lease; > > /* >@@ -1258,7 +1259,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp, > * the shared memory area whilst doing this. > */ > >- if (fsp_lease_type_is_exclusive(fsp)) { >+ if (fsp_lease & SMB2_LEASE_WRITE) { > /* > * There can't be any level2 oplocks, we're alone. > */ >-- >2.34.1 > > >From e5e78596f6243e3200303f27a863a9ffceee7770 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 15 Aug 2022 09:45:43 +0200 >Subject: [PATCH 8/9] s3:smbd: lease_match_break_fn() only needs > leases_db_get() once > >get_lease_type() will just call leases_db_get() again... > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >(cherry picked from commit 60ae7a5a2ed9a03d8693b9b455b7b3696386aeb1) >--- > source3/smbd/open.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index d95bab6d9a49..19b1ca8d618d 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -5483,7 +5483,7 @@ static bool lease_match_break_fn( > { > struct lease_match_break_state *state = private_data; > bool stale, equal; >- uint32_t e_lease_type; >+ uint32_t e_lease_type = SMB2_LEASE_NONE; > NTSTATUS status; > > stale = share_entry_stale_pid(e); >@@ -5500,7 +5500,7 @@ static bool lease_match_break_fn( > &e->client_guid, > &e->lease_key, > &state->id, >- NULL, /* current_state */ >+ &e_lease_type, /* current_state */ > NULL, /* breaking */ > NULL, /* breaking_to_requested */ > NULL, /* breaking_to_required */ >@@ -5511,9 +5511,9 @@ static bool lease_match_break_fn( > } else { > DBG_WARNING("Could not find version/epoch: %s\n", > nt_errstr(status)); >+ return false; > } > >- e_lease_type = get_lease_type(e, state->id); > if (e_lease_type == SMB2_LEASE_NONE) { > return false; > } >-- >2.34.1 > > >From 7bfc927f0d6766f7fdd7d122e15cfc3b4420023d Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 15 Aug 2022 09:56:15 +0200 >Subject: [PATCH 9/9] s3:smbd: let delay_for_oplock_fn() only call > leases_db_get() once > >get_lease_type() will just call leases_db_get() again for leases, >so only call it for oplocks. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125 > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Fri Aug 19 19:39:18 UTC 2022 on sn-devel-184 > >(cherry picked from commit d4f18f99d3a40a8df00beb006e2731959aa6fad9) >--- > source3/smbd/open.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index 19b1ca8d618d..825dd9e1761a 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -2529,7 +2529,7 @@ static bool delay_for_oplock_fn( > struct files_struct *fsp = state->fsp; > const struct smb2_lease *lease = state->lease; > bool e_is_lease = (e->op_type == LEASE_OPLOCK); >- uint32_t e_lease_type = get_lease_type(e, fsp->file_id); >+ uint32_t e_lease_type = SMB2_LEASE_NONE; > uint32_t break_to; > bool lease_is_breaking = false; > >@@ -2548,7 +2548,7 @@ static bool delay_for_oplock_fn( > &e->client_guid, > &e->lease_key, > &fsp->file_id, >- NULL, /* current_state */ >+ &e_lease_type, /* current_state */ > &lease_is_breaking, > NULL, /* breaking_to_requested */ > NULL, /* breaking_to_required */ >@@ -2590,6 +2590,8 @@ static bool delay_for_oplock_fn( > nt_errstr(status)); > smb_panic("leases_db_get() failed"); > } >+ } else { >+ e_lease_type = get_lease_type(e, fsp->file_id); > } > > if (!state->got_handle_lease && >-- >2.34.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=17488">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
jra
:
review+
slow
:
review+
Actions:
View
Attachments on
bug 15125
: 17488