diff options
author | Andreas Gruenbacher <agruenba@redhat.com> | 2019-03-14 11:48:48 -0400 |
---|---|---|
committer | Andreas Gruenbacher <agruenba@redhat.com> | 2019-05-07 16:33:44 -0400 |
commit | 71921ef85928e95e3d942c747c9d40443a5ff775 (patch) | |
tree | 55e56ea812ed519318d7f9d7dbd0c2ad5c27144d /fs/gfs2 | |
parent | b4b52b881cf08e13d110eac811d4becc0775abbf (diff) |
gfs2: Fix loop in gfs2_rbm_find (v2)
Fix the resource group wrap-around logic in gfs2_rbm_find that commit
e579ed4f44 broke. The bug can lead to unnecessary repeated scanning of the
same bitmaps; there is a risk that future changes will turn this into an
endless loop.
This is an updated version of commit 2d29f6b96d ("gfs2: Fix loop in
gfs2_rbm_find") which ended up being reverted because it introduced a
performance regression in iozone (see commit e74c98ca2d). Changes since v1:
- Simplify the wrap-around logic.
- Handle the case where each resource group only has a single bitmap block
(small filesystem).
- Update rd_extfail_pt whenever we scan the entire bitmap, even when we don't
start the scan at the very beginning of the bitmap.
Fixes: e579ed4f446e ("GFS2: Introduce rbm field bii")
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Diffstat (limited to 'fs/gfs2')
-rw-r--r-- | fs/gfs2/rgrp.c | 54 |
1 files changed, 25 insertions, 29 deletions
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c index 17a8d3b43990..52a4f340a867 100644 --- a/fs/gfs2/rgrp.c +++ b/fs/gfs2/rgrp.c | |||
@@ -1729,25 +1729,22 @@ fail: | |||
1729 | static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext, | 1729 | static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext, |
1730 | const struct gfs2_inode *ip, bool nowrap) | 1730 | const struct gfs2_inode *ip, bool nowrap) |
1731 | { | 1731 | { |
1732 | bool scan_from_start = rbm->bii == 0 && rbm->offset == 0; | ||
1732 | struct buffer_head *bh; | 1733 | struct buffer_head *bh; |
1733 | int initial_bii; | 1734 | int last_bii; |
1734 | u32 initial_offset; | ||
1735 | int first_bii = rbm->bii; | ||
1736 | u32 first_offset = rbm->offset; | ||
1737 | u32 offset; | 1735 | u32 offset; |
1738 | u8 *buffer; | 1736 | u8 *buffer; |
1739 | int n = 0; | 1737 | bool wrapped = false; |
1740 | int iters = rbm->rgd->rd_length; | ||
1741 | int ret; | 1738 | int ret; |
1742 | struct gfs2_bitmap *bi; | 1739 | struct gfs2_bitmap *bi; |
1743 | struct gfs2_extent maxext = { .rbm.rgd = rbm->rgd, }; | 1740 | struct gfs2_extent maxext = { .rbm.rgd = rbm->rgd, }; |
1744 | 1741 | ||
1745 | /* If we are not starting at the beginning of a bitmap, then we | 1742 | /* |
1746 | * need to add one to the bitmap count to ensure that we search | 1743 | * Determine the last bitmap to search. If we're not starting at the |
1747 | * the starting bitmap twice. | 1744 | * beginning of a bitmap, we need to search that bitmap twice to scan |
1745 | * the entire resource group. | ||
1748 | */ | 1746 | */ |
1749 | if (rbm->offset != 0) | 1747 | last_bii = rbm->bii - (rbm->offset == 0); |
1750 | iters++; | ||
1751 | 1748 | ||
1752 | while(1) { | 1749 | while(1) { |
1753 | bi = rbm_bi(rbm); | 1750 | bi = rbm_bi(rbm); |
@@ -1761,47 +1758,46 @@ static int gfs2_rbm_find(struct gfs2_rbm *rbm, u8 state, u32 *minext, | |||
1761 | WARN_ON(!buffer_uptodate(bh)); | 1758 | WARN_ON(!buffer_uptodate(bh)); |
1762 | if (state != GFS2_BLKST_UNLINKED && bi->bi_clone) | 1759 | if (state != GFS2_BLKST_UNLINKED && bi->bi_clone) |
1763 | buffer = bi->bi_clone + bi->bi_offset; | 1760 | buffer = bi->bi_clone + bi->bi_offset; |
1764 | initial_offset = rbm->offset; | ||
1765 | offset = gfs2_bitfit(buffer, bi->bi_bytes, rbm->offset, state); | 1761 | offset = gfs2_bitfit(buffer, bi->bi_bytes, rbm->offset, state); |
1766 | if (offset == BFITNOENT) | 1762 | if (offset == BFITNOENT) { |
1767 | goto bitmap_full; | 1763 | if (state == GFS2_BLKST_FREE && rbm->offset == 0) |
1764 | set_bit(GBF_FULL, &bi->bi_flags); | ||
1765 | goto next_bitmap; | ||
1766 | } | ||
1768 | rbm->offset = offset; | 1767 | rbm->offset = offset; |
1769 | if (ip == NULL) | 1768 | if (ip == NULL) |
1770 | return 0; | 1769 | return 0; |
1771 | 1770 | ||
1772 | initial_bii = rbm->bii; | ||
1773 | ret = gfs2_reservation_check_and_update(rbm, ip, | 1771 | ret = gfs2_reservation_check_and_update(rbm, ip, |
1774 | minext ? *minext : 0, | 1772 | minext ? *minext : 0, |
1775 | &maxext); | 1773 | &maxext); |
1776 | if (ret == 0) | 1774 | if (ret == 0) |
1777 | return 0; | 1775 | return 0; |
1778 | if (ret > 0) { | 1776 | if (ret > 0) |
1779 | n += (rbm->bii - initial_bii); | ||
1780 | goto next_iter; | 1777 | goto next_iter; |
1781 | } | ||
1782 | if (ret == -E2BIG) { | 1778 | if (ret == -E2BIG) { |
1783 | rbm->bii = 0; | 1779 | rbm->bii = 0; |
1784 | rbm->offset = 0; | 1780 | rbm->offset = 0; |
1785 | n += (rbm->bii - initial_bii); | ||
1786 | goto res_covered_end_of_rgrp; | 1781 | goto res_covered_end_of_rgrp; |
1787 | } | 1782 | } |
1788 | return ret; | 1783 | return ret; |
1789 | 1784 | ||
1790 | bitmap_full: /* Mark bitmap as full and fall through */ | ||
1791 | if ((state == GFS2_BLKST_FREE) && initial_offset == 0) | ||
1792 | set_bit(GBF_FULL, &bi->bi_flags); | ||
1793 | |||
1794 | next_bitmap: /* Find next bitmap in the rgrp */ | 1785 | next_bitmap: /* Find next bitmap in the rgrp */ |
1795 | rbm->offset = 0; | 1786 | rbm->offset = 0; |
1796 | rbm->bii++; | 1787 | rbm->bii++; |
1797 | if (rbm->bii == rbm->rgd->rd_length) | 1788 | if (rbm->bii == rbm->rgd->rd_length) |
1798 | rbm->bii = 0; | 1789 | rbm->bii = 0; |
1799 | res_covered_end_of_rgrp: | 1790 | res_covered_end_of_rgrp: |
1800 | if ((rbm->bii == 0) && nowrap) | 1791 | if (rbm->bii == 0) { |
1801 | break; | 1792 | if (wrapped) |
1802 | n++; | 1793 | break; |
1794 | wrapped = true; | ||
1795 | if (nowrap) | ||
1796 | break; | ||
1797 | } | ||
1803 | next_iter: | 1798 | next_iter: |
1804 | if (n >= iters) | 1799 | /* Have we scanned the entire resource group? */ |
1800 | if (wrapped && rbm->bii > last_bii) | ||
1805 | break; | 1801 | break; |
1806 | } | 1802 | } |
1807 | 1803 | ||
@@ -1811,8 +1807,8 @@ next_iter: | |||
1811 | /* If the extent was too small, and it's smaller than the smallest | 1807 | /* If the extent was too small, and it's smaller than the smallest |
1812 | to have failed before, remember for future reference that it's | 1808 | to have failed before, remember for future reference that it's |
1813 | useless to search this rgrp again for this amount or more. */ | 1809 | useless to search this rgrp again for this amount or more. */ |
1814 | if ((first_offset == 0) && (first_bii == 0) && | 1810 | if (wrapped && (scan_from_start || rbm->bii > last_bii) && |
1815 | (*minext < rbm->rgd->rd_extfail_pt)) | 1811 | *minext < rbm->rgd->rd_extfail_pt) |
1816 | rbm->rgd->rd_extfail_pt = *minext; | 1812 | rbm->rgd->rd_extfail_pt = *minext; |
1817 | 1813 | ||
1818 | /* If the maximum extent we found is big enough to fulfill the | 1814 | /* If the maximum extent we found is big enough to fulfill the |