| Message ID | 20230216151918.319585-2-axboe@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support REQ_NWOAIT in brd | expand |
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>
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; } /*
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(-)