mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2][FAT] miss-sync issues on sync mount (miss-sync on write)
@ 2005-09-14 20:34 Machida, Hiroyuki
  2005-09-15  2:48 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Machida, Hiroyuki @ 2005-09-14 20:34 UTC (permalink / raw)
  To: hirofumi; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 171 bytes --]

This patch fixes miss-sync issue on write() system call.
This updates inode attrs flags, mtime and ctime on every
comit_write call, due to locking.

---
Hiroyuki Machida


[-- Attachment #2: fat-sync-write.patch --]
[-- Type: text/plain, Size: 2467 bytes --]

Signed-off-by: Hiroyuki Machida <machida@sm.sony.co.jp>
 ---

 file.c  |   20 +-------------------
 inode.c |   21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 20 deletions(-)


--- linux-2.6.13.org/fs/fat/inode.c	2005-08-29 08:41:01.000000000 +0900
+++ linux-2.6.13/fs/fat/inode.c	2005-09-09 17:22:44.305189111 +0900
@@ -107,12 +107,31 @@ static sector_t _fat_bmap(struct address
 	return generic_block_bmap(mapping, block, fat_get_block);
 }
 
+static int fat_commit_write(struct file *file, struct page *page,
+		unsigned from, unsigned to)
+{
+	struct inode *inode = page->mapping->host;
+	loff_t pos = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
+	block_commit_write(page,from,to);
+	/*
+	 * No need to use i_size_read() here, the i_size
+	 * cannot change under us because we hold i_sem.
+	 */
+	if (pos > inode->i_size) {
+		i_size_write(inode, pos);
+	}
+	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
+	MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
+	mark_inode_dirty(inode);
+	return 0;
+}
+
 static struct address_space_operations fat_aops = {
 	.readpage	= fat_readpage,
 	.writepage	= fat_writepage,
 	.sync_page	= block_sync_page,
 	.prepare_write	= fat_prepare_write,
-	.commit_write	= generic_commit_write,
+	.commit_write	= fat_commit_write,
 	.bmap		= _fat_bmap
 };
 
--- linux-2.6.13.org/fs/fat/file.c	2005-08-29 08:41:01.000000000 +0900
+++ linux-2.6.13/fs/fat/file.c	2005-09-08 17:13:03.000000000 +0900
@@ -12,24 +12,6 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 
-static ssize_t fat_file_aio_write(struct kiocb *iocb, const char __user *buf,
-				  size_t count, loff_t pos)
-{
-	struct inode *inode = iocb->ki_filp->f_dentry->d_inode;
-	int retval;
-
-	retval = generic_file_aio_write(iocb, buf, count, pos);
-	if (retval > 0) {
-		inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
-		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-		mark_inode_dirty(inode);
-//		check the locking rules
-//		if (IS_SYNC(inode))
-//			fat_sync_inode(inode);
-	}
-	return retval;
-}
-
 static ssize_t fat_file_writev(struct file *filp, const struct iovec *iov,
 			       unsigned long nr_segs, loff_t *ppos)
 {
@@ -150,7 +132,7 @@ struct file_operations fat_file_operatio
 	.readv		= generic_file_readv,
 	.writev		= fat_file_writev,
 	.aio_read	= generic_file_aio_read,
-	.aio_write	= fat_file_aio_write,
+	.aio_write	= generic_file_aio_write,
 	.mmap		= generic_file_mmap,
 	.ioctl		= fat_generic_ioctl,
 	.fsync		= file_fsync,

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2][FAT] miss-sync issues on sync mount (miss-sync on write)
  2005-09-14 20:34 [PATCH 1/2][FAT] miss-sync issues on sync mount (miss-sync on write) Machida, Hiroyuki
@ 2005-09-15  2:48 ` Andrew Morton
  2005-09-15 12:43   ` OGAWA Hirofumi
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2005-09-15  2:48 UTC (permalink / raw)
  To: Machida, Hiroyuki; +Cc: hirofumi, linux-kernel

"Machida, Hiroyuki" <machida@sm.sony.co.jp> wrote:
>
> This patch fixes miss-sync issue on write() system call.
>  This updates inode attrs flags, mtime and ctime on every
>  comit_write call, due to locking.

This all seems wrong.

Why does fatfs have file_operations.write pointing at do_sync_write()
rather than generic_file_write()?

Why does fatfs have a custom .aio_write() rather than using
generic_file_aio_write()?

If fatfs can use all the standard library functions, all this inode
dirtying and O_SYNC/-o sync handling shoud just work.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2][FAT] miss-sync issues on sync mount (miss-sync on write)
  2005-09-15  2:48 ` Andrew Morton
@ 2005-09-15 12:43   ` OGAWA Hirofumi
  0 siblings, 0 replies; 3+ messages in thread
From: OGAWA Hirofumi @ 2005-09-15 12:43 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Machida, Hiroyuki, linux-kernel

Andrew Morton <akpm@osdl.org> writes:

> "Machida, Hiroyuki" <machida@sm.sony.co.jp> wrote:
>>
>> This patch fixes miss-sync issue on write() system call.
>>  This updates inode attrs flags, mtime and ctime on every
>>  comit_write call, due to locking.
>
> This all seems wrong.
>
> Why does fatfs have file_operations.write pointing at do_sync_write()
> rather than generic_file_write()?
>
> Why does fatfs have a custom .aio_write() rather than using
> generic_file_aio_write()?
>
> If fatfs can use all the standard library functions, all this inode
> dirtying and O_SYNC/-o sync handling shoud just work.

In fat_file_aio_write:

	retval = generic_file_aio_write(iocb, buf, count, pos);
	if (retval > 0) {
		inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
		mark_inode_dirty(inode);

He says about the above FAT specific flag and timestamp.  FAT needs to
add the ATTR_ARCH if data was modified, but current FAT is adding it
after generic_file_aio_write()/generic_file_writev(). (IIRC, some
other filesystems is also doing this.)

His patch seems good, although the some additional works was needed.
I applied the attached patch. Thanks, Hiroyuki-san.

Andrew, now fat_file_operations is the following. Is this OK?

struct file_operations fat_file_operations = {
	.llseek		= generic_file_llseek,
	.read		= do_sync_read,
	.write		= do_sync_write,
	.readv		= generic_file_readv,
	.writev		= generic_file_writev,
	.aio_read	= generic_file_aio_read,
	.aio_write	= generic_file_aio_write,
	.mmap		= generic_file_mmap,
	.ioctl		= fat_generic_ioctl,
	.fsync		= file_fsync,
	.sendfile	= generic_file_sendfile,
};
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>



[PATCH] FAT: miss-sync issues on sync mount (miss-sync on write)

This patch fixes miss-sync issue on write() system call.
This updates inode attrs flags, mtime and ctime on every
comit_write call, due to locking.

Signed-off-by: Hiroyuki Machida <machida@sm.sony.co.jp>
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 fs/fat/file.c  |   37 ++-----------------------------------
 fs/fat/inode.c |   15 ++++++++++++++-
 2 files changed, 16 insertions(+), 36 deletions(-)

diff -puN fs/fat/file.c~fat_arch-flags-fix fs/fat/file.c
--- linux-2.6.14-rc1/fs/fat/file.c~fat_arch-flags-fix	2005-09-15 20:52:21.000000000 +0900
+++ linux-2.6.14-rc1-hirofumi/fs/fat/file.c	2005-09-15 20:55:12.000000000 +0900
@@ -12,39 +12,6 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 
-static ssize_t fat_file_aio_write(struct kiocb *iocb, const char __user *buf,
-				  size_t count, loff_t pos)
-{
-	struct inode *inode = iocb->ki_filp->f_dentry->d_inode;
-	int retval;
-
-	retval = generic_file_aio_write(iocb, buf, count, pos);
-	if (retval > 0) {
-		inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
-		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-		mark_inode_dirty(inode);
-//		check the locking rules
-//		if (IS_SYNC(inode))
-//			fat_sync_inode(inode);
-	}
-	return retval;
-}
-
-static ssize_t fat_file_writev(struct file *filp, const struct iovec *iov,
-			       unsigned long nr_segs, loff_t *ppos)
-{
-	struct inode *inode = filp->f_dentry->d_inode;
-	int retval;
-
-	retval = generic_file_writev(filp, iov, nr_segs, ppos);
-	if (retval > 0) {
-		inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
-		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
-		mark_inode_dirty(inode);
-	}
-	return retval;
-}
-
 int fat_generic_ioctl(struct inode *inode, struct file *filp,
 		      unsigned int cmd, unsigned long arg)
 {
@@ -148,9 +115,9 @@ struct file_operations fat_file_operatio
 	.read		= do_sync_read,
 	.write		= do_sync_write,
 	.readv		= generic_file_readv,
-	.writev		= fat_file_writev,
+	.writev		= generic_file_writev,
 	.aio_read	= generic_file_aio_read,
-	.aio_write	= fat_file_aio_write,
+	.aio_write	= generic_file_aio_write,
 	.mmap		= generic_file_mmap,
 	.ioctl		= fat_generic_ioctl,
 	.fsync		= file_fsync,
diff -puN fs/fat/inode.c~fat_arch-flags-fix fs/fat/inode.c
--- linux-2.6.14-rc1/fs/fat/inode.c~fat_arch-flags-fix	2005-09-15 20:52:21.000000000 +0900
+++ linux-2.6.14-rc1-hirofumi/fs/fat/inode.c	2005-09-15 21:32:51.000000000 +0900
@@ -102,6 +102,19 @@ static int fat_prepare_write(struct file
 				  &MSDOS_I(page->mapping->host)->mmu_private);
 }
 
+static int fat_commit_write(struct file *file, struct page *page,
+			    unsigned from, unsigned to)
+{
+	struct inode *inode = page->mapping->host;
+	int err = generic_commit_write(file, page, from, to);
+	if (!err && !(MSDOS_I(inode)->i_attrs & ATTR_ARCH)) {
+		inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
+		MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
+		mark_inode_dirty(inode);
+	}
+	return err;
+}
+
 static sector_t _fat_bmap(struct address_space *mapping, sector_t block)
 {
 	return generic_block_bmap(mapping, block, fat_get_block);
@@ -112,7 +125,7 @@ static struct address_space_operations f
 	.writepage	= fat_writepage,
 	.sync_page	= block_sync_page,
 	.prepare_write	= fat_prepare_write,
-	.commit_write	= generic_commit_write,
+	.commit_write	= fat_commit_write,
 	.bmap		= _fat_bmap
 };
 
_

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2005-09-15 12:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-09-14 20:34 [PATCH 1/2][FAT] miss-sync issues on sync mount (miss-sync on write) Machida, Hiroyuki
2005-09-15  2:48 ` Andrew Morton
2005-09-15 12:43   ` OGAWA Hirofumi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®