pmem / pmem-redis

A version of Redis that uses persistent memory
BSD 3-Clause "New" or "Revised" License
113 stars 47 forks source link

RFC: Refine sdsmvtonvm(...) function to return more information when nvm_malloc failed. #13

Open BrytonLee opened 5 years ago

BrytonLee commented 5 years ago

https://github.com/pmem/pmem-redis/blob/cc54b5516d5106336a016cc36135141319a97690/src/sds.c#L106

 106 sds sdsmvtonvm(const sds s)
 107 {
 108     if(server.nvm_base && !is_nvm_addr(s))
 109     {
 110         size_t header_size = sdsheadersize(s);
 111         size_t total_size = header_size + sdsalloc(s) + 1;
 112         if(total_size >= server.sdsmv_threshold)
 113         {
 114             void* new_sh = nvm_malloc(total_size);
 115             if(!new_sh)
 116             {
 117                 //serverLog(LL_WARNING, "Can't allocate on NVM. Keep data in memory.");
 118                 return s;
 119             }
 120             void* sh = s - header_size;
 121             size_t used_size = header_size + sdslen(s) + 1;
 122             pmem_memcpy_persist(new_sh, sh, used_size);
 123             zfree(sh);
 124             return (char*)new_sh + header_size;
 125         }
 126     }
 127     return s;
 128 }

I would like to change the sdsmvtonvm() signature to return an error message when nvm_malloc failed. Otherwise, users who calls sdsmvtonvm() have no chance to check if it succeed or not.

Would you like to shed a light on that?

peifengsi commented 3 years ago

Hi, BrytonLee One alternative way is to check whether the return address belongs to nvm or not by calling is_nvm_addr().