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
|