Remove the small allocations

This is not compatible with multithreading and would require a complete
re-design.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
This commit is contained in:
Ronnie Sahlberg
2025-04-26 08:00:47 +10:00
parent 8047421868
commit edd7d9b843
7 changed files with 51 additions and 143 deletions

View File

@@ -48,7 +48,7 @@ static int finished;
* threads to demonstrate that multiple threads can very well write to the same
* context.
*/
#define NUM_THREADS 4
#define NUM_THREADS 8
const char *initiator = "iqn.2007-10.com.github:sahlberg:libiscsi:iscsi-inq";
@@ -90,6 +90,10 @@ static void *iscsi_read_thread(void *arg)
struct read_data *rd = arg;
struct scsi_task *task;
task = malloc(1024);
printf("iscsi_read_thread %d %p\n", rd->i, task);
free(task);
while (!finished) {
task = iscsi_read10_sync(rd->iscsi, rd->lun, 0,
512, 512,
@@ -243,6 +247,7 @@ int main(int argc, char *argv[])
iscsi_logout_sync(iscsi[i]);
iscsi_destroy_context(iscsi[i]);
}
printf("finished\n");
return 0;
}

View File

@@ -193,14 +193,10 @@ struct iscsi_context {
int log_level;
iscsi_log_fn log_fn;
int mallocs;
int reallocs;
int frees;
int smallocs;
void* smalloc_ptrs[SMALL_ALLOC_MAX_FREE];
int smalloc_free;
size_t smalloc_size;
int cache_allocations;
int mallocs; //needs protection?
int reallocs; //needs protection?
int frees; //needs protection?
int cache_allocations; //needs ptotection?
time_t next_reconnect;
int scsi_timeout;
@@ -395,9 +391,6 @@ void* iscsi_zmalloc(struct iscsi_context *iscsi, size_t size);
void* iscsi_realloc(struct iscsi_context *iscsi, void* ptr, size_t size);
void iscsi_free(struct iscsi_context *iscsi, void* ptr);
char* iscsi_strdup(struct iscsi_context *iscsi, const char* str);
void* iscsi_smalloc(struct iscsi_context *iscsi, size_t size);
void* iscsi_szmalloc(struct iscsi_context *iscsi, size_t size);
void iscsi_sfree(struct iscsi_context *iscsi, void* ptr);
uint32_t crc32c(uint8_t *buf, int len);
void crc32c_init(uint32_t *crc_ptr);

View File

@@ -281,7 +281,6 @@ void iscsi_reconnect_cb(struct iscsi_context *iscsi, int status,
{
struct iscsi_context *old_iscsi;
struct iscsi_pdu *tmp = NULL;
int i;
if (status != SCSI_STATUS_GOOD) {
int backoff = ++iscsi->old_iscsi->retry_cnt;
@@ -368,10 +367,6 @@ void iscsi_reconnect_cb(struct iscsi_context *iscsi, int status,
iscsi_free(old_iscsi, old_iscsi->opaque);
for (i = 0; i < old_iscsi->smalloc_free; i++) {
iscsi_free(old_iscsi, old_iscsi->smalloc_ptrs[i]);
}
iscsi->mallocs += old_iscsi->mallocs;
iscsi->frees += old_iscsi->frees;
@@ -468,10 +463,6 @@ static int reconnect(struct iscsi_context *iscsi, int force)
tmp_iscsi->reconnect_max_retries = iscsi->reconnect_max_retries;
if (iscsi->old_iscsi) {
int i;
for (i = 0; i < iscsi->smalloc_free; i++) {
iscsi_free(iscsi, iscsi->smalloc_ptrs[i]);
}
iscsi_free(iscsi, iscsi->opaque);
iscsi->old_iscsi->mallocs += iscsi->mallocs;

View File

@@ -115,42 +115,6 @@ char* iscsi_strdup(struct iscsi_context *iscsi, const char* str) {
return str2;
}
void* iscsi_smalloc(struct iscsi_context *iscsi, size_t size) {
void *ptr;
if (size > iscsi->smalloc_size) return NULL;
if (iscsi->smalloc_free > 0) {
ptr = iscsi->smalloc_ptrs[--iscsi->smalloc_free];
iscsi->smallocs++;
} else {
ptr = iscsi_malloc(iscsi, iscsi->smalloc_size);
}
return ptr;
}
void* iscsi_szmalloc(struct iscsi_context *iscsi, size_t size) {
void *ptr = iscsi_smalloc(iscsi, size);
if (ptr) {
memset(ptr, 0, size);
}
return ptr;
}
void iscsi_sfree(struct iscsi_context *iscsi, void* ptr) {
if (ptr == NULL) {
return;
}
if (!iscsi->cache_allocations) {
iscsi_free(iscsi, ptr);
} else if (iscsi->smalloc_free == SMALL_ALLOC_MAX_FREE) {
/* SMALL_ALLOC_MAX_FREE should be adjusted that this */
/* happens rarely */
ISCSI_LOG(iscsi, 6, "smalloc free == SMALLOC_MAX_FREE");
iscsi_free(iscsi, ptr);
} else {
iscsi->smalloc_ptrs[iscsi->smalloc_free++] = ptr;
}
}
static void
iscsi_srand_init(struct iscsi_context *iscsi) {
unsigned int seed;
@@ -201,7 +165,6 @@ struct iscsi_context *
iscsi_create_context(const char *initiator_name)
{
struct iscsi_context *iscsi;
size_t required = ISCSI_RAW_HEADER_SIZE + ISCSI_DIGEST_SIZE;
char *ca;
if (!initiator_name[0]) {
@@ -289,18 +252,6 @@ iscsi_create_context(const char *initiator_name)
iscsi->rdma_ack_timeout = atoi(getenv("LIBISCSI_RDMA_ACK_TIMEOUT"));
}
/* iscsi->smalloc_size is the size for small allocations. this should be
max(ISCSI_HEADER_SIZE, sizeof(struct iscsi_pdu), sizeof(struct iscsi_in_pdu))
rounded up to the next power of 2. */
required = MAX(required, sizeof(struct iscsi_pdu));
required = MAX(required, sizeof(struct iscsi_in_pdu));
iscsi->smalloc_size = 1;
while (iscsi->smalloc_size < required) {
iscsi->smalloc_size <<= 1;
}
ISCSI_LOG(iscsi, 5, "small allocation size is %u byte",
(uint32_t)iscsi->smalloc_size);
ca = getenv("LIBISCSI_CACHE_ALLOCATIONS");
if (!ca || atoi(ca) != 0) {
iscsi->cache_allocations = 1;
@@ -397,8 +348,6 @@ iscsi_set_targetname(struct iscsi_context *iscsi, const char *target_name)
int
iscsi_destroy_context(struct iscsi_context *iscsi)
{
int i;
if (iscsi == NULL) {
return 0;
}
@@ -419,16 +368,12 @@ iscsi_destroy_context(struct iscsi_context *iscsi)
iscsi->connect_data = NULL;
for (i=0;i<iscsi->smalloc_free;i++) {
iscsi_free(iscsi, iscsi->smalloc_ptrs[i]);
}
iscsi_free(iscsi, iscsi->opaque);
if (iscsi->mallocs != iscsi->frees) {
ISCSI_LOG(iscsi,1,"%d memory blocks lost at iscsi_destroy_context() after %d malloc(s), %d realloc(s), %d free(s) and %d reused small allocations",iscsi->mallocs-iscsi->frees,iscsi->mallocs,iscsi->reallocs,iscsi->frees,iscsi->smallocs);
ISCSI_LOG(iscsi,1,"%d memory blocks lost at iscsi_destroy_context() after %d malloc(s), %d realloc(s), %d free(s)",iscsi->mallocs-iscsi->frees,iscsi->mallocs,iscsi->reallocs,iscsi->frees);
} else {
ISCSI_LOG(iscsi,5,"memory is clean at iscsi_destroy_context() after %d mallocs, %d realloc(s), %d free(s) and %d reused small allocations",iscsi->mallocs,iscsi->reallocs,iscsi->frees,iscsi->smallocs);
ISCSI_LOG(iscsi,5,"memory is clean at iscsi_destroy_context() after %d mallocs, %d realloc(s), %d free(s)",iscsi->mallocs,iscsi->reallocs,iscsi->frees);
}
if (iscsi->old_iscsi) {

View File

@@ -539,7 +539,7 @@ iscsi_iser_new_pdu(struct iscsi_context *iscsi, __attribute__((unused))size_t si
struct iscsi_pdu *pdu;
struct iser_pdu *iser_pdu;
iser_pdu = iscsi_szmalloc(iscsi, sizeof(*iser_pdu));
iser_pdu = iscsi_zmalloc(iscsi, sizeof(*iser_pdu));
pdu = &iser_pdu->iscsi_pdu;
pdu->indata.data = NULL;
@@ -563,19 +563,10 @@ iscsi_iser_free_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
iser_pdu->desc = NULL;
}
if (pdu->outdata.size <= iscsi->smalloc_size) {
iscsi_sfree(iscsi, pdu->outdata.data);
} else {
iscsi_free(iscsi, pdu->outdata.data);
}
iscsi_free(iscsi, pdu->outdata.data);
pdu->outdata.data = NULL;
if (pdu->indata.size <= iscsi->smalloc_size) {
iscsi_sfree(iscsi, pdu->indata.data);
} else {
iscsi_free(iscsi, pdu->indata.data);
}
iscsi_free(iscsi, pdu->indata.data);
pdu->indata.data = NULL;
iscsi_mt_spin_lock(&iscsi->iscsi_lock);
@@ -584,7 +575,7 @@ iscsi_iser_free_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
}
iscsi_mt_spin_unlock(&iscsi->iscsi_lock);
iscsi_sfree(iscsi, iser_pdu);
iscsi_free(iscsi, iser_pdu);
}
/**
@@ -774,11 +765,7 @@ iser_prepare_read_cmd(struct iser_conn *iser_conn,struct iser_pdu *iser_pdu)
if (data_size > 0) {
if (task->iovector_in.iov == NULL) {
if (data_size <= iscsi->smalloc_size) {
iser_pdu->iscsi_pdu.indata.data = iscsi_smalloc(iscsi, data_size);
} else {
iser_pdu->iscsi_pdu.indata.data = iscsi_malloc(iscsi, data_size);
}
iser_pdu->iscsi_pdu.indata.data = iscsi_malloc(iscsi, data_size);
if (iser_pdu->iscsi_pdu.indata.data == NULL) {
iscsi_set_error(iscsi, "Failed to aloocate data buffer");
return -1;
@@ -1743,11 +1730,6 @@ void iscsi_init_iser_transport(struct iscsi_context *iscsi)
/* Update iSCSI params as per iSER transport */
iscsi->initiator_max_recv_data_segment_length = ISCSI_DEF_MAX_RECV_SEG_LEN;
iscsi->target_max_recv_data_segment_length = ISCSI_DEF_MAX_RECV_SEG_LEN;
/* ensure smalloc_size is enough for iser_pdu */
while (iscsi->smalloc_size < sizeof(struct iser_pdu)) {
iscsi->smalloc_size <<= 1;
}
}
#endif

View File

@@ -187,7 +187,7 @@ void iscsi_dump_pdu_header(struct iscsi_context *iscsi, unsigned char *data) {
struct iscsi_pdu*
iscsi_tcp_new_pdu(struct iscsi_context *iscsi, size_t size)
{
return iscsi_szmalloc(iscsi, size);
return iscsi_zmalloc(iscsi, size);
}
struct iscsi_pdu *
@@ -204,7 +204,7 @@ iscsi_allocate_pdu(struct iscsi_context *iscsi, enum iscsi_opcode opcode,
}
pdu->outdata.size = ISCSI_HEADER_SIZE(iscsi->header_digest);
pdu->outdata.data = iscsi_szmalloc(iscsi, pdu->outdata.size);
pdu->outdata.data = iscsi_zmalloc(iscsi, pdu->outdata.size);
if (pdu->outdata.data == NULL) {
iscsi_set_error(iscsi, "failed to allocate pdu header");
@@ -242,25 +242,17 @@ iscsi_tcp_free_pdu(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
return;
}
if (pdu->outdata.size <= iscsi->smalloc_size) {
iscsi_sfree(iscsi, pdu->outdata.data);
} else {
iscsi_free(iscsi, pdu->outdata.data);
}
iscsi_free(iscsi, pdu->outdata.data);
pdu->outdata.data = NULL;
if (pdu->indata.size <= iscsi->smalloc_size) {
iscsi_sfree(iscsi, pdu->indata.data);
} else {
iscsi_free(iscsi, pdu->indata.data);
}
iscsi_free(iscsi, pdu->indata.data);
pdu->indata.data = NULL;
if (iscsi->outqueue_current == pdu) {
iscsi->outqueue_current = NULL;
}
iscsi_sfree(iscsi, pdu);
iscsi_free(iscsi, pdu);
}
int
@@ -283,15 +275,9 @@ iscsi_add_data(struct iscsi_context *iscsi, struct iscsi_data *data,
}
if (data->size == 0) {
if (aligned <= iscsi->smalloc_size) {
data->data = iscsi_szmalloc(iscsi, aligned);
} else {
data->data = iscsi_malloc(iscsi, aligned);
}
data->data = iscsi_malloc(iscsi, aligned);
} else {
if (aligned > iscsi->smalloc_size) {
data->data = iscsi_realloc(iscsi, data->data, aligned);
}
data->data = iscsi_realloc(iscsi, data->data, aligned);
}
if (data->data == NULL) {
iscsi_set_error(iscsi, "failed to allocate buffer for %d "

View File

@@ -153,8 +153,8 @@ iscsi_add_to_outqueue(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
/* TODO QQQ need to immediately send for the non multithreading case too
* and for the Windows API too */
#if defined(HAVE_MULTITHREADING) && defined(HAVE_PTHREAD)
if (current == NULL && pdu == iscsi->outqueue) {
if(iscsi->multithreading_enabled) {
if(iscsi->multithreading_enabled) {
if (current == NULL && pdu == iscsi->outqueue) {
pthread_kill(iscsi->service_thread, SIGUSR1);
}
}
@@ -666,20 +666,23 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
struct iscsi_in_pdu *in;
ssize_t hdr_size, data_size, count, padding_size;
bool do_data_digest = (iscsi->data_digest != ISCSI_DATA_DIGEST_NONE);
int ret = -1;
do {
hdr_size = ISCSI_HEADER_SIZE(iscsi->header_digest);
if (iscsi->incoming == NULL) {
iscsi->incoming = iscsi_szmalloc(iscsi, sizeof(struct iscsi_in_pdu));
iscsi->incoming = iscsi_zmalloc(iscsi, sizeof(struct iscsi_in_pdu));
if (iscsi->incoming == NULL) {
iscsi_set_error(iscsi, "Out-of-memory: failed to malloc iscsi_in_pdu");
return -1;
goto finished;
}
}
if (iscsi->incoming->hdr == NULL) {
crc32c_init(&(iscsi->incoming->calculated_data_digest));
iscsi->incoming->hdr = iscsi_smalloc(iscsi, hdr_size);
iscsi->incoming->hdr = iscsi_malloc(iscsi, hdr_size);
if (iscsi->incoming->hdr == NULL) {
iscsi_set_error(iscsi, "Out-of-memory");
return -1;
goto finished;
}
}
in = iscsi->incoming;
@@ -694,7 +697,7 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
count, 0);
if (count == 0) {
/* remote side has closed the socket. */
return -1;
goto finished;
}
if (count < 0) {
if (errno == EINTR || errno == EAGAIN) {
@@ -702,7 +705,7 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
}
iscsi_set_error(iscsi, "read from socket failed, "
"errno:%d", errno);
return -1;
goto finished;
}
in->hdr_pos += count;
}
@@ -717,7 +720,7 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
if (data_size < 0 || data_size > (ssize_t)iscsi->initiator_max_recv_data_segment_length) {
iscsi_set_error(iscsi, "Invalid data size received from target (%d)", (int)data_size);
return -1;
goto finished;
}
if (data_size != 0) {
unsigned char padding_buf[3];
@@ -737,7 +740,7 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
in->data = iscsi_malloc(iscsi, data_size);
if (in->data == NULL) {
iscsi_set_error(iscsi, "Out-of-memory: failed to malloc iscsi_in_pdu->data(%d)", (int)data_size);
return -1;
goto finished;
}
}
buf = &in->data[in->data_pos];
@@ -748,13 +751,13 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
}
if (count == 0) {
/* remote side has closed the socket. */
return -1;
goto finished;
}
if (count < 0) {
if (errno == EINTR || errno == EAGAIN) {
break;
}
return -1;
goto finished;
}
in->data_pos += count;
}
@@ -770,13 +773,13 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
count = recv(iscsi->fd, (void *)(in->data_digest_buf + in->received_data_digest_bytes), ISCSI_DIGEST_SIZE - in->received_data_digest_bytes, 0);
if (count == 0) {
/* remote side has closed the socket. */
return -1;
goto finished;
}
if (count < 0) {
if (errno == EINTR || errno == EAGAIN) {
break;
}
return -1;
goto finished;
}
in->received_data_digest_bytes += count;
@@ -785,15 +788,17 @@ iscsi_read_from_socket(struct iscsi_context *iscsi)
}
}
iscsi->incoming = NULL;
iscsi->incoming = NULL;
if (iscsi_process_pdu(iscsi, in) != 0) {
iscsi_free_iscsi_in_pdu(iscsi, in);
return -1;
goto finished;
}
iscsi_free_iscsi_in_pdu(iscsi, in);
} while (iscsi->tcp_nonblocking && iscsi->waitpdu && iscsi->is_loggedin);
} while (iscsi->tcp_nonblocking && iscsi->waitpdu && iscsi->is_loggedin); //QQQ break the loop
return 0;
ret = 0;
finished:
return ret;
}
static int iscsi_pdu_update_headerdigest(struct iscsi_context *iscsi, struct iscsi_pdu *pdu)
@@ -1123,6 +1128,7 @@ iscsi_tcp_service(struct iscsi_context *iscsi, int revents)
return iscsi_service_reconnect_if_loggedin(iscsi);
}
}
if (revents & POLLOUT) {
if (iscsi_write_to_socket(iscsi) != 0) {
ISCSI_LOG(iscsi, 1, "%s", iscsi_get_error(iscsi));
@@ -1155,10 +1161,10 @@ static void iscsi_tcp_queue_pdu(struct iscsi_context *iscsi,
void
iscsi_free_iscsi_in_pdu(struct iscsi_context *iscsi, struct iscsi_in_pdu *in)
{
iscsi_sfree(iscsi, in->hdr);
iscsi_free(iscsi, in->hdr);
iscsi_free(iscsi, in->data);
in->data=NULL;
iscsi_sfree(iscsi, in);
iscsi_free(iscsi, in);
in=NULL;
}