- Fixes memory leaks in scst_user spotted by new SGV cache backend
authorvlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Tue, 25 Sep 2007 09:52:53 +0000 (09:52 +0000)
committervlnb <vlnb@d57e44dd-8a1f-0410-8b47-8ef2f437770f>
Tue, 25 Sep 2007 09:52:53 +0000 (09:52 +0000)
 - Version changed on -pre3
 - Minor fixes and cosmetics

git-svn-id: https://scst.svn.sourceforge.net/svnroot/scst/trunk@199 d57e44dd-8a1f-0410-8b47-8ef2f437770f

scst/include/scsi_tgt.h
scst/src/dev_handlers/scst_user.c
scst/src/scst_mem.c
usr/fileio/common.c
usr/fileio/fileio.c

index ec8f9b1..3c38fe2 100644 (file)
@@ -39,7 +39,7 @@
 /* Version numbers, the same as for the kernel */
 #define SCST_VERSION_CODE 0x000906
 #define SCST_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
-#define SCST_VERSION_STRING "0.9.6-pre2"
+#define SCST_VERSION_STRING "0.9.6-pre3"
 
 /*************************************************************
  ** States of command processing state machine
index 5ce61b5..b30883b 100644 (file)
@@ -361,7 +361,7 @@ static void dev_user_on_cached_mem_free(struct dev_user_cmd *ucmd)
 {
        TRACE_ENTRY();
 
-       TRACE_DBG("Preparing ON_CACHED_MEM_FREE (ucmd %p, h %d, ubuff %lx)",
+       TRACE_MEM("Preparing ON_CACHED_MEM_FREE (ucmd %p, h %d, ubuff %lx)",
                ucmd, ucmd->h, ucmd->ubuff);
 
        ucmd->user_cmd.cmd_h = ucmd->h;
@@ -400,18 +400,14 @@ static void dev_user_unmap_buf(struct dev_user_cmd *ucmd)
        return;
 }
 
-static void dev_user_free_sg_entries(struct scatterlist *sg, int sg_count,
-       void *priv)
+static void __dev_user_free_sg_entries(struct dev_user_cmd *ucmd)
 {
-       struct dev_user_cmd *ucmd = (struct dev_user_cmd*)priv;
-
        TRACE_ENTRY();
 
        sBUG_ON(ucmd->data_pages == NULL);
 
-       TRACE_MEM("Freeing data pages (ucmd=%p, ubuff=%lx, sg=%p, sg_count=%d, "
-               "buff_cached=%d)", ucmd, ucmd->ubuff, sg, sg_count,
-               ucmd->buff_cached);
+       TRACE_MEM("Freeing data pages (ucmd=%p, ubuff=%lx, buff_cached=%d)",
+               ucmd, ucmd->ubuff, ucmd->buff_cached);
 
        dev_user_unmap_buf(ucmd);
 
@@ -424,6 +420,19 @@ static void dev_user_free_sg_entries(struct scatterlist *sg, int sg_count,
        return;
 }
 
+static void dev_user_free_sg_entries(struct scatterlist *sg, int sg_count,
+       void *priv)
+{
+       struct dev_user_cmd *ucmd = (struct dev_user_cmd*)priv;
+
+       TRACE_MEM("Freeing data pages (sg=%p, sg_count=%d, priv %p)", sg,
+               sg_count, ucmd);
+
+       __dev_user_free_sg_entries(ucmd);
+
+       return;
+}
+
 static inline int is_buff_cached(struct dev_user_cmd *ucmd)
 {
        int mem_reuse_type = ucmd->dev->memory_reuse_type;
@@ -510,9 +519,8 @@ static int dev_user_alloc_sg(struct dev_user_cmd *ucmd, int cached_buff)
                                        cmd->tgt->sg_tablesize);
                                ll++;
                        }
-                       sgv_pool_free(ucmd->sgv);
-                       ucmd->sgv = NULL;
                        cmd->sg = NULL;
+                       /* sgv will be freed in dev_user_free_sgv() */
                        res = -1;
                }
        } else {
@@ -520,9 +528,9 @@ static int dev_user_alloc_sg(struct dev_user_cmd *ucmd, int cached_buff)
                        "sg_cnt %d, ubuff %lx, sgv %p", ucmd, ucmd->h,
                        ucmd->buff_cached, cmd->sg_cnt, ucmd->ubuff, ucmd->sgv);
                if (unlikely(cmd->sg_cnt == 0)) {
+                       TRACE_MEM("Refused allocation (ucmd %p)", ucmd);
+                       sBUG_ON(ucmd->sgv != NULL);
                        res = -1;
-                       if (ucmd->data_pages != NULL)
-                               dev_user_unmap_buf(ucmd);
                } else {
                        switch(ucmd->state & ~UCMD_STATE_MASK) {
                        case UCMD_STATE_BUF_ALLOCING:
@@ -530,8 +538,6 @@ static int dev_user_alloc_sg(struct dev_user_cmd *ucmd, int cached_buff)
                                break;
                        case UCMD_STATE_EXECING:
                                res = -1;
-                               if (ucmd->data_pages != NULL)
-                                       dev_user_unmap_buf(ucmd);
                                break;
                        default:
                                sBUG();
@@ -810,7 +816,12 @@ static void dev_user_free_sgv(struct dev_user_cmd *ucmd)
        if (ucmd->sgv != NULL) {
                sgv_pool_free(ucmd->sgv);
                ucmd->sgv = NULL;
+       } else if (ucmd->data_pages != NULL) {
+               /* We mapped pages, but for some reason didn't allocate them */
+               ucmd_get(ucmd, 0);
+               __dev_user_free_sg_entries(ucmd);
        }
+       return;
 }
 
 static void dev_user_on_free_cmd(struct scst_cmd *cmd)
@@ -822,7 +833,7 @@ static void dev_user_on_free_cmd(struct scst_cmd *cmd)
        if (unlikely(ucmd == NULL))
                goto out;
 
-       TRACE_DBG("ucmd %p, cmd %p, buff_cached %d, ubuff %lx", ucmd, ucmd->cmd,
+       TRACE_MEM("ucmd %p, cmd %p, buff_cached %d, ubuff %lx", ucmd, ucmd->cmd,
                ucmd->buff_cached, ucmd->ubuff);
 
        ucmd->cmd = NULL;
@@ -998,6 +1009,8 @@ static int dev_user_map_buf(struct dev_user_cmd *ucmd, unsigned long ubuff,
        if (unlikely(ubuff == 0))
                goto out_nomem;
 
+       sBUG_ON(ucmd->data_pages != NULL);
+
        ucmd->num_data_pages = num_pg;
 
        ucmd->data_pages = kzalloc(sizeof(*ucmd->data_pages)*ucmd->num_data_pages,
index 92a5955..4dc7311 100644 (file)
@@ -503,6 +503,8 @@ struct scatterlist *sgv_pool_alloc(struct sgv_pool *pool, unsigned int size,
        int no_cached = flags & SCST_POOL_ALLOC_NO_CACHED;
        int no_fail = ((gfp_mask & __GFP_NOFAIL) == __GFP_NOFAIL);
 
+       TRACE_ENTRY();
+
        sBUG_ON(size == 0);
 
        pages = ((size + PAGE_SIZE - 1) >> PAGE_SHIFT);
@@ -519,13 +521,8 @@ struct scatterlist *sgv_pool_alloc(struct sgv_pool *pool, unsigned int size,
                EXTRACHECKS_BUG_ON(obj->sg_count != 0);
                pages_to_alloc = (1 << order);
                cache = pool->caches[obj->order];
-               if (sgv_pool_hiwmk_check(pages_to_alloc, no_fail) != 0) {
-                       obj->sg_count = 0;
-                       if ((flags & SCST_POOL_RETURN_OBJ_ON_ALLOC_FAIL))
-                               goto out_return1;
-                       else
-                               goto out_fail_free_sg_entries;
-               }
+               if (sgv_pool_hiwmk_check(pages_to_alloc, no_fail) != 0)
+                       goto out_fail_free_sg_entries;
        } else if ((order < SGV_POOL_ELEMENTS) && !no_cached) {
                cache = pool->caches[order];
                obj = sgv_pool_cached_get(pool, order, gfp_mask);
@@ -579,13 +576,8 @@ struct scatterlist *sgv_pool_alloc(struct sgv_pool *pool, unsigned int size,
                        goto out_return;
                        
                obj->allocator_priv = priv;
-               if (sgv_pool_hiwmk_check(pages_to_alloc, no_fail) != 0) {
-                       obj->sg_count = 0;
-                       if ((flags & SCST_POOL_RETURN_OBJ_ON_ALLOC_FAIL))
-                               goto out_return1;
-                       else
-                               goto out_fail_free_sg_entries;
-               }
+               if (sgv_pool_hiwmk_check(pages_to_alloc, no_fail) != 0)
+                       goto out_fail_free_sg_entries;
        } else {
                int sz;
                pages_to_alloc = pages;
@@ -604,10 +596,8 @@ struct scatterlist *sgv_pool_alloc(struct sgv_pool *pool, unsigned int size,
                obj->sg_entries = obj->sg_entries_data;
                obj->allocator_priv = priv;
                
-               if (sgv_pool_hiwmk_check(pages_to_alloc, no_fail) != 0) {
-                       obj->sg_count = 0;
+               if (sgv_pool_hiwmk_check(pages_to_alloc, no_fail) != 0)
                        goto out_fail_free_sg_entries;
-               }
                TRACE_MEM("Big or no_cached sgv_obj %p (size %d)", obj, sz);            
        }
 
@@ -662,6 +652,7 @@ success:
                obj->sg_count, *count, obj->sg_entries[obj->orig_sg].length);
 
 out:
+       TRACE_EXIT_HRES(res);
        return res;
 
 out_return:
index 4c5ec43..75d3c16 100644 (file)
@@ -663,9 +663,9 @@ void *main_loop(void *arg)
 
        vcmd.fd = open_dev_fd(dev);
        if (vcmd.fd < 0) {
-               res = vcmd.fd;
+               res = -errno;
                PRINT_ERROR_PR("Unable to open file %s (%s)", dev->file_name,
-                       strerror(res));
+                       strerror(-res));
                goto out;
        }
 
index 5c6af50..6d11002 100644 (file)
@@ -277,9 +277,9 @@ int main(int argc, char **argv)
        TRACE_DBG("Opening file %s", dev.file_name);
        fd = open(dev.file_name, O_RDONLY|O_LARGEFILE);
        if (fd < 0) {
-               res = errno;
+               res = -errno;
                PRINT_ERROR_PR("Unable to open file %s (%s)", dev.file_name,
-                       strerror(res));
+                       strerror(-res));
                goto out_done;
        }
 
@@ -368,9 +368,9 @@ int main(int argc, char **argv)
        dev.scst_usr_fd = open(DEV_USER_PATH DEV_USER_NAME, O_RDWR | 
                (dev.non_blocking ? O_NONBLOCK : 0));
        if (dev.scst_usr_fd < 0) {
-               res = dev.scst_usr_fd;
+               res = -errno;
                PRINT_ERROR_PR("Unable to open SCST device %s (%s)",
-                       DEV_USER_PATH DEV_USER_NAME, strerror(res));
+                       DEV_USER_PATH DEV_USER_NAME, strerror(-res));
                goto out_done;
        }