RE: [PATCH v1 3/3] scsi: ufs: Make UPIU trace easier differentiate among CDB, OSF, and TM

[Avri Altman]

> 
> 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
> 
> 
> 
> 
>