apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: sdbm locks, second try
Date Fri, 04 May 2001 05:17:42 GMT
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/

Mime
View raw message