Patchset: Remove FAKE_LEVEL_II_OPLOCK type

[Jeremy Allison]

On Fri, Sep 13, 2013 at 06:53:06PM +0200, Stefan (metze) Metzmacher wrote:
> Hi Volker,
> 
> this looks really great, thanks! This looks like a big step in the direction
> of supporting leases:-)

Yes, with Metze's suggestions implemented please add a
"Reviewed-by: Jeremy Allison <jra at samba.org>

This is a really clever patchset !

> If possible I'd prefer the name 'read_oplocks' instead of 'level2_oplocks'
> for the new functions variable, so that we don't have to rename them
> when we get leases.

+1 - don't really care :-).

> What about adding a uint32_t and use it as flags field, we may want to
> store more information
> in future.

There's a really interesting command I learned about recently
(showing you can still teach an old dog new tricks :-) called
pahole.

It prints out the packing information on C structures defined
in .o elf files, and points out where these may be packed
together to reduce memory usage and cache lines.

Just for fun, here's the information on struct byte_range_lock
on my x86_64 box.

struct byte_range_lock {
        struct files_struct *      fsp;                  /*     0     8 */
        unsigned int               num_locks;            /*     8     4 */
        _Bool                      modified;             /*    12     1 */
        _Bool                      read_only;            /*    13     1 */

        /* XXX 2 bytes hole, try to pack */

        struct file_id             key;                  /*    16    24 */
        struct lock_struct *       lock_data;            /*    40     8 */
        struct db_record *         record;               /*    48     8 */

        /* size: 56, cachelines: 1, members: 7 */
        /* sum members: 54, holes: 1, sum holes: 2 */
        /* last cacheline: 56 bytes */
};

If we're changing the internals of struct byte_range_lock,
or any other structure in future, I'd recommend fixing them
so that pahole shows no holes.

But again, this is a change for another time IMHO - the
code is good as-is for me.

> I'd keep the optimization by using
> 
> if (EXCLUSIVE_OPLOCK_TYPE(fsp->ofsp->oplock_type)) {
>      return;
> }

+1 on this. This one would make a difference IMHO.

Cheers,

	Jeremy.