On Wed, May 02, 2001 at 09:42:10PM -0500, William A. Rowe, Jr. wrote: >... > I explicitly decided not to do stacked locks/refcounting. Win32 will not > allow you to 'promote' a sharelock into an exclusivelock. Ergo, it's not > portable. >... > Index: dbm/sdbm/sdbm.c > =================================================================== > RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm.c,v > retrieving revision 1.16 > diff -u -r1.16 sdbm.c > --- dbm/sdbm/sdbm.c 2001/05/01 05:37:05 1.16 > +++ dbm/sdbm/sdbm.c 2001/05/02 20:59:32
oops! That isn't the HEAD. In fact, 1.16 has the original locking patch, so this is really just a diff of changes in how you do the lock. Thus, it isn't showing what is being added to get the fine-grained locking. I tried to regenerate the patch against HEAD (do something, rather than just being a whiner), but I got too many conflicts and I'm not entirely sure what is intended, and what shouldn't be changed. [ hmm. just realized something. maybe rebuild the patch using: apply this patch against 1.16; generate a diff against 1.15 (the new patch); fetch a clean 1.18 and apply the new patch; tweak if needed, update patch if needed. that should do it. ] [ btw, your attached patch doesn't apply cleanly to sdbm_private.h ] I do see a number of changes regarding status handling and the use of ioerr(). However, every single use of ioerr() is *right* before returning the status code. I'd say the first change is to eliminate the ioerr() macro, and the apr_sdbm_error_get|clear functions. We give the caller the status, so there is no reason for apr_sdbm to remember it. (this may simplify the locking patch a bit) (btw, note that SDBM_IOERR is unused) Regarding the no-stack/refcount thing. That could be problematic. Why don't we just say that if the user calls apr_sdbm_lock, they get an exclusive lock. In other words, don't worry about promotion -- just give them the strongest lock possible. The *current* apr_sdbm_(un)lock functions would be renamed and kept private -- the "exclusive" flag is for internal use. The external function just calls it with the "exclusive" flag enable. The lock function can do two things: record a refcount, and record the type (so we can fail if an exclusive was attempted, but a shared already exists (the promotion thing you mention)). Another comment on the patch: I think the invalidation needs to occur at unlock time. As I mentioned before, a readonly operation could see that the cache has a "valid" page, but the bits could be stale. Hmm. Maybe it would just be best to never use the "cache" when fine-grained locking is enabled. (as a data buffer, yes) Maybe that is as simple as saying that pagbno and dirbno never get set (other than to -1), so they always get read in (while holding a read and/or read/write lock). I'd need to see the full patch to see how you're currently handling invalidation, though, to be sure. Looks like it can be done... just some details and assurances to do. And a lot of the latter :-) Cheers, -g -- Greg Stein, http://www.lyra.org/
