[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110519120816.GG8417@quack.suse.cz>
Date: Thu, 19 May 2011 14:08:16 +0200
From: Jan Kara <jack@...e.cz>
To: Christoph Hellwig <hch@...radead.org>
Cc: Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org,
linux-ext4@...r.kernel.org, Al Viro <viro@...IV.linux.org.uk>,
tytso@....edu
Subject: Re: [PATCH 2/3] vfs: Block mmapped writes while the fs is frozen
On Wed 18-05-11 14:12:06, Christoph Hellwig wrote:
> >
> > if (unlikely(ret < 0))
> > unlock_page(page);
> > + else {
> > + /*
> > + * Freezing in progress? We check after the page is marked
> > + * dirty and with page lock held so if the test here fails, we
> > + * are sure freezing code will wait during syncing until the
> > + * page fault is done - at that point page will be dirty and
> > + * unlocked so freezing code will write it and writeprotect it
> > + * again.
> > + */
> > + set_page_dirty(page);
> > + if (inode->i_sb->s_frozen != SB_UNFROZEN) {
> > + unlock_page(page);
> > + ret = -EAGAIN;
> > + goto out;
> > + }
> > + }
> > out:
> > return ret;
>
> The code structure looks a bit odd, why not:
>
> if (ret < 0)
> goto out_unlock;
>
> set_page_dirty(page);
> if (inode->i_sb->s_frozen != SB_UNFROZEN) {
> ret = -EAGAIN;
> goto out_unlock;
> }
>
> return 0;
>
> out_unlock:
> unlock_page(page);
> return ret;
> }
>
> Otherwise looks good,
>
> Reviewed-by: Christoph Hellwig <hch@....de>
Thanks, I've changed the flow as you suggested.
Honza
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists