linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: Jeff Layton <jlayton@redhat.com>
Cc: Steven Whitehouse <swhiteho@redhat.com>, Jan Kara <jack@suse.cz>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"J . Bruce Fields" <bfields@fieldses.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Matthew Wilcox <willy@infradead.org>,
	cluster-devel@redhat.com,
	Benjamin Marzinski <bmarzins@redhat.com>
Subject: Re: [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait
Date: Mon, 31 Jul 2017 08:38:41 -0400 (EDT)	[thread overview]
Message-ID: <1822812523.36420383.1501504721611.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <1501503761.4663.11.camel@redhat.com>

----- Original Message -----
| > If this can be called from anywhere without fs locks, then i_size is not
| > known. That has been a problem in the past since i_size may have changed
| > on another node. We avoid that in this case due to only changing i_size
| > under an exclusive lock, and also only having dirty pages when we have
| > an exclusive lock. There is another case though, if the inode is a block
| > device, i_size will be zero. That is the case for the address space that
| > looks after rgrps for GFS2. We do (luckily!) call
| > filemap_fdatawait_range() directly in that case. For "normal" inodes
| > though, the address space for metadata is backed by the block device
| > inode, so that looks like it might be an issue, since
| > fs/gfs2/glops.c:inode_go_sync() calls filemap_fdatawait() on the
| > metamapping. It might potentially be an issue in other cases too,
| > 
| > Steve.
| > 
| 
| Some of those do sound problematic.
| 
| Again though, we're only waiting on writeback here, and I assume with
| gfs2 that would only be pages that were written on the local node.
| 
| Is it possible to have pages under writeback and in still in the tree,
| but that are beyond the current i_size? It seems like that's the main
| worrisome case.
| 
| --
| Jeff Layton <jlayton@redhat.com>

Hi Jeff,

I believe the answer is yes.

I was recently "bitten" by a case where (whether due to a bug or not)
I had blocks allocated in a GFS2 file beyond i_size. I had implemented a
delete algorithm that used i_size, but I found cases where files couldn't
be deleted because of blocks hanging out past EOF. I'm not sure if they
can be in writeback, but possibly. It's already on my "to investigate"
list, but I haven't gotten to it yet. Yes, it seems like a bug. Yes, we
need to fix it. But now there may be lots of legacy file systems out in
the field that have this problem. Not sure if they can get to writeback
until I study the situation more closely.

I believe Ben Marzinski also may have come across a case in which we
can have blocks in writeback that are beyond i_size. See the commit
message on Ben's patch here:

https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?h=for-next&id=fd4c5748b8d3f7420e8932ed0bde3d53cc8acc9d

Regards,

Bob Peterson
Red Hat File Systems

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2017-07-31 12:38 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-26 17:55 [PATCH v2 0/4] mm/gfs2: extend file_* API, and convert gfs2 to errseq_t error reporting Jeff Layton
2017-07-26 17:55 ` [PATCH v2 1/4] mm: consolidate dax / non-dax checks for writeback Jeff Layton
2017-07-27  8:43   ` Jan Kara
2017-07-26 17:55 ` [PATCH v2 2/4] mm: add file_fdatawait_range and file_write_and_wait Jeff Layton
2017-07-26 19:13   ` Matthew Wilcox
2017-07-26 22:18     ` Jeff Layton
2017-07-26 19:50   ` Bob Peterson
2017-07-27  8:49   ` Jan Kara
2017-07-27 12:48     ` Jeff Layton
2017-07-31 11:27       ` Jeff Layton
2017-07-31 11:32         ` Steven Whitehouse
2017-07-31 11:44           ` Jeff Layton
2017-07-31 12:05             ` Steven Whitehouse
2017-07-31 12:22               ` Jeff Layton
2017-07-31 12:25                 ` Steven Whitehouse
2017-07-31 12:38                 ` Bob Peterson [this message]
2017-07-31 12:07             ` Jan Kara
2017-07-31 13:00               ` Jeff Layton
2017-07-31 13:32                 ` Jan Kara
2017-07-31 16:49   ` [PATCH v3] " Jeff Layton
2017-08-01  9:52     ` Jan Kara
2017-07-26 17:55 ` [PATCH v2 3/4] fs: convert sync_file_range to use errseq_t based error-tracking Jeff Layton
2017-07-26 17:55 ` [PATCH v2 4/4] gfs2: convert to errseq_t based writeback error reporting for fsync Jeff Layton
2017-07-26 19:21   ` Matthew Wilcox
2017-07-26 22:22     ` Jeff Layton
2017-07-27 12:47       ` Bob Peterson
2017-07-28 12:37         ` Steven Whitehouse
2017-07-28 12:47           ` Jeff Layton
2017-07-28 12:54             ` Steven Whitehouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1822812523.36420383.1501504721611.JavaMail.zimbra@redhat.com \
    --to=rpeterso@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=bfields@fieldses.org \
    --cc=bmarzins@redhat.com \
    --cc=cluster-devel@redhat.com \
    --cc=jack@suse.cz \
    --cc=jlayton@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mtosatti@redhat.com \
    --cc=swhiteho@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).