* [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
@ 2011-08-01 23:47 Rafael J. Wysocki
2011-08-02 0:17 ` Rafael J. Wysocki
0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2011-08-01 23:47 UTC (permalink / raw)
To: Josef Bacik
Cc: LKML, Linus Torvalds, Linux PM mailing list, Al Viro, Jan Kara
Hi,
Unfortunately s2disk (which is a user space tool for saving hibernate images)
doesn't work any more after your commit 02c24a82187d5a628c68edfe71ae60dc135cd178
(fs: push i_mutex and filemap_write_and_wait down into ->fsync() handlers).
The symptom is that the s2disk's resume counterpart cannot find the image in
the device even though s2disk considers it as successfully saved.
What happens is that s2disk opens a block device (disk partition to be precise)
directly and writes to it eventually calling fsync(). My interpretation of the
failure is that before commit 02c24a82187d5a628c68ed, when s2disk called fsync()
the VFS layer would run filemap_write_and_wait_range() that in turn would cause
the data to go to the disk, but after that commit this doesn't happen any more.
If I'm right, then not only s2disk, but any process writing directly to a block
device will have a problem with fsync().
Thanks,
Rafael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-01 23:47 [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk Rafael J. Wysocki
@ 2011-08-02 0:17 ` Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2011-08-02 0:17 UTC (permalink / raw)
To: Josef Bacik
Cc: LKML, Linus Torvalds, Linux PM mailing list, Al Viro, Jan Kara
On Tuesday, August 02, 2011, Rafael J. Wysocki wrote:
> Hi,
>
> Unfortunately s2disk (which is a user space tool for saving hibernate images)
> doesn't work any more after your commit 02c24a82187d5a628c68edfe71ae60dc135cd178
> (fs: push i_mutex and filemap_write_and_wait down into ->fsync() handlers).
> The symptom is that the s2disk's resume counterpart cannot find the image in
> the device even though s2disk considers it as successfully saved.
>
> What happens is that s2disk opens a block device (disk partition to be precise)
> directly and writes to it eventually calling fsync(). My interpretation of the
> failure is that before commit 02c24a82187d5a628c68ed, when s2disk called fsync()
> the VFS layer would run filemap_write_and_wait_range() that in turn would cause
> the data to go to the disk, but after that commit this doesn't happen any more.
>
> If I'm right, then not only s2disk, but any process writing directly to a block
> device will have a problem with fsync().
Well, I'm not sure if the patch below is the right fix, but it evidently makes
s2disk work for me again.
Thanks,
Rafael
---
fs/block_dev.c | 4 ++++
1 file changed, 4 insertions(+)
Index: linux-2.6/fs/block_dev.c
===================================================================
--- linux-2.6.orig/fs/block_dev.c
+++ linux-2.6/fs/block_dev.c
@@ -387,6 +387,10 @@ int blkdev_fsync(struct file *filp, loff
struct inode *bd_inode = filp->f_mapping->host;
struct block_device *bdev = I_BDEV(bd_inode);
int error;
+
+ error = filemap_write_and_wait_range(filp->f_mapping, start, end);
+ if (error)
+ return error;
/*
* There is no need to serialise calls to blkdev_issue_flush with
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
@ 2011-08-02 0:17 ` Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2011-08-02 0:17 UTC (permalink / raw)
To: Josef Bacik
Cc: Jan Kara, Linux PM mailing list, Linus Torvalds, LKML, Al Viro
On Tuesday, August 02, 2011, Rafael J. Wysocki wrote:
> Hi,
>
> Unfortunately s2disk (which is a user space tool for saving hibernate images)
> doesn't work any more after your commit 02c24a82187d5a628c68edfe71ae60dc135cd178
> (fs: push i_mutex and filemap_write_and_wait down into ->fsync() handlers).
> The symptom is that the s2disk's resume counterpart cannot find the image in
> the device even though s2disk considers it as successfully saved.
>
> What happens is that s2disk opens a block device (disk partition to be precise)
> directly and writes to it eventually calling fsync(). My interpretation of the
> failure is that before commit 02c24a82187d5a628c68ed, when s2disk called fsync()
> the VFS layer would run filemap_write_and_wait_range() that in turn would cause
> the data to go to the disk, but after that commit this doesn't happen any more.
>
> If I'm right, then not only s2disk, but any process writing directly to a block
> device will have a problem with fsync().
Well, I'm not sure if the patch below is the right fix, but it evidently makes
s2disk work for me again.
Thanks,
Rafael
---
fs/block_dev.c | 4 ++++
1 file changed, 4 insertions(+)
Index: linux-2.6/fs/block_dev.c
===================================================================
--- linux-2.6.orig/fs/block_dev.c
+++ linux-2.6/fs/block_dev.c
@@ -387,6 +387,10 @@ int blkdev_fsync(struct file *filp, loff
struct inode *bd_inode = filp->f_mapping->host;
struct block_device *bdev = I_BDEV(bd_inode);
int error;
+
+ error = filemap_write_and_wait_range(filp->f_mapping, start, end);
+ if (error)
+ return error;
/*
* There is no need to serialise calls to blkdev_issue_flush with
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 0:17 ` Rafael J. Wysocki
(?)
(?)
@ 2011-08-02 1:22 ` Linus Torvalds
2011-08-02 1:30 ` Al Viro
2011-08-02 1:30 ` Al Viro
-1 siblings, 2 replies; 14+ messages in thread
From: Linus Torvalds @ 2011-08-02 1:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Josef Bacik, LKML, Linux PM mailing list, Al Viro, Jan Kara
On Mon, Aug 1, 2011 at 2:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>
> Well, I'm not sure if the patch below is the right fix, but it evidently makes
> s2disk work for me again.
Looks right to me.
The only issue is whether we should use "db_mapping->i_mapping" or
"file->f_mapping". I think they are the same for block devices.
Al?
Also, what about some of the other fsync things that apparently
weren't updated to write back page caches. ps3flash_fsync? Others?
Linus
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 0:17 ` Rafael J. Wysocki
(?)
@ 2011-08-02 1:22 ` Linus Torvalds
-1 siblings, 0 replies; 14+ messages in thread
From: Linus Torvalds @ 2011-08-02 1:22 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM mailing list, Jan Kara, LKML, Josef Bacik, Al Viro
On Mon, Aug 1, 2011 at 2:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>
> Well, I'm not sure if the patch below is the right fix, but it evidently makes
> s2disk work for me again.
Looks right to me.
The only issue is whether we should use "db_mapping->i_mapping" or
"file->f_mapping". I think they are the same for block devices.
Al?
Also, what about some of the other fsync things that apparently
weren't updated to write back page caches. ps3flash_fsync? Others?
Linus
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:22 ` Linus Torvalds
@ 2011-08-02 1:30 ` Al Viro
2011-08-02 1:38 ` Linus Torvalds
` (3 more replies)
2011-08-02 1:30 ` Al Viro
1 sibling, 4 replies; 14+ messages in thread
From: Al Viro @ 2011-08-02 1:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rafael J. Wysocki, Josef Bacik, LKML, Linux PM mailing list, Jan Kara
On Mon, Aug 01, 2011 at 03:22:02PM -1000, Linus Torvalds wrote:
> On Mon, Aug 1, 2011 at 2:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> > Well, I'm not sure if the patch below is the right fix, but it evidently makes
> > s2disk work for me again.
>
> Looks right to me.
>
> The only issue is whether we should use "db_mapping->i_mapping" or
> "file->f_mapping". I think they are the same for block devices.
>
> Al?
Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
but what's wrong with using ->f_mapping here?)
> Also, what about some of the other fsync things that apparently
> weren't updated to write back page caches. ps3flash_fsync? Others?
ps3flash, IIRC, doesn't go through page cache on read and write...
If anything, we probably have instances that bother with pagecache
for no reason...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:22 ` Linus Torvalds
2011-08-02 1:30 ` Al Viro
@ 2011-08-02 1:30 ` Al Viro
1 sibling, 0 replies; 14+ messages in thread
From: Al Viro @ 2011-08-02 1:30 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux PM mailing list, Jan Kara, LKML, Josef Bacik
On Mon, Aug 01, 2011 at 03:22:02PM -1000, Linus Torvalds wrote:
> On Mon, Aug 1, 2011 at 2:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> > Well, I'm not sure if the patch below is the right fix, but it evidently makes
> > s2disk work for me again.
>
> Looks right to me.
>
> The only issue is whether we should use "db_mapping->i_mapping" or
> "file->f_mapping". I think they are the same for block devices.
>
> Al?
Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
but what's wrong with using ->f_mapping here?)
> Also, what about some of the other fsync things that apparently
> weren't updated to write back page caches. ps3flash_fsync? Others?
ps3flash, IIRC, doesn't go through page cache on read and write...
If anything, we probably have instances that bother with pagecache
for no reason...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:30 ` Al Viro
2011-08-02 1:38 ` Linus Torvalds
@ 2011-08-02 1:38 ` Linus Torvalds
2011-08-02 1:39 ` Al Viro
2011-08-02 1:39 ` Al Viro
3 siblings, 0 replies; 14+ messages in thread
From: Linus Torvalds @ 2011-08-02 1:38 UTC (permalink / raw)
To: Al Viro
Cc: Rafael J. Wysocki, Josef Bacik, LKML, Linux PM mailing list, Jan Kara
On Mon, Aug 1, 2011 at 3:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
> but what's wrong with using ->f_mapping here?)
My only issue was that from a "mindless conversion" standpoint, most
of the other users had been changed to use inode->i_mapping.
I dunno why. I agree that file->f_mapping looks cleaner and is what
the code used to do. But maybe there was some reason why the other
fsync methods had been changed to use
filemap_write_and_wait_range(inode->i_mapping, start, end);
instead.
Linus
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:30 ` Al Viro
@ 2011-08-02 1:38 ` Linus Torvalds
2011-08-02 1:38 ` Linus Torvalds
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Linus Torvalds @ 2011-08-02 1:38 UTC (permalink / raw)
To: Al Viro; +Cc: Linux PM mailing list, Jan Kara, LKML, Josef Bacik
On Mon, Aug 1, 2011 at 3:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
> but what's wrong with using ->f_mapping here?)
My only issue was that from a "mindless conversion" standpoint, most
of the other users had been changed to use inode->i_mapping.
I dunno why. I agree that file->f_mapping looks cleaner and is what
the code used to do. But maybe there was some reason why the other
fsync methods had been changed to use
filemap_write_and_wait_range(inode->i_mapping, start, end);
instead.
Linus
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:30 ` Al Viro
` (2 preceding siblings ...)
2011-08-02 1:39 ` Al Viro
@ 2011-08-02 1:39 ` Al Viro
2011-08-02 9:26 ` Rafael J. Wysocki
2011-08-02 9:26 ` Rafael J. Wysocki
3 siblings, 2 replies; 14+ messages in thread
From: Al Viro @ 2011-08-02 1:39 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rafael J. Wysocki, Josef Bacik, LKML, Linux PM mailing list, Jan Kara
On Tue, Aug 02, 2011 at 02:30:27AM +0100, Al Viro wrote:
> Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
> but what's wrong with using ->f_mapping here?)
BTW, I've put s-o-b: Rafael J. Wysocki <rjw@sisk.pl> in there. Rafael,
do you have any problems with that?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:30 ` Al Viro
2011-08-02 1:38 ` Linus Torvalds
2011-08-02 1:38 ` Linus Torvalds
@ 2011-08-02 1:39 ` Al Viro
2011-08-02 1:39 ` Al Viro
3 siblings, 0 replies; 14+ messages in thread
From: Al Viro @ 2011-08-02 1:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux PM mailing list, Jan Kara, LKML, Josef Bacik
On Tue, Aug 02, 2011 at 02:30:27AM +0100, Al Viro wrote:
> Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
> but what's wrong with using ->f_mapping here?)
BTW, I've put s-o-b: Rafael J. Wysocki <rjw@sisk.pl> in there. Rafael,
do you have any problems with that?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:39 ` Al Viro
2011-08-02 9:26 ` Rafael J. Wysocki
@ 2011-08-02 9:26 ` Rafael J. Wysocki
1 sibling, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2011-08-02 9:26 UTC (permalink / raw)
To: Al Viro
Cc: Linus Torvalds, Josef Bacik, LKML, Linux PM mailing list, Jan Kara
On Tuesday, August 02, 2011, Al Viro wrote:
> On Tue, Aug 02, 2011 at 02:30:27AM +0100, Al Viro wrote:
>
> > Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
> > but what's wrong with using ->f_mapping here?)
>
> BTW, I've put s-o-b: Rafael J. Wysocki <rjw@sisk.pl> in there. Rafael,
> do you have any problems with that?
Not at all.
Thanks a lot,
Rafael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
2011-08-02 1:39 ` Al Viro
@ 2011-08-02 9:26 ` Rafael J. Wysocki
2011-08-02 9:26 ` Rafael J. Wysocki
1 sibling, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2011-08-02 9:26 UTC (permalink / raw)
To: Al Viro
Cc: Jan Kara, Linux PM mailing list, Linus Torvalds, LKML, Josef Bacik
On Tuesday, August 02, 2011, Al Viro wrote:
> On Tue, Aug 02, 2011 at 02:30:27AM +0100, Al Viro wrote:
>
> > Applied (in file->f_mapping variant; it is equal to bdev->bd_mapping,
> > but what's wrong with using ->f_mapping here?)
>
> BTW, I've put s-o-b: Rafael J. Wysocki <rjw@sisk.pl> in there. Rafael,
> do you have any problems with that?
Not at all.
Thanks a lot,
Rafael
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk
@ 2011-08-01 23:47 Rafael J. Wysocki
0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2011-08-01 23:47 UTC (permalink / raw)
To: Josef Bacik
Cc: Jan Kara, Linux PM mailing list, Linus Torvalds, LKML, Al Viro
Hi,
Unfortunately s2disk (which is a user space tool for saving hibernate images)
doesn't work any more after your commit 02c24a82187d5a628c68edfe71ae60dc135cd178
(fs: push i_mutex and filemap_write_and_wait down into ->fsync() handlers).
The symptom is that the s2disk's resume counterpart cannot find the image in
the device even though s2disk considers it as successfully saved.
What happens is that s2disk opens a block device (disk partition to be precise)
directly and writes to it eventually calling fsync(). My interpretation of the
failure is that before commit 02c24a82187d5a628c68ed, when s2disk called fsync()
the VFS layer would run filemap_write_and_wait_range() that in turn would cause
the data to go to the disk, but after that commit this doesn't happen any more.
If I'm right, then not only s2disk, but any process writing directly to a block
device will have a problem with fsync().
Thanks,
Rafael
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-08-02 9:26 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-01 23:47 [Regression] Commit 02c24a82187d5a628c68edfe71ae60dc135cd178 breaks s2disk Rafael J. Wysocki
2011-08-02 0:17 ` Rafael J. Wysocki
2011-08-02 0:17 ` Rafael J. Wysocki
2011-08-02 1:22 ` Linus Torvalds
2011-08-02 1:22 ` Linus Torvalds
2011-08-02 1:30 ` Al Viro
2011-08-02 1:38 ` Linus Torvalds
2011-08-02 1:38 ` Linus Torvalds
2011-08-02 1:39 ` Al Viro
2011-08-02 1:39 ` Al Viro
2011-08-02 9:26 ` Rafael J. Wysocki
2011-08-02 9:26 ` Rafael J. Wysocki
2011-08-02 1:30 ` Al Viro
2011-08-01 23:47 Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.