Bart, On 2019/06/21 12:46, Bart Van Assche wrote: > On 6/20/19 5:58 PM, Damien Le Moal wrote: >> The REPORT_ZONES command is executed using scsi_execute_req(). Can we pass a >> vmalloc-ed buffer to that function ? It does look like it since scsi_execute_rq >> calls bio_rq_map_kern() which then calls bio_map_kern() which goes through the >> list of pages of the buffer. Just would like to confirm I understand this correctly. >> >> My concern with using vmalloc is that the worst case scenario will result in all >> pages of the buffer being non contiguous. In this case, since the report zones >> command cannot be split, we would need to limit the allocation to max_segments * >> page size, and that can be pretty small for some HBAs. >> >> Another reason I did not pursue the vmalloc route is that the processing of the >> report zones reply to transform zone information into struct blkzone is really >> painful to do with a vmalloced buffer as every page of the buffer needs to be >> kmap()/kunmap(). The code was like that when REPORT ZONES was processed as a >> BIO/request, but it was a lot of code for not much to be done. Or is there a >> more elegant solution for in-kernel mapping of a vmalloc buffer ? > > Hi Damien, > > I don't think that bio_rq_map_kern() works with vmalloc-ed buffers. How > about using is_vmalloc_addr() inside scsi_execute_req() to determine > whether or not the buffer passed to that function has been allocated > with vmalloc()? There may be other scsi_execute_req() callers that can > benefit from passing a vmalloc-ed buffer to that function. Sure, we could do that. But since most (if not all) users of scsi_execute_req() need only a very small buffer, I am not sure if this would be very useful. > Regarding the maximum segment size: is mpt3sas still the most popular > HBA? Is the maximum segment size 128 for that driver? Is 128 * 4 KB = > 512 KB big enough for the report zones buffer? Sure, it works, but compared to the target 1MB allocation that my patch has, that doubles the number of requests needed for reporting the zones of an entire drive. Since this is however not used a lot, I guess it is okay. But still, I do not like very much slowing down revalidate() nor regular blkdev_report_zones() calls... > Regarding the loop that calls sd_zbc_parse_report(): are you sure that > that kmap()/kunmap() calls would be necessary in that loop? No I am not sure. I will check again. Best regards. -- Damien Le Moal Western Digital Research
