On Wed, 03 Jan 2007 23:30:05 +0100
Guillaume Chazarain <[EMAIL PROTECTED]> wrote:
> Unfortunately, with your patch and not mine, the problem is still
> present: msync()
> does not return the error. Both pieces of code (yours and mine) are
> called for the
> same mapping though, albeit yours more frequently.
>
> My interpretation (based more on imagination than experience) is that
> __writeback_single_inode() ends up calling __block_write_full_page() which
> sets the page flags (your patch), then calls wait_on_page_writeback_range()
> which clears the flags but returns the error as its return value. And this
> error code is dropped by sync_sb_inodes() (my patch not applied).
>
> With my patch, wait_on_page_writeback_range() would get the error code
> by some other mean, and sync_sb_inodes() would re-put it in the flags for
> msync() later.
hm, something like that.
There's a lot of sloppiness in there. Do these two patches fix things up?
From: Andrew Morton <[EMAIL PROTECTED]>
Guillame points out that sync_sb_inodes() is failing to propagate error codes
back. Fix that, and make several other void-returning functions not drop
reportable error codes.
Cc: Guillaume Chazarain <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
fs/fs-writeback.c | 55 ++++++++++++++++++++++++++----------
include/linux/writeback.h | 6 +--
2 files changed, 44 insertions(+), 17 deletions(-)
diff -puN fs/fs-writeback.c~sync_sb_inodes-propagate-errors fs/fs-writeback.c
--- a/fs/fs-writeback.c~sync_sb_inodes-propagate-errors
+++ a/fs/fs-writeback.c
@@ -302,15 +302,18 @@ __writeback_single_inode(struct inode *i
* on the writer throttling path, and we get decent balancing between many
* throttled threads: we don't want them all piling up on __wait_on_inode.
*/
-static void
+static int
sync_sb_inodes(struct super_block *sb, struct writeback_control *wbc)
{
const unsigned long start = jiffies; /* livelock avoidance */
+ int ret = 0;
if (!wbc->for_kupdate || list_empty(&sb->s_io))
list_splice_init(&sb->s_dirty, &sb->s_io);
while (!list_empty(&sb->s_io)) {
+ int err;
+
struct inode *inode = list_entry(sb->s_io.prev,
struct inode, i_list);
struct address_space *mapping = inode->i_mapping;
@@ -365,7 +368,9 @@ sync_sb_inodes(struct super_block *sb, s
BUG_ON(inode->i_state & I_FREEING);
__iget(inode);
pages_skipped = wbc->pages_skipped;
- __writeback_single_inode(inode, wbc);
+ err = __writeback_single_inode(inode, wbc);
+ if (!ret)
+ ret = err;
if (wbc->sync_mode == WB_SYNC_HOLD) {
inode->dirtied_when = jiffies;
list_move(&inode->i_list, &sb->s_dirty);
@@ -386,7 +391,7 @@ sync_sb_inodes(struct super_block *sb, s
if (wbc->nr_to_write <= 0)
break;
}
- return; /* Leave any unwritten inodes on s_io */
+ return ret; /* Leave any unwritten inodes on s_io */
}
/*
@@ -408,10 +413,10 @@ sync_sb_inodes(struct super_block *sb, s
* sync_sb_inodes will seekout the blockdev which matches `bdi'. Maybe not
* super-efficient but we're about to do a ton of I/O...
*/
-void
-writeback_inodes(struct writeback_control *wbc)
+int writeback_inodes(struct writeback_control *wbc)
{
struct super_block *sb;
+ int ret = 0;
might_sleep();
spin_lock(&sb_lock);
@@ -429,9 +434,13 @@ restart:
*/
if (down_read_trylock(&sb->s_umount)) {
if (sb->s_root) {
+ int err;
+
spin_lock(&inode_lock);
- sync_sb_inodes(sb, wbc);
+ err = sync_sb_inodes(sb, wbc);
spin_unlock(&inode_lock);
+ if (!ret)
+ ret = err;
}
up_read(&sb->s_umount);
}
@@ -443,6 +452,7 @@ restart:
break;
}
spin_unlock(&sb_lock);
+ return ret;
}
/*
@@ -456,7 +466,7 @@ restart:
* We add in the number of potentially dirty inodes, because each inode write
* can dirty pagecache in the underlying blockdev.
*/
-void sync_inodes_sb(struct super_block *sb, int wait)
+int sync_inodes_sb(struct super_block *sb, int wait)
{
struct writeback_control wbc = {
.sync_mode = wait ? WB_SYNC_ALL : WB_SYNC_HOLD,
@@ -465,14 +475,16 @@ void sync_inodes_sb(struct super_block *
};
unsigned long nr_dirty = global_page_state(NR_FILE_DIRTY);
unsigned long nr_unstable = global_page_state(NR_UNSTABLE_NFS);
+ int ret;
wbc.nr_to_write = nr_dirty + nr_unstable +
(inodes_stat.nr_inodes - inodes_stat.nr_unused) +
nr_dirty + nr_unstable;
wbc.nr_to_write += wbc.nr_to_write / 2; /* Bit more for luck */
spin_lock(&inode_lock);
- sync_sb_inodes(sb, &wbc);
+ ret = sync_sb_inodes(sb, &wbc);
spin_unlock(&inode_lock);
+ return ret;
}
/*
@@ -508,13 +520,16 @@ static void set_sb_syncing(int val)
* outstanding dirty inodes, the writeback goes block-at-a-time within the
* filesystem's write_inode(). This is extremely slow.
*/
-static void __sync_inodes(int wait)
+static int __sync_inodes(int wait)
{
struct super_block *sb;
+ int ret = 0;
spin_lock(&sb_lock);
restart:
list_for_each_entry(sb, &super_blocks, s_list) {
+ int err;
+
if (sb->s_syncing)
continue;
sb->s_syncing = 1;
@@ -522,8 +537,12 @@ restart:
spin_unlock(&sb_lock);
down_read(&sb->s_umount);
if (sb->s_root) {
- sync_inodes_sb(sb, wait);
- sync_blockdev(sb->s_bdev);
+ err = sync_inodes_sb(sb, wait);
+ if (!ret)
+ ret = err;
+ err = sync_blockdev(sb->s_bdev);
+ if (!ret)
+ ret = err;
}
up_read(&sb->s_umount);
spin_lock(&sb_lock);
@@ -531,17 +550,25 @@ restart:
goto restart;
}
spin_unlock(&sb_lock);
+ return ret;
}
-void sync_inodes(int wait)
+int sync_inodes(int wait)
{
+ int ret;
+
set_sb_syncing(0);
- __sync_inodes(0);
+ ret = __sync_inodes(0);
if (wait) {
+ int err;
+
set_sb_syncing(0);
- __sync_inodes(1);
+ err = __sync_inodes(1);
+ if (!ret)
+ ret = err;
}
+ return ret;
}
/**
diff -puN include/linux/writeback.h~sync_sb_inodes-propagate-errors
include/linux/writeback.h
--- a/include/linux/writeback.h~sync_sb_inodes-propagate-errors
+++ a/include/linux/writeback.h
@@ -64,11 +64,11 @@ struct writeback_control {
/*
* fs/fs-writeback.c
*/
-void writeback_inodes(struct writeback_control *wbc);
+int writeback_inodes(struct writeback_control *wbc);
void wake_up_inode(struct inode *inode);
int inode_wait(void *);
-void sync_inodes_sb(struct super_block *, int wait);
-void sync_inodes(int wait);
+int sync_inodes_sb(struct super_block *, int wait);
+int sync_inodes(int wait);
/* writeback.h requires fs.h; it, too, is not included from here. */
static inline void wait_on_inode(struct inode *inode)
_
From: Andrew Morton <[EMAIL PROTECTED]>
block_write_full_page() forgot to propagate ENPSOC into the address_space.
Cc: Guillaume Chazarain <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
fs/buffer.c | 1 +
1 files changed, 1 insertion(+)
diff -puN fs/buffer.c~block_write_full_page-handle-enospc fs/buffer.c
--- a/fs/buffer.c~block_write_full_page-handle-enospc
+++ a/fs/buffer.c
@@ -1740,6 +1740,7 @@ recover:
} while ((bh = bh->b_this_page) != head);
SetPageError(page);
BUG_ON(PageWriteback(page));
+ mapping_set_error(page->mapping, err);
set_page_writeback(page);
unlock_page(page);
do {
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
