* [PATCH] raw: fsync method is now required
@ 2010-04-04 0:45 Anton Blanchard
2010-04-05 20:39 ` Andrew Morton
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Anton Blanchard @ 2010-04-04 0:45 UTC (permalink / raw)
To: Jan Kara, Christoph Hellwig, Alexander Viro, Jens Axboe,
Andrew Morton
Cc: linux-kernel
Commit 148f948ba877f4d3cdef036b1ff6d9f68986706a (vfs: Introduce new helpers for syncing after writing to O_SYNC file or IS_SYNC inode) broke the raw driver.
We now call through generic_file_aio_write -> generic_write_sync ->
vfs_fsync_range. vfs_fsync_range has:
if (!fop || !fop->fsync) {
ret = -EINVAL;
goto out;
}
But drivers/char/raw.c doesn't set an fsync method.
We have two options: fix it or remove the raw driver completely. I'm happy
to do either, the fact this has been broken for so long suggests it is
rarely used.
The patch below adds an fsync method to the raw driver. My knowledge of the
block layer is pretty sketchy so this could do with a once over.
If we instead decide to remove the raw driver, this patch might still be
useful as a backport to 2.6.33 and 2.6.32.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-2.6/drivers/char/raw.c
===================================================================
--- linux-2.6.orig/drivers/char/raw.c 2010-03-19 11:46:30.000000000 +1100
+++ linux-2.6/drivers/char/raw.c 2010-04-04 10:34:36.000000000 +1000
@@ -247,6 +247,7 @@ static const struct file_operations raw_
.aio_read = generic_file_aio_read,
.write = do_sync_write,
.aio_write = blkdev_aio_write,
+ .fsync = block_fsync,
.open = raw_open,
.release= raw_release,
.ioctl = raw_ioctl,
Index: linux-2.6/fs/block_dev.c
===================================================================
--- linux-2.6.orig/fs/block_dev.c 2010-04-04 08:33:10.000000000 +1000
+++ linux-2.6/fs/block_dev.c 2010-04-04 10:34:36.000000000 +1000
@@ -404,7 +404,7 @@ static loff_t block_llseek(struct file *
* NULL first argument is nfsd_sync_dir() and that's not a directory.
*/
-static int block_fsync(struct file *filp, struct dentry *dentry, int datasync)
+int block_fsync(struct file *filp, struct dentry *dentry, int datasync)
{
struct block_device *bdev = I_BDEV(filp->f_mapping->host);
int error;
@@ -418,6 +418,7 @@ static int block_fsync(struct file *filp
error = 0;
return error;
}
+EXPORT_SYMBOL(block_fsync);
/*
* pseudo-fs
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h 2010-04-04 08:33:15.000000000 +1000
+++ linux-2.6/include/linux/fs.h 2010-04-04 10:34:36.000000000 +1000
@@ -2212,6 +2212,7 @@ extern int generic_segment_checks(const
/* fs/block_dev.c */
extern ssize_t blkdev_aio_write(struct kiocb *iocb, const struct iovec *iov,
unsigned long nr_segs, loff_t pos);
+extern int block_fsync(struct file *filp, struct dentry *dentry, int datasync);
/* fs/splice.c */
extern ssize_t generic_file_splice_read(struct file *, loff_t *,
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] raw: fsync method is now required
2010-04-04 0:45 [PATCH] raw: fsync method is now required Anton Blanchard
@ 2010-04-05 20:39 ` Andrew Morton
2010-04-05 20:42 ` Christoph Hellwig
2010-04-06 15:14 ` Jeff Moyer
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2010-04-05 20:39 UTC (permalink / raw)
To: Anton Blanchard
Cc: Jan Kara, Christoph Hellwig, Alexander Viro, Jens Axboe,
linux-kernel
On Sun, 4 Apr 2010 10:45:23 +1000
Anton Blanchard <anton@samba.org> wrote:
>
> Commit 148f948ba877f4d3cdef036b1ff6d9f68986706a (vfs: Introduce new helpers for syncing after writing to O_SYNC file or IS_SYNC inode) broke the raw driver.
>
> We now call through generic_file_aio_write -> generic_write_sync ->
> vfs_fsync_range. vfs_fsync_range has:
>
> if (!fop || !fop->fsync) {
> ret = -EINVAL;
> goto out;
> }
>
> But drivers/char/raw.c doesn't set an fsync method.
So if you run fsync() against a /dev/rawX fd you get -EINVAL? erk.
> We have two options: fix it or remove the raw driver completely. I'm happy
> to do either, the fact this has been broken for so long suggests it is
> rarely used.
Well. It shows that fsync() is rarely used.
> The patch below adds an fsync method to the raw driver. My knowledge of the
> block layer is pretty sketchy so this could do with a once over.
>
> If we instead decide to remove the raw driver, this patch might still be
> useful as a backport to 2.6.33 and 2.6.32.
People go all whiny when I talk about removing raw.c. I gave up.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] raw: fsync method is now required
2010-04-05 20:39 ` Andrew Morton
@ 2010-04-05 20:42 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2010-04-05 20:42 UTC (permalink / raw)
To: Andrew Morton
Cc: Anton Blanchard, Jan Kara, Christoph Hellwig, Alexander Viro,
Jens Axboe, linux-kernel
On Mon, Apr 05, 2010 at 01:39:02PM -0700, Andrew Morton wrote:
> So if you run fsync() against a /dev/rawX fd you get -EINVAL? erk.
>
> > We have two options: fix it or remove the raw driver completely. I'm happy
> > to do either, the fact this has been broken for so long suggests it is
> > rarely used.
>
> Well. It shows that fsync() is rarely used.
The new O_SYNC implementation uses fsync, so O_SYNC users are also
affected now. Still shows that either one is relatively rarely used..
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] raw: fsync method is now required
2010-04-04 0:45 [PATCH] raw: fsync method is now required Anton Blanchard
2010-04-05 20:39 ` Andrew Morton
@ 2010-04-06 15:14 ` Jeff Moyer
2010-04-06 15:34 ` Jan Kara
2010-04-06 17:44 ` Christoph Hellwig
3 siblings, 0 replies; 6+ messages in thread
From: Jeff Moyer @ 2010-04-06 15:14 UTC (permalink / raw)
To: Anton Blanchard
Cc: Jan Kara, Christoph Hellwig, Alexander Viro, Jens Axboe,
Andrew Morton, linux-kernel
Anton Blanchard <anton@samba.org> writes:
> Commit 148f948ba877f4d3cdef036b1ff6d9f68986706a (vfs: Introduce new helpers for syncing after writing to O_SYNC file or IS_SYNC inode) broke the raw driver.
>
> We now call through generic_file_aio_write -> generic_write_sync ->
> vfs_fsync_range. vfs_fsync_range has:
>
> if (!fop || !fop->fsync) {
> ret = -EINVAL;
> goto out;
> }
>
> But drivers/char/raw.c doesn't set an fsync method.
>
> We have two options: fix it or remove the raw driver completely. I'm happy
> to do either, the fact this has been broken for so long suggests it is
> rarely used.
>
> The patch below adds an fsync method to the raw driver. My knowledge of the
> block layer is pretty sketchy so this could do with a once over.
>
> If we instead decide to remove the raw driver, this patch might still be
> useful as a backport to 2.6.33 and 2.6.32.
Databases like to use the raw interface. Thanks for fixing this!
> Signed-off-by: Anton Blanchard <anton@samba.org>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Tested-by: Jeff Moyer <jmoyer@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] raw: fsync method is now required
2010-04-04 0:45 [PATCH] raw: fsync method is now required Anton Blanchard
2010-04-05 20:39 ` Andrew Morton
2010-04-06 15:14 ` Jeff Moyer
@ 2010-04-06 15:34 ` Jan Kara
2010-04-06 17:44 ` Christoph Hellwig
3 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2010-04-06 15:34 UTC (permalink / raw)
To: Anton Blanchard
Cc: Jan Kara, Christoph Hellwig, Alexander Viro, Jens Axboe,
Andrew Morton, linux-kernel
On Sun 04-04-10 10:45:23, Anton Blanchard wrote:
> Commit 148f948ba877f4d3cdef036b1ff6d9f68986706a (vfs: Introduce new helpers for syncing after writing to O_SYNC file or IS_SYNC inode) broke the raw driver.
>
> We now call through generic_file_aio_write -> generic_write_sync ->
> vfs_fsync_range. vfs_fsync_range has:
>
> if (!fop || !fop->fsync) {
> ret = -EINVAL;
> goto out;
> }
>
> But drivers/char/raw.c doesn't set an fsync method.
>
> We have two options: fix it or remove the raw driver completely. I'm happy
> to do either, the fact this has been broken for so long suggests it is
> rarely used.
>
> The patch below adds an fsync method to the raw driver. My knowledge of the
> block layer is pretty sketchy so this could do with a once over.
Thanks for catching this. The patch looks fine.
> If we instead decide to remove the raw driver, this patch might still be
> useful as a backport to 2.6.33 and 2.6.32.
Oracle databases still use raw devices so I don't think we should remove
them.
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] raw: fsync method is now required
2010-04-04 0:45 [PATCH] raw: fsync method is now required Anton Blanchard
` (2 preceding siblings ...)
2010-04-06 15:34 ` Jan Kara
@ 2010-04-06 17:44 ` Christoph Hellwig
3 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2010-04-06 17:44 UTC (permalink / raw)
To: Anton Blanchard
Cc: Jan Kara, Christoph Hellwig, Alexander Viro, Jens Axboe,
Andrew Morton, linux-kernel
On Sun, Apr 04, 2010 at 10:45:23AM +1000, Anton Blanchard wrote:
> +int block_fsync(struct file *filp, struct dentry *dentry, int datasync)
Can you rename this to blkdev_fsync now that it's exported?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-04-06 17:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-04 0:45 [PATCH] raw: fsync method is now required Anton Blanchard
2010-04-05 20:39 ` Andrew Morton
2010-04-05 20:42 ` Christoph Hellwig
2010-04-06 15:14 ` Jeff Moyer
2010-04-06 15:34 ` Jan Kara
2010-04-06 17:44 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox