(Removed Cc invalid emails to Nicholas and Andrzej)
Hi,
On Fri, Dec 20, 2024, Homura Akemi wrote:
Hi Thinh,
On 2024-12-11 0:31 UTC, Thinh Nguyen Thinh.Nguyen@synopsys.com wrote:
- Fix Data Corruption
Properly increment the "len" base on the command requested length instead of the SG entry length.
If you're using File backend, then you need to fix target_core_file. If you're using other backend such as Ramdisk, then you need a similar fix there.
I am trying to do some basic rw aging test with this serie on my gadget board. When it comes to target_core_iblock, the rw code is less similar to the one in target_core_file or ramdisk. Could you just spend some extra time explaining what cause the Data Corruption issue and how can this fix it ? So that I could perform similar fix in target_core_iblock on my own, so a full test could start soonner.
When we prepare a new generic command for read/write, we call to target_alloc_sgl(). This will allocate PAGE_SIZE SG entries enough to handle the se_cmd read/write base on its length. The total length of all the SG entries combine will be at least se_cmd->data_length.
The typical block size is 512 byte. A page size is typically 4KB. So, the se_cmd->data_length may not be a multiple of PAGE_SiZE. In target_core_file, it execute_rw() with this logic:
for_each_sg(sgl, sg, sgl_nents, i) { bvec_set_page(&aio_cmd->bvecs[i], sg_page(sg), sg->length, sg->offset); len += sg->length; }
// It sets the length to be the iter to be total length of the // allocated SG entries and not the requested command length:
iov_iter_bvec(&iter, is_write, aio_cmd->bvecs, sgl_nents, len);
aio_cmd->cmd = cmd; aio_cmd->len = len; aio_cmd->iocb.ki_pos = cmd->t_task_lba * dev->dev_attrib.block_size; aio_cmd->iocb.ki_filp = file; aio_cmd->iocb.ki_complete = cmd_rw_aio_complete; aio_cmd->iocb.ki_flags = IOCB_DIRECT;
if (is_write && (cmd->se_cmd_flags & SCF_FUA)) aio_cmd->iocb.ki_flags |= IOCB_DSYNC;
// So when we do f_op read/write, we may do more than needed and may // write bogus data from the extra SG entry length.
if (is_write) ret = file->f_op->write_iter(&aio_cmd->iocb, &iter); else ret = file->f_op->read_iter(&aio_cmd->iocb, &iter);
I did not review target_core_iblock. It may or may not do things properly.
BR, Thinh