>>>> Was referring to this: >>>> -- >>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >>>> index 3345f866178e..dbc402587431 100644 >>>> --- a/drivers/nvme/host/core.c >>>> +++ b/drivers/nvme/host/core.c >>>> @@ -781,6 +781,7 @@ static blk_status_t nvme_setup_discard(struct nvme_ns >>>> *ns, struct request *req, >>>> range = page_address(ns->ctrl->discard_page); >>>> } >>>> >>>> + segments = min(segments, queue_max_discard_segments(req->q)); >>> >>> That can't work. >>> >>> In case of queue_max_discard_segments(req->q) == 1, the request still >>> can have more than one bios since the normal merge is taken for discard >>> IOs. >> >> Ah, I see, the bios are contiguous though right? > > Yes, the merge is just like normal RW. > >> We could add a contiguity check in the loop and conditionally >> increment n, but maybe that would probably be more complicated... > > That is more complicated than this patch, and the same pattern > has been applied on virtio-blk. > I'd very much keep the pattern in virio-blk in nvme that is easier to read and simple than conditional increment of the n, unless there is a strong reason for not doing that, which I failed to understand ... -ck
