All of lore.kernel.org
 help / color / mirror / Atom feed
From: "majianpeng" <majianpeng@gmail.com>
To: NeilBrown <neilb@suse.de>
Cc: linux-raid <linux-raid@vger.kernel.org>
Subject: Re: Re: md:When opened /proc/mdstat, increase the refcount of
Date: Mon, 27 Feb 2012 09:21:50 +0800	[thread overview]
Message-ID: <201202270921472184397@gmail.com> (raw)
In-Reply-To: 20120226220120.3dbe3a3b@notabene.brown

my patch is:
---
 fs/proc/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 84fd323..78dbb03 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -317,6 +317,7 @@ static int proc_reg_open(struct inode *inode, struct file *file)
 	int (*open)(struct inode *, struct file *);
 	int (*release)(struct inode *, struct file *);
 	struct pde_opener *pdeo;
+	const struct file_operations *fops;
 
 	/*
 	 * What for, you ask? Well, we can have open, rmmod, remove_proc_entry
@@ -339,7 +340,12 @@ static int proc_reg_open(struct inode *inode, struct file *file)
 		return -ENOENT;
 	}
 	pde->pde_users++;
-	open = pde->proc_fops->open;
+	/*
+	 *If proc_fops defined in module and used .owner = THIS_MODULE,so open must
+	 *get a reference of module.
+	 */
+	fops = fops_get(pde->proc_fops);
+	open = fops->open;
 	release = pde->proc_fops->release;
 	spin_unlock(&pde->pde_unload_lock);
 
@@ -354,8 +360,10 @@ static int proc_reg_open(struct inode *inode, struct file *file)
 		/* Strictly for "too late" ->release in proc_reg_release(). */
 		pdeo->release = release;
 		list_add(&pdeo->lh, &pde->pde_openers);
-	} else
+	} else	{
+		fops_put(pde->proc_fops);
 		kfree(pdeo);
+	}
 	__pde_users_dec(pde);
 	spin_unlock(&pde->pde_unload_lock);
 	return rv;
@@ -402,6 +410,7 @@ static int proc_reg_release(struct inode *inode, struct file *file)
 	}
 	pde->pde_users++;
 	release = pde->proc_fops->release;
+	fops_put(pde->proc_fops);
 	if (pdeo) {
 		list_del(&pdeo->lh);
 		kfree(pdeo);

I tested ok,but Al Viro said deadlock, I did not tested.

------------------				 
majianpeng
2012-02-27

-------------------------------------------------------------
发件人:NeilBrown
发送日期:2012-02-26 19:01:32
收件人:majianpeng
抄送:linux-raid
主题:Re: md:When opened /proc/mdstat, increase the refcount of

On Sun, 26 Feb 2012 16:33:25 +0800 "majianpeng" <majianpeng@gmail.com> wrote:

> I wrote a letter to maintainer of procfs. He's answer is follow:
> >No.  Read remove_proc_entry(), especially the part under ->pde_unload_lock.
> >The whole damn point of that stuff is that opened file on procfs does *not*
> >pin the module down; IO in progress does, but that's it.
> >You can't deadlock rmmod foobar </proc/crap/foobar; with your patch we'll
> >be back to a pile of deadlocks in there.
> >IOW, NAK.

Hmmm... that sounds like Al Viro.

I see the point of that code now.  And your patch would be open to the same
problem wouldn't it. i.e.
    rmmod md_mod < /proc/mdstat

would deadlock??

So we still need to find a fix that actually works correctly.

That probably means moving the call to 'poll_wait' into procfs code, and using
a wait_queue_head which is managed by procfs.

So maybe:
 - put a wait_queue_head in 'struct proc_dir_entry'
 - have proc_reg_poll call poll_wait passing that wait_queue_head
 - write a function e.g. proc_poll_wake which calls wake_up on that
   wait_queue_head
 - have md.c save the return value from proc_create, and call the above
   function instead of calling "wake_up(&md_event_waiters)"

That should work.

NeilBrown

      reply	other threads:[~2012-02-27  1:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-26  4:55 md:When opened /proc/mdstat, increase the refcount of majianpeng
2012-02-26  5:26 ` NeilBrown
2012-02-26  5:39   ` majianpeng
2012-02-26  8:36     ` NeilBrown
2012-02-26  8:33 ` majianpeng
2012-02-26 11:01   ` NeilBrown
2012-02-27  1:21     ` majianpeng [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=201202270921472184397@gmail.com \
    --to=majianpeng@gmail.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    /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.