| Message ID | 20210415050448.267306-5-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: add full read-write support for subpage | expand |
On 4/15/21 1:04 AM, Qu Wenruo wrote: > The new function, submit_eb_subpage(), will submit all the dirty extent > buffers in the page. > > The major difference between submit_eb_page() and submit_eb_subpage() > is: > - How to grab extent buffer > Now we use find_extent_buffer_nospinlock() other than using > page::private. > > All other different handling is already done in functions like > lock_extent_buffer_for_io() and write_one_eb(). > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/extent_io.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 95 insertions(+) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index c068c2fcba09..7d1fca9b87f0 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -4323,6 +4323,98 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb, > return ret; > } > > +/* > + * Submit one subpage btree page. > + * > + * The main difference between submit_eb_page() is: > + * - Page locking > + * For subpage, we don't rely on page locking at all. > + * > + * - Flush write bio > + * We only flush bio if we may be unable to fit current extent buffers into > + * current bio. > + * > + * Return >=0 for the number of submitted extent buffers. > + * Return <0 for fatal error. > + */ > +static int submit_eb_subpage(struct page *page, > + struct writeback_control *wbc, > + struct extent_page_data *epd) > +{ > + struct btrfs_fs_info *fs_info = btrfs_sb(page->mapping->host->i_sb); > + int submitted = 0; > + u64 page_start = page_offset(page); > + int bit_start = 0; > + int nbits = BTRFS_SUBPAGE_BITMAP_SIZE; > + int sectors_per_node = fs_info->nodesize >> fs_info->sectorsize_bits; > + int ret; > + > + /* Lock and write each dirty extent buffers in the range */ > + while (bit_start < nbits) { > + struct btrfs_subpage *subpage = (struct btrfs_subpage *)page->private; > + struct extent_buffer *eb; > + unsigned long flags; > + u64 start; > + > + /* > + * Take private lock to ensure the subpage won't be detached > + * halfway. > + */ > + spin_lock(&page->mapping->private_lock); > + if (!PagePrivate(page)) { > + spin_unlock(&page->mapping->private_lock); > + break; > + } > + spin_lock_irqsave(&subpage->lock, flags); writepages doesn't get called with irq context, so you can just do spin_lock_irq()/spin_unlock_irq(). > + if (!((1 << bit_start) & subpage->dirty_bitmap)) { Can we make this a helper so it's more clear what's going on here? Thanks, Josef
On 2021/4/16 上午3:27, Josef Bacik wrote: > On 4/15/21 1:04 AM, Qu Wenruo wrote: >> The new function, submit_eb_subpage(), will submit all the dirty extent >> buffers in the page. >> >> The major difference between submit_eb_page() and submit_eb_subpage() >> is: >> - How to grab extent buffer >> Now we use find_extent_buffer_nospinlock() other than using >> page::private. >> >> All other different handling is already done in functions like >> lock_extent_buffer_for_io() and write_one_eb(). >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/extent_io.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 95 insertions(+) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index c068c2fcba09..7d1fca9b87f0 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -4323,6 +4323,98 @@ static noinline_for_stack int >> write_one_eb(struct extent_buffer *eb, >> return ret; >> } >> +/* >> + * Submit one subpage btree page. >> + * >> + * The main difference between submit_eb_page() is: >> + * - Page locking >> + * For subpage, we don't rely on page locking at all. >> + * >> + * - Flush write bio >> + * We only flush bio if we may be unable to fit current extent >> buffers into >> + * current bio. >> + * >> + * Return >=0 for the number of submitted extent buffers. >> + * Return <0 for fatal error. >> + */ >> +static int submit_eb_subpage(struct page *page, >> + struct writeback_control *wbc, >> + struct extent_page_data *epd) >> +{ >> + struct btrfs_fs_info *fs_info = btrfs_sb(page->mapping->host->i_sb); >> + int submitted = 0; >> + u64 page_start = page_offset(page); >> + int bit_start = 0; >> + int nbits = BTRFS_SUBPAGE_BITMAP_SIZE; >> + int sectors_per_node = fs_info->nodesize >> >> fs_info->sectorsize_bits; >> + int ret; >> + >> + /* Lock and write each dirty extent buffers in the range */ >> + while (bit_start < nbits) { >> + struct btrfs_subpage *subpage = (struct btrfs_subpage >> *)page->private; >> + struct extent_buffer *eb; >> + unsigned long flags; >> + u64 start; >> + >> + /* >> + * Take private lock to ensure the subpage won't be detached >> + * halfway. >> + */ >> + spin_lock(&page->mapping->private_lock); >> + if (!PagePrivate(page)) { >> + spin_unlock(&page->mapping->private_lock); >> + break; >> + } >> + spin_lock_irqsave(&subpage->lock, flags); > > writepages doesn't get called with irq context, so you can just do > spin_lock_irq()/spin_unlock_irq(). But this spinlock is used in endio function. If we don't use irqsave variant here, won't an endio interruption call sneak in and screw up everything? > >> + if (!((1 << bit_start) & subpage->dirty_bitmap)) { > > Can we make this a helper so it's more clear what's going on here? Thanks, That makes sense. Thanks, Qu > > Josef
On 4/15/21 7:28 PM, Qu Wenruo wrote: > > > On 2021/4/16 上午3:27, Josef Bacik wrote: >> On 4/15/21 1:04 AM, Qu Wenruo wrote: >>> The new function, submit_eb_subpage(), will submit all the dirty extent >>> buffers in the page. >>> >>> The major difference between submit_eb_page() and submit_eb_subpage() >>> is: >>> - How to grab extent buffer >>> Now we use find_extent_buffer_nospinlock() other than using >>> page::private. >>> >>> All other different handling is already done in functions like >>> lock_extent_buffer_for_io() and write_one_eb(). >>> >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> fs/btrfs/extent_io.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 95 insertions(+) >>> >>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >>> index c068c2fcba09..7d1fca9b87f0 100644 >>> --- a/fs/btrfs/extent_io.c >>> +++ b/fs/btrfs/extent_io.c >>> @@ -4323,6 +4323,98 @@ static noinline_for_stack int >>> write_one_eb(struct extent_buffer *eb, >>> return ret; >>> } >>> +/* >>> + * Submit one subpage btree page. >>> + * >>> + * The main difference between submit_eb_page() is: >>> + * - Page locking >>> + * For subpage, we don't rely on page locking at all. >>> + * >>> + * - Flush write bio >>> + * We only flush bio if we may be unable to fit current extent >>> buffers into >>> + * current bio. >>> + * >>> + * Return >=0 for the number of submitted extent buffers. >>> + * Return <0 for fatal error. >>> + */ >>> +static int submit_eb_subpage(struct page *page, >>> + struct writeback_control *wbc, >>> + struct extent_page_data *epd) >>> +{ >>> + struct btrfs_fs_info *fs_info = btrfs_sb(page->mapping->host->i_sb); >>> + int submitted = 0; >>> + u64 page_start = page_offset(page); >>> + int bit_start = 0; >>> + int nbits = BTRFS_SUBPAGE_BITMAP_SIZE; >>> + int sectors_per_node = fs_info->nodesize >> >>> fs_info->sectorsize_bits; >>> + int ret; >>> + >>> + /* Lock and write each dirty extent buffers in the range */ >>> + while (bit_start < nbits) { >>> + struct btrfs_subpage *subpage = (struct btrfs_subpage >>> *)page->private; >>> + struct extent_buffer *eb; >>> + unsigned long flags; >>> + u64 start; >>> + >>> + /* >>> + * Take private lock to ensure the subpage won't be detached >>> + * halfway. >>> + */ >>> + spin_lock(&page->mapping->private_lock); >>> + if (!PagePrivate(page)) { >>> + spin_unlock(&page->mapping->private_lock); >>> + break; >>> + } >>> + spin_lock_irqsave(&subpage->lock, flags); >> >> writepages doesn't get called with irq context, so you can just do >> spin_lock_irq()/spin_unlock_irq(). > > But this spinlock is used in endio function. > If we don't use irqsave variant here, won't an endio interruption call > sneak in and screw up everything? > No, you use irqsave if the function can be called under irq. So in the endio call you do irqsave. This function can't be called by an interrupt handler, so just _irq() is fine because you just need to disable irq's. Thanks, Josef
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index c068c2fcba09..7d1fca9b87f0 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -4323,6 +4323,98 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb, return ret; } +/* + * Submit one subpage btree page. + * + * The main difference between submit_eb_page() is: + * - Page locking + * For subpage, we don't rely on page locking at all. + * + * - Flush write bio + * We only flush bio if we may be unable to fit current extent buffers into + * current bio. + * + * Return >=0 for the number of submitted extent buffers. + * Return <0 for fatal error. + */ +static int submit_eb_subpage(struct page *page, + struct writeback_control *wbc, + struct extent_page_data *epd) +{ + struct btrfs_fs_info *fs_info = btrfs_sb(page->mapping->host->i_sb); + int submitted = 0; + u64 page_start = page_offset(page); + int bit_start = 0; + int nbits = BTRFS_SUBPAGE_BITMAP_SIZE; + int sectors_per_node = fs_info->nodesize >> fs_info->sectorsize_bits; + int ret; + + /* Lock and write each dirty extent buffers in the range */ + while (bit_start < nbits) { + struct btrfs_subpage *subpage = (struct btrfs_subpage *)page->private; + struct extent_buffer *eb; + unsigned long flags; + u64 start; + + /* + * Take private lock to ensure the subpage won't be detached + * halfway. + */ + spin_lock(&page->mapping->private_lock); + if (!PagePrivate(page)) { + spin_unlock(&page->mapping->private_lock); + break; + } + spin_lock_irqsave(&subpage->lock, flags); + if (!((1 << bit_start) & subpage->dirty_bitmap)) { + spin_unlock_irqrestore(&subpage->lock, flags); + spin_unlock(&page->mapping->private_lock); + bit_start++; + continue; + } + + start = page_start + bit_start * fs_info->sectorsize; + bit_start += sectors_per_node; + + /* + * Here we just want to grab the eb without touching extra + * spin locks. So here we call find_extent_buffer_nospinlock(). + */ + eb = find_extent_buffer_nospinlock(fs_info, start); + spin_unlock_irqrestore(&subpage->lock, flags); + spin_unlock(&page->mapping->private_lock); + + /* + * The eb has already reached 0 refs thus find_extent_buffer() + * doesn't return it. We don't need to write back such eb + * anyway. + */ + if (!eb) + continue; + + ret = lock_extent_buffer_for_io(eb, epd); + if (ret == 0) { + free_extent_buffer(eb); + continue; + } + if (ret < 0) { + free_extent_buffer(eb); + goto cleanup; + } + ret = write_one_eb(eb, wbc, epd); + free_extent_buffer(eb); + if (ret < 0) + goto cleanup; + submitted++; + } + return submitted; + +cleanup: + /* We hit error, end bio for the submitted extent buffers */ + end_write_bio(epd, ret); + return ret; +} + /* * Submit all page(s) of one extent buffer. * @@ -4355,6 +4447,9 @@ static int submit_eb_page(struct page *page, struct writeback_control *wbc, if (!PagePrivate(page)) return 0; + if (btrfs_sb(page->mapping->host->i_sb)->sectorsize < PAGE_SIZE) + return submit_eb_subpage(page, wbc, epd); + spin_lock(&mapping->private_lock); if (!PagePrivate(page)) { spin_unlock(&mapping->private_lock);
The new function, submit_eb_subpage(), will submit all the dirty extent buffers in the page. The major difference between submit_eb_page() and submit_eb_subpage() is: - How to grab extent buffer Now we use find_extent_buffer_nospinlock() other than using page::private. All other different handling is already done in functions like lock_extent_buffer_for_io() and write_one_eb(). Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent_io.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)