| Message ID | 855a8376fa0d8e63e066ac323a985fe7bc1e562f.1601495426.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve preemptive ENOSPC flushing | expand |
On 30.09.20 г. 23:01 ч., Josef Bacik wrote: > If we're flushing space for tickets then we have > space_info->reclaim_size set and we do not need to do background > reclaim. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> I'm fine with this but check my remak below. Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/space-info.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c > index 98207ea57a3d..518749093bc5 100644 > --- a/fs/btrfs/space-info.c > +++ b/fs/btrfs/space-info.c > @@ -805,6 +805,13 @@ static inline int need_preemptive_reclaim(struct btrfs_fs_info *fs_info, > if ((space_info->bytes_used + space_info->bytes_reserved) >= thresh) > return 0; > > + /* > + * We have tickets queued, bail so we don't compete with the async > + * flushers. > + */ > + if (space_info->reclaim_size) > + return 0; > + nit: So where do we draw the line if this check should be in this function or in __reserve_bytes so that we eliminate the case where a preemptive reclaim is scheduled only to return instantly because of tickets. Though the 'if' in __reserve_bytes is getting slitghly out of hand :) > if (!btrfs_calc_reclaim_metadata_size(fs_info, space_info)) > return 0; > >
On 10/1/20 9:23 AM, Nikolay Borisov wrote: > > > On 30.09.20 г. 23:01 ч., Josef Bacik wrote: >> If we're flushing space for tickets then we have >> space_info->reclaim_size set and we do not need to do background >> reclaim. >> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > I'm fine with this but check my remak below. > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> >> --- >> fs/btrfs/space-info.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c >> index 98207ea57a3d..518749093bc5 100644 >> --- a/fs/btrfs/space-info.c >> +++ b/fs/btrfs/space-info.c >> @@ -805,6 +805,13 @@ static inline int need_preemptive_reclaim(struct btrfs_fs_info *fs_info, >> if ((space_info->bytes_used + space_info->bytes_reserved) >= thresh) >> return 0; >> >> + /* >> + * We have tickets queued, bail so we don't compete with the async >> + * flushers. >> + */ >> + if (space_info->reclaim_size) >> + return 0; >> + > > nit: So where do we draw the line if this check should be in this > function or in __reserve_bytes so that we eliminate the case where a > preemptive reclaim is scheduled only to return instantly because of > tickets. Though the 'if' in __reserve_bytes is getting slitghly out of > hand :) > Keep in mind this is also used by the background flushing thread to tell when it's time to stop flushing, which is why the check is in here. Thanks, Josef
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index 98207ea57a3d..518749093bc5 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c @@ -805,6 +805,13 @@ static inline int need_preemptive_reclaim(struct btrfs_fs_info *fs_info, if ((space_info->bytes_used + space_info->bytes_reserved) >= thresh) return 0; + /* + * We have tickets queued, bail so we don't compete with the async + * flushers. + */ + if (space_info->reclaim_size) + return 0; + if (!btrfs_calc_reclaim_metadata_size(fs_info, space_info)) return 0;
If we're flushing space for tickets then we have space_info->reclaim_size set and we do not need to do background reclaim. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/space-info.c | 7 +++++++ 1 file changed, 7 insertions(+)