On 10/04/2020 08:18, Christoph Hellwig wrote:
> On Fri, Apr 10, 2020 at 01:53:49AM +0900, Johannes Thumshirn wrote:
>> + if (req_op(rq) == REQ_OP_ZONE_APPEND) {
>> + ret = sd_zbc_prepare_zone_append(cmd, &lba, nr_blocks);
>> + if (ret) {
>> + scsi_free_sgtables(cmd);
>> + return ret;
>> + }
>> + }
>
> So actually.
>
> I've been trying to understand the lifetime of the sgtables. Shouldn't
> we free them in the midayer ->init_command returned BLK_STS_*RESOURCE
> instead? It seems like this just catches one particular error instead
> of the whole category? The end of scsi_queue_rq seem like a particular
> good place, as that also releases the resources for the "hard" errors.
>
I did have this in a private version but it leaked the sgtables in case
we had an inconsistent wp_ofst cache (I injected a zone-reset via
/dev/sg so the driver didn't see it).
The code that handles freeing of the sgtables in scsi_queue_rq's error
handling won't get called on BLK_STS_*RESOURCE, which makes perfect
sense at least for BLK_STS_RESOURCE, as if you can't allocate the memory
for the sgtables, there's no need to free it.
The only code in SCSI being a special snowflake there is
sd_zbc_prepare_zone_append(), this is why I ended up freeing the
sgtables after it returned an error.
