Re: [PATCH 4/7] Implement get_pts_ns() and put_pts_ns()

[sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org]

Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| Quoting sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org (sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
| > 
| > From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
| > Subject: [PATCH 4/7]: Implement get_pts_ns() and put_pts_ns()
| > 
| > Implement get_pts_ns() and put_pts_ns() interfaces.
| > 
| > Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
| > ---
| >  include/linux/devpts_fs.h |   21 ++++++++++++++++++++-
| >  1 file changed, 20 insertions(+), 1 deletion(-)
| > 
| > Index: 2.6.25-rc5-mm1/include/linux/devpts_fs.h
| > ===================================================================
| > --- 2.6.25-rc5-mm1.orig/include/linux/devpts_fs.h	2008-03-24 20:04:31.000000000 -0700
| > +++ 2.6.25-rc5-mm1/include/linux/devpts_fs.h	2008-03-24 20:05:05.000000000 -0700
| > @@ -27,13 +27,26 @@ struct pts_namespace {
| >  extern struct pts_namespace init_pts_ns;
| > 
| >  #ifdef CONFIG_UNIX98_PTYS
| > -
| >  int devpts_new_index(void);
| >  void devpts_kill_index(int idx);
| >  int devpts_pty_new(struct tty_struct *tty);      /* mknod in devpts */
| >  struct tty_struct *devpts_get_tty(int number);	 /* get tty structure */
| >  void devpts_pty_kill(int number);		 /* unlink */
| > 
| > +static inline void free_pts_ns(struct kref *ns_kref) { }
| > +
| > +static inline struct pts_namespace *get_pts_ns(struct pts_namespace *ns)
| > +{
| > +	if (ns && (ns != &init_pts_ns))
| > +		kref_get(&ns->kref);
| > +	return ns;
| > +}
| > +static inline void put_pts_ns(struct pts_namespace *ns)
| > +{
| > +	if (ns && (ns != &init_pts_ns))
| > +		kref_put(&ns->kref, free_pts_ns);
| 
| This isn't right, or I'm not thinking right.  Don't you somewhere
| need to
| 
| 	1. rcu_assign ns->mnt->mnt_sb->s_fs_info to NULL
| 	2. wait a grace period
| 	3. call free_pts_ns and check the refcount on the ns again?
| 
| and then do pts_ns_from_inode() under an rcu_read_lock and grab
| a ref to the ns?

Yes, we need the rcu to grab the reference to pts_ns.