-
Notifications
You must be signed in to change notification settings - Fork 232
Implement TRIM Support for Dataset Management Command in BBSSD #174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -858,6 +858,89 @@ static uint64_t ssd_write(struct ssd *ssd, NvmeRequest *req) | |||||
| return maxlat; | ||||||
| } | ||||||
|
|
||||||
| static uint64_t ssd_trim(struct ssd *ssd, NvmeRequest *req) | ||||||
| { | ||||||
| struct ssdparams *spp = &ssd->sp; | ||||||
| NvmeDsmRange *ranges = req->dsm_ranges; | ||||||
| int nr_ranges = req->dsm_nr_ranges; | ||||||
| // uint32_t attributes = req->dsm_attributes; | ||||||
|
|
||||||
| int total_trimmed_pages = 0; | ||||||
| int total_already_invalid = 0; | ||||||
| int total_out_of_bounds = 0; | ||||||
|
|
||||||
| if (!ranges || nr_ranges <= 0) { | ||||||
| printf("TRIM: Invalid ranges or count\n"); | ||||||
|
||||||
| printf("TRIM: Invalid ranges or count\n"); | |
| ftl_err("TRIM: Invalid ranges or count\n"); |
Copilot
AI
Jul 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latency variable (lat) is not initialized for the DSM case when req->dsm_ranges is empty, leading to undefined behavior. Consider setting lat = 0; before the conditional or restoring the default assignment.
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -62,6 +62,9 @@ static void nvme_process_sq_io(void *opaque, int index_poller) | |||||||
| req = QTAILQ_FIRST(&sq->req_list); | ||||||||
| QTAILQ_REMOVE(&sq->req_list, req, entry); | ||||||||
| memset(&req->cqe, 0, sizeof(req->cqe)); | ||||||||
| req->dsm_ranges = NULL; | ||||||||
| req->dsm_nr_ranges = 0; | ||||||||
| req->dsm_attributes = 0; | ||||||||
| /* Coperd: record req->stime at earliest convenience */ | ||||||||
| req->expire_time = req->stime = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); | ||||||||
| req->cqe.cid = cmd.cid; | ||||||||
|
|
@@ -73,18 +76,29 @@ static void nvme_process_sq_io(void *opaque, int index_poller) | |||||||
| } | ||||||||
|
|
||||||||
| status = nvme_io_cmd(n, &cmd, req); | ||||||||
| if (1 && status == NVME_SUCCESS) { | ||||||||
| if (status == NVME_SUCCESS) { | ||||||||
| req->status = status; | ||||||||
|
|
||||||||
| int rc = femu_ring_enqueue(n->to_ftl[index_poller], (void *)&req, 1); | ||||||||
| if (rc != 1) { | ||||||||
| femu_err("enqueue failed, ret=%d\n", rc); | ||||||||
| // Clean up DSM ranges on enqueue failure | ||||||||
| if (req->dsm_ranges) { | ||||||||
| g_free(req->dsm_ranges); | ||||||||
| req->dsm_ranges = NULL; | ||||||||
| req->dsm_nr_ranges = 0; | ||||||||
| } | ||||||||
| } | ||||||||
| } else if (status == NVME_SUCCESS) { | ||||||||
| /* Normal I/Os that don't need delay emulation */ | ||||||||
| req->status = status; | ||||||||
| } else { | ||||||||
| femu_err("Error IO processed!\n"); | ||||||||
| femu_err("Error IO processed! opcode=0x%x, status=0x%x\n", | ||||||||
| cmd.opcode, status); | ||||||||
| req->status = status; | ||||||||
|
|
||||||||
| // Clean up DSM ranges on error | ||||||||
| if (req->dsm_ranges) { | ||||||||
| g_free(req->dsm_ranges); | ||||||||
| req->dsm_ranges = NULL; | ||||||||
| req->dsm_nr_ranges = 0; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| processed++; | ||||||||
|
|
@@ -282,41 +296,109 @@ uint16_t nvme_rw(FemuCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd, NvmeRequest *req) | |||||||
| static uint16_t nvme_dsm(FemuCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd, | ||||||||
| NvmeRequest *req) | ||||||||
| { | ||||||||
| uint32_t dw10 = le32_to_cpu(cmd->cdw10); | ||||||||
| uint32_t dw11 = le32_to_cpu(cmd->cdw11); | ||||||||
| uint32_t cdw10 = le32_to_cpu(cmd->cdw10); | ||||||||
| uint32_t cdw11 = le32_to_cpu(cmd->cdw11); | ||||||||
| uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1); | ||||||||
| uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2); | ||||||||
| uint16_t nr_ranges; | ||||||||
| NvmeDsmRange *ranges = NULL; | ||||||||
| int i; | ||||||||
|
|
||||||||
| if (dw11 & NVME_DSMGMT_AD) { | ||||||||
| uint16_t nr = (dw10 & 0xff) + 1; | ||||||||
| // Extract number of ranges from CDW10 (bits 7:0, 0-based) | ||||||||
| nr_ranges = (cdw10 & 0xFF) + 1; | ||||||||
|
|
||||||||
| // Validate range count - NVMe supports up to 256 ranges | ||||||||
| if (nr_ranges > 256) { | ||||||||
| femu_err("DSM: Invalid range count %u (max 256)\n", nr_ranges); | ||||||||
| nvme_set_error_page(n, req->sq->sqid, cmd->cid, NVME_INVALID_FIELD, | ||||||||
| offsetof(NvmeCmd, cdw10), nr_ranges, ns->id); | ||||||||
| return NVME_INVALID_FIELD | NVME_DNR; | ||||||||
| } | ||||||||
|
|
||||||||
| // Check if any deallocate operation is requested | ||||||||
| bool has_deallocate = (cdw11 & NVME_DSMGMT_AD) != 0; | ||||||||
| // bool has_idr = (cdw11 & NVME_DSMGMT_IDR) != 0; | ||||||||
| // bool has_idw = (cdw11 & NVME_DSMGMT_IDW) != 0; | ||||||||
|
|
||||||||
| // femu_debug("DSM: nr_ranges=%u, AD=%d, IDR=%d, IDW=%d\n", | ||||||||
| // nr_ranges, has_deallocate, has_idr, has_idw); | ||||||||
|
|
||||||||
| // If no deallocate attribute, no need to process further for TRIM | ||||||||
| if (!has_deallocate) { | ||||||||
| femu_err("DSM: No deallocate attribute set, skipping\n"); | ||||||||
|
||||||||
| femu_err("DSM: No deallocate attribute set, skipping\n"); | |
| femu_debug("DSM: No deallocate attribute set, skipping\n"); | |
| req->cmd_opcode = cmd->opcode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The counters
total_trimmed_pages,total_already_invalid, andtotal_out_of_boundsare never used or logged. Remove them or emit a summary log to reduce dead code and improve observability.