On Thu, Jun 13, 2019 at 10:24:05AM +0100, Filipe Manana wrote:
> On Thu, Jun 6, 2019 at 2:24 PM Filipe Manana <fdman...@kernel.org> wrote:
> >
> > On Mon, May 13, 2019 at 6:59 PM David Sterba <dste...@suse.cz> wrote:
> > >
> > > On Mon, May 13, 2019 at 05:43:55PM +0100, Filipe Manana wrote:
> > > > On Mon, May 13, 2019 at 5:31 PM David Sterba <dste...@suse.cz> wrote:
> > > > >
> > > > > On Mon, Apr 22, 2019 at 04:44:09PM +0100, fdman...@kernel.org wrote:
> > > > > > From: Filipe Manana <fdman...@suse.com>
> > > > > > --- a/fs/btrfs/send.c
> > > > > > +++ b/fs/btrfs/send.c
> > > > > > @@ -6869,9 +6869,23 @@ long btrfs_ioctl_send(struct file *mnt_file,
> > > > > > struct btrfs_ioctl_send_args *arg)
> > > > > > if (ret)
> > > > > > goto out;
> > > > > >
> > > > > > + mutex_lock(&fs_info->balance_mutex);
> > > > > > + if (test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) {
> > > > > > + mutex_unlock(&fs_info->balance_mutex);
> > > > > > + btrfs_warn_rl(fs_info,
> > > > > > + "Can not run send because a balance operation is in
> > > > > > progress");
> > > > > > + ret = -EAGAIN;
> > > > > > + goto out;
> > > > > > + }
> > > > > > + fs_info->send_in_progress++;
> > > > > > + mutex_unlock(&fs_info->balance_mutex);
> > > > >
> > > > > This would be better in a helper that hides that the balance mutex
> > > > > from
> > > > > send.
> > > >
> > > > Given the large number of cleanup patches that open code helpers that
> > > > had only one caller, this somewhat surprises me.
> > >
> > > Fair point, though I'd object that there are cases where the function
> > > name says in short what happens without the implementation details and
> > > this helps code readability. I struck me when I saw 'send_in_progress
> > > protected by balance_mutex'. You can find functions that are called just
> > > once, that's not an anti-pattern in general.
> > >
> > > I'll take a fresh look later, the setup phase of btrfs_ioctl_send is not
> > > exactly short so the added check does not stand out.
> >
> > So, several weeks passed, and this prevents a quite serious bug from
> > happening.
> > Any progress on that or was I supposed to do something about it?
> >
> > Thanks.
>
> Ping.
Sorry, missed that. I'll apply the patch without changes, the open coding
of the check is not a big deal.
