LKML Archive on
help / color / mirror / Atom feed
From: Oleg Nesterov <>
To: Hugh Dickins <>
Cc: Andrew Morton <>,
	Miklos Szeredi <>,
Subject: Re: [PATCH] mmap_region: cleanup, remove unneeded file != NULL check
Date: Mon, 11 Feb 2008 13:34:05 +0300	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Sorry for delay,

On 02/06, Hugh Dickins wrote:
> On Sun, 3 Feb 2008, Oleg Nesterov wrote:
> > mmap_region() checks "file != NULL" when we know "file && vma_merge() == T".
> > Also, swap these if/else branches, imho make the code a bit more readable.
> > 
> > Signed-off-by: Oleg Nesterov <>
> Acked-with-a-but-by: Hugh Dickins <>
> But my but is this: you can go one step further, it's silly to be
> repeating the "if (correct_wcount) atomic_inc..." in both the if
> and the else clauses.

Ah. Shame on me. Of course, I noticed these 2 correct_wcount's, but
didn't realize we can safely use "inode" after fput(). Not only the
caller should have a reference, vma_merge() requires that vm_file == file,
so fput(file) can't destroy the last reference.

> For several minutes I thought that must indicate we already had a
> bug there.  Eventually I realized not: we need deny_write_access()
> above to test and deny atomically, then once we've merged or linked
> the vma it's securely denying in the vma itself: so before returning
> we need to undo our temporary denial.  A brief comment might be
> worthwhile, perhaps something like
> 	/* Once vma denies write, undo our temporary denial count */
> 	if (correct_wcount)
> 		atomic_inc(&inode->i_writecount);

Thanks Hugh, I'll redo this cleanup.


      reply	other threads:[~2008-02-11 10:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-03 18:01 Oleg Nesterov
2008-02-06 20:22 ` Hugh Dickins
2008-02-11 10:34   ` Oleg Nesterov [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:

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

  git send-email \ \ \ \ \ \ \
    --subject='Re: [PATCH] mmap_region: cleanup, remove unneeded file '\!'= NULL check' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).