* [PATCH v2] btrfs: don't return EINTR
@ 2012-04-18 8:27 Arne Jansen
2012-04-18 14:22 ` Arne Jansen
2012-04-18 14:36 ` Josef Bacik
0 siblings, 2 replies; 4+ messages in thread
From: Arne Jansen @ 2012-04-18 8:27 UTC (permalink / raw)
To: linux-btrfs
It is basically a good thing if we are interruptible when waiting for
free space, but the generality in which it is implemented currently
leads to system calls being interruptible that are not documented this
way. For example git can't handle interrupted unlink(), leading to
corrupt repos under space pressure.
Instead we raise the bar to only be interruptible by SIGKILL.
Thanks to David Sterba for suggesting this.
Signed-off-by: Arne Jansen <sensille@gmx.net>
---
fs/btrfs/extent-tree.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 2b35f8d..10d4bb7 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3769,13 +3769,10 @@ again:
*/
if (current->journal_info)
return -EAGAIN;
- ret = wait_event_interruptible(space_info->wait,
- !space_info->flush);
- /* Must have been interrupted, return */
- if (ret) {
- printk(KERN_DEBUG "btrfs: %s returning -EINTR\n", __func__);
+ ret = wait_event_killable(space_info->wait, !space_info->flush);
+ /* Must have been killed, return */
+ if (ret)
return -EINTR;
- }
spin_lock(&space_info->lock);
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs: don't return EINTR
2012-04-18 8:27 [PATCH v2] btrfs: don't return EINTR Arne Jansen
@ 2012-04-18 14:22 ` Arne Jansen
2012-04-18 14:36 ` Josef Bacik
1 sibling, 0 replies; 4+ messages in thread
From: Arne Jansen @ 2012-04-18 14:22 UTC (permalink / raw)
To: linux-btrfs; +Cc: Josef Bacik, Chris Mason
On 18.04.2012 10:27, Arne Jansen wrote:
> It is basically a good thing if we are interruptible when waiting for
> free space, but the generality in which it is implemented currently
> leads to system calls being interruptible that are not documented this
> way. For example git can't handle interrupted unlink(), leading to
> corrupt repos under space pressure.
> Instead we raise the bar to only be interruptible by SIGKILL.
> Thanks to David Sterba for suggesting this.
>
> Signed-off-by: Arne Jansen <sensille@gmx.net>
So, is this patch a candidate for the next rc? ;)
-Arne
> ---
> fs/btrfs/extent-tree.c | 9 +++------
> 1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 2b35f8d..10d4bb7 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3769,13 +3769,10 @@ again:
> */
> if (current->journal_info)
> return -EAGAIN;
> - ret = wait_event_interruptible(space_info->wait,
> - !space_info->flush);
> - /* Must have been interrupted, return */
> - if (ret) {
> - printk(KERN_DEBUG "btrfs: %s returning -EINTR\n", __func__);
> + ret = wait_event_killable(space_info->wait, !space_info->flush);
> + /* Must have been killed, return */
> + if (ret)
> return -EINTR;
> - }
>
> spin_lock(&space_info->lock);
> }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs: don't return EINTR
2012-04-18 8:27 [PATCH v2] btrfs: don't return EINTR Arne Jansen
2012-04-18 14:22 ` Arne Jansen
@ 2012-04-18 14:36 ` Josef Bacik
2012-04-18 16:32 ` Chris Mason
1 sibling, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2012-04-18 14:36 UTC (permalink / raw)
To: Arne Jansen; +Cc: linux-btrfs
On Wed, Apr 18, 2012 at 10:27:16AM +0200, Arne Jansen wrote:
> It is basically a good thing if we are interruptible when waiting for
> free space, but the generality in which it is implemented currently
> leads to system calls being interruptible that are not documented this
> way. For example git can't handle interrupted unlink(), leading to
> corrupt repos under space pressure.
> Instead we raise the bar to only be interruptible by SIGKILL.
> Thanks to David Sterba for suggesting this.
>
> Signed-off-by: Arne Jansen <sensille@gmx.net>
> ---
> fs/btrfs/extent-tree.c | 9 +++------
> 1 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 2b35f8d..10d4bb7 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3769,13 +3769,10 @@ again:
> */
> if (current->journal_info)
> return -EAGAIN;
> - ret = wait_event_interruptible(space_info->wait,
> - !space_info->flush);
> - /* Must have been interrupted, return */
> - if (ret) {
> - printk(KERN_DEBUG "btrfs: %s returning -EINTR\n", __func__);
> + ret = wait_event_killable(space_info->wait, !space_info->flush);
> + /* Must have been killed, return */
> + if (ret)
> return -EINTR;
> - }
>
> spin_lock(&space_info->lock);
> }
Ok I like this one,
Reviewed-by: Josef Bacik <josef@redhat.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] btrfs: don't return EINTR
2012-04-18 14:36 ` Josef Bacik
@ 2012-04-18 16:32 ` Chris Mason
0 siblings, 0 replies; 4+ messages in thread
From: Chris Mason @ 2012-04-18 16:32 UTC (permalink / raw)
To: Josef Bacik; +Cc: Arne Jansen, linux-btrfs
On Wed, Apr 18, 2012 at 10:36:11AM -0400, Josef Bacik wrote:
> On Wed, Apr 18, 2012 at 10:27:16AM +0200, Arne Jansen wrote:
> > It is basically a good thing if we are interruptible when waiting for
> > free space, but the generality in which it is implemented currently
> > leads to system calls being interruptible that are not documented this
> > way. For example git can't handle interrupted unlink(), leading to
> > corrupt repos under space pressure.
> > Instead we raise the bar to only be interruptible by SIGKILL.
> > Thanks to David Sterba for suggesting this.
> >
> > Signed-off-by: Arne Jansen <sensille@gmx.net>
> > ---
> > fs/btrfs/extent-tree.c | 9 +++------
> > 1 files changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 2b35f8d..10d4bb7 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -3769,13 +3769,10 @@ again:
> > */
> > if (current->journal_info)
> > return -EAGAIN;
> > - ret = wait_event_interruptible(space_info->wait,
> > - !space_info->flush);
> > - /* Must have been interrupted, return */
> > - if (ret) {
> > - printk(KERN_DEBUG "btrfs: %s returning -EINTR\n", __func__);
> > + ret = wait_event_killable(space_info->wait, !space_info->flush);
> > + /* Must have been killed, return */
> > + if (ret)
> > return -EINTR;
> > - }
> >
> > spin_lock(&space_info->lock);
> > }
>
> Ok I like this one,
>
> Reviewed-by: Josef Bacik <josef@redhat.com>
Perfect.
-chris
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-04-18 16:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-18 8:27 [PATCH v2] btrfs: don't return EINTR Arne Jansen
2012-04-18 14:22 ` Arne Jansen
2012-04-18 14:36 ` Josef Bacik
2012-04-18 16:32 ` Chris Mason
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).