Skip to content

Commit 6773da8

Browse files
Christoph HellwigChandan Babu R
authored andcommitted
xfs: fix error returns from xfs_bmapi_write
xfs_bmapi_write can return 0 without actually returning a mapping in mval in two different cases: 1) when there is absolutely no space available to do an allocation 2) when converting delalloc space, and the allocation is so small that it only covers parts of the delalloc extent before the range requested by the caller Callers at best can handle one of these cases, but in many cases can't cope with either one. Switch xfs_bmapi_write to always return a mapping or return an error code instead. For case 1) above ENOSPC is the obvious choice which is very much what the callers expect anyway. For case 2) there is no really good error code, so pick a funky one from the SysV streams portfolio. This fixes the reproducer here: https://lore.kernel.org/linux-xfs/CAEJPjCvT3Uag-pMTYuigEjWZHn1sGMZ0GCjVVCv29tNHK76Cgg@mail.gmail.com0/ which uses reserved blocks to create file systems that are gravely out of space and thus cause at least xfs_file_alloc_space to hang and trigger the lack of ENOSPC handling in xfs_dquot_disk_alloc. Note that this patch does not actually make any caller but xfs_alloc_file_space deal intelligently with case 2) above. Signed-off-by: Christoph Hellwig <[email protected]> Reported-by: 刘通 <[email protected]> Reviewed-by: "Darrick J. Wong" <[email protected]> Signed-off-by: Chandan Babu R <[email protected]>
1 parent 5ce5674 commit 6773da8

File tree

10 files changed

+57
-74
lines changed

10 files changed

+57
-74
lines changed

fs/xfs/libxfs/xfs_attr_remote.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,6 @@ xfs_attr_rmtval_set_blk(
625625
if (error)
626626
return error;
627627

628-
ASSERT(nmap == 1);
629628
ASSERT((map->br_startblock != DELAYSTARTBLOCK) &&
630629
(map->br_startblock != HOLESTARTBLOCK));
631630

fs/xfs/libxfs/xfs_bmap.c

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4217,8 +4217,10 @@ xfs_bmapi_allocate(
42174217
} else {
42184218
error = xfs_bmap_alloc_userdata(bma);
42194219
}
4220-
if (error || bma->blkno == NULLFSBLOCK)
4220+
if (error)
42214221
return error;
4222+
if (bma->blkno == NULLFSBLOCK)
4223+
return -ENOSPC;
42224224

42234225
if (bma->flags & XFS_BMAPI_ZERO) {
42244226
error = xfs_zero_extent(bma->ip, bma->blkno, bma->length);
@@ -4397,6 +4399,15 @@ xfs_bmapi_finish(
43974399
* extent state if necessary. Details behaviour is controlled by the flags
43984400
* parameter. Only allocates blocks from a single allocation group, to avoid
43994401
* locking problems.
4402+
*
4403+
* Returns 0 on success and places the extent mappings in mval. nmaps is used
4404+
* as an input/output parameter where the caller specifies the maximum number
4405+
* of mappings that may be returned and xfs_bmapi_write passes back the number
4406+
* of mappings (including existing mappings) it found.
4407+
*
4408+
* Returns a negative error code on failure, including -ENOSPC when it could not
4409+
* allocate any blocks and -ENOSR when it did allocate blocks to convert a
4410+
* delalloc range, but those blocks were before the passed in range.
44004411
*/
44014412
int
44024413
xfs_bmapi_write(
@@ -4525,10 +4536,16 @@ xfs_bmapi_write(
45254536
ASSERT(len > 0);
45264537
ASSERT(bma.length > 0);
45274538
error = xfs_bmapi_allocate(&bma);
4528-
if (error)
4539+
if (error) {
4540+
/*
4541+
* If we already allocated space in a previous
4542+
* iteration return what we go so far when
4543+
* running out of space.
4544+
*/
4545+
if (error == -ENOSPC && bma.nallocs)
4546+
break;
45294547
goto error0;
4530-
if (bma.blkno == NULLFSBLOCK)
4531-
break;
4548+
}
45324549

45334550
/*
45344551
* If this is a CoW allocation, record the data in
@@ -4566,7 +4583,6 @@ xfs_bmapi_write(
45664583
if (!xfs_iext_next_extent(ifp, &bma.icur, &bma.got))
45674584
eof = true;
45684585
}
4569-
*nmap = n;
45704586

45714587
error = xfs_bmap_btree_to_extents(tp, ip, bma.cur, &bma.logflags,
45724588
whichfork);
@@ -4577,7 +4593,22 @@ xfs_bmapi_write(
45774593
ifp->if_nextents > XFS_IFORK_MAXEXT(ip, whichfork));
45784594
xfs_bmapi_finish(&bma, whichfork, 0);
45794595
xfs_bmap_validate_ret(orig_bno, orig_len, orig_flags, orig_mval,
4580-
orig_nmap, *nmap);
4596+
orig_nmap, n);
4597+
4598+
/*
4599+
* When converting delayed allocations, xfs_bmapi_allocate ignores
4600+
* the passed in bno and always converts from the start of the found
4601+
* delalloc extent.
4602+
*
4603+
* To avoid a successful return with *nmap set to 0, return the magic
4604+
* -ENOSR error code for this particular case so that the caller can
4605+
* handle it.
4606+
*/
4607+
if (!n) {
4608+
ASSERT(bma.nallocs >= *nmap);
4609+
return -ENOSR;
4610+
}
4611+
*nmap = n;
45814612
return 0;
45824613
error0:
45834614
xfs_bmapi_finish(&bma, whichfork, error);
@@ -4685,9 +4716,6 @@ xfs_bmapi_convert_one_delalloc(
46854716
if (error)
46864717
goto out_finish;
46874718

4688-
error = -ENOSPC;
4689-
if (WARN_ON_ONCE(bma.blkno == NULLFSBLOCK))
4690-
goto out_finish;
46914719
if (WARN_ON_ONCE(!xfs_valid_startblock(ip, bma.got.br_startblock))) {
46924720
xfs_bmap_mark_sick(ip, whichfork);
46934721
error = -EFSCORRUPTED;

fs/xfs/libxfs/xfs_da_btree.c

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2297,8 +2297,8 @@ xfs_da_grow_inode_int(
22972297
struct xfs_inode *dp = args->dp;
22982298
int w = args->whichfork;
22992299
xfs_rfsblock_t nblks = dp->i_nblocks;
2300-
struct xfs_bmbt_irec map, *mapp;
2301-
int nmap, error, got, i, mapi;
2300+
struct xfs_bmbt_irec map, *mapp = &map;
2301+
int nmap, error, got, i, mapi = 1;
23022302

23032303
/*
23042304
* Find a spot in the file space to put the new block.
@@ -2314,14 +2314,7 @@ xfs_da_grow_inode_int(
23142314
error = xfs_bmapi_write(tp, dp, *bno, count,
23152315
xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
23162316
args->total, &map, &nmap);
2317-
if (error)
2318-
return error;
2319-
2320-
ASSERT(nmap <= 1);
2321-
if (nmap == 1) {
2322-
mapp = &map;
2323-
mapi = 1;
2324-
} else if (nmap == 0 && count > 1) {
2317+
if (error == -ENOSPC && count > 1) {
23252318
xfs_fileoff_t b;
23262319
int c;
23272320

@@ -2339,16 +2332,13 @@ xfs_da_grow_inode_int(
23392332
args->total, &mapp[mapi], &nmap);
23402333
if (error)
23412334
goto out_free_map;
2342-
if (nmap < 1)
2343-
break;
23442335
mapi += nmap;
23452336
b = mapp[mapi - 1].br_startoff +
23462337
mapp[mapi - 1].br_blockcount;
23472338
}
2348-
} else {
2349-
mapi = 0;
2350-
mapp = NULL;
23512339
}
2340+
if (error)
2341+
goto out_free_map;
23522342

23532343
/*
23542344
* Count the blocks we got, make sure it matches the total.

fs/xfs/scrub/quota_repair.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ xrep_quota_item_fill_bmap_hole(
7777
irec, &nmaps);
7878
if (error)
7979
return error;
80-
if (nmaps != 1)
81-
return -ENOSPC;
8280

8381
dq->q_blkno = XFS_FSB_TO_DADDR(mp, irec->br_startblock);
8482

@@ -444,10 +442,6 @@ xrep_quota_data_fork(
444442
XFS_BMAPI_CONVERT, 0, &nrec, &nmap);
445443
if (error)
446444
goto out;
447-
if (nmap != 1) {
448-
error = -ENOSPC;
449-
goto out;
450-
}
451445
ASSERT(nrec.br_startoff == irec.br_startoff);
452446
ASSERT(nrec.br_blockcount == irec.br_blockcount);
453447

fs/xfs/scrub/rtbitmap_repair.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,6 @@ xrep_rtbitmap_data_mappings(
108108
0, &map, &nmaps);
109109
if (error)
110110
return error;
111-
if (nmaps != 1)
112-
return -EFSCORRUPTED;
113111

114112
/* Commit new extent and all deferred work. */
115113
error = xrep_defer_finish(sc);

fs/xfs/xfs_bmap_util.c

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -721,33 +721,32 @@ xfs_alloc_file_space(
721721
if (error)
722722
goto error;
723723

724-
error = xfs_bmapi_write(tp, ip, startoffset_fsb,
725-
allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
726-
&nimaps);
727-
if (error)
728-
goto error;
729-
730-
ip->i_diflags |= XFS_DIFLAG_PREALLOC;
731-
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
732-
733-
error = xfs_trans_commit(tp);
734-
xfs_iunlock(ip, XFS_ILOCK_EXCL);
735-
if (error)
736-
break;
737-
738724
/*
739725
* If the allocator cannot find a single free extent large
740726
* enough to cover the start block of the requested range,
741-
* xfs_bmapi_write will return 0 but leave *nimaps set to 0.
727+
* xfs_bmapi_write will return -ENOSR.
742728
*
743729
* In that case we simply need to keep looping with the same
744730
* startoffset_fsb so that one of the following allocations
745731
* will eventually reach the requested range.
746732
*/
747-
if (nimaps) {
733+
error = xfs_bmapi_write(tp, ip, startoffset_fsb,
734+
allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
735+
&nimaps);
736+
if (error) {
737+
if (error != -ENOSR)
738+
goto error;
739+
error = 0;
740+
} else {
748741
startoffset_fsb += imapp->br_blockcount;
749742
allocatesize_fsb -= imapp->br_blockcount;
750743
}
744+
745+
ip->i_diflags |= XFS_DIFLAG_PREALLOC;
746+
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
747+
748+
error = xfs_trans_commit(tp);
749+
xfs_iunlock(ip, XFS_ILOCK_EXCL);
751750
}
752751

753752
return error;

fs/xfs/xfs_dquot.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,6 @@ xfs_dquot_disk_alloc(
357357
goto err_cancel;
358358

359359
ASSERT(map.br_blockcount == XFS_DQUOT_CLUSTER_SIZE_FSB);
360-
ASSERT(nmaps == 1);
361360
ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
362361
(map.br_startblock != HOLESTARTBLOCK));
363362

fs/xfs/xfs_iomap.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -322,14 +322,6 @@ xfs_iomap_write_direct(
322322
if (error)
323323
goto out_unlock;
324324

325-
/*
326-
* Copy any maps to caller's array and return any error.
327-
*/
328-
if (nimaps == 0) {
329-
error = -ENOSPC;
330-
goto out_unlock;
331-
}
332-
333325
if (unlikely(!xfs_valid_startblock(ip, imap->br_startblock))) {
334326
xfs_bmap_mark_sick(ip, XFS_DATA_FORK);
335327
error = xfs_alert_fsblock_zero(ip, imap);

fs/xfs/xfs_reflink.c

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -430,13 +430,6 @@ xfs_reflink_fill_cow_hole(
430430
if (error)
431431
return error;
432432

433-
/*
434-
* Allocation succeeded but the requested range was not even partially
435-
* satisfied? Bail out!
436-
*/
437-
if (nimaps == 0)
438-
return -ENOSPC;
439-
440433
convert:
441434
return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);
442435

@@ -499,13 +492,6 @@ xfs_reflink_fill_delalloc(
499492
error = xfs_trans_commit(tp);
500493
if (error)
501494
return error;
502-
503-
/*
504-
* Allocation succeeded but the requested range was not even
505-
* partially satisfied? Bail out!
506-
*/
507-
if (nimaps == 0)
508-
return -ENOSPC;
509495
} while (cmap->br_startoff + cmap->br_blockcount <= imap->br_startoff);
510496

511497
return xfs_reflink_convert_unwritten(ip, imap, cmap, convert_now);

fs/xfs/xfs_rtalloc.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -709,8 +709,6 @@ xfs_growfs_rt_alloc(
709709
nmap = 1;
710710
error = xfs_bmapi_write(tp, ip, oblocks, nblocks - oblocks,
711711
XFS_BMAPI_METADATA, 0, &map, &nmap);
712-
if (!error && nmap < 1)
713-
error = -ENOSPC;
714712
if (error)
715713
goto out_trans_cancel;
716714
/*

0 commit comments

Comments
 (0)