linux-kernel - Re: [PATCH RFC v3 04/22] scsi: core: Add support to send reserved commands

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Oct 2022 18:18:17 +0900
From:   Damien Le Moal <damien.lemoal@...nsource.wdc.com>
To:     John Garry <john.garry@...wei.com>, axboe@...nel.dk,
        jejb@...ux.ibm.com, martin.petersen@...cle.com,
        jinpu.wang@...ud.ionos.com, hare@...e.de, bvanassche@....org,
        hch@....de, ming.lei@...hat.com, niklas.cassel@....com
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-ide@...r.kernel.org, linux-scsi@...r.kernel.org,
        linuxarm@...wei.com
Subject: Re: [PATCH RFC v3 04/22] scsi: core: Add support to send reserved
 commands

On 10/27/22 18:13, John Garry wrote:
> On 27/10/2022 02:21, Damien Le Moal wrote:
>>>   +    if (blk_mq_is_reserved_rq(rq)) {
>>> +        struct scsi_device *sdev = cmd->device;
>> This variable is not really needed. You can call:
>>        
>>         scsi_device_unbusy(cmd->device, cmd);
>>
>> No ?
> 
> ok, your suggestion is good
> 
>>
>>> +
>>> +        scsi_mq_uninit_cmd(cmd);
>>> +        scsi_device_unbusy(sdev, cmd);
>>> +        __blk_mq_end_request(rq, 0);
>>> +
>>> +        return;
>>> +    }
>>> +
>>>       INIT_LIST_HEAD(&cmd->eh_entry);
>>>         atomic_inc(&cmd->device->iodone_cnt);
>>> @@ -1718,6 +1728,21 @@ static blk_status_t scsi_queue_rq(struct
>>> blk_mq_hw_ctx *hctx,
>>>       blk_status_t ret;
>>>       int reason;
>>>   +    if (blk_mq_is_reserved_rq(req)) {
>>> +        if (!(req->rq_flags & RQF_DONTPREP)) {
>>> +            ret = scsi_prepare_cmd(req);
>>> +            if (ret != BLK_STS_OK)
>>> +                goto out_dec_host_busy;
>>> +
>>> +            req->rq_flags |= RQF_DONTPREP;
>>> +        } else {
>>> +            clear_bit(SCMD_STATE_COMPLETE, &cmd->state);
>>> +        }
>>> +        blk_mq_start_request(req);
>>> +
>>> +        return shost->hostt->reserved_queuecommand(shost, cmd);
>>> +    }
>>> +
>>>       WARN_ON_ONCE(cmd->budget_token < 0);
>>>         /*
>>> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
>>> index 91678c77398e..a39f36aa0b0d 100644
>>> --- a/include/scsi/scsi_host.h
>>> +++ b/include/scsi/scsi_host.h
>>> @@ -73,6 +73,7 @@ struct scsi_host_template {
>>>        * STATUS: REQUIRED
>>>        */
>>>       int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
>>> +    int (*reserved_queuecommand)(struct Scsi_Host *, struct
>>> scsi_cmnd *);
>> Nit: This op name sound like something returning a bool... May be a
>> straight "queue_reserved_command" name would be clearer ?
> 
> or queuecommand_reserved ? I'm just trying to have the name a variant of
> "queuecommand".

I figured that :)
queuereservedcommand ? (hard to read...)
queuecommand_reserved is OK I guess.

> 
>>
> 
> thanks,
> John

-- 
Damien Le Moal
Western Digital Research