Re: [PATCH v8 11/12] zram: fix crashes with cpu hotplug multistate

Ming Lei <[email protected]>
Newsgroups org.kernel.vger.linux-spdx,org.kernel.vger.linux-block,org.kernel.vger.linux-doc,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.live-patching
Message-ID <YYFRjpJcIaZ1AQRS@T590>
On Tue, Nov 02, 2021 at 03:15:15PM +0100, Petr Mladek wrote:
> On Tue 2021-10-26 23:37:30, Ming Lei wrote:
> > On Tue, Oct 26, 2021 at 10:48:18AM +0200, Petr Mladek wrote:
> > > Below are more details about the livepatch code. I hope that it will
> > > help you to see if zram has similar problems or not.
> > > 
> > > We have kobject in three structures: klp_func, klp_object, and
> > > klp_patch, see include/linux/livepatch.h.
> > > 
> > > These structures have to be statically defined in the module sources
> > > because they define what is livepatched, see
> > > samples/livepatch/livepatch-sample.c
> > > 
> > > The kobject is used there to show information about the patch, patched
> > > objects, and patched functions, in sysfs. And most importantly,
> > > the sysfs interface can be used to disable the livepatch.
> > > 
> > > The problem with static structures is that the module must stay
> > > in the memory as long as the sysfs interface exists. It can be
> > > solved in module_exit() callback. It could wait until the sysfs
> > > interface is destroyed.
> > > 
> > > kobject API does not support this scenario. The relase() callbacks
> > 
> > kobject_delete() is for supporting this scenario, that is why we don't
> > need to grab module refcnt before calling show()/store() of the
> > kobject's attributes.
> > 
> > kobject_delete() can be called in module_exit(), then any show()/store()
> > will be done after kobject_delete() returns.
> 
> I am a bit confused. I do not see kobject_delete() anywhere in kernel
> sources.
> 
> I see only kobject_del() and kobject_put(). AFAIK, they do _not_
> guarantee that either the sysfs interface was destroyed or
> the release callbacks were called. For example, see
> schedule_delayed_work(&kobj->release, delay) in kobject_release().

After kobject_del() returns, no one can call run into show()/store(),
and all pending show()/store() are drained meantime. But yes, the release
handler may still be called later, and the kobject has to be freed
during or before module_exit().

https://lore.kernel.org/lkml/[email protected]/

> 
> By other words, anyone could still be using either the sysfs interface
> or the related structures after kobject_del() or kobject_put()
> returns.

No, no one can do that after kobject_del() returns.

> 
> IMHO, kobject API does not support static structures and module
> removal.

But so far klp_patch can only be defined as static instance, and it
depends on the implementation, especially the release handler.


Thanks,
Ming
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.