Remove serverids_exist from parse_share_modes?

Jeremy Allison jra at samba.org
Tue May 15 11:30:40 MDT 2012


On Tue, May 15, 2012 at 04:41:05PM +0200, Volker Lendecke wrote:
> Hi!
> 
> Under
> 
> https://git.samba.org/?p=vl/samba.git/.git;a=shortlog;h=refs/heads/wip
> 
> find a patchset that removes the serverids_exist check from
> parse_share_modes. It replaces the central check by checks
> when conflicts happen. Thus it removes the assumption that
> the data from get_share_mode_lock & friends only contains
> entries with live processes attached. In a normally
> functioning server, this is certainly still the case because
> everybody is supposed to clean up behind itself. The
> different behavior shows when smbds crash hard.
> 
> There are two reasons for this change. For busy files (for
> example a popular directory everybody watches with notify)
> we are beating the serverid.tdb pretty hard by checking
> every entry on every open and close possibly multiple times.
> This is a penalty for well-functioning servers. In this
> particular case those checks are not strictly required
> anyway because no oplocks are granted that would need to be
> revoked and no share mode locks will happen. So we could
> live with stale entries. The second reason for this change
> is a requirement from the implementation of durable file
> handles. With DH we can not clean up all entries without
> processes behind, this needs to be more fine-grained.
> 
> I have tried to catch all cases where we rely on proper
> cleanup. The cleanup3 test plays some tricks with
> locking.tdb, forcing smbd to excercise some of the new code.
> Unfortunately we can't run this in make test because we have
> no access to the server's locking.tdb from smbtorture, so we
> have to rely on manual tests for this new functionality.
> 
> There's one downside so far: For crashing servers the code
> leaves stale entries behind as long as they do not conflict
> with other opens. I am not sure it is worth a regular
> cleanup process which would put quite some load on the tdb
> files whenever it is run on large and busy servers.
> 
> Comments?

I love this patchset (as we discussed @ SambaXP :-).

If you can get someone to review before I get to it please
feel free to push (I have to finish reviewing Andrews ACL
patches first), otherwise I'll put this patchset next on
my review list !

Great work.

Jeremy.


More information about the samba-technical mailing list