LKML Archive on
help / color / mirror / Atom feed
From: Duncan Sands <>
To: Alexey Dobriyan <>
Subject: Re: remove_proc_entry and read_proc
Date: Fri, 2 Feb 2007 08:31:57 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Alexey,

> I believe, barriers not needed, not now.
> This scheme relies on the fact that remove_proc_entry() will be the only
> place that will clear ->proc_fops and, once cleared, ->proc_fops will
> never be resurrected. Clearing of ->proc_fops will eventually propagate
> to CPU doing first check, thus preveting refcount bumps from this CPU.
> What can be missed is some "rogue" readers or writers¹. Big deal.

I don't understand you.  Without memory barriers, remove_proc_entry will
most of the time, but not all of the time, wait for all readers and writers
to finish before exiting.  Since the whole point of your patch was to ensure
that all readers and writers finish before remove_proc_entry exits, I don't
understand why you don't just put the memory barriers in and make it correct.

Also, I do consider it a big deal:

> ¹ Sigh, modules should do removals of proc entries first. And I should
> check for that.

Modules should of course call remove_proc_entry before exiting.  However
right now, even with your patch, a read or write method can still be
running when remove_proc_entry returns [1], so could still be running when
the module is removed (if they sleep; I guess this applies mostly to
write methods).  This is very bad - why not put in memory barriers and
fix it?  Also, plenty of proc read and write methods access private data
that is allocated before calling create_proc_entry and freed after calling
remove_proc_entry.  If a read or write method is still running after
remove_proc_entry returns, then it can access freed memory - very bad.



[1] proc_get_inode does a try_module_get, so it is possible that module
unloading is not a problem - not sure.

  reply	other threads:[~2007-02-02  7:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-01 16:09 Alexey Dobriyan
2007-02-02  7:31 ` Duncan Sands [this message]
2007-02-05 11:39   ` Alexey Dobriyan
2007-02-05 12:05     ` Duncan Sands
  -- strict thread matches above, loose matches on Subject: below --
2007-01-31 10:54 Duncan Sands
2007-01-31 18:42 ` Alexey Dobriyan
2007-01-31 19:26   ` Duncan Sands
2007-02-01 10:15     ` Duncan Sands

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: remove_proc_entry and read_proc' \

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