RE: [PATCH v1 3/3] scsi: ufs: Make UPIU trace easier differentiate among CDB, OSF, and TM
From: Avri Altman
Date: Tue Dec 08 2020 - 03:37:12 EST
>
> On Mon, 2020-12-07 at 07:57 +0000, Avri Altman wrote:
> > > TP_printk(
> > > - "%s: %s: HDR:%s, CDB:%s",
> > > + "%s: %s: HDR:%s, %s:%s",
> > > __get_str(str), __get_str(dev_name),
> > > __print_hex(__entry->hdr, sizeof(__entry->hdr)),
> > > + __get_str(tsf_type),
> >
> > This breaks what current parsers expects.
> > Why str is not enough to distinguish between the command?
> >
> > Thanks,
> > Avri
>
> Hi Avri
> Tt donesn't break original CDB parser. for the CDB, it is still the
> same as before. Here just make Transaction Specific Fields in the UPIU
> package much clearer.
It does breaks our current parser that expects "CDB:" for all upiu types
>
> I mentioned in the commits message:
>
> Transaction Specific Fields (TSF) in the UPIU package could be CDB
> (SCSI/UFS Command Descriptor Block), OSF (Opcode Specific Field), and
> TM I/O parameter (Task Management Input/Output Parameter). But we
> didn't differenciate them. we take all of these as CDB. This is wrong.
>
> I want to make it clearer and make UPIU trace in line with the Spec.
> what's more, how do you filter OSF, TM parameters with current UPIU
> trace? you take all of them as CDB? if so, I think, it's better to
> change parser.
Indeed, it is just a small change, but breaking user-space is not an acceptable approach.
Also, the upiu tracer was never meant to be human-readable: it just dump the upiu,
Which contains all the info required to parse it anyway,
So breaking user-space just to making it more readable doesn't really make sense?
Looking at the previous 2 patches of this series, I was hoping that you will do the same for
Command upiu, as well?
Again - same comment: if you are doing that need to change the str not to break current parsers.
Thanks,
Avri
>
> Thanks,
> Bean
>
>
>
>
>