* [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored
@ 2013-09-20 3:06 Darrick J. Wong
2013-09-20 3:28 ` Mike Snitzer
2013-09-20 8:06 ` Mel Gorman
0 siblings, 2 replies; 6+ messages in thread
From: Darrick J. Wong @ 2013-09-20 3:06 UTC (permalink / raw)
To: Kent Overstreet, Andrew Morton
Cc: Mel Gorman, Jens Axboe, Jan Kara, Hannes Reinecke, linux-kernel
The "force" parameter in __blk_queue_bounce was being ignored, which means that
stable page snapshots are not always happening (on ext3). This of course
leads to DIF disks reporting checksum errors, so fix this regression.
The regression was introduced in commit 6bc454d1 (bounce: Refactor
__blk_queue_bounce to not use bi_io_vec)
Reported-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
mm/bounce.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/bounce.c b/mm/bounce.c
index c9f0a43..5a7d58f 100644
--- a/mm/bounce.c
+++ b/mm/bounce.c
@@ -204,6 +204,8 @@ static void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig,
struct bio_vec *to, *from;
unsigned i;
+ if (force)
+ goto bounce;
bio_for_each_segment(from, *bio_orig, i)
if (page_to_pfn(from->bv_page) > queue_bounce_pfn(q))
goto bounce;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored
2013-09-20 3:06 [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored Darrick J. Wong
@ 2013-09-20 3:28 ` Mike Snitzer
2013-09-20 8:06 ` Mel Gorman
1 sibling, 0 replies; 6+ messages in thread
From: Mike Snitzer @ 2013-09-20 3:28 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Kent Overstreet, Andrew Morton, Mel Gorman, Jens Axboe, Jan Kara,
Hannes Reinecke, linux-kernel@vger.kernel.org
On Thu, Sep 19, 2013 at 11:06 PM, Darrick J. Wong
<darrick.wong@oracle.com> wrote:
> The "force" parameter in __blk_queue_bounce was being ignored, which means that
> stable page snapshots are not always happening (on ext3). This of course
> leads to DIF disks reporting checksum errors, so fix this regression.
>
> The regression was introduced in commit 6bc454d1 (bounce: Refactor
> __blk_queue_bounce to not use bi_io_vec)
Should probably add "Cc: stable@vger.kernel.org # v3.10+" no?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored
2013-09-20 3:06 [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored Darrick J. Wong
2013-09-20 3:28 ` Mike Snitzer
@ 2013-09-20 8:06 ` Mel Gorman
2013-09-20 8:28 ` Mel Gorman
1 sibling, 1 reply; 6+ messages in thread
From: Mel Gorman @ 2013-09-20 8:06 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Kent Overstreet, Andrew Morton, Jens Axboe, Jan Kara,
Hannes Reinecke, linux-kernel
On Thu, Sep 19, 2013 at 08:06:02PM -0700, Darrick J. Wong wrote:
> The "force" parameter in __blk_queue_bounce was being ignored, which means that
> stable page snapshots are not always happening (on ext3). This of course
> leads to DIF disks reporting checksum errors, so fix this regression.
>
> The regression was introduced in commit 6bc454d1 (bounce: Refactor
> __blk_queue_bounce to not use bi_io_vec)
>
> Reported-by: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
I have no means of testing it but it looks right and thanks for checking
DIF disks.
Acked-by: Mel Gorman <mgorman@suse.de>
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored
2013-09-20 8:06 ` Mel Gorman
@ 2013-09-20 8:28 ` Mel Gorman
2013-09-20 16:54 ` Darrick J. Wong
0 siblings, 1 reply; 6+ messages in thread
From: Mel Gorman @ 2013-09-20 8:28 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Kent Overstreet, Andrew Morton, Jens Axboe, Jan Kara,
Hannes Reinecke, linux-kernel
On Fri, Sep 20, 2013 at 09:06:37AM +0100, Mel Gorman wrote:
> On Thu, Sep 19, 2013 at 08:06:02PM -0700, Darrick J. Wong wrote:
> > The "force" parameter in __blk_queue_bounce was being ignored, which means that
> > stable page snapshots are not always happening (on ext3). This of course
> > leads to DIF disks reporting checksum errors, so fix this regression.
> >
> > The regression was introduced in commit 6bc454d1 (bounce: Refactor
> > __blk_queue_bounce to not use bi_io_vec)
> >
> > Reported-by: Mel Gorman <mgorman@suse.de>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> I have no means of testing it but it looks right and thanks for checking
> DIF disks.
>
> Acked-by: Mel Gorman <mgorman@suse.de>
>
That said the two checks are now redundant. They could just be deleted
and depend entirely on the following check within the loop
if (page_to_pfn(page) <= queue_bounce_pfn(q) && !force)
continue;
with an update to the comment explaining that the check is for pages
below the bounce pfn or for bios that require stable writes
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored
2013-09-20 8:28 ` Mel Gorman
@ 2013-09-20 16:54 ` Darrick J. Wong
2013-09-24 8:33 ` Mel Gorman
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2013-09-20 16:54 UTC (permalink / raw)
To: Mel Gorman
Cc: Kent Overstreet, Andrew Morton, Jens Axboe, Jan Kara,
Hannes Reinecke, linux-kernel
On Fri, Sep 20, 2013 at 09:28:07AM +0100, Mel Gorman wrote:
> On Fri, Sep 20, 2013 at 09:06:37AM +0100, Mel Gorman wrote:
> > On Thu, Sep 19, 2013 at 08:06:02PM -0700, Darrick J. Wong wrote:
> > > The "force" parameter in __blk_queue_bounce was being ignored, which means that
> > > stable page snapshots are not always happening (on ext3). This of course
> > > leads to DIF disks reporting checksum errors, so fix this regression.
> > >
> > > The regression was introduced in commit 6bc454d1 (bounce: Refactor
> > > __blk_queue_bounce to not use bi_io_vec)
> > >
> > > Reported-by: Mel Gorman <mgorman@suse.de>
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > I have no means of testing it but it looks right and thanks for checking
> > DIF disks.
> >
> > Acked-by: Mel Gorman <mgorman@suse.de>
> >
>
> That said the two checks are now redundant. They could just be deleted
> and depend entirely on the following check within the loop
>
> if (page_to_pfn(page) <= queue_bounce_pfn(q) && !force)
> continue;
>
> with an update to the comment explaining that the check is for pages
> below the bounce pfn or for bios that require stable writes
I'm under the impression that the point of the first looping-check is to avoid
the bio_clone_bioset() if we're not going to bounce anything. But I could be
wrong. Maybe Kent can shed some light on this?
--D
>
> --
> Mel Gorman
> SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored
2013-09-20 16:54 ` Darrick J. Wong
@ 2013-09-24 8:33 ` Mel Gorman
0 siblings, 0 replies; 6+ messages in thread
From: Mel Gorman @ 2013-09-24 8:33 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Kent Overstreet, Andrew Morton, Jens Axboe, Jan Kara,
Hannes Reinecke, linux-kernel
On Fri, Sep 20, 2013 at 09:54:07AM -0700, Darrick J. Wong wrote:
> On Fri, Sep 20, 2013 at 09:28:07AM +0100, Mel Gorman wrote:
> > On Fri, Sep 20, 2013 at 09:06:37AM +0100, Mel Gorman wrote:
> > > On Thu, Sep 19, 2013 at 08:06:02PM -0700, Darrick J. Wong wrote:
> > > > The "force" parameter in __blk_queue_bounce was being ignored, which means that
> > > > stable page snapshots are not always happening (on ext3). This of course
> > > > leads to DIF disks reporting checksum errors, so fix this regression.
> > > >
> > > > The regression was introduced in commit 6bc454d1 (bounce: Refactor
> > > > __blk_queue_bounce to not use bi_io_vec)
> > > >
> > > > Reported-by: Mel Gorman <mgorman@suse.de>
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > >
> > > I have no means of testing it but it looks right and thanks for checking
> > > DIF disks.
> > >
> > > Acked-by: Mel Gorman <mgorman@suse.de>
> > >
> >
> > That said the two checks are now redundant. They could just be deleted
> > and depend entirely on the following check within the loop
> >
> > if (page_to_pfn(page) <= queue_bounce_pfn(q) && !force)
> > continue;
> >
> > with an update to the comment explaining that the check is for pages
> > below the bounce pfn or for bios that require stable writes
>
> I'm under the impression that the point of the first looping-check is to avoid
> the bio_clone_bioset() if we're not going to bounce anything. But I could be
> wrong.
I doubt it, avoiding the clone makes sense and I was wrong about the
check being redundant anyway. Trying to do the clone within the loop
when the first bio that needs bouncing is encountered turns into a
mess.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-24 8:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-20 3:06 [PATCH] mm: Fix a regression where MS_SNAP_STABLE (stable pages snapshotting) was ignored Darrick J. Wong
2013-09-20 3:28 ` Mike Snitzer
2013-09-20 8:06 ` Mel Gorman
2013-09-20 8:28 ` Mel Gorman
2013-09-20 16:54 ` Darrick J. Wong
2013-09-24 8:33 ` Mel Gorman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).