Re: [PATCH 2/2] xfs_db: consolidate set_iocur_type behavior

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

 



On 8/21/20 9:52 AM, Darrick J. Wong wrote:
> On Thu, Aug 20, 2020 at 07:09:45PM -0500, Eric Sandeen wrote:
>> Right now there are 3 cases to type_f: inode type, type with fields,
>> and a default.  The first two were added to address issues with handling
>> V5 metadata.
>>
>> The first two already use some version of set_cur, which handles all
>> of the validation etc. There's no reason to leave the open-coded bits
>> at the end, just send every non-inode type through set_cur and be done
>> with it.
>>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> ---
>>
>>  io.c |   28 +++++-----------------------
>>  1 file changed, 5 insertions(+), 23 deletions(-)
>>
>> diff --git a/db/io.c b/db/io.c
>> index 884da599..235191f5 100644
>> --- a/db/io.c
>> +++ b/db/io.c
>> @@ -603,33 +603,15 @@ set_iocur_type(
>>  				iocur_top->boff / mp->m_sb.sb_inodesize);
>>  		ino = XFS_AGINO_TO_INO(mp, xfs_daddr_to_agno(mp, b), agino);
>>  		set_cur_inode(ino);
>> -		return;
>> -	}
>> -
>> -	/* adjust buffer size for types with fields & hence fsize() */
>> -	if (type->fields) {
>> -		int bb_count;	/* type's size in basic blocks */
>> +	} else  {
> 
> Two spaces    ^^ between the else and the paren.
> 
> I also wonder, why not leave the "return;" at the end of the
> "if (type->typnm == TYP_INODE)" part and then you can reduce the
> indenting level of the rest of the function?

Oh, that might be nice.  I'll see how that looks.

> <shrug> maintainer's prerogative on that one though.
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> 
> /me also wonders why dquots don't get the same "save the buffer offset"
> treatment as inodes, but that's a separate question. :)

hmmmm good question tho.

Thanks for the reviews,
-Eric

> --D
> 
>> +		int bb_count = 1;	/* type's size in basic blocks */
>>  
>> -		bb_count = BTOBB(byteize(fsize(type->fields,
>> +		/* adjust buffer size for types with fields & hence fsize() */
>> +		if (type->fields)
>> +			bb_count = BTOBB(byteize(fsize(type->fields,
>>  					       iocur_top->data, 0, 0)));
>>  		set_cur(type, iocur_top->bb, bb_count, DB_RING_IGN, NULL);
>>  	}
>> -	iocur_top->typ = type;
>> -
>> -	/* verify the buffer if the type has one. */
>> -	if (!bp)
>> -		return;
>> -	if (!type->bops) {
>> -		bp->b_ops = NULL;
>> -		bp->b_flags |= LIBXFS_B_UNCHECKED;
>> -		return;
>> -	}
>> -	if (!(bp->b_flags & LIBXFS_B_UPTODATE))
>> -		return;
>> -	bp->b_error = 0;
>> -	bp->b_ops = type->bops;
>> -	bp->b_ops->verify_read(bp);
>> -	bp->b_flags &= ~LIBXFS_B_UNCHECKED;
>>  }
>>  
>>  static void
>>
> 



[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