All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Mason <chris.mason-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
To: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>,
	linux-cifs <linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-fsdevel
	<linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Mingming Cao <mcao-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>,
	Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: Re: stable page writes: wait_on_page_writeback and packet signing
Date: Thu, 10 Mar 2011 04:26:31 -0800 (PST)	[thread overview]
Message-ID: <1299717690-sup-2613@think> (raw)
In-Reply-To: <AANLkTikK8MOm-m9XsOA4YGRe=E9bJTDh4iEYXtZumNmv-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Excerpts from Steve French's message of 2011-03-09 17:13:06 -0500:
> On Wed, Mar 9, 2011 at 3:58 PM, Chris Mason <chris.mason-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> > Excerpts from Dave Chinner's message of 2011-03-09 16:51:48 -0500:
> >> On Wed, Mar 09, 2011 at 01:44:24PM -0600, Steve French wrote:
> >> > Have alternative approaches, other than using wait_on_page_writeback,
> >> > been considered for solving the stable page write problem in similar
> >> > cases (since only about 1 out of 5 linux file systems uses this call
> >> > today).
> >>
> >> I think that is incorrect. write_cache_pages() does:
> >>
> >>  929                         lock_page(page);
> >> .....
> >>  950                         if (PageWriteback(page)) {
> >>  951                                 if (wbc->sync_mode != WB_SYNC_NONE)
> >>  952                                         wait_on_page_writeback(page);
> >>  953                                 else
> >>  954                                         goto continue_unlock;
> >>  955                         }
> >>  956
> >>  957                         BUG_ON(PageWriteback(page));
> >>  958                         if (!clear_page_dirty_for_io(page))
> >>  959                                 goto continue_unlock;
> >>  960
> >>  961                         trace_wbc_writepage(wbc, mapping->backing_dev_info);
> >>  962                         ret = (*writepage)(page, wbc, data);
> >>
> >> so every filesystem using the generic_writepages code already does
> >> this check and wait before .writepage is called. Hence only the
> >> filesystems that do not use generic_writepages() or
> >> mpage_writepages() need a specific check, and that means most
> >> filesystems are actually waiting on writeback pages correctly.
> >
> > But checking here just means we don't start writeback on a page that is
> > writeback, which is a good idea but not really related to stable pages?
> >
> > stable pages means we don't let mmap'd pages or file_write muck around
> > with the pages while they are in writeback, so we need to wait in
> > file_write and page_mkwrite.
> 
> Isn't the file_write case covered by the i_mutex as
> Documentation/filesystems/Locking implies (for write_begin/write_end).
> 

Does cifs take i_mutex before writepage?  The disk based filesystems
don't.  So, i_mutex protects file_write from other procs jumping into
file_write, but it doesn't protect writeback from file_write jumping in
and changing the pages while they are being sent to storage (or over the
wire).

Basically the model needs to be:

file_write:
	lock the page
	wait on page writeback

	< new writeback cannot start because of the page lock >
	copy_from_user
	unlock the page

We also use page_mkwrite to get notified when userland wants to change
some page it has given to mmap.  That needs to wait on page writeback as
well.

-chris

  parent reply	other threads:[~2011-03-10 12:26 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-09 19:44 stable page writes: wait_on_page_writeback and packet signing Steve French
     [not found] ` <AANLkTinFx9KGKDWSdUvFSvT4S6f9QjBzX=6Uo17oO89+-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-03-09 21:51   ` Dave Chinner
2011-03-09 21:58     ` Chris Mason
2011-03-09 22:13       ` Steve French
     [not found]         ` <AANLkTikK8MOm-m9XsOA4YGRe=E9bJTDh4iEYXtZumNmv-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-03-10 12:26           ` Chris Mason [this message]
2011-03-10 13:16             ` Jeff Layton
     [not found]               ` <20110310081638.0f8275d4-xSBYVWDuneFaJnirhKH9O4GKTjYczspe@public.gmane.org>
2011-03-10 13:32                 ` Chris Mason
2011-03-10 13:47                   ` Jeff Layton
     [not found]                     ` <20110310084724.658fe5d7-xSBYVWDuneFaJnirhKH9O4GKTjYczspe@public.gmane.org>
2011-03-10 13:58                       ` Chris Mason
2011-03-11 12:11                         ` Jeff Layton
     [not found]                           ` <20110311071143.01b407b6-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2011-03-11 12:56                             ` Chris Mason
2011-03-11 13:42                               ` Jeff Layton
     [not found]                                 ` <20110311084221.4ac6bd11-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2011-03-11 16:00                                   ` Chris Mason
2011-03-09 23:46       ` Dave Chinner
2011-03-09 22:01     ` Steve French
2011-03-09 23:54       ` Jeff Layton
     [not found]         ` <20110309185427.7858c29b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-03-10  0:33           ` Steve French
     [not found]             ` <AANLkTi=pXHjE6tNMm0_nO=Cn3nGH8oZ6Xhm1STh8x1Xe-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-03-10  1:30               ` Jeff Layton
2011-03-10 13:53                 ` Steve French
     [not found]                 ` <20110309203044.4fd0498e-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2011-03-11 11:53                   ` Jeff Layton
     [not found]       ` <AANLkTinDmqah6pQnHugoVxh-gDq+6+MDMuh-TyVAQ7LP-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-03-10  1:41         ` Trond Myklebust
     [not found]           ` <1299721264.2976.3.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
2011-03-10  7:34             ` Christoph Hellwig
2011-03-10 13:44             ` Steve French
2011-03-09 23:45     ` Jeff Layton
2011-03-10  2:12       ` Jeff Layton

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=1299717690-sup-2613@think \
    --to=chris.mason-qhclzuegtsvqt0dzr+alfa@public.gmane.org \
    --cc=david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org \
    --cc=jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mcao-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org \
    --cc=smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.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 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.