From f04f423ddda8c2f6eae73911b3da4b914d8cb251 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Sun, 29 Sep 2024 10:44:06 +0200 Subject: [PATCH 1/5] cache_obj: refactor: pull out extension signal to client --- bin/varnishd/cache/cache_obj.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c index 9ffa373f3bf..aa4ef2ac4d3 100644 --- a/bin/varnishd/cache/cache_obj.c +++ b/bin/varnishd/cache/cache_obj.c @@ -258,6 +258,17 @@ ObjExtend(struct worker *wrk, struct objcore *oc, ssize_t l, int final) /*==================================================================== */ +static inline void +objSignalFetchLocked(const struct objcore *oc, uint64_t l) +{ + if (oc->boc->transit_buffer > 0) { + assert(oc->flags & OC_F_TRANSIENT); + /* Signal the new client position */ + oc->boc->delivered_so_far = l; + PTOK(pthread_cond_signal(&oc->boc->cond)); + } +} + uint64_t ObjWaitExtend(const struct worker *wrk, const struct objcore *oc, uint64_t l, enum boc_state_e *statep) @@ -272,13 +283,8 @@ ObjWaitExtend(const struct worker *wrk, const struct objcore *oc, uint64_t l, while (1) { rv = oc->boc->fetched_so_far; assert(l <= rv || oc->boc->state == BOS_FAILED); - if (oc->boc->transit_buffer > 0) { - assert(oc->flags & OC_F_TRANSIENT); - /* Signal the new client position */ - oc->boc->delivered_so_far = l; - PTOK(pthread_cond_signal(&oc->boc->cond)); - } state = oc->boc->state; + objSignalFetchLocked(oc, l); if (rv > l || state >= BOS_FINISHED) break; (void)Lck_CondWait(&oc->boc->cond, &oc->boc->mtx); From c76901cd4c9410f01df7219291537e5e3bdb71a4 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Sun, 29 Sep 2024 10:47:13 +0200 Subject: [PATCH 2/5] cache_obj: Add a more generic boc extension notification facility This commit prepares a more generic busy object extension notification facility in preparation of the asynchronous iteration facility introduced with the next commit. It makes more sense when looked at in context of that, but the changes constitute a fairly independent part and thus have been separated. Background To support streaming of busy objects (delivery to a client while the body is being fetched), the Object API provides ObjWaitExtend(), which is called by storage iterators to learn the available amount of body data and to wait for more if all available data has been processed (= sent to the client, usually). The other end of the facility is ObjExtend(), which is called by the fetch side of storage to update the available amount of body data and wake up any clients blocking in ObjWaitExtend(). This facility recently got extended by a blocking operation in the other direction, where the writing side blocks if the amount of unsent data exceeds the amount configured via the transit_buffer. Why this change? The existing facility is based on the model of blocking threads. In order to support asynchronous iterators, where a single thread may serve multiple requests, we need a different, non-blocking model with notifications. Implementation The basic implementation idea is to introduce a variant of ObjWaitExtend() which, rather than blocking on a condition variable, registers a callback function to be called when the condition variable got signalled. This is ObjVAIGetExtend(): It returns the updated extension, if available, _or_ registers the callback. To implement the actual callback, we add to struct boc a queue (struct vai_q_head), whose elements are basically the notification callback with two pointers: the caller gets a private pointer as well as vai_hdl is an opaque handle owned by storage. ObjExtend() now also works the list of registered callbacks. ObjVAICancel() removes a callback when the caller is no longer interested or needs to reclaim the queue entry. --- bin/varnishd/cache/cache.h | 14 ++++++ bin/varnishd/cache/cache_obj.c | 76 +++++++++++++++++++++++++++-- bin/varnishd/cache/cache_obj.h | 20 ++++++++ bin/varnishd/cache/cache_varnishd.h | 4 ++ 4 files changed, 109 insertions(+), 5 deletions(-) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index b52cbab01a9..f6a0bc4f182 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -286,6 +286,10 @@ enum boc_state_e { #include "tbl/boc_state.h" }; +// cache_obj.h vai notify +struct vai_qe; +VSLIST_HEAD(vai_q_head, vai_qe); + struct boc { unsigned magic; #define BOC_MAGIC 0x70c98476 @@ -298,6 +302,7 @@ struct boc { uint64_t fetched_so_far; uint64_t delivered_so_far; uint64_t transit_buffer; + struct vai_q_head vai_q_head; }; /* Object core structure --------------------------------------------- @@ -770,6 +775,15 @@ int ObjGetDouble(struct worker *, struct objcore *, enum obj_attr, double *); int ObjGetU64(struct worker *, struct objcore *, enum obj_attr, uint64_t *); int ObjCheckFlag(struct worker *, struct objcore *, enum obj_flags of); +/*==================================================================== + * ObjVAI...(): Asynchronous Iteration + * + * see comments in cache_obj.c for usage + */ + +typedef void *vai_hdl; +typedef void vai_notify_cb(vai_hdl, void *priv); + /* cache_req_body.c */ ssize_t VRB_Iterate(struct worker *, struct vsl_log *, struct req *, objiterate_f *func, void *priv); diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c index aa4ef2ac4d3..c5f2e54fc8a 100644 --- a/bin/varnishd/cache/cache_obj.c +++ b/bin/varnishd/cache/cache_obj.c @@ -231,6 +231,29 @@ obj_extend_condwait(const struct objcore *oc) (void)Lck_CondWait(&oc->boc->cond, &oc->boc->mtx); } +// notify of an extension of the boc or state change +//lint -sem(obj_boc_notify_Unlock, thread_unlock) + +static void +obj_boc_notify_Unlock(struct boc *boc) +{ + struct vai_qe *qe, *next; + + PTOK(pthread_cond_broadcast(&boc->cond)); + qe = VSLIST_FIRST(&boc->vai_q_head); + VSLIST_FIRST(&boc->vai_q_head) = NULL; + while (qe != NULL) { + CHECK_OBJ(qe, VAI_Q_MAGIC); + AN(qe->flags & VAI_QF_INQUEUE); + qe->flags &= ~VAI_QF_INQUEUE; + next = VSLIST_NEXT(qe, list); + VSLIST_NEXT(qe, list) = NULL; + qe->cb(qe->hdl, qe->priv); + qe = next; + } + Lck_Unlock(&boc->mtx); +} + void ObjExtend(struct worker *wrk, struct objcore *oc, ssize_t l, int final) { @@ -241,14 +264,13 @@ ObjExtend(struct worker *wrk, struct objcore *oc, ssize_t l, int final) AN(om->objextend); assert(l >= 0); - Lck_Lock(&oc->boc->mtx); if (l > 0) { + Lck_Lock(&oc->boc->mtx); obj_extend_condwait(oc); om->objextend(wrk, oc, l); oc->boc->fetched_so_far += l; - PTOK(pthread_cond_broadcast(&oc->boc->cond)); + obj_boc_notify_Unlock(oc->boc); } - Lck_Unlock(&oc->boc->mtx); assert(oc->boc->state < BOS_FINISHED); if (final && om->objtrimstore != NULL) @@ -294,6 +316,51 @@ ObjWaitExtend(const struct worker *wrk, const struct objcore *oc, uint64_t l, *statep = state; return (rv); } + +// get a new extension _or_ register a notification +uint64_t +ObjVAIGetExtend(struct worker *wrk, const struct objcore *oc, uint64_t l, + enum boc_state_e *statep, struct vai_qe *qe) +{ + enum boc_state_e state; + uint64_t rv; + + (void) wrk; + CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); + CHECK_OBJ_NOTNULL(oc->boc, BOC_MAGIC); + CHECK_OBJ_NOTNULL(qe, VAI_Q_MAGIC); + Lck_Lock(&oc->boc->mtx); + rv = oc->boc->fetched_so_far; + assert(l <= rv || oc->boc->state == BOS_FAILED); + state = oc->boc->state; + objSignalFetchLocked(oc, l); + if (l == rv && state < BOS_FINISHED && + (qe->flags & VAI_QF_INQUEUE) == 0) { + qe->flags |= VAI_QF_INQUEUE; + VSLIST_INSERT_HEAD(&oc->boc->vai_q_head, qe, list); + } + Lck_Unlock(&oc->boc->mtx); + if (statep != NULL) + *statep = state; + return (rv); +} + +void +ObjVAICancel(struct worker *wrk, struct boc *boc, struct vai_qe *qe) +{ + + (void) wrk; + CHECK_OBJ_NOTNULL(boc, BOC_MAGIC); + CHECK_OBJ_NOTNULL(qe, VAI_Q_MAGIC); + + Lck_Lock(&boc->mtx); + // inefficient, but should be rare + if ((qe->flags & VAI_QF_INQUEUE) != 0) + VSLIST_REMOVE(&boc->vai_q_head, qe, vai_qe, list); + qe->flags = 0; + Lck_Unlock(&boc->mtx); +} + /*==================================================================== */ @@ -319,8 +386,7 @@ ObjSetState(struct worker *wrk, const struct objcore *oc, Lck_Lock(&oc->boc->mtx); oc->boc->state = next; - PTOK(pthread_cond_broadcast(&oc->boc->cond)); - Lck_Unlock(&oc->boc->mtx); + obj_boc_notify_Unlock(oc->boc); } /*==================================================================== diff --git a/bin/varnishd/cache/cache_obj.h b/bin/varnishd/cache/cache_obj.h index 1f936a53485..f6ee8618e62 100644 --- a/bin/varnishd/cache/cache_obj.h +++ b/bin/varnishd/cache/cache_obj.h @@ -50,6 +50,26 @@ typedef void *objsetattr_f(struct worker *, struct objcore *, enum obj_attr attr, ssize_t len, const void *ptr); typedef void objtouch_f(struct worker *, struct objcore *, vtim_real now); +/* called by Obj/storage to notify that the lease function (vai_lease_f) can be + * called again after a -EAGAIN / -ENOBUFS return value + * NOTE: + * - the callback gets executed by an arbitrary thread + * - WITH the boc mtx held + * so it should never block and be efficient + */ + +/* notify entry added to struct boc::vai_q_head */ +struct vai_qe { + unsigned magic; +#define VAI_Q_MAGIC 0x573e27eb + unsigned flags; +#define VAI_QF_INQUEUE (1U<<0) + VSLIST_ENTRY(vai_qe) list; + vai_notify_cb *cb; + vai_hdl hdl; + void *priv; +}; + struct obj_methods { /* required */ objfree_f *objfree; diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index 31fd57eb6e4..743029617d0 100644 --- a/bin/varnishd/cache/cache_varnishd.h +++ b/bin/varnishd/cache/cache_varnishd.h @@ -347,6 +347,10 @@ void *ObjSetAttr(struct worker *, struct objcore *, enum obj_attr, int ObjCopyAttr(struct worker *, struct objcore *, struct objcore *, enum obj_attr attr); void ObjBocDone(struct worker *, struct objcore *, struct boc **); +// VAI +uint64_t ObjVAIGetExtend(struct worker *, const struct objcore *, uint64_t, + enum boc_state_e *, struct vai_qe *); +void ObjVAICancel(struct worker *, struct boc *, struct vai_qe *); int ObjSetDouble(struct worker *, struct objcore *, enum obj_attr, double); int ObjSetU64(struct worker *, struct objcore *, enum obj_attr, uint64_t); From bd88aad6f95976feda921e78f7f37d8d88adcb3a Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Sun, 29 Sep 2024 10:50:44 +0200 Subject: [PATCH 3/5] cache_obj: Add an asynchronous iteration API This commit adds a new object iteration API to support asynchronous IO. Background To process object bodies, the Object API so far provides ObjIterate(), which calls a storage specific iterator function. It in turn calls a caller-provided objiterate_f function on individual, contigious segments of data (extents). In turn, objiterate_f gets called with either no flags, or one of OBJ_ITER_FLUSH and OBJ_ITER_END. The storage iterator uses these flags to signal lifetime of the provided entents: They remain valid until a flag is present, so the caller may delay use until an extent is provided with a flag sent. Or, seen from the other end, objiterate_f needs to ensure it does not use any previously received extent when a flag is seen. objiterate_f can not make any assumption as to if or when it is going to be called, if the storage iterator function needs time to retrieve data or a streaming fetch is in progress, then so be it, objiterate_f may eventually get called or not. Or, again seen from the other end, the storage iterator function assumes being called from a thread and may block at any time. Why this change? The model described above is fundamentally incompatible with asynchronous, event driven IO models, where a single thread might serve multiple requests in parallel to benefit from efficiency gains and thus no called function must ever block. This additional API is intended to provide an interface suitable for such asynchronous models. As before, also the asynchronous iterator is owned by a storage specific implementation, but now, instead of using a thread for its state, that state exists in a data structure opaque to the caller. API Usage The basic model for the API is that the storage engine "leases" to the caller a number of extents, which the caller is then free to use until it returns the leases to the storage engine. The storage engine can also signal to the caller that it can not return more extents unless some are returned or that it simply can not return any at this time for other reasons (for example, because it is waiting for data on a streaming fetch). In both cases, the storage engine promises to call the caller's notification function when it is ready to provide more extents or iteration has ended. The API consists of four functions: - ObjVAIinit() requests an asynchronous iteration on an object. The caller provides an optional workspace for the storage engine to use for its state, and the notification callback / private pointer introduced with the previous commit. Its use is explained below. ObjVAIinit() returns either an opaque handle owned jointly by the Object layer in Varnish-Cache and the storage engine, or NULL if the storage engine does not provide asynchronous iteration. All other API functions work on the handle returned by ObjVAIinit(): - ObjVAIlease() returns the next extents from the object body in a caller-prodived array. Eeach extent is a struct viov, which contains a struct iovec (see iovec(3type) / readv(2)) with the actual extent, a flags field to signal the last extent (mirroring OBJ_ITER_END) and an integer identifying the lease. The "lease" integer (uint64_t) is opaque to the caller and needs to be returned as-is later, but is guaranteed by storage to be a multiple of 8. This can be used by the caller to temporily stash a tiny amount of additional state into the lease. ObjVAIlease either returns a positive integer with a number of available leases, zero if the end of the object has been reached, or a negative integer for "call again later" and error conditions: -EAGAIN signals that no more data is available at this point, and the storage engine will call the notification function when the condition changes. -ENOBUFS behaves identically, but also requires the caller to return more leases. -EPIPE mirrors BOS_FAILED on the busy object. Any other -(errno) can be used by the storage engine to signal other error conditions. - ObjVAIreturn() returns leases to the storage when the caller is done with them For efficiency, leases should be returned in batches, and latest if ObjVAIlease() requests so by returning -ENOBUFS. - ObjVAIfini() finalizes iteration. The handle must not be used thereafter. Implementation One particular aspect of the implementation is that the storage engine returns the "lease", "return" and "fini" functions to be used with the handle. This allows the storage engine to provide functions tailored to the attributes of the storage object, for example streaming fetches require more elaborate handling than settled storage objects. Consequently, the vai_hdl which is, by design, opaque to the caller, is not entirely opaque to the object layer: The implementation requires it to start with a struct vai_hdl_preamble containing the function pointers to be called by ObjVAIlease(), ObjVAIreturn() and ObjVAIfini(). The return function pointer vai_return is optional. More details about the implementation will become clear with the next commit, which implements SML's synchronous iterator using the new API. --- bin/varnishd/cache/cache.h | 16 ++++++ bin/varnishd/cache/cache_obj.c | 94 ++++++++++++++++++++++++++++++++++ bin/varnishd/cache/cache_obj.h | 80 ++++++++++++++++++++++++++++- 3 files changed, 189 insertions(+), 1 deletion(-) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index f6a0bc4f182..ca62005dc9a 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -42,6 +42,7 @@ #include #include #include +#include #include "vdef.h" #include "vrt.h" @@ -784,6 +785,21 @@ int ObjCheckFlag(struct worker *, struct objcore *, enum obj_flags of); typedef void *vai_hdl; typedef void vai_notify_cb(vai_hdl, void *priv); +struct viov { + unsigned magic; +#define VIOV_MAGIC 0x7a107a10 + unsigned flags; +#define VIOV_F_END 1 // last VIOV + uint64_t lease; + struct iovec iov; +}; + +vai_hdl ObjVAIinit(struct worker *, struct objcore *, struct ws *, + vai_notify_cb *, void *); +int ObjVAIlease(struct worker *, vai_hdl, struct viov *, int); +void ObjVAIreturn(struct worker *, vai_hdl, uint64_t *, int); +void ObjVAIfini(struct worker *, vai_hdl *); + /* cache_req_body.c */ ssize_t VRB_Iterate(struct worker *, struct vsl_log *, struct req *, objiterate_f *func, void *priv); diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c index c5f2e54fc8a..683c5a4f395 100644 --- a/bin/varnishd/cache/cache_obj.c +++ b/bin/varnishd/cache/cache_obj.c @@ -183,6 +183,100 @@ ObjIterate(struct worker *wrk, struct objcore *oc, return (om->objiterator(wrk, oc, priv, func, final)); } +/*==================================================================== + * ObjVAI...(): Asynchronous Iteration + * + * + * ObjVAIinit() returns an opaque handle, or NULL if not supported + * + * A VAI handle must not be used concurrently + * + * the vai_notify_cb(priv) will be called asynchronously by the storage + * engine when a -EAGAIN / -ENOBUFS condition is over and ObjVAIlease() + * can be called again. + * + * Note: + * - the callback gets executed by an arbitrary thread + * - WITH the boc mtx held + * so it should never block and only do minimal work + * + * ObjVAIlease() fills the viov array passed in with leases. returns: + * + * -EAGAIN: nothing available at the moment, storage will notify, no use to + * call again until notification + * -ENOBUFS: caller needs to return leases, storage will notify + * -EPIPE: BOS_FAILED for busy object + * -(errno): other problem, fatal + * 0: EOF + * n: number of viovs filled + * + * struct viov: + * + * the returned leases can be used by the caller until returned with + * ObjVAIreturn(). The storage guarantees that the lease member is a + * multiple of 8 (that is, the lower three bits are zero). These can be + * used by the caller between lease and return, but must be returned to + * zero before returning. + * + * ObjVAIreturn() returns leases + * + * it must be called with an array of lease values from viovs + * received when the caller can guarantee that they are no longer accessed + * + * ObjVAIfini() finalized iteration + * + * it must be called when iteration is done, irrespective of error status + */ + +vai_hdl +ObjVAIinit(struct worker *wrk, struct objcore *oc, struct ws *ws, + vai_notify_cb *cb, void *cb_priv) +{ + const struct obj_methods *om = obj_getmethods(oc); + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + + if (om->vai_init == NULL) + return (NULL); + return (om->vai_init(wrk, oc, ws, cb, cb_priv)); +} + +int +ObjVAIlease(struct worker *wrk, vai_hdl vhdl, struct viov *viov, int n) +{ + struct vai_hdl_preamble *vaip = vhdl; + + AN(vaip); + assert(vaip->magic2 == VAI_HDL_PREAMBLE_MAGIC2); + AN(vaip->vai_lease); + return (vaip->vai_lease(wrk, vhdl, viov, n)); +} + +void +ObjVAIreturn(struct worker *wrk, vai_hdl vhdl, uint64_t *leases, int n) +{ + struct vai_hdl_preamble *vaip = vhdl; + + AN(vaip); + assert(vaip->magic2 == VAI_HDL_PREAMBLE_MAGIC2); + /* vai_return is optional */ + if (vaip->vai_return == NULL) + return; + vaip->vai_return(wrk, vhdl, leases, n); +} + +void +ObjVAIfini(struct worker *wrk, vai_hdl *vhdlp) +{ + AN(vhdlp); + struct vai_hdl_preamble *vaip = *vhdlp; + + AN(vaip); + assert(vaip->magic2 == VAI_HDL_PREAMBLE_MAGIC2); + AN(vaip->vai_lease); + return (vaip->vai_fini(wrk, vhdlp)); +} + /*==================================================================== * ObjGetSpace() * diff --git a/bin/varnishd/cache/cache_obj.h b/bin/varnishd/cache/cache_obj.h index f6ee8618e62..7ab7a8cc3de 100644 --- a/bin/varnishd/cache/cache_obj.h +++ b/bin/varnishd/cache/cache_obj.h @@ -70,6 +70,83 @@ struct vai_qe { void *priv; }; +#define VAI_ASSERT_LEASE(x) AZ((x) & 0x7) + +/* + * start an iteration. the ws can we used (reserved) by storage + * the void * will be passed as the second argument to vai_notify_cb + */ +typedef vai_hdl vai_init_f(struct worker *, struct objcore *, struct ws *, + vai_notify_cb *, void *); + +/* + * lease io vectors from storage + * + * vai_hdl is from vai_init_f + * viov / viovcnt is space provided by the caller to return leases + * + * return: + * -EAGAIN: nothing available at the moment, storage will notify, no use to + * call again until notification + * -ENOBUFS: caller needs to return leases, storage will notify + * -EPIPE: BOS_FAILED for busy object + * -(errno): other problem, fatal + * 0: EOF + * n: number of viovs filled + */ +typedef int vai_lease_f(struct worker *, vai_hdl, struct viov *viov, int viovcnt); + +/* + * return leases + */ +typedef void vai_return_f(struct worker *,vai_hdl, uint64_t *leases, int leasecnt); + +/* + * finish iteration, vai_return_f must have been called on all leases + */ +typedef void vai_fini_f(struct worker *, vai_hdl *); + +/* + * vai_hdl must start with this preamble such that when cast to it, cache_obj.c + * has access to the methods. + * + * The first magic is owned by storage, the second magic is owned by cache_obj.c + * and must be initialized to VAI_HDL_PREAMBLE_MAGIC2 + * + */ + +struct vai_hdl_preamble { + unsigned magic; // owned by storage + unsigned magic2; +#define VAI_HDL_PREAMBLE_MAGIC2 0x7a15d162 + vai_lease_f *vai_lease; + vai_return_f *vai_return; // optional + uintptr_t reserve[4]; // abi fwd compat + vai_fini_f *vai_fini; +}; + +#define INIT_VAI_HDL(to, x) do { \ + (void)memset(to, 0, sizeof *(to)); \ + (to)->preamble.magic = (x); \ + (to)->preamble.magic2 = VAI_HDL_PREAMBLE_MAGIC2; \ +} while (0) + +#define CHECK_VAI_HDL(obj, x) do { \ + assert(obj->preamble.magic == (x)); \ + assert(obj->preamble.magic2 == VAI_HDL_PREAMBLE_MAGIC2);\ +} while (0) + +#define CHECK_VAI_HDL_NOTNULL(obj, x) do { \ + AN(obj); \ + CHECK_VAI_HDL(obj, x); \ +} while (0) + +#define CAST_VAI_HDL_NOTNULL(obj, ptr, x) do { \ + AN(ptr); \ + (obj) = (ptr); \ + CHECK_VAI_HDL(obj, x); \ +} while (0) + struct obj_methods { /* required */ objfree_f *objfree; @@ -84,5 +161,6 @@ struct obj_methods { objslim_f *objslim; objtouch_f *objtouch; objsetstate_f *objsetstate; + /* async iteration (VAI) */ + vai_init_f *vai_init; }; - From da158fa31e02ba2422a8ce0060787d811d88d6fc Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Sun, 29 Sep 2024 10:54:09 +0200 Subject: [PATCH 4/5] storage_simple: Implement asynchronous iteration and use it for the iterator This commit implements the asynchronous iteration API defined and described in previous commits for the simple storage layer and reimplements the synchronous iterator with it. This commit message does not provide background information, please refer to the two previous commits. Implementation sml_ai_init() initializes the handle and choses either a simple or more elaborate "boc" lease function depending on whether or not a streaming fetch is ongoing (boc present). sml_ai_lease_simple() is just that, dead simple. It iterates the storage segment list and fills the struct viov array provided by the caller. It is a good starting point into the implementation. sml_ai_lease_boc() handles the busy case and is more elaborate due to the nature of streaming fetches. It first calls ObjVAIGetExtend() to get the current extent. If no data is available, it returns the appropriate value. Other than that, is basically does the same things as sml_ai_lease_simple() with these exceptions: It also needs to return partial extends ("fragments") and it needs to handle the case where the last available segment is reached, in which case there is no successor to store for the next invocation, but also the last segment could get returned and thus freed before the next invocation. sml_ai_return() is only used for the "boc" case. It removes returned full segments from the list and then frees them outside the boc mtx. sml_ai_fini() is straight forward and should not need explanation. Implementation of sml_iterator() using the new API To reimplement the existing synchronous iterator based on the new API, we first need a little facility to block waiting for a notification. This is struct sml_notify with the four sml_notify* functions. sml_notify() is the callback, sml_notify_wait() blocks waiting for a notification to arrive. Until it runs out of work, the iterator performs these steps: ObjVAIlease() is called repeatedly until either the viov array is full or a negative value is returned. This allows the rest of the code to react to the next condition appropriately by sending an OBJ_ITER_FLUSH with the last lease only. Calling func() on each extent is trivial, the complications only come from handling OBJ_ITER_FLUSH, "just in time" returns and error handling. --- bin/varnishd/storage/storage_persistent.c | 1 + bin/varnishd/storage/storage_simple.c | 527 +++++++++++++++++----- bin/varnishd/storage/storage_simple.h | 1 - bin/varnishtest/tests/c00111.vtc | 5 +- 4 files changed, 424 insertions(+), 110 deletions(-) diff --git a/bin/varnishd/storage/storage_persistent.c b/bin/varnishd/storage/storage_persistent.c index df2fc9128e7..9b37de27590 100644 --- a/bin/varnishd/storage/storage_persistent.c +++ b/bin/varnishd/storage/storage_persistent.c @@ -692,6 +692,7 @@ smp_init(void) smp_oc_realmethods.objsetattr = SML_methods.objsetattr; smp_oc_realmethods.objtouch = LRU_Touch; smp_oc_realmethods.objfree = smp_oc_objfree; + smp_oc_realmethods.vai_init = SML_methods.vai_init; } /*-------------------------------------------------------------------- diff --git a/bin/varnishd/storage/storage_simple.c b/bin/varnishd/storage/storage_simple.c index 1dea4dc3940..835a8f42fda 100644 --- a/bin/varnishd/storage/storage_simple.c +++ b/bin/varnishd/storage/storage_simple.c @@ -31,6 +31,8 @@ #include "config.h" +#include + #include "cache/cache_varnishd.h" #include "cache/cache_obj.h" @@ -304,130 +306,440 @@ sml_objfree(struct worker *wrk, struct objcore *oc) wrk->stats->n_object--; } +// kept for reviewers - XXX remove later +#undef VAI_DBG + +struct sml_hdl { + struct vai_hdl_preamble preamble; +#define SML_HDL_MAGIC 0x37dfd996 + struct vai_qe qe; + struct ws *ws; // NULL is malloc() + struct objcore *oc; + struct object *obj; + const struct stevedore *stv; + struct boc *boc; + + struct storage *st; // updated by _lease() + struct storage *last; // if st == NULL + + // only for _lease_boc() + uint64_t st_off; // already returned fragment of current st + uint64_t avail, returned; +}; + +static inline void +sml_ai_viov_fill(struct viov *viov, struct storage *st) +{ + INIT_OBJ(viov, VIOV_MAGIC); + viov->iov.iov_base = st->ptr; + viov->iov.iov_len = st->len; + viov->lease = (uintptr_t)st; + VAI_ASSERT_LEASE(viov->lease); +} + +static int +sml_ai_lease_simple(struct worker *wrk, vai_hdl vhdl, struct viov *viov, int viovcnt) +{ + struct sml_hdl *hdl; + struct storage *st, *next; + int r = 0; + + (void) wrk; + CAST_VAI_HDL_NOTNULL(hdl, vhdl, SML_HDL_MAGIC); + AN(viov); + AN(viovcnt); + + AZ(hdl->st_off); + st = hdl->st; + while (st != NULL && viovcnt > 0) { + next = VTAILQ_PREV(st, storagehead, list); + CHECK_OBJ_ORNULL(next, STORAGE_MAGIC); + sml_ai_viov_fill(viov, st); + viov->flags = (next == NULL) ? VIOV_F_END : 0; + + viov++; + r++; + viovcnt--; + st = next; + } + hdl->st = st; + return (r); +} + +/* + * on leases while streaming (with a boc): + * + * SML uses the lease return facility to implement the "free behind" for + * OC_F_TRANSIENT objects. When streaming, we also return leases on + * fragments of sts, but we must only "free behind" when we are done with the + * last fragment. + * + * So we use a magic lease to signal "this is only a fragment", which we ignore + * on returns + */ + +// not using 0 to be able to catch accidental null pointers +static const uint64_t sml_ai_lease_frag = 0x8; + +static int +sml_ai_lease_boc(struct worker *wrk, vai_hdl vhdl, struct viov *viov, int viovcnt) +{ + struct sml_hdl *hdl; + enum boc_state_e state; + struct storage *next; + int r = 0; + + CAST_VAI_HDL_NOTNULL(hdl, vhdl, SML_HDL_MAGIC); + AN(viov); + AN(viovcnt); + + CHECK_VAI_HDL_NOTNULL(hdl, SML_HDL_MAGIC); + AN(viov); + AN(viovcnt); + if (hdl->avail == hdl->returned) { + hdl->avail = ObjVAIGetExtend(wrk, hdl->oc, hdl->returned, + &state, &hdl->qe); + if (state == BOS_FAILED) + return (-EPIPE); + else if (state == BOS_FINISHED) + (void)0; + else if (hdl->avail == hdl->returned) { + // ObjVAIGetExtend() has scheduled a notification + if (hdl->boc->transit_buffer > 0) + return (-ENOBUFS); + else + return (-EAGAIN); + } + else + assert(state < BOS_FINISHED); + } + Lck_Lock(&hdl->boc->mtx); + if (hdl->st == NULL && hdl->last != NULL) { + /* when the "last" st completed, we did not yet have a next, so + * resume from there. Because "last" might have been returned and + * deleted, we can not just use the pointer, but rather need to + * iterate the st list. + * if we can not find "last", it also has been returned and + * deleted, and the current write head (VTAILQ_LAST) is our next + * st, which can also be null if we are done. + */ + VTAILQ_FOREACH_REVERSE(next, &hdl->obj->list, storagehead, list) { + if (next == hdl->last) { + hdl->st = VTAILQ_PREV(next, storagehead, list); + break; + } + } + } + hdl->last = NULL; + if (hdl->st == NULL) { + assert(hdl->returned == 0 || hdl->avail == hdl->returned); + hdl->st = VTAILQ_LAST(&hdl->obj->list, storagehead); + } + if (hdl->st == NULL) + assert(hdl->avail == hdl->returned); + + while (viovcnt > 0 && hdl->avail > hdl->returned) { + CHECK_OBJ_NOTNULL(hdl->st, STORAGE_MAGIC); // ObjVAIGetExtend ensures + assert(hdl->st_off <= hdl->st->space); + size_t av = hdl->avail - hdl->returned; + size_t l = hdl->st->space - hdl->st_off; + AN(l); + if (l > av) + l = av; + INIT_OBJ(viov, VIOV_MAGIC); + viov->iov.iov_base = hdl->st->ptr + hdl->st_off; + viov->iov.iov_len = l; + if (hdl->st_off + l == hdl->st->space) { + next = VTAILQ_PREV(hdl->st, storagehead, list); + AZ(hdl->last); + if (next == NULL) + hdl->last = hdl->st; + else + CHECK_OBJ(next, STORAGE_MAGIC); +#ifdef VAI_DBG + VSLb(wrk->vsl, SLT_Debug, "off %zu + l %zu == space st %p next st %p stvprv %p", + hdl->st_off, l, hdl->st, next, hdl->boc->stevedore_priv); +#endif + viov->lease = (uintptr_t)hdl->st; + hdl->st_off = 0; + hdl->st = next; + } + else { + viov->lease = sml_ai_lease_frag; + hdl->st_off += l; + } + hdl->returned += l; + VAI_ASSERT_LEASE(viov->lease); + r++; + viov++; + viovcnt--; + } + + Lck_Unlock(&hdl->boc->mtx); + return (r); +} + +static void v_matchproto_(vai_return_f) +sml_ai_return(struct worker *wrk, vai_hdl vhdl, uint64_t *leases, int leasecnt) +{ + struct storage *st; + struct sml_hdl *hdl; + uint64_t *p, *end; + + (void) wrk; + CAST_VAI_HDL_NOTNULL(hdl, vhdl, SML_HDL_MAGIC); + AN(leases); + AN(leasecnt); + + // callback is only registered if needed + assert(hdl->boc != NULL && (hdl->oc->flags & OC_F_TRANSIENT) != 0); + + // make this a two-stage process to avoid free under the lock + Lck_Lock(&hdl->boc->mtx); + for (p = leases, end = leases + leasecnt; p < end; p++) { + if (*p == sml_ai_lease_frag) + continue; + CAST_OBJ_NOTNULL(st, (void *)*p, STORAGE_MAGIC); + VTAILQ_REMOVE(&hdl->obj->list, st, list); + if (st == hdl->boc->stevedore_priv) + hdl->boc->stevedore_priv = trim_once; + } + Lck_Unlock(&hdl->boc->mtx); + + for (p = leases; p < end; p++) { + if (*p == sml_ai_lease_frag) + continue; + CAST_OBJ_NOTNULL(st, (void *)*p, STORAGE_MAGIC); + sml_stv_free(hdl->stv, st); + } +} + +static void v_matchproto_(vai_fini_f) +sml_ai_fini(struct worker *wrk, vai_hdl *vai_hdlp) +{ + struct sml_hdl *hdl; + + AN(vai_hdlp); + CAST_VAI_HDL_NOTNULL(hdl, *vai_hdlp, SML_HDL_MAGIC); + *vai_hdlp = NULL; + + if (hdl->boc != NULL) { + ObjVAICancel(wrk, hdl->boc, &hdl->qe); + HSH_DerefBoc(wrk, hdl->oc); + hdl->boc = NULL; + } + + if (hdl->ws != NULL) + WS_Release(hdl->ws, 0); + else + free(hdl); +} + +static vai_hdl v_matchproto_(vai_init_f) +sml_ai_init(struct worker *wrk, struct objcore *oc, struct ws *ws, + vai_notify_cb *notify, void *notify_priv) +{ + struct sml_hdl *hdl; + const size_t sz = sizeof *hdl; + + if (ws != NULL && WS_ReserveSize(ws, (unsigned)sz)) + hdl = WS_Reservation(ws); + else { + hdl = malloc(sz); + ws = NULL; + } + + AN(hdl); + INIT_VAI_HDL(hdl, SML_HDL_MAGIC); + hdl->preamble.vai_lease = sml_ai_lease_simple; + hdl->preamble.vai_fini = sml_ai_fini; + hdl->ws = ws; + + hdl->oc = oc; + hdl->obj = sml_getobj(wrk, oc); + CHECK_OBJ_NOTNULL(hdl->obj, OBJECT_MAGIC); + hdl->stv = oc->stobj->stevedore; + CHECK_OBJ_NOTNULL(hdl->stv, STEVEDORE_MAGIC); + + hdl->st = VTAILQ_LAST(&hdl->obj->list, storagehead); + CHECK_OBJ_ORNULL(hdl->st, STORAGE_MAGIC); + + hdl->boc = HSH_RefBoc(oc); + if (hdl->boc == NULL) + return (hdl); + /* we only initialize notifications if we have a boc, so + * any wrong attempt triggers magic checks. + */ + hdl->preamble.vai_lease = sml_ai_lease_boc; + if ((hdl->oc->flags & OC_F_TRANSIENT) != 0) + hdl->preamble.vai_return = sml_ai_return; + hdl->qe.magic = VAI_Q_MAGIC; + hdl->qe.cb = notify; + hdl->qe.hdl = hdl; + hdl->qe.priv = notify_priv; + return (hdl); +} + +/* + * trivial notification to allow the iterator to simply block + */ +struct sml_notify { + unsigned magic; +#define SML_NOTIFY_MAGIC 0x4589af31 + unsigned hasmore; + pthread_mutex_t mtx; + pthread_cond_t cond; +}; + +static void +sml_notify_init(struct sml_notify *sn) +{ + + INIT_OBJ(sn, SML_NOTIFY_MAGIC); + AZ(pthread_mutex_init(&sn->mtx, NULL)); + AZ(pthread_cond_init(&sn->cond, NULL)); +} + +static void +sml_notify_fini(struct sml_notify *sn) +{ + + CHECK_OBJ_NOTNULL(sn, SML_NOTIFY_MAGIC); + AZ(pthread_mutex_destroy(&sn->mtx)); + AZ(pthread_cond_destroy(&sn->cond)); +} + +static void v_matchproto_(vai_notify_cb) +sml_notify(vai_hdl hdl, void *priv) +{ + struct sml_notify *sn; + + (void) hdl; + CAST_OBJ_NOTNULL(sn, priv, SML_NOTIFY_MAGIC); + AZ(pthread_mutex_lock(&sn->mtx)); + sn->hasmore = 1; + AZ(pthread_cond_signal(&sn->cond)); + AZ(pthread_mutex_unlock(&sn->mtx)); + +} + +static void +sml_notify_wait(struct sml_notify *sn) +{ + + CHECK_OBJ_NOTNULL(sn, SML_NOTIFY_MAGIC); + AZ(pthread_mutex_lock(&sn->mtx)); + while (sn->hasmore == 0) + AZ(pthread_cond_wait(&sn->cond, &sn->mtx)); + AN(sn->hasmore); + sn->hasmore = 0; + AZ(pthread_mutex_unlock(&sn->mtx)); +} + static int v_matchproto_(objiterator_f) sml_iterator(struct worker *wrk, struct objcore *oc, void *priv, objiterate_f *func, int final) { - struct boc *boc; - enum boc_state_e state; - struct object *obj; - struct storage *st; - struct storage *checkpoint = NULL; - const struct stevedore *stv; - ssize_t checkpoint_len = 0; - ssize_t len = 0; - int ret = 0, ret2; - ssize_t ol; - ssize_t nl; - ssize_t sl; - void *p; - ssize_t l; - unsigned u; - - obj = sml_getobj(wrk, oc); - CHECK_OBJ_NOTNULL(obj, OBJECT_MAGIC); - stv = oc->stobj->stevedore; - CHECK_OBJ_NOTNULL(stv, STEVEDORE_MAGIC); + struct sml_notify sn; + const int vaiomax = 256; + struct viov *vio, *end, viov[vaiomax]; + uint64_t ret[vaiomax]; + unsigned u, uu; + vai_hdl hdl; + int nn, n, r, r2, nret; - boc = HSH_RefBoc(oc); + (void) final; // phase out? + sml_notify_init(&sn); + hdl = ObjVAIinit(wrk, oc, NULL, sml_notify, &sn); + AN(hdl); - if (boc == NULL) { - VTAILQ_FOREACH_REVERSE_SAFE( - st, &obj->list, storagehead, list, checkpoint) { + r = u = nret = 0; - u = 0; - if (VTAILQ_PREV(st, storagehead, list) == NULL) + do { + n = 0; + do { + nn = ObjVAIlease(wrk, hdl, viov + n, vaiomax - n); + if (nn <= 0) + break; + n += nn; + } while (n < vaiomax); + assert(n >= 0); + assert(n <= vaiomax); + + /* + * n contains the number of viovs leased + * nn is the wait/return action or 0 + * + * nn tells us if to flush + */ + for (vio = viov, end = viov + n; vio < end; vio++) { + CHECK_OBJ(vio, VIOV_MAGIC); + AZ(u & OBJ_ITER_END); + if (vio->flags & VIOV_F_END) { + assert(vio == end - 1); u |= OBJ_ITER_END; - if (final) - u |= OBJ_ITER_FLUSH; - if (ret == 0 && st->len > 0) - ret = func(priv, u, st->ptr, st->len); - if (final) { - VTAILQ_REMOVE(&obj->list, st, list); - sml_stv_free(stv, st); - } else if (ret) + } + else + AZ(vio->flags); + + // flush if it is the last IOV and we will wait + // or if we need space in the return leases array + uu = u; + if ((vio == end - 1 && nn < 0) || nret == vaiomax) + uu |= OBJ_ITER_FLUSH; + r = func(priv, uu, vio->iov.iov_base, vio->iov.iov_len); + if (r != 0) break; - } - return (ret); - } - p = NULL; - l = 0; + // whenever we have flushed, return leases + if ((uu & OBJ_ITER_FLUSH) && nret > 0) { + ObjVAIreturn(wrk, hdl, ret, nret); + nret = 0; + } - u = 0; - if (boc->fetched_so_far == 0) { - ret = func(priv, OBJ_ITER_FLUSH, NULL, 0); - if (ret) - return (ret); - } - while (1) { - ol = len; - nl = ObjWaitExtend(wrk, oc, ol, &state); - if (state == BOS_FAILED) { - ret = -1; - break; + assert(nret < vaiomax); + ret[nret++] = vio->lease; } - if (nl == ol) { - assert(state == BOS_FINISHED); - break; - } - assert(nl > ol); - Lck_Lock(&boc->mtx); - AZ(VTAILQ_EMPTY(&obj->list)); - if (checkpoint == NULL) { - st = VTAILQ_LAST(&obj->list, storagehead); - sl = 0; - } else { - st = checkpoint; - sl = checkpoint_len; - ol -= checkpoint_len; + + if (n == 0 && nn < 0) { + r2 = func(priv, OBJ_ITER_FLUSH, NULL, 0); + if (r == 0) + r = r2; } - while (st != NULL) { - if (st->len > ol) { - p = st->ptr + ol; - l = st->len - ol; - len += l; - break; - } - ol -= st->len; - assert(ol >= 0); - nl -= st->len; - assert(nl > 0); - sl += st->len; - st = VTAILQ_PREV(st, storagehead, list); - if (final && checkpoint != NULL) { - if (checkpoint == boc->stevedore_priv) - boc->stevedore_priv = trim_once; - else - VTAILQ_REMOVE(&obj->list, checkpoint, list); - sml_stv_free(stv, checkpoint); + + // for the break of the for loop above, we still need to collect returns + for (vio++; vio < end; vio++) { + CHECK_OBJ(vio, VIOV_MAGIC); + if (nret == vaiomax) { + ObjVAIreturn(wrk, hdl, ret, nret); + nret = 0; } - checkpoint = st; - checkpoint_len = sl; + ret[nret++] = vio->lease; } - CHECK_OBJ_NOTNULL(obj, OBJECT_MAGIC); - CHECK_OBJ_NOTNULL(st, STORAGE_MAGIC); - st = VTAILQ_PREV(st, storagehead, list); - if (st != NULL && st->len == 0) - st = NULL; - Lck_Unlock(&boc->mtx); - assert(l > 0 || state == BOS_FINISHED); - u = 0; - if (st == NULL || final) - u |= OBJ_ITER_FLUSH; - if (st == NULL && state == BOS_FINISHED) - u |= OBJ_ITER_END; - ret = func(priv, u, p, l); - if (ret) - break; - } - HSH_DerefBoc(wrk, oc); + if (n == 0 && nn == -ENOBUFS && nret > 0) { + ObjVAIreturn(wrk, hdl, ret, nret); + nret = 0; + } + if (r == 0 && (nn == -ENOBUFS || nn == -EAGAIN)) { + sml_notify_wait(&sn); + } + else if (r == 0 && nn < 0) + r = -1; + } while (nn != 0 && r == 0); + if ((u & OBJ_ITER_END) == 0) { - ret2 = func(priv, OBJ_ITER_END, NULL, 0); - if (ret == 0) - ret = ret2; + r2 = func(priv, OBJ_ITER_END, NULL, 0); + if (r == 0) + r = r2; } - return (ret); + + if (nret > 0) + ObjVAIreturn(wrk, hdl, ret, nret); + + ObjVAIfini(wrk, &hdl); + sml_notify_fini(&sn); + + return (r); } /*-------------------------------------------------------------------- @@ -734,6 +1046,7 @@ const struct obj_methods SML_methods = { .objgetattr = sml_getattr, .objsetattr = sml_setattr, .objtouch = LRU_Touch, + .vai_init = sml_ai_init }; static void diff --git a/bin/varnishd/storage/storage_simple.h b/bin/varnishd/storage/storage_simple.h index 881e0b8571c..894de38e153 100644 --- a/bin/varnishd/storage/storage_simple.h +++ b/bin/varnishd/storage/storage_simple.h @@ -39,7 +39,6 @@ struct storage { unsigned magic; #define STORAGE_MAGIC 0x1a4e51c0 - VTAILQ_ENTRY(storage) list; void *priv; diff --git a/bin/varnishtest/tests/c00111.vtc b/bin/varnishtest/tests/c00111.vtc index 706ee7041a7..996d5d258a8 100644 --- a/bin/varnishtest/tests/c00111.vtc +++ b/bin/varnishtest/tests/c00111.vtc @@ -15,7 +15,8 @@ client c1 { } -run varnish v1 -vsl_catchup -varnish v1 -expect fetch_failed == 1 +# with vai, this no longer fails systematically (which is good) +varnish v1 -expect fetch_failed <= 1 varnish v1 -cliok "param.set transit_buffer 4k" @@ -26,4 +27,4 @@ client c2 { varnish v1 -vsl_catchup varnish v1 -expect s_fetch == 2 -varnish v1 -expect fetch_failed == 1 +varnish v1 -expect fetch_failed <= 1 From d435babc783fa73f430fd8fc620c285ecb7c9906 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Sat, 12 Oct 2024 23:45:49 +0200 Subject: [PATCH 5/5] =?UTF-8?q?Who=20said=20they=20wanted=20to=20keep=2032?= =?UTF-8?q?bit=20alive=20as=20an=20option=3F=20=F0=9F=A4=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- bin/varnishd/storage/storage_simple.c | 29 +++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/bin/varnishd/storage/storage_simple.c b/bin/varnishd/storage/storage_simple.c index 835a8f42fda..b3e5283c48c 100644 --- a/bin/varnishd/storage/storage_simple.c +++ b/bin/varnishd/storage/storage_simple.c @@ -327,13 +327,34 @@ struct sml_hdl { uint64_t avail, returned; }; +static inline uint64_t +st2lease(const struct storage *st) +{ + uint64_t r = (uintptr_t)st; + + if (sizeof(void *) < 8) + r <<= 1; + + return (r); +} + +static inline struct storage * +lease2st(uint64_t l) +{ + + if (sizeof(void *) < 8) + l >>= 1; + + return ((void *)l); +} + static inline void sml_ai_viov_fill(struct viov *viov, struct storage *st) { INIT_OBJ(viov, VIOV_MAGIC); viov->iov.iov_base = st->ptr; viov->iov.iov_len = st->len; - viov->lease = (uintptr_t)st; + viov->lease = st2lease(st); VAI_ASSERT_LEASE(viov->lease); } @@ -460,7 +481,7 @@ sml_ai_lease_boc(struct worker *wrk, vai_hdl vhdl, struct viov *viov, int viovcn VSLb(wrk->vsl, SLT_Debug, "off %zu + l %zu == space st %p next st %p stvprv %p", hdl->st_off, l, hdl->st, next, hdl->boc->stevedore_priv); #endif - viov->lease = (uintptr_t)hdl->st; + viov->lease = st2lease(hdl->st); hdl->st_off = 0; hdl->st = next; } @@ -499,7 +520,7 @@ sml_ai_return(struct worker *wrk, vai_hdl vhdl, uint64_t *leases, int leasecnt) for (p = leases, end = leases + leasecnt; p < end; p++) { if (*p == sml_ai_lease_frag) continue; - CAST_OBJ_NOTNULL(st, (void *)*p, STORAGE_MAGIC); + CAST_OBJ_NOTNULL(st, lease2st(*p), STORAGE_MAGIC); VTAILQ_REMOVE(&hdl->obj->list, st, list); if (st == hdl->boc->stevedore_priv) hdl->boc->stevedore_priv = trim_once; @@ -509,7 +530,7 @@ sml_ai_return(struct worker *wrk, vai_hdl vhdl, uint64_t *leases, int leasecnt) for (p = leases; p < end; p++) { if (*p == sml_ai_lease_frag) continue; - CAST_OBJ_NOTNULL(st, (void *)*p, STORAGE_MAGIC); + CAST_OBJ_NOTNULL(st, lease2st(*p), STORAGE_MAGIC); sml_stv_free(hdl->stv, st); } }