[PATCH] Update two optimisations that check for oplocks to the lease area
Jeremy Allison
jra at samba.org
Fri May 5 06:29:21 UTC 2017
On Thu, May 04, 2017 at 12:26:48PM +0200, Ralph Böhme wrote:
> Hi!
>
> This is the one I talked about in my talk.
>
> I took another look and found another plase where we need this.
>
> Both combined eat 12%CPU in perf, not including syscalls.
>
> Please review carefully and push if ok. Thanks!
OK, there were some flaws in the last patch in this set
(found by the smb2.lease tests).
Can you try this version of your patch instead ? It changes
the assertion in the last patch (num_oplocks one) to ensure
that all existing leases in the array are exclusive (remember
with leases the client can open multiple times with compatible
leases so long as the client guid is the same - it's the multi-handle
advantage leases have over oplocks).
The logic in the assertion now looks like:
if (fsp_lease_type_is_exclusive(fsp)) {
/*
* If we're fully exclusive, we don't need a brlock entry
*/
remove_stale_share_mode_entries(d);
for (i=0; i<d->num_share_modes; i++) {
struct share_mode_entry *e = &d->share_modes[i];
uint32_t e_lease_type = get_lease_type(d, e);
SMB_ASSERT(lease_type_is_exclusive(e_lease_type));
}
return true;
}
instead of checking for only one entry.
Please push if happy !
Jeremy.
-------------- next part --------------
>From 3ef89d36c145341b230c67752434da521d87df05 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 May 2017 11:50:01 +0200
Subject: [PATCH 1/4] s3/locking: add const to fsp_lease_type
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12766
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/locking/leases_util.c | 2 +-
source3/locking/proto.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/locking/leases_util.c b/source3/locking/leases_util.c
index cb307c88d36..5d07ff9dd7b 100644
--- a/source3/locking/leases_util.c
+++ b/source3/locking/leases_util.c
@@ -46,7 +46,7 @@ uint32_t map_oplock_to_lease_type(uint16_t op_type)
return ret;
}
-uint32_t fsp_lease_type(struct files_struct *fsp)
+uint32_t fsp_lease_type(const struct files_struct *fsp)
{
if (fsp->oplock_type == LEASE_OPLOCK) {
return fsp->lease->lease.lease_state;
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 967af02bb26..44ab315660b 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -257,6 +257,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);
+uint32_t fsp_lease_type(const struct files_struct *fsp);
#endif /* _LOCKING_PROTO_H_ */
--
2.11.0
>From e33e9d19d183aacb7b652538d5b8069b4621e307 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 May 2017 11:50:56 +0200
Subject: [PATCH 2/4] s3/locking: helper functions for lease types
Add some helper functions that will be used to update a bunch of checks
for exclusive oplocks to the lease area.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12766
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/locking/leases_util.c | 17 +++++++++++++++++
source3/locking/proto.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/source3/locking/leases_util.c b/source3/locking/leases_util.c
index 5d07ff9dd7b..af1e8374128 100644
--- a/source3/locking/leases_util.c
+++ b/source3/locking/leases_util.c
@@ -53,3 +53,20 @@ uint32_t fsp_lease_type(const struct files_struct *fsp)
}
return map_oplock_to_lease_type(fsp->oplock_type);
}
+
+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(const struct files_struct *fsp)
+{
+ uint32_t lease_type = fsp_lease_type(fsp);
+
+ return lease_type_is_exclusive(lease_type);
+}
diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index 44ab315660b..5d2326a40aa 100644
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -258,5 +258,7 @@ 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(const struct files_struct *fsp);
+uint32_t lease_type_is_exclusive(uint32_t lease_type);
+bool fsp_lease_type_is_exclusive(const struct files_struct *fsp);
#endif /* _LOCKING_PROTO_H_ */
--
2.11.0
>From 75caef3d655b56d6af92cca7b7d768041f3f7404 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 20 Apr 2017 21:37:37 +0200
Subject: [PATCH 3/4] s3/smbd: update exclusive oplock optimisation to the
lease area
This is similar to 9533a55ee5ffe430589dcea845851b84876ef656 but this
time in the contend_level2_oplocks_begin_default() function.
The idea of the optimisation is to avoid expensive db queries in
locking.tdb if we *know* we're the only open.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12766
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/oplock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 417eda530e3..cd92c8334f0 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -1044,7 +1044,7 @@ static void contend_level2_oplocks_begin_default(files_struct *fsp,
* the shared memory area whilst doing this.
*/
- if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
+ if (fsp_lease_type_is_exclusive(fsp)) {
/*
* There can't be any level2 oplocks, we're alone.
*/
--
2.11.0
>From adbed0363662b5331096be65c2856cfa53d86a7c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 May 2017 11:52:16 +0200
Subject: [PATCH 4/4] s3/smbd: update exclusive oplock optimisation to the
lease area
Update an optimisation in update_num_read_oplocks() that checks for
exclusive oplocks to the lease area.
The idea of the optimisation is to avoid expensive db queries in
brlock.tdb if we *know* we're the only open.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12766
Signed-off-by: Ralph Boehme <slow at samba.org>
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/oplock.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index cd92c8334f0..aa060ad3747 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -166,13 +166,18 @@ bool update_num_read_oplocks(files_struct *fsp, struct share_mode_lock *lck)
uint32_t num_read_oplocks = 0;
uint32_t i;
- if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type)) {
+ if (fsp_lease_type_is_exclusive(fsp)) {
/*
- * If we're the only one, we don't need a brlock entry
+ * If we're fully exclusive, we don't need a brlock entry
*/
remove_stale_share_mode_entries(d);
- SMB_ASSERT(d->num_share_modes == 1);
- SMB_ASSERT(EXCLUSIVE_OPLOCK_TYPE(d->share_modes[0].op_type));
+
+ for (i=0; i<d->num_share_modes; i++) {
+ struct share_mode_entry *e = &d->share_modes[i];
+ uint32_t e_lease_type = get_lease_type(d, e);
+
+ SMB_ASSERT(lease_type_is_exclusive(e_lease_type));
+ }
return true;
}
--
2.11.0
More information about the samba-technical
mailing list