It currently returns a page, but callers just check for NULL/page to gauge success. Clean this up and return the appropriate error directly instead.
Cc: stable@vger.kernel.org # 5.10+ Signed-off-by: Jens Axboe axboe@kernel.dk --- drivers/block/brd.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c index 20acc4a1fd6d..15a148d5aad9 100644 --- a/drivers/block/brd.c +++ b/drivers/block/brd.c @@ -78,11 +78,9 @@ static struct page *brd_lookup_page(struct brd_device *brd, sector_t sector) }
/* - * Look up and return a brd's page for a given sector. - * If one does not exist, allocate an empty page, and insert that. Then - * return it. + * Insert a new page for a given sector, if one does not already exist. */ -static struct page *brd_insert_page(struct brd_device *brd, sector_t sector) +static int brd_insert_page(struct brd_device *brd, sector_t sector) { pgoff_t idx; struct page *page; @@ -90,7 +88,7 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector)
page = brd_lookup_page(brd, sector); if (page) - return page; + return 0;
/* * Must use NOIO because we don't want to recurse back into the @@ -99,11 +97,11 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector) gfp_flags = GFP_NOIO | __GFP_ZERO | __GFP_HIGHMEM; page = alloc_page(gfp_flags); if (!page) - return NULL; + return -ENOMEM;
if (radix_tree_preload(GFP_NOIO)) { __free_page(page); - return NULL; + return -ENOMEM; }
spin_lock(&brd->brd_lock); @@ -120,8 +118,7 @@ static struct page *brd_insert_page(struct brd_device *brd, sector_t sector) spin_unlock(&brd->brd_lock);
radix_tree_preload_end(); - - return page; + return 0; }
/* @@ -174,16 +171,17 @@ static int copy_to_brd_setup(struct brd_device *brd, sector_t sector, size_t n) { unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT; size_t copy; + int ret;
copy = min_t(size_t, n, PAGE_SIZE - offset); - if (!brd_insert_page(brd, sector)) - return -ENOSPC; + ret = brd_insert_page(brd, sector); + if (ret) + return ret; if (copy < n) { sector += copy >> SECTOR_SHIFT; - if (!brd_insert_page(brd, sector)) - return -ENOSPC; + ret = brd_insert_page(brd, sector); } - return 0; + return ret; }
/*
Looks good:
Reviewed-by: Christoph Hellwig hch@lst.de
Cc: stable@vger.kernel.org # 5.10+
But why is this a stable candidate?
On 2/16/23 9:05 AM, Christoph Hellwig wrote:
Looks good:
Reviewed-by: Christoph Hellwig hch@lst.de
Cc: stable@vger.kernel.org # 5.10+
But why is this a stable candidate?
Only because the other patches depend on it.
On Thu, Feb 16, 2023 at 09:12:33AM -0700, Jens Axboe wrote:
On 2/16/23 9:05 AM, Christoph Hellwig wrote:
Looks good:
Reviewed-by: Christoph Hellwig hch@lst.de
Cc: stable@vger.kernel.org # 5.10+
But why is this a stable candidate?
Only because the other patches depend on it.
But none of those is stable material either as I can tell. It's a fairly simple and nice to have enhancement, but not really a grave bug or regression fix.
On 2/16/23 9:14 AM, Christoph Hellwig wrote:
On Thu, Feb 16, 2023 at 09:12:33AM -0700, Jens Axboe wrote:
On 2/16/23 9:05 AM, Christoph Hellwig wrote:
Looks good:
Reviewed-by: Christoph Hellwig hch@lst.de
Cc: stable@vger.kernel.org # 5.10+
But why is this a stable candidate?
Only because the other patches depend on it.
But none of those is stable material either as I can tell. It's a fairly simple and nice to have enhancement, but not really a grave bug or regression fix.
It causes a big perf regression when swapping between IO backends, and even caused confusion for that initial reporter. So while it's not a very important crash fix, I do think shoving into stable is prudent.
Looks good, Reviewed-by: Johannes Thumshirn johannes.thumshirn@wdc.com
linux-stable-mirror@lists.linaro.org
-
Christoph Hellwig -
Jens Axboe -
Johannes Thumshirn