* [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs
@ 2012-11-08 12:41 Jan Kara
2012-11-08 12:41 ` [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem Jan Kara
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jan Kara @ 2012-11-08 12:41 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, dchinner, Jan Kara
When user asks for O_NONBLOCK behavior for a file descriptor, return
EAGAIN instead of blocking on a frozen filesystem.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/btrfs/file.c | 3 ++-
fs/cifs/file.c | 3 ++-
fs/fuse/file.c | 3 ++-
fs/ntfs/file.c | 3 ++-
fs/ocfs2/file.c | 3 ++-
fs/xfs/xfs_file.c | 3 ++-
include/linux/fs.h | 10 ++++++++++
mm/filemap.c | 3 ++-
8 files changed, 24 insertions(+), 7 deletions(-)
This is a second version of patches to fix BSD process accounting deadlocks
on frozen filesystem. This time I used O_NONBLOCK to indicate write shouldn't
block on frozen filesystem as Dave Chinner suggested.
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 9ab1bed..6eb2e30 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1411,7 +1411,8 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
ssize_t err = 0;
size_t count, ocount;
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
mutex_lock(&inode->i_mutex);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index edb25b4..1629e47 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2448,7 +2448,8 @@ cifs_writev(struct kiocb *iocb, const struct iovec *iov,
BUG_ON(iocb->ki_pos != pos);
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
/*
* We need to hold the sem to be sure nobody modifies lock list
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 78d2837..641df9e 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -947,7 +947,8 @@ static ssize_t fuse_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
return err;
count = ocount;
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
mutex_lock(&inode->i_mutex);
/* We can write back this queue in page reclaim */
diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 1ecf464..028b349 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -2118,7 +2118,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
BUG_ON(iocb->ki_pos != pos);
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
mutex_lock(&inode->i_mutex);
ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
mutex_unlock(&inode->i_mutex);
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 5a4ee77..93ef34d 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2265,7 +2265,8 @@ static ssize_t ocfs2_file_aio_write(struct kiocb *iocb,
if (iocb->ki_left == 0)
return 0;
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
appending = file->f_flags & O_APPEND ? 1 : 0;
direct_io = file->f_flags & O_DIRECT ? 1 : 0;
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index aa473fa..7d8af61 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -771,7 +771,8 @@ xfs_file_aio_write(
if (ocount == 0)
return 0;
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
if (XFS_FORCED_SHUTDOWN(ip->i_mount)) {
ret = -EIO;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b33cfc9..2325e95 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1401,6 +1401,16 @@ static inline int sb_start_write_trylock(struct super_block *sb)
return __sb_start_write(sb, SB_FREEZE_WRITE, false);
}
+/*
+ * sb_start_write() for writing into a file. When file has O_NONBLOCK set,
+ * we use trylock semantics, otherwise we block on frozen filesystem.
+ */
+static inline int sb_start_file_write(struct file *file)
+{
+ return __sb_start_write(file->f_mapping->host->i_sb, SB_FREEZE_WRITE,
+ !(file->f_flags & O_NONBLOCK));
+}
+
/**
* sb_start_pagefault - get write access to a superblock from a page fault
* @sb: the super we write to
diff --git a/mm/filemap.c b/mm/filemap.c
index 83efee7..3b2812b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2527,7 +2527,8 @@ ssize_t generic_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
BUG_ON(iocb->ki_pos != pos);
- sb_start_write(inode->i_sb);
+ if (!sb_start_file_write(file))
+ return -EAGAIN;
mutex_lock(&inode->i_mutex);
ret = __generic_file_aio_write(iocb, iov, nr_segs, &iocb->ki_pos);
mutex_unlock(&inode->i_mutex);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem
2012-11-08 12:41 [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Jan Kara
@ 2012-11-08 12:41 ` Jan Kara
2012-11-12 5:43 ` Dave Chinner
2012-11-10 9:23 ` [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Marco Stornelli
2012-11-12 5:42 ` Dave Chinner
2 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2012-11-08 12:41 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, dchinner, Jan Kara
When BSD process accounting is enabled and logs information to a filesystem
which gets frozen, system easily becomes unusable because each attempt to
account process information blocks. Thus e.g. every task gets blocked in exit.
It seems better to drop accounting information (which can already happen when
filesystem is running out of space) instead of locking system up. So we
open the accounting file with O_NONBLOCK.
Reported-and-tested-by: Nikola Ciprich <nikola.ciprich@linuxbox.cz>
Signed-off-by: Jan Kara <jack@suse.cz>
---
kernel/acct.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/kernel/acct.c b/kernel/acct.c
index 051e071..a061116 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -201,7 +201,8 @@ static int acct_on(struct filename *pathname)
struct bsd_acct_struct *acct = NULL;
/* Difference from BSD - they don't do O_APPEND */
- file = file_open_name(pathname, O_WRONLY|O_APPEND|O_LARGEFILE, 0);
+ file = file_open_name(pathname,
+ O_WRONLY|O_APPEND|O_LARGEFILE|O_NONBLOCK, 0);
if (IS_ERR(file))
return PTR_ERR(file);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem
2012-11-08 12:41 ` [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem Jan Kara
@ 2012-11-12 5:43 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2012-11-12 5:43 UTC (permalink / raw)
To: Jan Kara; +Cc: Al Viro, linux-fsdevel, dchinner
On Thu, Nov 08, 2012 at 01:41:38PM +0100, Jan Kara wrote:
> When BSD process accounting is enabled and logs information to a filesystem
> which gets frozen, system easily becomes unusable because each attempt to
> account process information blocks. Thus e.g. every task gets blocked in exit.
>
> It seems better to drop accounting information (which can already happen when
> filesystem is running out of space) instead of locking system up. So we
> open the accounting file with O_NONBLOCK.
>
> Reported-and-tested-by: Nikola Ciprich <nikola.ciprich@linuxbox.cz>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> kernel/acct.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/acct.c b/kernel/acct.c
> index 051e071..a061116 100644
> --- a/kernel/acct.c
> +++ b/kernel/acct.c
> @@ -201,7 +201,8 @@ static int acct_on(struct filename *pathname)
> struct bsd_acct_struct *acct = NULL;
>
> /* Difference from BSD - they don't do O_APPEND */
> - file = file_open_name(pathname, O_WRONLY|O_APPEND|O_LARGEFILE, 0);
> + file = file_open_name(pathname,
> + O_WRONLY|O_APPEND|O_LARGEFILE|O_NONBLOCK, 0);
> if (IS_ERR(file))
> return PTR_ERR(file);
Looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs
2012-11-08 12:41 [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Jan Kara
2012-11-08 12:41 ` [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem Jan Kara
@ 2012-11-10 9:23 ` Marco Stornelli
2012-11-12 5:42 ` Dave Chinner
2 siblings, 0 replies; 6+ messages in thread
From: Marco Stornelli @ 2012-11-10 9:23 UTC (permalink / raw)
To: Jan Kara; +Cc: Al Viro, linux-fsdevel, dchinner
Il 08/11/2012 13:41, Jan Kara ha scritto:
> When user asks for O_NONBLOCK behavior for a file descriptor, return
> EAGAIN instead of blocking on a frozen filesystem.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
It seems good. Since we have changed a bit the behavior, what do you
think to replace the uninterruptible sleeping with killable sleeping in
this context?
Marco
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs
2012-11-08 12:41 [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Jan Kara
2012-11-08 12:41 ` [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem Jan Kara
2012-11-10 9:23 ` [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Marco Stornelli
@ 2012-11-12 5:42 ` Dave Chinner
2012-11-12 10:45 ` Jan Kara
2 siblings, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2012-11-12 5:42 UTC (permalink / raw)
To: Jan Kara; +Cc: Al Viro, linux-fsdevel, dchinner
On Thu, Nov 08, 2012 at 01:41:37PM +0100, Jan Kara wrote:
> When user asks for O_NONBLOCK behavior for a file descriptor, return
> EAGAIN instead of blocking on a frozen filesystem.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/btrfs/file.c | 3 ++-
> fs/cifs/file.c | 3 ++-
> fs/fuse/file.c | 3 ++-
> fs/ntfs/file.c | 3 ++-
> fs/ocfs2/file.c | 3 ++-
> fs/xfs/xfs_file.c | 3 ++-
> include/linux/fs.h | 10 ++++++++++
> mm/filemap.c | 3 ++-
> 8 files changed, 24 insertions(+), 7 deletions(-)
Looks good, though you missed generic_file_splice_write(). Shouldn't
this also return EAGAIN?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs
2012-11-12 5:42 ` Dave Chinner
@ 2012-11-12 10:45 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2012-11-12 10:45 UTC (permalink / raw)
To: Dave Chinner; +Cc: Jan Kara, Al Viro, linux-fsdevel, dchinner
On Mon 12-11-12 16:42:43, Dave Chinner wrote:
> On Thu, Nov 08, 2012 at 01:41:37PM +0100, Jan Kara wrote:
> > When user asks for O_NONBLOCK behavior for a file descriptor, return
> > EAGAIN instead of blocking on a frozen filesystem.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > fs/btrfs/file.c | 3 ++-
> > fs/cifs/file.c | 3 ++-
> > fs/fuse/file.c | 3 ++-
> > fs/ntfs/file.c | 3 ++-
> > fs/ocfs2/file.c | 3 ++-
> > fs/xfs/xfs_file.c | 3 ++-
> > include/linux/fs.h | 10 ++++++++++
> > mm/filemap.c | 3 ++-
> > 8 files changed, 24 insertions(+), 7 deletions(-)
>
> Looks good, though you missed generic_file_splice_write(). Shouldn't
> this also return EAGAIN?
Yes, it should. Thanks for spotting this. I've also noticed ocfs2 misses
freeze protection in splice_write at all. I'll fix that in a separate
patch and send v3.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-11-12 10:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-08 12:41 [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Jan Kara
2012-11-08 12:41 ` [PATCH 2/2 v2] fs: Fix hang with BSD accounting on frozen filesystem Jan Kara
2012-11-12 5:43 ` Dave Chinner
2012-11-10 9:23 ` [PATCH 1/2 v2] fs: Return EAGAIN when O_NONBLOCK write should block on frozen fs Marco Stornelli
2012-11-12 5:42 ` Dave Chinner
2012-11-12 10:45 ` Jan Kara
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).