All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Tejun Heo <tj@kernel.org>
Cc: Yijing Wang <wangyijing@huawei.com>,
	lizefan@huawei.com, linux-kernel@vger.kernel.org,
	Weng Meiling <wengmeiling.weng@huawei.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] sysfs: driver core: Fix glue dir race condition
Date: Thu, 6 Nov 2014 09:22:46 -0800	[thread overview]
Message-ID: <20141106172246.GA20192@kroah.com> (raw)
In-Reply-To: <20141106165547.GG25642@htj.dyndns.org>

On Thu, Nov 06, 2014 at 11:55:47AM -0500, Tejun Heo wrote:
> Maybe "fix glue dir race condition by not removing them" is a better
> title?
> 
> On Thu, Nov 06, 2014 at 04:16:38PM +0800, Yijing Wang wrote:
> > There is a race condition when removing glue directory.
> > It can be reproduced in following test:
> > 
> > path 1: Add first child device
> > device_add()
> > 	get_device_parent()
> > 		/*find parent from glue_dirs.list*/
> > 		list_for_each_entry(k, &dev->class->p->glue_dirs.list, entry)
> > 			if (k->parent == parent_kobj) {
> > 				kobj = kobject_get(k);
> > 				break;
> > 			}
> > 		....
> > 		class_dir_create_and_add()
> > 
> > path2: Remove last child device under glue dir
> > device_del()
> > 	cleanup_device_parent()
> > 		cleanup_glue_dir()
> > 			kobject_put(glue_dir);
> > 
> > If path2 has been called cleanup_glue_dir(), but not
> > call kobject_put(glue_dir), the glue dir is still
> > in parent's kset list. Meanwhile, path1 find the glue
> > dir from the glue_dirs.list. Path2 may release glue dir
> > before path1 call kobject_get(). So kernel will report
> > the warning and bug_on.
> > 
> > This fix keep glue dir around once it created suggested
> > by Tejun Heo.
> 
> I think you prolly want to explain why this is okay / desired.
> e.g. list how the glue dir is used and how many of them are there and
> explain that there's no real benefit in removing them.

I'd really _like_ to remove them if at all possible, as if there isn't
any "children" in the subdirectory, there shouldn't be a need for that
directory to be there.

This seems to be the "classic" problem we have of a kref in a list that
can be found while the last instance could be removed at the same time.
I hate to just throw another lock at the problem, but wouldn't a lock to
protect the list of glue_dirs be the answer here?

thanks,

greg k-h

  reply	other threads:[~2014-11-06 17:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-06  8:16 [PATCH] sysfs: driver core: Fix glue dir race condition Yijing Wang
2014-11-06 16:55 ` Tejun Heo
2014-11-06 17:22   ` Greg KH [this message]
2014-11-07  1:44     ` Yijing Wang
2014-11-07  2:46       ` Greg KH
2014-11-07  3:12         ` Yijing Wang
2014-11-07  5:51           ` Greg KH
2014-11-07  1:22   ` Yijing Wang

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=20141106172246.GA20192@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=stable@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=wangyijing@huawei.com \
    --cc=wengmeiling.weng@huawei.com \
    /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.