| Message ID | 20190626014759.15285-2-damien.lemoal@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix zone revalidation memory allocation failures | expand |
On Wed, Jun 26, 2019 at 10:47:57AM +0900, Damien Le Moal wrote: > @@ -1501,9 +1502,14 @@ struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len, > unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT; > unsigned long start = kaddr >> PAGE_SHIFT; > const int nr_pages = end - start; > + bool is_vmalloc = is_vmalloc_addr(data); > + struct page *page; > int offset, i; > struct bio *bio; > > + if (is_vmalloc) > + invalidate_kernel_vmap_range(data, len); That is not correct. The submission path needs an unconditional flush_kernel_vmap_range call, and the read completion path will additionally need the invalidate_kernel_vmap_range call.
Christoph, On 2019/06/26 15:10, Christoph Hellwig wrote: > On Wed, Jun 26, 2019 at 10:47:57AM +0900, Damien Le Moal wrote: >> @@ -1501,9 +1502,14 @@ struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len, >> unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT; >> unsigned long start = kaddr >> PAGE_SHIFT; >> const int nr_pages = end - start; >> + bool is_vmalloc = is_vmalloc_addr(data); >> + struct page *page; >> int offset, i; >> struct bio *bio; >> >> + if (is_vmalloc) >> + invalidate_kernel_vmap_range(data, len); > > That is not correct. > > The submission path needs an unconditional flush_kernel_vmap_range call, > and the read completion path will additionally need the > invalidate_kernel_vmap_range call. > I mimicked what XFS and DM do with vmalloc-ed buffers. I guess I missed something. So in this case, the allocation is in sd_zbc.c, where the completion is too. So I think it may be better to have flush_kernel_vmap_range() right after the allocation before scsi_execute_req() is called and do the invalidate_kernel_vmap_range() before scanning the report zones output for transformation into struct blk_zone ? And do not do anything in bio_map_kern beside the change from virt_to_page() to vmalloc_to_page() ?
diff --git a/block/bio.c b/block/bio.c index ce797d73bb43..46e0b970e287 100644 --- a/block/bio.c +++ b/block/bio.c @@ -16,6 +16,7 @@ #include <linux/workqueue.h> #include <linux/cgroup.h> #include <linux/blk-cgroup.h> +#include <linux/highmem.h> #include <trace/events/block.h> #include "blk.h" @@ -1501,9 +1502,14 @@ struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len, unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT; unsigned long start = kaddr >> PAGE_SHIFT; const int nr_pages = end - start; + bool is_vmalloc = is_vmalloc_addr(data); + struct page *page; int offset, i; struct bio *bio; + if (is_vmalloc) + invalidate_kernel_vmap_range(data, len); + bio = bio_kmalloc(gfp_mask, nr_pages); if (!bio) return ERR_PTR(-ENOMEM); @@ -1518,7 +1524,11 @@ struct bio *bio_map_kern(struct request_queue *q, void *data, unsigned int len, if (bytes > len) bytes = len; - if (bio_add_pc_page(q, bio, virt_to_page(data), bytes, + if (is_vmalloc) + page = vmalloc_to_page(data); + else + page = virt_to_page(data); + if (bio_add_pc_page(q, bio, page, bytes, offset) < bytes) { /* we don't support partial mappings */ bio_put(bio);
To allow the SCSI subsystem scsi_execute_req() function to issue requests using large buffers that are better allocated with vmalloc() rather than kmalloc(), modify bio_map_kern() to allow passing a buffer allocated with the vmalloc() function. To do so, simply test the buffer address using is_vmalloc_addr() and use vmalloc_to_page() instead of virt_to_page() to obtain the pages of vmalloc-ed buffers. Fixes: 515ce6061312 ("scsi: sd_zbc: Fix sd_zbc_report_zones() buffer allocation") Fixes: e76239a3748c ("block: add a report_zones method") Cc: stable@vger.kernel.org Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- block/bio.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)