[PATCH] Fix !mapping_cap_writeback_dirty(inode->i_mapping)

From: OGAWA Hirofumi
Date: Sat Oct 29 2005 - 03:42:44 EST


Andrew Morton <akpm@xxxxxxxx> writes:

> Isn't write_inode_now() buggy? If !mapping_cap_writeback_dirty() we
> should still write the inode itself?
>
> diff -puN fs/fs-writeback.c~write_inode_now-write-inode-if-not-bdi_cap_no_writeback fs/fs-writeback.c
> --- devel/fs/fs-writeback.c~write_inode_now-write-inode-if-not-bdi_cap_no_writeback 2005-10-11 21:13:25.000000000 -0700
> +++ devel-akpm/fs/fs-writeback.c 2005-10-11 21:13:40.000000000 -0700
> @@ -558,7 +558,7 @@ int write_inode_now(struct inode *inode,
> };
>
> if (!mapping_cap_writeback_dirty(inode->i_mapping))
> - return 0;
> + wbc.nr_to_write = 0;
>
> might_sleep();
> spin_lock(&inode_lock);

You are right, and I was wrong.

Yes, if block device has BDI_CAP_NO_WRITEBACK and inode->i_mapping
was changing to bd_inode->i_mapping,
the !mapping_cap_writeback_dirty(inode->i_mapping) is true, so we need
to write the inode itself of block special file.

I think we need to fix sync_sb_inodes() too. What do you think?

Since wbc.nr_to_write includes the information on how many pages were
written, we can't change wbc.nr_to_write in sync_sb_inodes().

This patch fixed the following case,

# mke2fs -m0 -F file
# mount -t ext2 file mnt -o loop
# cd mnt
# mknod ram b 1 0
# cat ram
# cd ..
# umount mnt
# e2fsck -f file
"mnt/ram" wasn't flushed and it was corrupted

Thanks.
--
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>



Signed-off-by: OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
---

fs/fs-writeback.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff -puN fs/fs-writeback.c~device-inode-write-fix fs/fs-writeback.c
--- linux-2.6.14/fs/fs-writeback.c~device-inode-write-fix 2005-10-29 08:06:28.000000000 +0900
+++ linux-2.6.14-hirofumi/fs/fs-writeback.c 2005-10-29 08:06:28.000000000 +0900
@@ -151,7 +151,8 @@ static int write_inode(struct inode *ino
* Called under inode_lock.
*/
static int
-__sync_single_inode(struct inode *inode, struct writeback_control *wbc)
+__sync_single_inode(struct inode *inode, struct writeback_control *wbc,
+ int inode_only)
{
unsigned dirty;
struct address_space *mapping = inode->i_mapping;
@@ -168,7 +169,9 @@ __sync_single_inode(struct inode *inode,

spin_unlock(&inode_lock);

- ret = do_writepages(mapping, wbc);
+ ret = 0;
+ if (!inode_only)
+ ret = do_writepages(mapping, wbc);

/* Don't write the inode if only I_DIRTY_PAGES was set */
if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
@@ -177,7 +180,7 @@ __sync_single_inode(struct inode *inode,
ret = err;
}

- if (wait) {
+ if (!inode_only && wait) {
int err = filemap_fdatawait(mapping);
if (ret == 0)
ret = err;
@@ -242,7 +245,7 @@ __sync_single_inode(struct inode *inode,
*/
static int
__writeback_single_inode(struct inode *inode,
- struct writeback_control *wbc)
+ struct writeback_control *wbc, int inode_only)
{
wait_queue_head_t *wqh;

@@ -267,7 +270,7 @@ __writeback_single_inode(struct inode *i
spin_lock(&inode_lock);
} while (inode->i_state & I_LOCK);
}
- return __sync_single_inode(inode, wbc);
+ return __sync_single_inode(inode, wbc, inode_only);
}

/*
@@ -304,6 +307,7 @@ static void
sync_sb_inodes(struct super_block *sb, struct writeback_control *wbc)
{
const unsigned long start = jiffies; /* livelock avoidance */
+ int inode_only;

if (!wbc->for_kupdate || list_empty(&sb->s_io))
list_splice_init(&sb->s_dirty, &sb->s_io);
@@ -315,7 +319,8 @@ sync_sb_inodes(struct super_block *sb, s
struct backing_dev_info *bdi = mapping->backing_dev_info;
long pages_skipped;

- if (!bdi_cap_writeback_dirty(bdi)) {
+ inode_only = 0;
+ if (!mapping_cap_writeback_dirty(mapping)) {
list_move(&inode->i_list, &sb->s_dirty);
if (sb == blockdev_superblock) {
/*
@@ -324,12 +329,7 @@ sync_sb_inodes(struct super_block *sb, s
*/
continue;
}
- /*
- * Dirty memory-backed inode against a filesystem other
- * than the kernel-internal bdev filesystem. Skip the
- * entire superblock.
- */
- break;
+ inode_only = 1;
}

if (wbc->nonblocking && bdi_write_congested(bdi)) {
@@ -363,7 +363,7 @@ 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);
+ __writeback_single_inode(inode, wbc, inode_only);
if (wbc->sync_mode == WB_SYNC_HOLD) {
inode->dirtied_when = jiffies;
list_move(&inode->i_list, &sb->s_dirty);
@@ -551,18 +551,18 @@ void sync_inodes(int wait)

int write_inode_now(struct inode *inode, int sync)
{
- int ret;
struct writeback_control wbc = {
.nr_to_write = LONG_MAX,
.sync_mode = WB_SYNC_ALL,
};
+ int ret, inode_only = 0;

if (!mapping_cap_writeback_dirty(inode->i_mapping))
- return 0;
+ inode_only = 1;

might_sleep();
spin_lock(&inode_lock);
- ret = __writeback_single_inode(inode, &wbc);
+ ret = __writeback_single_inode(inode, &wbc, inode_only);
spin_unlock(&inode_lock);
if (sync)
wait_on_inode(inode);
@@ -586,7 +586,7 @@ int sync_inode(struct inode *inode, stru
int ret;

spin_lock(&inode_lock);
- ret = __writeback_single_inode(inode, wbc);
+ ret = __writeback_single_inode(inode, wbc, 0);
spin_unlock(&inode_lock);
return ret;
}
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/