Hi Christoph, On Tue, Aug 6, 2019 at 7:30 AM Christoph Hellwig <hch@xxxxxx> wrote: > On Mon, Aug 05, 2019 at 02:27:21PM +0200, Andreas Gruenbacher wrote: > > Christoph, > > > > thanks again for this patch and the rest of the patch queue. There's > > one minor bug here (see below). With that and the gfs2_walk_metadata > > fix I've just posted to cluster-devel, this is now all working nicely. > > Skipping through the full quote this was a missing set_page_dirty, > right? Looks fine to me and sorry for messing this up. here are the changes we currently need on top of what you've posted on July 1. On top of the page dirtying which you patch accidentally dropped in gfs2_unstuffer_page, there are two places in which we also need to call iomap_page_create to attach an iomap_page structure to the pages. The first place is in gfs2_unstuffer_page, which converts an inline (stuffed) file into a regular file. This is implemented in a filesystem specific way, and I don't think there is any point in trying to make this more generic. The second place is in gfs2_page_mkwrite. This function should eventually be changed to call iomap_page_mkwrite instead, but we can just fix it as below just to get this working. Currently, iomap_page_create is a static function in fs/iomap/buffered-io.c, so we need to export it before these changes will work. I'm still trying to track down consistency problems with a 1k blocksize in xfstests generic/263 and generic/300, and that is with the mmap locking issue fixed that Dave Chinner has pointed out [*]. This problem existed even before your changes, so your changes seem to be working correctly. Thanks again, Andreas [*] https://lore.kernel.org/linux-fsdevel/20190906205241.2292-1-agruenba@xxxxxxxxxx/ --- fs/gfs2/bmap.c | 2 ++ fs/gfs2/file.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c index bf5c494d25ef..48b458c49fa1 100644 --- a/fs/gfs2/bmap.c +++ b/fs/gfs2/bmap.c @@ -93,6 +93,8 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh, set_buffer_uptodate(bh); gfs2_trans_add_data(ip->i_gl, bh); } else { + iomap_page_create(inode, page); + set_page_dirty(page); gfs2_ordered_add_inode(ip); } diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index 997b326247e2..30fd180e199d 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -516,6 +516,8 @@ static vm_fault_t gfs2_page_mkwrite(struct vm_fault *vmf) out_uninit: gfs2_holder_uninit(&gh); if (ret == 0) { + if (!gfs2_is_jdata(ip)) + iomap_page_create(inode, page); set_page_dirty(page); wait_for_stable_page(page); } -- 2.20.1
