Re: [PATCH 5/5] brd: use memcpy_{to,from]_page in brd_rw_bvec

Yu Kuai
2025-04-22T11:18:09Z

在 2025/04/21 15:26, Christoph Hellwig 写道:
> Use the proper helpers to copy to/from potential highmem pages, which
> do a local instead of atomic kmap underneath, and perform
> flush_dcache_page where needed. This also simplifies the code so much
> that the separate read write helpers are not required any more.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/block/brd.c | 57 ++++++++++-----------------------------------
> 1 file changed, 12 insertions(+), 45 deletions(-)
>
LGTM

Reviewed-by: Yu Kuai <yukuai3@huawei.com>

> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index 580b2d8ce99c..79e96221f887 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -99,43 +99,6 @@ static void brd_free_pages(struct brd_device *brd)
> xa_destroy(&brd->brd_pages);
> }
>
> -/*
> - * Copy n bytes from src to the brd starting at sector. Does not sleep.
> - */
> -static void copy_to_brd(struct brd_device *brd, const void *src,
> - sector_t sector, size_t n)
> -{
> - struct page *page;
> - void *dst;
> - unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT;
> -
> - page = brd_lookup_page(brd, sector);
> - BUG_ON(!page);
> -
> - dst = kmap_atomic(page);
> - memcpy(dst + offset, src, n);
> - kunmap_atomic(dst);
> -}
> -
> -/*
> - * Copy n bytes to dst from the brd starting at sector. Does not sleep.
> - */
> -static void copy_from_brd(void *dst, struct brd_device *brd,
> - sector_t sector, size_t n)
> -{
> - struct page *page;
> - void *src;
> - unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT;
> -
> - page = brd_lookup_page(brd, sector);
> - if (page) {
> - src = kmap_atomic(page);
> - memcpy(dst, src + offset, n);
> - kunmap_atomic(src);
> - } else
> - memset(dst, 0, n);
> -}
> -
> /*
> * Process a single segment. The segment is capped to not cross page boundaries
> * in both the bio and the brd backing memory.
> @@ -146,7 +109,8 @@ static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
> sector_t sector = bio->bi_iter.bi_sector;
> u32 offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
> blk_opf_t opf = bio->bi_opf;
> - void *mem;
> + struct page *page;
> + void *kaddr;
>
> bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
>
> @@ -168,15 +132,18 @@ static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
> }
> }
>
> - mem = bvec_kmap_local(&bv);
> - if (!op_is_write(opf)) {
> - copy_from_brd(mem, brd, sector, bv.bv_len);
> - flush_dcache_page(bv.bv_page);
> + page = brd_lookup_page(brd, sector);
> +
> + kaddr = bvec_kmap_local(&bv);
> + if (op_is_write(opf)) {
> + BUG_ON(!page);
> + memcpy_to_page(page, offset, kaddr, bv.bv_len);
> + } else if (page) {
> + memcpy_from_page(kaddr, page, offset, bv.bv_len);
> } else {
> - flush_dcache_page(bv.bv_page);
> - copy_to_brd(brd, mem, sector, bv.bv_len);
> + memset(kaddr, 0, bv.bv_len);
> }
> - kunmap_local(mem);
> + kunmap_local(kaddr);
>
> bio_advance_iter_single(bio, &bio->bi_iter, bv.bv_len);
> return true;
>