| Message ID | 20190603100602.19362-4-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Further FITRIM improvements | expand |
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
On 2019/6/3 下午6:06, Nikolay Borisov wrote: > Currently the first megabyte on a device housing a btrfs filesystem is > exempt from allocation and trimming. Currently this is not a problem > since 'start' is set to 1m at the beginning of btrfs_trim_free_extents > and find_first_clear_extent_bit always returns a range that is >= start. > However, in a follow up patch find_first_clear_extent_bit will be > changed such that it will return a range containing 'start' and this > range may very well be 0...>=1M so 'start'. > > Future proof the sole user of find_first_clear_extent_bit by setting > 'start' after the function is called. No functional changes. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Doesn't that previous patch already address this by: + u64 start = SZ_1M, len = 0, end = 0; Thanks, Qu > --- > fs/btrfs/extent-tree.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index d8c5febf7636..5a11e4988243 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -11183,6 +11183,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed) > * to the caller to trim the value to the size of the device. > */ > end = min(end, device->total_bytes - 1); > + > + /* Ensure we skip first mb in case we have a bootloader there */ > + start = max_t(u64, start, SZ_1M); > + > len = end - start + 1; > > /* We didn't find any extents */ >
On 5.06.19 г. 12:14 ч., Qu Wenruo wrote: > > > On 2019/6/3 下午6:06, Nikolay Borisov wrote: >> Currently the first megabyte on a device housing a btrfs filesystem is >> exempt from allocation and trimming. Currently this is not a problem >> since 'start' is set to 1m at the beginning of btrfs_trim_free_extents >> and find_first_clear_extent_bit always returns a range that is >= start. >> However, in a follow up patch find_first_clear_extent_bit will be >> changed such that it will return a range containing 'start' and this >> range may very well be 0...>=1M so 'start'. >> >> Future proof the sole user of find_first_clear_extent_bit by setting >> 'start' after the function is called. No functional changes. >> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> > > Doesn't that previous patch already address this by: > > + u64 start = SZ_1M, len = 0, end = 0; No, because with the changes introduced in the next patch start can actually be made to point to 0 for example. One of the self-test cases covers this, e.g. : find_first_clear_extent_bit(&tree, SZ_512K, &start, &end, + CHUNK_TRIMMED | CHUNK_ALLOCATED); > > Thanks, > Qu > > >> --- >> fs/btrfs/extent-tree.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index d8c5febf7636..5a11e4988243 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -11183,6 +11183,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed) >> * to the caller to trim the value to the size of the device. >> */ >> end = min(end, device->total_bytes - 1); >> + >> + /* Ensure we skip first mb in case we have a bootloader there */ >> + start = max_t(u64, start, SZ_1M); >> + >> len = end - start + 1; >> >> /* We didn't find any extents */ >> >
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index d8c5febf7636..5a11e4988243 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -11183,6 +11183,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed) * to the caller to trim the value to the size of the device. */ end = min(end, device->total_bytes - 1); + + /* Ensure we skip first mb in case we have a bootloader there */ + start = max_t(u64, start, SZ_1M); + len = end - start + 1; /* We didn't find any extents */
Currently the first megabyte on a device housing a btrfs filesystem is exempt from allocation and trimming. Currently this is not a problem since 'start' is set to 1m at the beginning of btrfs_trim_free_extents and find_first_clear_extent_bit always returns a range that is >= start. However, in a follow up patch find_first_clear_extent_bit will be changed such that it will return a range containing 'start' and this range may very well be 0...>=1M so 'start'. Future proof the sole user of find_first_clear_extent_bit by setting 'start' after the function is called. No functional changes. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/extent-tree.c | 4 ++++ 1 file changed, 4 insertions(+)