Re: [PATCH 01/12] ubd: untagle discard vs write zeroes not support handling

[Damien Le Moal <dlemoal@xxxxxxxxxx>]

On 5/29/24 14:04, Christoph Hellwig wrote:
> Discard and Write Zeroes are different operation and implemented
> by different fallocate opcodes for ubd.  If one fails the other one
> can work and vice versa.
> 
> Split the code to disable the operations in ubd_handler to only
> disable the operation that actually failed.
> 
> Fixes: 50109b5a03b4 ("um: Add support for DISCARD in the UBD Driver")
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>  arch/um/drivers/ubd_kern.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
> index ef805eaa9e013d..a79a3b7c33a647 100644
> --- a/arch/um/drivers/ubd_kern.c
> +++ b/arch/um/drivers/ubd_kern.c
> @@ -471,9 +471,14 @@ static void ubd_handler(void)
>               for (count = 0; count < n/sizeof(struct io_thread_req *); 
> count++) {
>                       struct io_thread_req *io_req = (*irq_req_buffer)[count];
>  
> -                     if ((io_req->error == BLK_STS_NOTSUPP) && 
> (req_op(io_req->req) == REQ_OP_DISCARD)) {
> -                             blk_queue_max_discard_sectors(io_req->req->q, 
> 0);
> -                             
> blk_queue_max_write_zeroes_sectors(io_req->req->q, 0);
> +                     if (io_req->error == BLK_STS_NOTSUPP) {
> +                             struct request_queue *q = io_req->req->q;
> +
> +                             if (req_op(io_req->req) == REQ_OP_DISCARD)
> +                                     blk_queue_max_discard_sectors(q, 0);
> +                             if (req_op(io_req->req) == REQ_OP_WRITE_ZEROES)

Nit: this can be an "else if".

Otherwise, looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>

> +                                     blk_queue_max_write_zeroes_sectors(q,
> +                                                     0);
>                       }
>                       blk_mq_end_request(io_req->req, io_req->error);
>                       kfree(io_req);

-- 
Damien Le Moal
Western Digital Research