Increasing response times for byte range unlock requests.

[Jeremy Allison]

On Tue, Jun 24, 2014 at 10:05:58PM -0700, Jeremy Allison wrote:
> On Wed, Jun 25, 2014 at 10:20:37AM +0530, Hemanth Thummala wrote:
> > Hi Jeremy,
> > 
> > Clients are running SMB2. We haven't tested with SMB1 clients.
> > 
> > I think I  have identified the problem. When the blocking lock requests are
> > reprocessed successfully, we are not removing them from pending queue in
> > function reprocess_blocked_smb2_lock().
> > 
> > With the following changes, we did not observe any further degradation in
> > unlock responses.
> > 
> > bash-4.0$ diff -up smbd/smb2_lock.c.orig smbd/smb2_lock.c
> > --- smbd/smb2_lock.c.orig       2014-06-24 21:43:25.300420642 -0700
> > +++ smbd/smb2_lock.c    2014-06-24 21:43:33.290457365 -0700
> > @@ -745,6 +745,10 @@ static void reprocess_blocked_smb2_lock(
> >                 if (NT_STATUS_IS_ERR(status)) {
> >                         break;
> >                 }
> > +               else {
> > +                       /* Removed it from pending queue.*/
> > +                       remove_pending_lock(state, blr);
> > +               }
> >         }
> > 
> >         if(blr->lock_num == state->lock_count) {
> > 
> > Please let me know if this change looks fine.
> > 
> > I will also raise a bug and update all the details.
> 
> Yes, in looking over the code I was coming to
> the same conclusion (we're not removing the pending
> lock records correctly :-).
> 
> At first glance your fix looks right, but only
> because SMB2 wait lock requests can only ever
> have one element :-).
> 
> I'll check this code path in SMB1 also to make
> sure this is also being done correctly there.
> 
> Please raise a bug and I'll get patches prepared
> for master, 4.1.x and 4.0.x (all active branches
> we'll be doing releases on - 3.6.x is security
> maintanence only right now but your fix should
> do the jobs there).
> 
> Thanks a *LOT* for your help on this !

Actually I think remove_pending_lock() inside smb2_lock.c
is incorrect, due to the code it tries to execute
after the comment:

/* Remove the locks we already got. */

Having said that, again we get away with it
because SMB2 blocking locks can only ever
have one waitable element (Windows behavior
note:

3.3.5.14.2
Processing Locks
If the Locks array has more than one entry and the Flags field in any of these entries does not
have SMB2_LOCKFLAG_FAIL_IMMEDIATELY set, the server SHOULD<296> fail the request with
STATUS_INVALID_PARAMETER.

:-).

But that code really shouldn't be there :-).

Jeremy.