linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Amir G." <amir73il@users.sourceforge.net>
To: Jan Kara <jack@suse.cz>
Cc: Ted Tso <tytso@mit.edu>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/3] Rewrite ext4_page_mkwrite, fix fs freezing
Date: Wed, 11 May 2011 13:15:36 +0300	[thread overview]
Message-ID: <BANLkTik7pmNpiWR4jZAZALf58S+015dYAw@mail.gmail.com> (raw)
In-Reply-To: <20110511094311.GB5057@quack.suse.cz>

On Wed, May 11, 2011 at 12:43 PM, Jan Kara <jack@suse.cz> wrote:
>  Hi Amir,
>
> On Wed 11-05-11 10:33:17, Amir G. wrote:
>> Can you provide a bit of a wider scope review of how this related to the
>> work on stable pages.
>  It is related in the sense that Darrick had to return locked page from
> ext4_page_mkwrite() to avoid races and the result (although working) is
> really like scratching your left ear with your right hand (not sure if
> English has this idiom ;). So with my patches, what Darrick needs to do is
> simply achieved by updating block_page_mkwrite() (__block_page_mkwrite() in
> fact).
>
>> For example, when to the pages get unlocked?
>  That's handled by mm code in mm/memory.c. You can read do_wp_page() but
> it's not a light reading ;)

This is why I asked for the cheat sheet ;-)

>
>> If the pages supposed to be stable during writeback, how is this related
>> to returning locked pages from page_mkwrite?
>  Returning locked page is needed to avoid races with page writeback - for
> stable pages we want to do wait_on_page_writeback() to be sure that there's
> no IO happening while we make the page writeable for user. But once we
> release page lock, writepage can come and start the writeback. The basic
> scheme of the race we want to avoid is:
>  do_wp_page()
>    ext4_page_mkwrite()
>      wait_on_page_writeback()
>      unlock_page()
>                                        clear_page_dirty_for_io()
>                                          page_mkclean()
>                                        writepage()
>    maybe_mkwrite()
>
> Because this results in writeable page under writeback... We have to make
> sure page_mkclean() happens *after* maybe_mkwrite() from do_wp_page() in
> this scenario.
>
>> Is the page going to stay locked until writeback?
>  No, until the page fault is finished.
>
>> Do I understand correctly that a page will be marked read-only after
>> writeback completes, so page_mkwrite will be called again on next write?
>  Page is marked read-only before writeback is started in
> clear_page_dirty_for_io().

OK. that makes sense. I wasn't sure my move-on-write hook,
which is called from ext4_page_mkwrite() is going to be called on every
writeback.
The use case is:
data = mmap(file);
data[0] = 1;
take snapshot 1;
data[0] = 2;
take snapshot 2;
data[0] = 3;
...

That use case doesn't specify when writeback of first page of file happens
and I don't care which version of data[0] is seen by the snapshots, as long
as the snapshot view is internally consistent (it doesn't change over time or
after reboot).

So if ext4_page_mkwrite() is called after every writeback and the page is stable
during writeback, snapshots should be OK.

>
>                                                                Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

      reply	other threads:[~2011-05-11 15:40 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-10 22:29 [PATCH 0/3] Rewrite ext4_page_mkwrite, fix fs freezing Jan Kara
2011-05-10 22:29 ` [PATCH 1/3] fs: Create __block_page_mkwrite() helper passing error values back Jan Kara
2011-05-17 15:09   ` Christoph Hellwig
2011-05-18  7:35     ` Jan Kara
2011-05-10 22:29 ` [PATCH 2/3] ext4: Rewrite ext4_page_mkwrite() to return locked page Jan Kara
2011-05-10 22:29 ` [PATCH 3/3] ext4: Block mmapped writes while the fs is frozen Jan Kara
2011-05-17 15:11   ` Christoph Hellwig
2011-05-18  7:56     ` Jan Kara
2011-05-18  8:07       ` Christoph Hellwig
2011-05-18 14:03         ` Eric Sandeen
2011-05-18 15:25           ` Jan Kara
2011-05-18 16:40             ` Eric Sandeen
2011-05-11  0:30 ` [PATCH 0/3] Rewrite ext4_page_mkwrite, fix fs freezing Ted Ts'o
2011-05-11  9:27   ` Jan Kara
2011-05-11 16:48     ` Darrick J. Wong
2011-05-11 17:03       ` Jan Kara
2011-05-11  7:33 ` Amir G.
2011-05-11  9:43   ` Jan Kara
2011-05-11 10:15     ` Amir G. [this message]

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=BANLkTik7pmNpiWR4jZAZALf58S+015dYAw@mail.gmail.com \
    --to=amir73il@users.sourceforge.net \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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).