Re: [PATCH] btrfs: use preallocated page for super block write

From: David Sterba <dsterba@suse.cz>
To: Nikolay Borisov <nborisov@suse.com>
Cc: David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org, willy@infradead.org
Subject: Re: [PATCH] btrfs: use preallocated page for super block write
Date: Thu, 9 Jun 2022 14:26:30 +0200	[thread overview]
Message-ID: <20220609122630.GS20633@twin.jikos.cz> (raw)
In-Reply-To: <1c8faef3-6ccb-6eb8-6f42-d52faf8f74e1@suse.com>

On Thu, Jun 09, 2022 at 10:30:06AM +0300, Nikolay Borisov wrote:
> 
> 
> On 7.06.22 г. 18:42 ч., David Sterba wrote:
> > Currently the super block page is from the mapping of the block device,
> > this is result of direct conversion from the previous buffer_head to bio
> > conversion.  We don't use the page cache or the mapping anywhere else,
> > the page is a temporary space for the associated bio.
> > 
> > Allocate the page at device allocation time, also to avoid any later
> > allocation problems when writing the super block. This simplifies the
> > page reference tracking, but the page lock is still used as waiting
> > mechanism for the write and write error is tracked in the page.
> > 
> > This was inspired by Matthew's question
> > https://lore.kernel.org/all/Yn%2FtxWbij5voeGOB@casper.infradead.org/
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> 
> nit: I think it's important to remark in the changelog that with this 

If you say it's important, it's not a 'nit'

> change sb writing becomes sequential as opposed to parallel with the old 
> code. This also means that wait_dev_supers can be simplified because the 
> max_mirror's loop is not needed, at least for waiting, since for each 
> device we at most need to wait for the last write to it, as all previous 
> ones have been serialized by the pagelock.

Though it works that way, the serialization was not intentional and I
did not realize that.  find_get_page always provides a different page
and bio was allocated each time.

Either we keep it that way at the cost of slower superblock write or it
needs to be reworked, basically preallocating 3 pages. The write is
paralellized over all devices, I'm not sure how much the per-device
serialization would hurt but on rotational devices it could be
noticeable as the offsets are far and the seek penalty would add up.