Re: [PATCH 4/5] xfs: push the ilock into xfs_zero_eof

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

 



On Mon, Mar 26, 2012 at 05:14:25PM -0400, Christoph Hellwig wrote:
> Instead of calling xfs_zero_eof with the ilock held only take it internally
> for the minimall required critical section around xfs_bmapi_read.  This
> also requires changing the calling convention for xfs_zero_last_block
> slightly.  The actual zeroing operation is still serialized by the iolock,
> which must be taken exclusively over the call to xfs_zero_eof.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
.....
> +	struct xfs_mount	*mp = ip->i_mount;
> +	xfs_fileoff_t		last_fsb = XFS_B_TO_FSBT(mp, isize);
> +	int			zero_offset = XFS_B_FSB_OFFSET(mp, isize);
> +	int			zero_len;
> +	int			nimaps = 1;
> +	int			error = 0;
> +	struct xfs_bmbt_irec	imap;
>  
> -	last_fsb = XFS_B_TO_FSBT(mp, isize);
> -	nimaps = 1;
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	error = xfs_bmapi_read(ip, last_fsb, 1, &imap, &nimaps, 0);
> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);

Does that even need to be an exclusive lock? a shared lock is all
that is needed to do a lookup, and this is just a lookup...

....

> @@ -521,23 +503,18 @@ xfs_zero_eof(
>  	while (start_zero_fsb <= end_zero_fsb) {
>  		nimaps = 1;
>  		zero_count_fsb = end_zero_fsb - start_zero_fsb + 1;
> +
> +		xfs_ilock(ip, XFS_ILOCK_EXCL);
>  		error = xfs_bmapi_read(ip, start_zero_fsb, zero_count_fsb,
>  					  &imap, &nimaps, 0);
> -		if (error) {
> -			ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_IOLOCK_EXCL));
> +		xfs_iunlock(ip, XFS_ILOCK_EXCL);

Same question - it is a read lookup so why do we need exclusive
locking here?

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux