Re: [PATCH] xfs: fix extent busy updating

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Darrick,

> On Jan 4, 2023, at 8:24 AM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> 
> On Tue, Jan 03, 2023 at 11:32:17AM -0800, Wengang Wang wrote:
>> In xfs_extent_busy_update_extent() case 6 and 7, whenever bno is modified on
>> extent busy, the relavent length has to be modified accordingly.
>> 
>> Signed-off-by: Wengang Wang <wen.gang.wang@xxxxxxxxxx>
>> ---
>> fs/xfs/xfs_extent_busy.c | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
>> index ad22a003f959..f3d328e4a440 100644
>> --- a/fs/xfs/xfs_extent_busy.c
>> +++ b/fs/xfs/xfs_extent_busy.c
>> @@ -236,6 +236,7 @@ xfs_extent_busy_update_extent(
>> 		 *
>> 		 */
>> 		busyp->bno = fend;
>> +		busyp->length = bend - fend;
> 
> Looks correct to me, but how did you find this?

I was working with a UEK5 XFS bug where busy blocks (contained in extent busy) are allocated to regular files unexpectedly.
When I was trying to fix that problem (still reuse busy blocks for directories), the problem here is exposed.


>  Is there some sort of
> test case we could attach to this?

Hm.. I can only reproduce this with my patch. Well, the idea is that, for example,

1) we have an extent busy in the busy tree:    (bno=100, len=200)
2) allocate blocks for directories from above extent busy (multiple times)
3) after the allocations, above extent busy finally becomes  (bno=300, len=200)  though it should become (bno=300, len=0) and should be removed from the busy tree.
4) the block 300 (in that AG) is used as metadata (directory blocks containing dir entries) and then that block is freed
5) insert the new extent busy (bno=300, len=1) to the busy tree,
in function xfs_extent_busy_insert():

 61         while (*rbp) {
 62                 parent = *rbp;
 63                 busyp = rb_entry(parent, struct xfs_extent_busy, rb_node);
 64
 65                 if (new->bno < busyp->bno) {
 66                         rbp = &(*rbp)->rb_left;
 67                         ASSERT(new->bno + new->length <= busyp->bno);
 68                 } else if (new->bno > busyp->bno) {
 69                         rbp = &(*rbp)->rb_right;
 70                         ASSERT(bno >= busyp->bno + busyp->length);
 71                 } else {
 72                         ASSERT(0);
 73                 }
 74         }

Note that node (bno=300, len=200) already exists in the tree, the code hits line 72, the “else” case, and enters infinite loop.

thanks,
wengang

> 
> --D
> 
>> 	} else if (bbno < fbno) {
>> 		/*
>> 		 * Case 8:
>> -- 
>> 2.21.0 (Apple Git-122.2)
>> 





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux