| Message ID | 1605672156-29051-1-git-send-email-kaixuxia@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] btrfs: return EAGAIN when receiving a pending signal in the defrag loops | expand |
On 18/11/20 12:02 pm, xiakaixu1987@gmail.com wrote: > From: Kaixu Xia <kaixuxia@tencent.com> > > The variable ret is overwritten by the following variable defrag_count. > Actually the code should return EAGAIN when receiving a pending signal > in the defrag loops. > > Reported-by: Tosk Robot <tencent_os_robot@tencent.com> > Signed-off-by: Kaixu Xia <kaixuxia@tencent.com> > --- > v2 > -return EAGAIN instead of remove the EAGAIN error. Sorry I might have missed in v1. Why was EAGAIN needed here? Return of defrag_count rather makes sense to me as of now. Thanks, Anand > > fs/btrfs/ioctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 69a384145dc6..6f13db6d30bd 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1519,7 +1519,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, > if (btrfs_defrag_cancelled(fs_info)) { > btrfs_debug(fs_info, "defrag_file cancelled"); > ret = -EAGAIN; > - break; > + goto out_ra; > } > > if (!should_defrag_range(inode, (u64)i << PAGE_SHIFT, >
On Wed, Nov 18, 2020 at 12:02:36PM +0800, xiakaixu1987@gmail.com wrote: > From: Kaixu Xia <kaixuxia@tencent.com> > > The variable ret is overwritten by the following variable defrag_count. > Actually the code should return EAGAIN when receiving a pending signal > in the defrag loops. This lacks explanation why is EAGAIN supposed to be the right return value. This is about semantics of the FITRIM ioctl, changing that would affect userspace applications.
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 69a384145dc6..6f13db6d30bd 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1519,7 +1519,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, if (btrfs_defrag_cancelled(fs_info)) { btrfs_debug(fs_info, "defrag_file cancelled"); ret = -EAGAIN; - break; + goto out_ra; } if (!should_defrag_range(inode, (u64)i << PAGE_SHIFT,