lkml.org 
[lkml]   [2021]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 2/3] scsi: ufs: Let command trace only for the cmd != null case
From
Date
On 5/23/21 2:14 PM, Bean Huo wrote:
> + opcode = cmd->cmnd[0];
> + if ((opcode == READ_10) || (opcode == WRITE_10)) {
> + /*
> + * Currently we only fully trace read(10) and write(10)
> + * commands
> + */
> + if (cmd->request && cmd->request->bio)
> + lba = cmd->request->bio->bi_iter.bi_sector;

Why does the lba assignment occur inside the if-statement for the
READ_10 and WRITE_10 cases? Has it been considered to move that
assignment before this if-statement?

Does 'lba' represent an offset in units of 512 bytes (sector_t) or an
LBA (logical block address)? In the former case, please rename the
variable 'lba' into 'sector' or 'start_sector'. In the latter case,
please use sectors_to_logical().

Why are READ_16 and WRITE_16 ignored?

Please remove the 'if (cmd->request)' checks since these are not necessary.

> + } else if (opcode == UNMAP) {
> + if (cmd->request) {
> + lba = scsi_get_lba(cmd);
> + transfer_len = blk_rq_bytes(cmd->request);
> }
> }

The name of the variable 'transfer_len' is wrong since blk_rq_bytes()
returns the number of bytes affected on the storage medium instead of
the number of bytes transferred from the host to the storage controller.

> /**
> - * Describes the ufs rpmb wlun.
> - * Used only to send uac.
> + * Describes the ufs rpmb wlun. Used only to send uac.
> */

Is this change related to the rest of this patch?

Thanks,

Bart.

\
 
 \ /
  Last update: 2021-05-24 03:33    [W:0.066 / U:1.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site