| Message ID | CA+eEYqX6OkHEF0AhQ5E7DbSF16So7W0wiff=2uhgm9dmtsQGjQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: clear page dirty before invalidate page | expand |
On Tue, 2019-07-23 at 15:55 +0800, erqi chen wrote: > From: Erqi Chen <chenerqi@gmail.com> > > clear_page_dirty_for_io(page) before mapping->a_ops->invalidatepage(). > invalidatepage() clears page's private flag, if dirty flag is not > cleared, the page may cause BUG_ON failure in ceph_set_page_dirty(). > > Fixes: https://tracker.ceph.com/issues/40862 > Signed-off-by: Erqi Chen chenerqi@gmail.com > --- > fs/ceph/addr.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index e078cc5..5ad63bf 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -914,9 +914,11 @@ static int ceph_writepages_start(struct > address_space *mapping, > dout("%p page eof %llu\n", > page, ceph_wbc.i_size); > if (ceph_wbc.size_stable || > - page_offset(page) >= i_size_read(inode)) > - mapping->a_ops->invalidatepage(page, > - 0, PAGE_SIZE); > + page_offset(page) >= i_size_read(inode)) { > + if (clear_page_dirty_for_io(page)) > + mapping->a_ops->invalidatepage(page, > + 0, PAGE_SIZE); > + } > unlock_page(page); > continue; > } > -- > 1.8.3.1 Looks good. I'll plan to do a bit of testing with this today and then merge it into the testing branch. Thanks,
On Tue, 2019-07-23 at 15:55 +0800, erqi chen wrote: > From: Erqi Chen <chenerqi@gmail.com> > > clear_page_dirty_for_io(page) before mapping->a_ops->invalidatepage(). > invalidatepage() clears page's private flag, if dirty flag is not > cleared, the page may cause BUG_ON failure in ceph_set_page_dirty(). > > Fixes: https://tracker.ceph.com/issues/40862 > Signed-off-by: Erqi Chen chenerqi@gmail.com > --- > fs/ceph/addr.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index e078cc5..5ad63bf 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -914,9 +914,11 @@ static int ceph_writepages_start(struct > address_space *mapping, > dout("%p page eof %llu\n", > page, ceph_wbc.i_size); > if (ceph_wbc.size_stable || > - page_offset(page) >= i_size_read(inode)) > - mapping->a_ops->invalidatepage(page, > - 0, PAGE_SIZE); > + page_offset(page) >= i_size_read(inode)) { > + if (clear_page_dirty_for_io(page)) > + mapping->a_ops->invalidatepage(page, > + 0, PAGE_SIZE); It might be cleaner to just add: && clear_page_dirty_for_io(page) to the existing if statement -- that will reduce the level of indentation (which is already pretty far here). > + } > unlock_page(page); > continue; > } > -- > 1.8.3.1 This patch looks good at first glance, but there is some whitespace damage here and it does not apply. It looks like you may have cut and pasted it into an email? Could you resend it? Maybe look into using the git-send-email script to send it. Thanks,
On Tue, 2019-07-23 at 15:55 +0800, erqi chen wrote: > From: Erqi Chen <chenerqi@gmail.com> > > clear_page_dirty_for_io(page) before mapping->a_ops->invalidatepage(). > invalidatepage() clears page's private flag, if dirty flag is not > cleared, the page may cause BUG_ON failure in ceph_set_page_dirty(). > > Fixes: https://tracker.ceph.com/issues/40862 > Signed-off-by: Erqi Chen chenerqi@gmail.com > --- > fs/ceph/addr.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index e078cc5..5ad63bf 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -914,9 +914,11 @@ static int ceph_writepages_start(struct > address_space *mapping, > dout("%p page eof %llu\n", > page, ceph_wbc.i_size); > if (ceph_wbc.size_stable || > - page_offset(page) >= i_size_read(inode)) > - mapping->a_ops->invalidatepage(page, > - 0, PAGE_SIZE); > + page_offset(page) >= i_size_read(inode)) { > + if (clear_page_dirty_for_io(page)) > + mapping->a_ops->invalidatepage(page, > + 0, PAGE_SIZE); > + } > unlock_page(page); > continue; > } > -- > 1.8.3.1 Thanks for the patch! This one looks good. I'm running some tests and they look good so far. I will plan to merge this into the testing branch later today assuming there are no issues.
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c index e078cc5..5ad63bf 100644 --- a/fs/ceph/addr.c +++ b/fs/ceph/addr.c @@ -914,9 +914,11 @@ static int ceph_writepages_start(struct address_space *mapping, dout("%p page eof %llu\n", page, ceph_wbc.i_size); if (ceph_wbc.size_stable || - page_offset(page) >= i_size_read(inode)) - mapping->a_ops->invalidatepage(page, - 0, PAGE_SIZE); + page_offset(page) >= i_size_read(inode)) { + if (clear_page_dirty_for_io(page)) + mapping->a_ops->invalidatepage(page, + 0, PAGE_SIZE); + } unlock_page(page); continue;