linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: Tejun Heo <tj@kernel.org>,
	rafael@kernel.org, davem@davemloft.net, kuba@kernel.org,
	ast@kernel.org, andriin@fb.com, daniel@iogearbox.net,
	atenart@kernel.org, alobakin@pm.me, weiwan@google.com,
	ap420073@gmail.com, jeyu@kernel.org, ngupta@vflare.org,
	sergey.senozhatsky.work@gmail.com, minchan@kernel.org,
	axboe@kernel.dk, mbenes@suse.com, jpoimboe@redhat.com,
	tglx@linutronix.de, keescook@chromium.org, jikos@kernel.org,
	rostedt@goodmis.org, peterz@infradead.org,
	linux-block@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] sysfs: fix kobject refcount to address races with kobject removal
Date: Fri, 23 Jul 2021 13:14:10 +0200	[thread overview]
Message-ID: <YPqkgqxXQI1qYaxv@kroah.com> (raw)
In-Reply-To: <20210722213137.jegpykf2ddwmmck5@garbanzo>

On Thu, Jul 22, 2021 at 02:31:37PM -0700, Luis Chamberlain wrote:
> On Wed, Jul 21, 2021 at 01:30:29PM +0200, Greg KH wrote:
> > On Thu, Jul 01, 2021 at 03:48:16PM -0700, Luis Chamberlain wrote:
> > > On Fri, Jun 25, 2021 at 02:56:03PM -0700, Luis Chamberlain wrote:
> > > > On Thu, Jun 24, 2021 at 01:09:03PM +0200, Greg KH wrote:
> > > > > thanks for making this change and sticking with it!
> > > > > 
> > > > > Oh, and with this change, does your modprobe/rmmod crazy test now work?
> > > > 
> > > > It does but I wrote a test_syfs driver and I believe I see an issue with
> > > > this. I'll debug a bit more and see what it was, and I'll then also use
> > > > the driver to demo the issue more clearly, and then verification can be
> > > > an easy selftest test.
> > > 
> > > OK my conclusion based on a new selftest driver I wrote is we can drop
> > > this patch safely. The selftest will cover this corner case well now.
> > > 
> > > In short: the kernfs active reference will ensure the store operation
> > > still exists. The kernfs mutex is not enough, but if the driver removes
> > > the operation prior to getting the active reference, the write will just
> > > fail. The deferencing inside of the sysfs operation is abstract to
> > > kernfs, and while kernfs can't do anything to prevent a driver from
> > > doing something stupid, it at least can ensure an open file ensure the
> > > op is not removed until the operation completes.
> > 
> > Ok, so all is good?
> 
> It would seem to be the case.
> 
> > Then why is your zram test code blowing up so badly?
> 
> I checked the logs for the backtrace where the crash did happen
> and we did see clear evidence of the race we feared here. The *first*
> bug that happened was the CPU hotplug race:
> 
> [132004.787099] Error: Removing state 61 which has instances left.
> [132004.787124] WARNING: CPU: 17 PID: 9307 at ../kernel/cpu.c:1879 __cpuhp_remove_state_cpuslocked+0x1c4/0x1d0

I do not understand what this issue is, is it fixed?  Why is a cpu being
hot unplugged at the same time a zram?

thanks,

greg k-h

  reply	other threads:[~2021-07-23 11:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23 21:50 [PATCH v4] sysfs: fix kobject refcount to address races with kobject removal Luis Chamberlain
2021-06-23 22:59 ` Kees Cook
2021-06-24  1:09   ` Luis Chamberlain
2021-06-24 11:06   ` Greg KH
2021-06-24 11:09 ` Greg KH
2021-06-25 21:55   ` Luis Chamberlain
2021-07-01 22:48     ` Luis Chamberlain
2021-07-02  1:04       ` Luis Chamberlain
2021-07-21 11:30       ` Greg KH
2021-07-22 21:31         ` Luis Chamberlain
2021-07-23 11:14           ` Greg KH [this message]
2021-07-23 17:35             ` Luis Chamberlain
2021-07-01  2:27 ` [sysfs] 1c04296f8f: suspend-stress.fail kernel test robot
2021-07-01 15:59   ` Luis Chamberlain

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=YPqkgqxXQI1qYaxv@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alobakin@pm.me \
    --cc=andriin@fb.com \
    --cc=ap420073@gmail.com \
    --cc=ast@kernel.org \
    --cc=atenart@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jeyu@kernel.org \
    --cc=jikos@kernel.org \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbenes@suse.com \
    --cc=mcgrof@kernel.org \
    --cc=minchan@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ngupta@vflare.org \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=weiwan@google.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 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).