On Mon, Jul 02, 2018 at 01:03:41PM +0300, Nikolay Borisov wrote:
>
>
> On 29.06.2018 15:35, David Sterba wrote:
> > On Wed, Jun 27, 2018 at 04:38:22PM +0300, Nikolay Borisov wrote:
> >> The purpose of the function is to free all the pages comprising an
> >> extent buffer. This can be achieved with a simple for loop rather than
> >> the slitghly more involved 'do {} while' construct. So rewrite the
> >> loop using a 'for' construct. Additionally we can never have an
> >> extent_buffer that is 0 pages so remove the check for index == 0. No
> >> functional changes.
> >
> > The reversed loop makes sense, the first page is special and used for
> > locking the whole extent buffer's pages, as can be seen eg.
> > 897ca6e9b4fef86d5dfb6b31 or 4f2de97acee6532b36dd6e99 . Also see current
> > alloc_extent_buffer for the locking and managing the private bit.
>
> Turns out page[0] is not special and it's not used for any special
> locking whatsoever. In csum_dirty_buffer this page is used so that when
> we submit a multipage eb then we only perform csum calculation once, i.e
> when the first page (page[0]) is submitted.
> btrfs_release_extent_buffer_page OTOH no longer takes an index but just
> an EB and iterates all the pages.
>
> The fact that page0 is unlocked last in alloc_extent_buffer seems to be
> an artefact of the code refactoring rather than a deliberate behavior.
> Furthermore I've been running tests with sequential unlocking (and
> complete removal of the SetPageChecked/ClearPageChecked function from
> alloc_extent_buffer) of the pages and didn't observe any problems
> whatsoever on both 4g and 1g configuration ( the latter is more likely
> to trigger premature page eviction hence trigger any lurking races
> between alloc_extent_buffer and btree_releasepage. I will be sending a
> patch after I do a bit more testing but so far the results are good.
With more patch archeology I agree that the page zero is not used in any
sense that would require the reversed loop. Patch added to misc-next.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
