On 2016-07-13 12:38, David Sterba wrote:
On Tue, Jun 21, 2016 at 11:16:59AM -0400, Austin S. Hemmelgarn wrote:
Currently, balance operations are run synchronously in the foreground.
This is nice for interactive management, but is kind of crappy when you
start looking at automation and similar things.
Yeah, people have been complaining about the lack of backgrounding
support over the time.
TBH, I actually understand why it was implemented the way it was in the
first place, it's a lot easier to work with something running in the
foreground, and having the ability to run in the background adds
complexity, so blocking like it does currently makes sense for an
initial implementation. Beyond the initial implementation, I don't
think it's bothered anyone enough that they wanted to fix it themselves.
This patch adds an option to `btrfs balance start` to tell it to
daemonize prior to running the balance operation, thus allowing us to
preform balances asynchronously. The two biggest use cases I have for
this are starting a balance on a remote server without establishing a
full shell session, and being able to background the balance in a
recovery shell (which usually has no job control) so I can still get
progress information.
Because it simply daemonizes prior to calling the balance ioctl, this
doesn't actually need any kernel support.
We could also add the kernel support, but this would need to extend the
ioctl flags. Unfortunatelly older kernels would ignore it and always let
balance run in foreground (due to lack of checks for the known flags).
So at the moment forking a process seems to be an option.
Ideally, I'd like to see kernel support for this too so that tools that
want to manage it don't need to spawn a thread or fork a new process
just to run it in the background. That said, it's a lot quicker to
implement in userspace, and we'd need such an implementation anyway to
support old kernels.
Signed-off-by: Austin S. Hemmelgarn <ahferro...@gmail.com>
---
This works as is, but there are two specific things I would love to
eventually fix but don't have the time to fix right now:
* There is no way to get any feedback from the balance operation.
Does this mean that 'btrfs balance status' is not sufficient? Or I don't
understand what you mean.
`btrfs balance status` calls still work. What I was referring to is the
lack of the usual 'Done, had to relocate X out of Y chunks.' output on
completion. I probably should have worded this differently in retrospect.
As mentioned below though, in an ideal situation, this would have the
ability to log the results, but doing that the right way is more code
than I'm willing to deal with for a first pass at this.
* Because of how everything works, trying to start a new balance with
--background while one iw already running won't return an error but
won't queue or start a new balance either.
The first one is more a utility item than anything else, and probably
would not be hard to add. Ideally, it should be output to a user
specified file, and this should work even for a normal foreground balance.
The second is very much a UX issue, but can't be easily sovled without
doing some creative process monitoring from the parrent processes.
Currently, starting a second balance will return immediatelly with "in
progress" message, this is returned by the balance iotctl itself. In
this case it would be the child process and communicating it back would
need a pipe or somesuch. But, the parent can check the balance status by
itself, before calling fork, right? Unless I'm missing something, this
should address your concerns.
What I had been thinking was just having the parent wait and see that
the child is still running after a short time (100ms maybe), or has
exited with a 0 exit status. I'm hesitant to add an extra ioctl just to
check if a balance is running because of TOCTOU races, I'd rather have
it deterministically start the balance or not based solely on the
balance ioctl, especially since fork() is expensive enough to make such
a race rather easy to hit). Hopefully, I'll have some time in the near
future to do an updated version that properly handles this.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
