Samba oplock level II problem

[Jeremy Allison]

On Tue, Feb 01, 2011 at 03:10:16PM +0300, Pavel Shilovsky wrote:
> 2011/2/1 Jeremy Allison <jra at samba.org>:
> > On Tue, Feb 01, 2011 at 12:21:29AM +0300, Pavel Shilovsky wrote:
> >> >
> >>
> >> Please, look at the second variant of the patch - it has extra debug
> >> message if initial_break_processing returns NULL:
> >> http://git.etersoft.ru/people/piastry/packages/?p=samba4.git;a=commitdiff;h=569ee7e3553b4e2fb0b098cca9d1a32ddc7afee6
> >
> > Got it - pushed (a similar patch) thanks !
> >
> 
> Today, I noticed that now we can have several fid with different
> oplock types in the same time. That's why we need to process
> conted_level2_oplocks_begin_default not only for level II current
> oplock type but for none oplock too (because another clients can have
> level II oplock to be broken in this call).
> 
> So, I created the patch to fix it:
> http://git.etersoft.ru/people/piastry/packages/?p=samba4.git;a=commitdiff;h=0f1a9d80fc28b2e9f8ace21819bf9af4d9da0580

I don't think this patch is needed.

If a client asks for NO_OPLOCK but the file already has
a level2 oplock granted we silently and internally grant
an oplock type of FAKE_LEVEL_II_OPLOCK.

So all entries in the open file table can be in one of
3 states:

1). Exclusive or batch oplock - only one allowed.
2). Level2 or fake level2 oplocks - multiple allowed.
3). no oplock - multiple allowed.

Note the only way we can have a combination of
Level2 or fake level2 and no-oplocks is in the
case where we're in transition from state (2) to
state (3) (and we haven't finished yet).

In that case someone (an active smbd) has already
called contend_level2_oplocks_begin_default() and
we're in the process of transitioning from state (2)
to state (3) - and we don't need to send out another
(duplicate) set of messages.

In any other case we're either in state (1) where
we dont' need to send messages or in state (3) where
we also don't need to send messages.

The FAKE_LEVEL_II_OPLOCK which is granted when a
client asks for NO_OPLOCK saves us from needing
to make this change:

-       if (!LEVEL_II_OPLOCK_TYPE(fsp->oplock_type))
+       if (EXCLUSIVE_OPLOCK_TYPE(fsp->oplock_type) ||
+           BATCH_OPLOCK_TYPE(fsp->oplock_type))
                return;

- the single check for LEVEL_II_OPLOCK_TYPE (which
covers both LEVEL_II_OPLOCK and FAKE_LEVEL_II_OPLOCK
types is correct here I think.

If you disagree please explain your reasoning further.

Jeremy.