Re: [PATCH v3 3/3] scsi: Set allocation length to 255 for ATA Information VPD page

[Maciej W. Rozycki]

Martin,

> >  So I do have all the reasons to conclude this value has indeed been 
> > arbitrarily chosen, don't I?
> 
> The SAT spec defines the contents of the first part of the page. The
> trailing 512 bytes are defined in the ATA spec.

 I think that would best be reflected in code somehow as lone `64' doesn't 
say anything really.

> > If you insist that the value of 64 stay, then please come up with a
> > suitable macro name to define along with SCSI_VPD_PG_LEN that reflects
> > the meaning of 64 in this context and I'll be happy to update 3/3
> > accordingly, but please explain why the value of 64 is any better than
> > 255 here and the inconsistency with the buffer size justified.
> 
> Can please you try the following patch?

 I have tried it and it's neutral, that is with 1/3 applied the HBA still 
works and with 1/3 removed it still breaks (2/3 and 3/3 obviously don't 
build anymore).  Unsurprisingly, as it's the call to `scsi_get_vpd_page' 
rather than `scsi_get_vpd_buf' that causes an issue here.

 I think the latter function isn't called in my setup, and it's not clear 
to me anymore after so long why I didn't poke at it.  It looks to me like 
the `retry_pg' code there can be gone now with your update in place as it 
duplicates buffer sizing, and with that included it'll be a nice clean-up.

 NB you'll need to adjust drivers/scsi/mpt3sas/mpt3sas_scsih.c accordingly 
if we are to move forward with this change, as it's another user of the 
SCSI_VPD_PG_LEN macro.

 Given what has been said in the discussion so far would you consider 2/3
and 3/3 unnecessary?  In the course of verifying your change I have looked 
through our code again and found that inline magic numbers are there in 
several though not all places where `scsi_get_vpd_page' gets called, so I 
think it would make sense to get rid of them all at once with a single 
self-contained change.

 Thank you for your input and the extra fix.

  Maciej