Re: [PATCH v1 1/4] ata: libata: Remove redundant sense_buffer memsets
From: Niklas Cassel
Date: Thu Jun 20 2024 - 07:51:55 EST
On Tue, Jun 18, 2024 at 07:58:56PM +0000, Igor Pylypiv wrote:
>
> I think we should explicitly memset buffers before passing them to
> atapi_eh_request_sense() in atapi_eh_clear_ua() and zpready() so that
> atapi_eh_request_sense() can have the same behavior as ata_eh_request_sense()
> with regards to sense buffer expectations i.e. both functions will expect
> buffers that are already memeset to zero.
Well, you could argue that:
static bool ata_eh_request_sense(struct ata_queued_cmd *qc)
doesn't take a sense_buffer, but:
unsigned int atapi_eh_request_sense(struct ata_device *dev,
u8 *sense_buf, u8 dfl_sense_key)
does, so it makes sense for atapi_eh_request_sense() to memset() the buffer.
> I think that it is still benefitial to remove the redundant memset() from
> the ata_eh_analyze_tf() -> atapi_eh_request_sense() path?
atapi_eh_request_sense() should only be called when ATA_SENSE bit is set,
so this is only called in special circumstances, so it is not like the
memset() is in the hot path.
If you ask me, I think that the current code is fine.
Kind regards,
Niklas