You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2006/07/26 16:42:55 UTC

Re: svn commit: r425734 - in /httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem: config5.m4 mod_plainmem.c mod_scoreboard.c mod_sharedmem.c sharedmem_util.c slotmem.h

On 26.07.2006 15:42, jfclere@apache.org wrote:
> Author: jfclere
> Date: Wed Jul 26 06:42:43 2006
> New Revision: 425734
> 
> URL: http://svn.apache.org/viewvc?rev=425734&view=rev
> Log:
> Add ap_slotmem_attach() to the slotmem_storage_method.
> Cut mod_sharemem.c in 2 so that its features could be
> used outside httpd.

> 
> Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c?rev=425734&r1=425733&r2=425734&view=diff
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c (original)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c Wed Jul 26 06:42:43 2006
> @@ -98,6 +98,36 @@
>      *new = res;
>      return APR_SUCCESS;
>  }
> +
> +static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t item_size, int item_num, apr_pool_t *pool)

Why using item_size and item_num as call by value parameters? Shouldn't that be
pointers?

> +{
> +    void *slotmem = NULL;
> +    ap_slotmem_t *res;
> +    ap_slotmem_t *next = globallistmem;
> +    char *fname;
> +    apr_status_t rv;
> +
> +    fname = ap_server_root_relative(pool, name);
> +
> +    /* first try to attach to existing slotmem */
> +    if (next) {
> +        for (;;) {
> +            if (strcmp(next->name, fname) == 0) {
> +                /* we already have it */
> +                *new = next;
> +                *item_size = next->size;
> +                *item_num = next->num;

See comment above.


> 
> Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c?rev=425734&r1=425733&r2=425734&view=diff
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c (original)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c Wed Jul 26 06:42:43 2006
> @@ -84,6 +84,12 @@
>      *new = res;
>      return APR_SUCCESS;
>  }
> +
> +static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t *item_size, int *item_num, apr_pool_t *pool)
> +{
> +    return(ap_slotmem_create(new, name, item_size, item_num, pool));

This does not work. You need to provide ap_slotmem_create with values for
item_size and item_num. It does not return them. This can get difficult since
these values get not stored in the scoreboard. There is not globallistmem here.

> 
> Added: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c?rev=425734&view=auto
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c (added)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c Wed Jul 26 06:42:43 2006

> +static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t *item_size, int *item_num, apr_pool_t *pool)
> +{
> +    void *slotmem = NULL;
> +    void *ptr;
> +    ap_slotmem_t *res;
> +    ap_slotmem_t *next = globallistmem;
> +    struct sharedslotdesc desc;
> +    char *fname;
> +    apr_status_t rv;
> +
> +    fname = ap_server_root_relative(pool, name);
> +
> +    /* first try to attach to existing slotmem */
> +    if (next) {
> +        for (;;) {
> +            if (strcmp(next->name, fname) == 0) {
> +                /* we already have it */
> +                *new = next;
> +                *item_size = next->size;
> +                *item_num = next->num;
> +                return APR_SUCCESS;
> +            }
> +            if (next->next)
> +                break;
> +            next = next->next;
> +        }
> +    }
> +
> +    /* first try to attach to existing shared memory */
> +    res = (ap_slotmem_t *) apr_pcalloc(globalpool, sizeof(ap_slotmem_t));
> +    rv = apr_shm_attach(&res->shm, fname, globalpool);
> +    if (rv != APR_SUCCESS)
> +        return rv;
> +
> +    /* Read the description of the slotmem */
> +    ptr = apr_shm_baseaddr_get(res->shm);
> +    memcpy(&desc, ptr, sizeof(desc));
> +    ptr = ptr + sizeof(desc);
> +
> +    /* For the chained slotmem stuff */
> +    res->name = apr_pstrdup(globalpool, fname);
> +    res->base = ptr;
> +    res->size = desc.item_size;
> +    res->num = desc.item_num;
> +    res->next = NULL;
> +    if (globallistmem==NULL)
> +        globallistmem = res;
> +    else
> +        next->next = res;
> +
> +    *new = res;
> +    *item_size = desc.item_size;
> +    *item_num = desc.item_num;
> +    return APR_SUCCESS;

Too much copy and paste? We already found it above and what we are searching for
is in *new isn't it? (Maybe we should also set *new to NULL in the beginning to
have a defined return value?)

> 
> Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h?rev=425734&r1=425733&r2=425734&view=diff
> ==============================================================================
> --- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h (original)
> +++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h Wed Jul 26 06:42:43 2006
> @@ -65,6 +65,17 @@
>  AP_DECLARE(apr_status_t) (* ap_slotmem_create)(ap_slotmem_t **new, const char *name, apr_size_t item_size, int item_num, apr_pool_t *pool);
>  
>  /**
> + * attach to an existing slotmem.
> + * This would attach to  shared memory, basically.
> + * @param pointer to store the address of the scoreboard.
> + * @param name is a key used for debugging and in mod_status output or allow another process to share this space.
> + * @param item_size size of each idem
> + * @param item_num max number of idem.

Guess it should be item above :-)?


Regards

Rüdiger



Re: svn commit: r425734 - in /httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem: config5.m4 mod_plainmem.c mod_scoreboard.c mod_sharedmem.c sharedmem_util.c slotmem.h

Posted by Ruediger Pluem <rp...@apache.org>.
On 26.07.2006 17:39, Jean-frederic Clere wrote:
> Ruediger Pluem wrote:

>>
>>
>> Too much copy and paste? We already found it above and what we are
>> searching for
>> is in *new isn't it? (Maybe we should also set *new to NULL in the
>> beginning to
>> have a defined return value?)
>>  
>>
> No... There is a typo:
> if (next->next)
> should be:
> if (!next->next)
> To stop on the last item of the list.
> It really attaches to the shared memory and has to read the max number
> of slot available.

Sounds reasonable. Looks like I got confused :-).

Regards

Rüdiger


Re: svn commit: r425734 - in /httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem: config5.m4 mod_plainmem.c mod_scoreboard.c mod_sharedmem.c sharedmem_util.c slotmem.h

Posted by Jean-frederic Clere <jf...@gmail.com>.
Ruediger Pluem wrote:

>On 26.07.2006 15:42, jfclere@apache.org wrote:
>  
>
>>Author: jfclere
>>Date: Wed Jul 26 06:42:43 2006
>>New Revision: 425734
>>
>>URL: http://svn.apache.org/viewvc?rev=425734&view=rev
>>Log:
>>Add ap_slotmem_attach() to the slotmem_storage_method.
>>Cut mod_sharemem.c in 2 so that its features could be
>>used outside httpd.
>>    
>>
>
>  
>
>>Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c?rev=425734&r1=425733&r2=425734&view=diff
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c (original)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_plainmem.c Wed Jul 26 06:42:43 2006
>>@@ -98,6 +98,36 @@
>>     *new = res;
>>     return APR_SUCCESS;
>> }
>>+
>>+static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t item_size, int item_num, apr_pool_t *pool)
>>    
>>
>
>Why using item_size and item_num as call by value parameters? Shouldn't that be
>pointers?
>  
>
Yep.

>  
>
>>+{
>>+    void *slotmem = NULL;
>>+    ap_slotmem_t *res;
>>+    ap_slotmem_t *next = globallistmem;
>>+    char *fname;
>>+    apr_status_t rv;
>>+
>>+    fname = ap_server_root_relative(pool, name);
>>+
>>+    /* first try to attach to existing slotmem */
>>+    if (next) {
>>+        for (;;) {
>>+            if (strcmp(next->name, fname) == 0) {
>>+                /* we already have it */
>>+                *new = next;
>>+                *item_size = next->size;
>>+                *item_num = next->num;
>>    
>>
>
>See comment above.
>
>
>  
>
>>Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c?rev=425734&r1=425733&r2=425734&view=diff
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c (original)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/mod_scoreboard.c Wed Jul 26 06:42:43 2006
>>@@ -84,6 +84,12 @@
>>     *new = res;
>>     return APR_SUCCESS;
>> }
>>+
>>+static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t *item_size, int *item_num, apr_pool_t *pool)
>>+{
>>+    return(ap_slotmem_create(new, name, item_size, item_num, pool));
>>    
>>
>
>This does not work. You need to provide ap_slotmem_create with values for
>item_size and item_num. It does not return them. This can get difficult since
>these values get not stored in the scoreboard. There is not globallistmem here.
>  
>
Right... Well I have used proxy_lb_workers() for the number of slot mem. 
I have the problem is other places to get proxy_lb_workers() in 
pre-config. I don't how to make this clean for the moment.

>  
>
>>Added: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c?rev=425734&view=auto
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c (added)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/sharedmem_util.c Wed Jul 26 06:42:43 2006
>>    
>>
>
>  
>
>>+static apr_status_t ap_slotmem_attach(ap_slotmem_t **new, const char *name, apr_size_t *item_size, int *item_num, apr_pool_t *pool)
>>+{
>>+    void *slotmem = NULL;
>>+    void *ptr;
>>+    ap_slotmem_t *res;
>>+    ap_slotmem_t *next = globallistmem;
>>+    struct sharedslotdesc desc;
>>+    char *fname;
>>+    apr_status_t rv;
>>+
>>+    fname = ap_server_root_relative(pool, name);
>>+
>>+    /* first try to attach to existing slotmem */
>>+    if (next) {
>>+        for (;;) {
>>+            if (strcmp(next->name, fname) == 0) {
>>+                /* we already have it */
>>+                *new = next;
>>+                *item_size = next->size;
>>+                *item_num = next->num;
>>+                return APR_SUCCESS;
>>+            }
>>+            if (next->next)
>>+                break;
>>+            next = next->next;
>>+        }
>>+    }
>>+
>>+    /* first try to attach to existing shared memory */
>>+    res = (ap_slotmem_t *) apr_pcalloc(globalpool, sizeof(ap_slotmem_t));
>>+    rv = apr_shm_attach(&res->shm, fname, globalpool);
>>+    if (rv != APR_SUCCESS)
>>+        return rv;
>>+
>>+    /* Read the description of the slotmem */
>>+    ptr = apr_shm_baseaddr_get(res->shm);
>>+    memcpy(&desc, ptr, sizeof(desc));
>>+    ptr = ptr + sizeof(desc);
>>+
>>+    /* For the chained slotmem stuff */
>>+    res->name = apr_pstrdup(globalpool, fname);
>>+    res->base = ptr;
>>+    res->size = desc.item_size;
>>+    res->num = desc.item_num;
>>+    res->next = NULL;
>>+    if (globallistmem==NULL)
>>+        globallistmem = res;
>>+    else
>>+        next->next = res;
>>+
>>+    *new = res;
>>+    *item_size = desc.item_size;
>>+    *item_num = desc.item_num;
>>+    return APR_SUCCESS;
>>    
>>
>
>Too much copy and paste? We already found it above and what we are searching for
>is in *new isn't it? (Maybe we should also set *new to NULL in the beginning to
>have a defined return value?)
>  
>
No... There is a typo:
if (next->next)
should be:
if (!next->next)
To stop on the last item of the list.
It really attaches to the shared memory and has to read the max number 
of slot available.

>  
>
>>Modified: httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h
>>URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h?rev=425734&r1=425733&r2=425734&view=diff
>>==============================================================================
>>--- httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h (original)
>>+++ httpd/httpd/branches/httpd-proxy-scoreboard/modules/mem/slotmem.h Wed Jul 26 06:42:43 2006
>>@@ -65,6 +65,17 @@
>> AP_DECLARE(apr_status_t) (* ap_slotmem_create)(ap_slotmem_t **new, const char *name, apr_size_t item_size, int item_num, apr_pool_t *pool);
>> 
>> /**
>>+ * attach to an existing slotmem.
>>+ * This would attach to  shared memory, basically.
>>+ * @param pointer to store the address of the scoreboard.
>>+ * @param name is a key used for debugging and in mod_status output or allow another process to share this space.
>>+ * @param item_size size of each idem
>>+ * @param item_num max number of idem.
>>    
>>
>
>Guess it should be item above :-)?
>  
>
Instead of idem. Oops

Many thanks

Jean-Frederic

>
>Regards
>
>Rüdiger
>
>
>
>  
>