[PATCH] Update two optimisations that check for oplocks to the lease area

Jeremy Allison jra at samba.org
Thu May 4 11:25:04 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!

Reviewed carefully :-) and pushed ! Thanks Ralph,
nice work.

> From 56f1da66edd62ffb7192a462664227fe9c0ae3f2 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>
> ---
>  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 cb307c8..5d07ff9 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 967af02..44ab315 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.9.3
> 
> 
> From 4c224ecd9f91fb89298ee91161e866643d494902 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>
> ---
>  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 5d07ff9..af1e837 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 44ab315..5d2326a 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.9.3
> 
> 
> From b704c881885498cf308cd867106a4a33b530fd28 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>
> ---
>  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 417eda5..cd92c83 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.9.3
> 
> 
> From 2b47711a7a93a7a92f04d5083de6939b8b4685d6 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>
> ---
>  source3/smbd/oplock.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
> index cd92c83..87ca53c 100644
> --- a/source3/smbd/oplock.c
> +++ b/source3/smbd/oplock.c
> @@ -166,13 +166,17 @@ 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)) {
> +		uint32_t lease_type;
> +
>  		/*
>  		 * If we're the only one, 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));
> +
> +		lease_type = get_lease_type(d, 0);
> +		SMB_ASSERT(lease_type_is_exclusive(lease_type));
>  		return true;
>  	}
>  
> -- 
> 2.9.3
> 




More information about the samba-technical mailing list