Bug 12628 - is_locked optimization does not carry over from oplocks to leases.
Summary: is_locked optimization does not carry over from oplocks to leases.
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.6.0rc4
Hardware: x64 Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-10 15:06 UTC by Jacek Tomaka
Modified: 2017-07-31 07:53 UTC (History)
2 users (show)

See Also:


Attachments
Proposed fix for master. (8.04 KB, patch)
2017-03-14 20:39 UTC, Jeremy Allison
no flags Details
git-am fix for 4.6.next. (8.47 KB, patch)
2017-03-15 23:51 UTC, Jeremy Allison
slow: review+
Details
git-am fix for 4.5.next. (8.36 KB, patch)
2017-03-16 00:03 UTC, Jeremy Allison
slow: review+
Details
git-am fix for 4.4.next. (8.20 KB, patch)
2017-03-16 00:14 UTC, Jeremy Allison
slow: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Tomaka 2017-03-10 15:06:15 UTC
There is an optimization made to is_locked that checks if a file is oplocked and if that is the case, it does not need to do more extensive checks. It has been committed as :  https://github.com/samba-team/samba/commit/822fcec39d6597524a94b0e8b9c244d5eb62b972

Unfortunately this optimization does not carry over to leases. So when smb2 leases = yes (and in more or less recent versions of samba it is a default setting) there is much more time spent on locking. 

e.g. 

smb2 leases = yes

Skipping user change - already user
mid 7189, CreditCharge: 1, NeededCharge: 1
smbd_smb2_read: testfile.dat - fnum 3860045402
smb2: read size (4096) too small for minimum aio_read of 0
seqnum=1, fsp->brlock_seqnum=1
is_posix_locked: File testfile.dat, offset = 1933574144, count = 4096, type = READ
posix_lock_in_range: offset_out = 1933574144, count_out = 4096
posix_fcntl_getlock 12 1933574144 4096 0
fcntl_getlock fd=12 op=5 offset=1933574144 count=4096 type=0
fcntl_getlock: fd 12 is returned info 2 pid 0
posix_fcntl_getlock: Lock query call successful
brl_locktest: posix start=1933574144 len=4096 unlocked for fnum 3860045402 file testfile.dat
strict_lock_default: flavour = WINDOWS_LOCK brl start=1933574144 len=4096 unlocked for fnum 3860045402 file testfile.dat
read_file (testfile.dat): pos = 1933574144, size = 4096, returned 4096
smbd_smb2_read: file testfile.dat, fnum 3860045402, offset=1933574144 len=4096 returned 4096
smbd_smb2_read: fnum 3860045402, file testfile.dat, length=4096 offset=1933574144 read=4096
smbd_smb2_request_done_ex: idx[1] status[NT_STATUS_OK] body[16] dyn[yes:4096] at ../source3/smbd/smb2_read.c:164
smb2_set_operation_credit: requested 1, charge 1, granted 1, current possible/max 354/512, total granted/max/low/range 159/8192/7190/159
smbd_smb2_request idx[1] of 5 vectors
smb2_validate_sequence_number: clearing id 7190 (position 7190) from bitmap
smbd_smb2_request_dispatch: opcode[SMB2_OP_READ] mid = 7190



smb2 leases = no

Skipping user change - already user
mid 11722, CreditCharge: 1, NeededCharge: 1
smbd_smb2_read: testfile.dat - fnum 1413864810
smb2: read size (4096) too small for minimum aio_read of 0
is_locked: optimisation - exclusive oplock on file testfile.dat
read_file (testfile.dat): pos = 138412032, size = 4096, returned 4096
smbd_smb2_read: file testfile.dat, fnum 1413864810, offset=138412032 len=4096 returned 4096
smbd_smb2_read: fnum 1413864810, file testfile.dat, length=4096 offset=138412032 read=4096
smbd_smb2_request_done_ex: idx[1] status[NT_STATUS_OK] body[16] dyn[yes:4096] at ../source3/smbd/smb2_read.c:155
smb2_set_operation_credit: requested 1, charge 1, granted 1, current possible/max 354/512, total granted/max/low/range 159/8192/11723/159
smbd_smb2_request idx[1] of 5 vectors
smb2_validate_sequence_number: clearing id 11723 (position 3531) from bitmap
smbd_smb2_request_dispatch: opcode[SMB2_OP_READ] mid = 11723
Comment 1 Jeremy Allison 2017-03-14 20:39:14 UTC
Created attachment 13059 [details]
Proposed fix for master.

Can you check this fix please ?
Comment 2 Jeremy Allison 2017-03-15 23:51:56 UTC
Created attachment 13073 [details]
git-am fix for 4.6.next.

Cherry-picked from master.
Comment 3 Jeremy Allison 2017-03-16 00:03:52 UTC
Created attachment 13074 [details]
git-am fix for 4.5.next.

Back-ported from master.
Comment 4 Jeremy Allison 2017-03-16 00:14:21 UTC
Created attachment 13075 [details]
git-am fix for 4.4.next.

Back-ported from master.
Comment 5 Ralph Böhme 2017-03-16 05:15:31 UTC
Reassigning to Karolin for inclusion in 4.4, 4.5 and 4.6.
Comment 6 Karolin Seeger 2017-03-23 11:23:09 UTC
Pushed to autobuild-v4-{6,5}.
v4-4-test is in the security fixes only mode.
Comment 7 Karolin Seeger 2017-03-28 10:00:43 UTC
(In reply to Karolin Seeger from comment #6)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 8 Stefan Metzmacher 2017-07-12 06:48:31 UTC
(In reply to Karolin Seeger from comment #6)

This landed in v4-4-test somehow, I'll revert it from there.
Comment 9 Karolin Seeger 2017-07-31 07:10:07 UTC
(In reply to Stefan Metzmacher from comment #8)
Explicitly requested by Ralph!
Comment 10 Ralph Böhme 2017-07-31 07:53:05 UTC
(In reply to Karolin Seeger from comment #9)
Hm, sorry, I just acked the backports done by Jeremy without looking at version numbers.