Re: [BUG, PATCH 03/10] sysfs: added __compat_only_sysfs_link_entry_to_kobj()

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Sat, Nov 07, 2015 at 12:55:43PM +0200, Jarkko Sakkinen wrote:
> On Fri, Nov 06, 2015 at 06:55:18PM -0800, Jeremiah Mahler wrote:
> > Jarkko,
> > 
> > On Fri, Oct 16, 2015 at 09:40:22PM +0300, Jarkko Sakkinen wrote:
> > > Added a new function __compat_only_sysfs_link_group_to_kobj() that adds
> > > a symlink from attribute or group to a kobject. This needed for
> > > maintaining backwards compatibility with PPI attributes in the TPM
> > > driver.
> > > 
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > ---
> > >  fs/sysfs/group.c      | 44 ++++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/sysfs.h | 11 +++++++++++
> > >  2 files changed, 55 insertions(+)
> > > 
> > > diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> > > index 39a0199..e123659 100644
> > > --- a/fs/sysfs/group.c
> > > +++ b/fs/sysfs/group.c
> > > @@ -352,3 +352,47 @@ void sysfs_remove_link_from_group(struct kobject *kobj, const char *group_name,
> > >  	}
> > >  }
> > >  EXPORT_SYMBOL_GPL(sysfs_remove_link_from_group);
> > > +
> > > +/**
> > > + * __compat_only_sysfs_link_entry_to_kobj - add a symlink to a kobject pointing
> > > + * to a group or an attribute
> > > + * @kobj:		The kobject containing the group.
> > > + * @target_kobj:	The target kobject.
> > > + * @target_name:	The name of the target group or attribute.
> > > + */
> > > +int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj,
> > > +				      struct kobject *target_kobj,
> > > +				      const char *target_name)
> > > +{
> > > +	struct kernfs_node *target;
> > > +	struct kernfs_node *entry;
> > > +	struct kernfs_node *link;
> > > +
> > > +	/*
> > > +	 * We don't own @target_kobj and it may be removed at any time.
> > > +	 * Synchronize using sysfs_symlink_target_lock. See sysfs_remove_dir()
> > > +	 * for details.
> > > +	 */
> > > +	spin_lock(&sysfs_symlink_target_lock);
> > > +	target = target_kobj->sd;
> > > +	if (target)
> > > +		kernfs_get(target);
> > > +	spin_unlock(&sysfs_symlink_target_lock);
> > > +	if (!target)
> > > +		return -ENOENT;
> > > +
> > > +	entry = kernfs_find_and_get(target_kobj->sd, target_name);
> > > +	if (!entry) {
> > > +		kernfs_put(target);
> > > +		return -ENOENT;
> > > +	}
> > > +
> > 
> > On an Acer C720 this call to kernfs_find_and_get fails resulting in
> > a failed resume after suspend.
> > 
> > Apparently it can't find an object for the name "ppi".
> > 
> > This bug does not appear until the next patch is applied which
> > calls __compat_only_sysfs_link_entry_to_kobj.
> 
> I think I might have found something thanks to your help
> 
> First I found this old bug:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1096511
> 
> What is happening is that DSM is not found and therefore tpm_add_ppi()
> does not add ppi to sysfs groups array.
> 
> __compat_only_sysfs_link_entry_to_kobj() is called after tpm_add_ppi
> unconditionally for TPM1.
> 
> I'll implement a fix for this ASAP.
> 
> Thank you for great effort on finding tis!

I pushed a fix over here:

https://github.com/jsakkine/linux-tpmdd/tree/fixes

I'm at the moment in process testing it. Can you run 'acpidump' on your
laptop and send the output me in private? I'll just verify that my
deduction for the root cause is correct.

> > -- 
> > - Jeremiah Mahler

/Jarkko