Re: [PATCH v3 2/9] ide: Do not set the RQF_PREEMPT flag for sense requests

[Hannes Reinecke <hare@suse.de>]

On 11/23/20 4:17 AM, Bart Van Assche wrote:
> RQF_PREEMPT is used for two different purposes in the legacy IDE code:
> 1. To mark power management requests.
> 2. To mark requests that should preempt another request. An (old)
>     explanation of that feature is as follows:
>     "The IDE driver in the Linux kernel normally uses a series of busywait
>     delays during its initialization. When the driver executes these
>     busywaits, the kernel does nothing for the duration of the wait. The
>     time spent in these waits could be used for other initialization
>     activities, if they could be run concurrently with these waits.
> 
>     More specifically, busywait-style delays such as udelay() in module
>     init functions inhibit kernel preemption because the Big Kernel Lock
>     is held, while yielding APIs such as schedule_timeout() allow preemption.
>     This is true because the kernel handles the BKL specially and releases
>     and reacquires it across reschedules allowed by the current thread.
> 
>     This IDE-preempt specification requires that the driver eliminate these
>     busywaits and replace them with a mechanism that allows other work to
>     proceed while the IDE driver is initializing."
> 
> Since I haven't found an implementation of (2), do not set the PREEMPT
> flag for sense requests. This patch causes sense requests to be
> postponed while a drive is suspended instead of being submitted to
> ide_queue_rq().
> 
> If it would ever be necessary to restore the IDE PREEMPT functionality,
> that can be done by introducing a new flag in struct ide_request.
> 
> This patch is a first step towards removing the PREEMPT flag from the
> block layer.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Can Guo <cang@codeaurora.org>
> Cc: Stanley Chu <stanley.chu@mediatek.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/ide/ide-atapi.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
> index 2162bc80f09e..013ad33fbbc8 100644
> --- a/drivers/ide/ide-atapi.c
> +++ b/drivers/ide/ide-atapi.c
> @@ -223,7 +223,6 @@ void ide_prep_sense(ide_drive_t *drive, struct request *rq)
>   	sense_rq->rq_disk = rq->rq_disk;
>   	sense_rq->cmd_flags = REQ_OP_DRV_IN;
>   	ide_req(sense_rq)->type = ATA_PRIV_SENSE;
> -	sense_rq->rq_flags |= RQF_PREEMPT;
>   
>   	req->cmd[0] = GPCMD_REQUEST_SENSE;
>   	req->cmd[4] = cmd_len;
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer