[Top] [All Lists]

Re: [PATCH] kill superflous buffer locking

To: Lachlan McIlroy <lachlan@xxxxxxx>
Subject: Re: [PATCH] kill superflous buffer locking
From: Christoph Hellwig <hch@xxxxxx>
Date: Wed, 28 Nov 2007 10:48:02 +0100
Cc: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx, xfs-dev <xfs-dev@xxxxxxx>
In-reply-to: <474CD2BA.8070204@xxxxxxx>
References: <20070924184926.GA20661@xxxxxx> <4716DD79.6040309@xxxxxxx> <474CD2BA.8070204@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.3.28i
On Wed, Nov 28, 2007 at 01:30:18PM +1100, Lachlan McIlroy wrote:
> Christoph,
> We've fixed the source of the assertion (that was the bugs in
> xfs_buf_associate_memory()) so I'm pushing your buffer lock
> removal patch back in again.
> While looking through it I found a couple of issues:
> - It called unlock_page() before calls to PagePrivate() and
>   PageUptodate().  I think the page needs to be locked during
>   these calls so I moved the unlock_page() further down.

This doesn't really matter at all.  XFS is the only user of the
address_space the pages reside in and we never have overlapping
buffers.  That's the reason why we can remove the buffer locking.
Now if there was a variant of find_or_create_page that didn't set
pages locked at all we could happily use it and get rid of the last
place we deal with locked pages.

> - Unlocking the pages as we go can cause a double unlock in the
>   error handling for a NULL page in the XBF_READ_AHEAD case so I
>   removed the unlocking code for that case.

Indeed.  Thanks for spotting this.

<Prev in Thread] Current Thread [Next in Thread>