On 08/28/18 20:56, Damien Le Moal wrote: > On Tue, 2018-08-28 at 20:23 -0700, Bart Van Assche wrote: >> On 08/28/18 17:57, Damien Le Moal wrote: >>> If yes, then range < 0 is not possible, but range == 0 is definitely still >>> possible and so the +1 is needed. >> >> Have you considered to leave out the "+ 1" and to handle range == 0 >> separately? > > Sure I can do that. But again what about zbd_find_zone() ? Shouldn't it be > changed to look for a zone with at least io_u->buflen bytes rather than only > min_bs ? Otherwise, range can be negative and the test "if (io_u->offset + > io_u->buflen > zb->wp)" at the end of the DDIR_READ case could terminate reads > with eof prematuraly. I think that it's better to shrink a read request than to modify zbd_find_zone(). I think that making zbd_find_zone() look for a zone that is large enough would confuse random reads if the random map is enabled because the random generator would repeatedly propose to submit I/O to the remainder of a zone if the random map is almost full. >>> Looking at get_next_buflen(), io_u->buflen can be larger than min_bs. So >>> either we need to modify zbd_find_zone() to return a zone with at least >>> io_u->buflen bytes in it, or we need to handle the range < 0 case... >> >> The range < 0 case is already handled by the following code at the end >> of the DDIR_WRITE case: > > That is fine, but that is for the DDIR_WRITE case. There is no such code in > the DDIR_READ case. Reads I/Os buflen are not modified based on the zone wp > position... Do you think it is better to add a similar I/O size reduction code > for reads or change zbd_find_zone() as suggested above ? I think the latter is > better to ensure consistent sizes for reads. Modifying the DDIR_READ case such that read requests for a zone with less data than io_u->buflen are shrunk sounds fine to me. Bart.
