From: Trond Myklebust <Trond.Myklebust@netapp.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-nfs@vger.kernel.org, Wu Fengguang <fengguang.wu@intel.com>,
Peter Zijlstra <peterz@infradead.org>, Jan Kara <jack@suse.cz>,
Steve Rago <sar@nec-labs.com>, Jens Axboe <jens.axboe@oracle.com>,
Peter Staubach <staubach@redhat.com>,
Arjan van de Ven <arjan@infradead.org>,
Ingo Molnar <mingo@elte.hu>,
linux-fsdevel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH 09/12] NFS: Replace __nfs_write_mapping with sync_inode()
Date: Tue, 26 Jan 2010 09:02:52 -0500 [thread overview]
Message-ID: <1264514573.3615.90.camel@localhost> (raw)
In-Reply-To: <20100126112148.GA25170@infradead.org>
On Tue, 2010-01-26 at 06:21 -0500, Christoph Hellwig wrote:
> On Mon, Jan 25, 2010 at 05:15:45PM -0500, Trond Myklebust wrote:
> > int nfs_wb_nocommit(struct inode *inode)
> > {
> > - return nfs_write_mapping(inode->i_mapping, FLUSH_NOCOMMIT);
> > + return filemap_write_and_wait(inode->i_mapping);
>
> Any point in keeping this as a wrapper for a single well-documented
> caller? Also taking i_mutex around it seems a bit questionable these
> days given that filemap_write_and_wait avoids lifelocks with writing
> applications okay and we use it without i_mutex all over the place.
>
Agreed, but just out of curiosity. Is there any reason why we shouldn't
use filemap_flush() + filemap_fdatawait() here instead? We're not really
interested in doing a full data integrity flush, but just want to make
sure that pages which were dirtied before the stat() syscall are flushed
to disk so that the server updates the c/mtime for us.
Cheers
Trond
WARNING: multiple messages have this Message-ID (diff)
From: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
To: Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Wu Fengguang
<fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>,
Steve Rago <sar-a+KepyhlMvJWk0Htik3J/w@public.gmane.org>,
Jens Axboe <jens.axboe-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>,
Peter Staubach <staubach-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
Arjan van de Ven <arjan-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>,
Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
Subject: Re: [PATCH 09/12] NFS: Replace __nfs_write_mapping with sync_inode()
Date: Tue, 26 Jan 2010 09:02:52 -0500 [thread overview]
Message-ID: <1264514573.3615.90.camel@localhost> (raw)
In-Reply-To: <20100126112148.GA25170-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On Tue, 2010-01-26 at 06:21 -0500, Christoph Hellwig wrote:
> On Mon, Jan 25, 2010 at 05:15:45PM -0500, Trond Myklebust wrote:
> > int nfs_wb_nocommit(struct inode *inode)
> > {
> > - return nfs_write_mapping(inode->i_mapping, FLUSH_NOCOMMIT);
> > + return filemap_write_and_wait(inode->i_mapping);
>
> Any point in keeping this as a wrapper for a single well-documented
> caller? Also taking i_mutex around it seems a bit questionable these
> days given that filemap_write_and_wait avoids lifelocks with writing
> applications okay and we use it without i_mutex all over the place.
>
Agreed, but just out of curiosity. Is there any reason why we shouldn't
use filemap_flush() + filemap_fdatawait() here instead? We're not really
interested in doing a full data integrity flush, but just want to make
sure that pages which were dirtied before the stat() syscall are flushed
to disk so that the server updates the c/mtime for us.
Cheers
Trond
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2010-01-26 14:03 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-25 22:15 [PATCH 00/12] Re: [PATCH] improve the performance of large sequential write NFS workloads Trond Myklebust
[not found] ` <20100125221544.16750.70574.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-01-25 22:15 ` [PATCH 03/12] NFS: Cleanup - move nfs_write_inode() into fs/nfs/write.c Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 07/12] NFS: Ensure inode is always marked I_DIRTY_DATASYNC, if it has unstable pages Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 01/12] VM: Split out the accounting of unstable writes from BDI_RECLAIMABLE Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 08/12] NFS: Simplify nfs_wb_page_cancel() Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 02/12] VM: Don't call bdi_stat(BDI_UNSTABLE) on non-nfs backing-devices Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 04/12] NFS: Reduce the number of unnecessary COMMIT calls Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 06/12] NFS: Run COMMIT as an asynchronous RPC call when wbc->for_background is set Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 10/12] NFS: Simplify nfs_wb_page() Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
[not found] ` <20100125221545.16750.19154.stgit-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-10 18:51 ` J. R. Okajima
2010-03-10 18:51 ` J. R. Okajima
2010-03-10 19:31 ` Trond Myklebust
2010-03-10 19:31 ` Trond Myklebust
[not found] ` <1268249482.3096.76.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-10 20:18 ` Trond Myklebust
2010-03-10 20:18 ` Trond Myklebust
[not found] ` <1268252300.3096.81.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-11 4:45 ` J. R. Okajima
2010-03-11 4:45 ` J. R. Okajima
2010-03-11 14:26 ` Trond Myklebust
2010-03-11 14:26 ` Trond Myklebust
[not found] ` <1268317582.3354.9.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-12 4:22 ` J. R. Okajima
2010-03-12 4:22 ` J. R. Okajima
2010-03-17 16:49 ` Christoph Hellwig
2010-03-17 16:49 ` Christoph Hellwig
2010-03-17 17:26 ` Trond Myklebust
2010-03-17 17:52 ` Jeff Layton
2010-03-17 17:58 ` Trond Myklebust
[not found] ` <1268848682.8335.5.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2010-03-17 18:08 ` Jeff Layton
2010-03-17 18:08 ` Jeff Layton
2010-01-25 22:15 ` [PATCH 09/12] NFS: Replace __nfs_write_mapping with sync_inode() Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-26 11:21 ` Christoph Hellwig
2010-01-26 11:21 ` Christoph Hellwig
2010-01-26 14:02 ` Trond Myklebust [this message]
2010-01-26 14:02 ` Trond Myklebust
2010-01-26 23:17 ` Trond Myklebust
2010-01-26 23:17 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 12/12] NFS: Remove requirement for inode->i_mutex from nfs_invalidate_mapping Trond Myklebust
2010-01-25 22:15 ` Trond Myklebust
2010-01-25 22:15 ` [PATCH 05/12] VM/NFS: The VM must tell the filesystem when to free reclaimable pages Trond Myklebust
2010-01-25 22:15 ` [PATCH 11/12] NFS: Clean up nfs_sync_mapping Trond Myklebust
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=1264514573.3615.90.camel@localhost \
--to=trond.myklebust@netapp.com \
--cc=arjan@infradead.org \
--cc=fengguang.wu@intel.com \
--cc=hch@infradead.org \
--cc=hch@lst.de \
--cc=jack@suse.cz \
--cc=jens.axboe@oracle.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=sar@nec-labs.com \
--cc=staubach@redhat.com \
--cc=viro@ZenIV.linux.org.uk \
/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 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.