| Message ID | 1445020843-9382-4-git-send-email-jarkko.sakkinen@linux.intel.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
Am Freitag, 16. Oktober 2015, 20:40:22 schrieb Jarkko Sakkinen: > 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> @Greg: Can I get an acked-by or something from you as the sysfs maintainer? Thanks, Peter ------------------------------------------------------------------------------
On Sun, Oct 18, 2015 at 03:37:16AM +0200, Peter Hüwe wrote: > Am Freitag, 16. Oktober 2015, 20:40:22 schrieb Jarkko Sakkinen: > > 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> > @Greg: Can I get an acked-by or something from you as the sysfs maintainer? I think everyone was otherwise OK with this with only one contradiction as I read previous dicussions about this patch: - This name was propsed by Tejun Heo - Greg doubted whether it is a good name. We can fixup to another name if someone says what it would be. > Thanks, > Peter /Jarkko ------------------------------------------------------------------------------
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; > + } > + > + link = kernfs_create_link(kobj->sd, target_name, entry); > + if (IS_ERR(link) && PTR_ERR(link) == -EEXIST) > + sysfs_warn_dup(kobj->sd, target_name); > + > + kernfs_put(entry); > + kernfs_put(target); > + return IS_ERR(link) ? PTR_ERR(link) : 0; > +} > +EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj); > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h > index 9f65758..ea090ea 100644 > --- a/include/linux/sysfs.h > +++ b/include/linux/sysfs.h > @@ -268,6 +268,9 @@ int sysfs_add_link_to_group(struct kobject *kobj, const char *group_name, > struct kobject *target, const char *link_name); > void sysfs_remove_link_from_group(struct kobject *kobj, const char *group_name, > const char *link_name); > +int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj, > + struct kobject *target_kobj, > + const char *target_name); > > void sysfs_notify(struct kobject *kobj, const char *dir, const char *attr); > > @@ -451,6 +454,14 @@ static inline void sysfs_remove_link_from_group(struct kobject *kobj, > { > } > > +static inline int __compat_only_sysfs_link_entry_to_kobj( > + struct kobject *kobj, > + struct kobject *target_kobj, > + const char *target_name) > +{ > + return 0; > +} > + > static inline void sysfs_notify(struct kobject *kobj, const char *dir, > const char *attr) > { > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ One problem with this patch is that it adds a bunch of code but none of it is used. So if there is a bug, git bisect will point to a later patch instead of this one.
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. [...]
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! > -- > - Jeremiah Mahler /Jarkko ------------------------------------------------------------------------------
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 ------------------------------------------------------------------------------
Jarkko, On Sat, Nov 07, 2015 at 01:41:37PM +0200, Jarkko Sakkinen wrote: > 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, > > > [...] > > > > + 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 > This fix does work since it effectively avoids the call to __compat_only_sysfs_link_entry_to_kobj(). Have you tested cases where __compat_only_sysfs_link_entry_to_kobj() is actually used? [...]
On Sat, Nov 07, 2015 at 10:08:56AM -0800, Jeremiah Mahler wrote: > Jarkko, > > On Sat, Nov 07, 2015 at 01:41:37PM +0200, Jarkko Sakkinen wrote: > > 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, > > > > > [...] > > > > > + 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 > > > This fix does work since it effectively avoids the call to > __compat_only_sysfs_link_entry_to_kobj(). > > Have you tested cases where __compat_only_sysfs_link_entry_to_kobj() > is actually used? Yes, of course I have. And I checked your DSDT and my assumption was correct. There was no DSM in the ACPI object. However, there is probably another regression but it is caused by some patch that was added earlier. I strongly believe it is not caused by any of my 4.4 patches. I think what was happening with you was that __compat_only_sysfs_link_entry_to_kobj() was returning -ENOENT, which it should do when target is not found. This was propagated to tpm_tis and it probably messes up clean up somehow. I have to test my hypothesis as soon as possible. The fix that I pushed is still valid no matter which way the things are. > [...] > > -- > - Jeremiah Mahler /Jarkko ------------------------------------------------------------------------------
Jarkko, On Sun, Nov 08, 2015 at 12:31:09AM +0200, Jarkko Sakkinen wrote: > On Sat, Nov 07, 2015 at 10:08:56AM -0800, Jeremiah Mahler wrote: > > Jarkko, > > > > On Sat, Nov 07, 2015 at 01:41:37PM +0200, Jarkko Sakkinen wrote: > > > 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, > > > > > [...] > > > I pushed a fix over here: > > > > > > https://github.com/jsakkine/linux-tpmdd/tree/fixes > > > > > This fix does work since it effectively avoids the call to > > __compat_only_sysfs_link_entry_to_kobj(). > > > > Have you tested cases where __compat_only_sysfs_link_entry_to_kobj() > > is actually used? > > Yes, of course I have. And I checked your DSDT and my assumption was > correct. There was no DSM in the ACPI object. > > However, there is probably another regression but it is caused by some > patch that was added earlier. I strongly believe it is not caused by any > of my 4.4 patches. > > I think what was happening with you was that > __compat_only_sysfs_link_entry_to_kobj() was returning -ENOENT, which it > should do when target is not found. This was propagated to tpm_tis and > it probably messes up clean up somehow. > > I have to test my hypothesis as soon as possible. The fix that I pushed > is still valid no matter which way the things are. > > > [...] > > > > -- > > - Jeremiah Mahler > > /Jarkko It sounds like you have the problem figured out and have a good fix. If I can do anything else to help let me know :-)
On Sat, Nov 07, 2015 at 03:11:47PM -0800, Jeremiah Mahler wrote: > Jarkko, > > On Sun, Nov 08, 2015 at 12:31:09AM +0200, Jarkko Sakkinen wrote: > > On Sat, Nov 07, 2015 at 10:08:56AM -0800, Jeremiah Mahler wrote: > > > Jarkko, > > > > > > On Sat, Nov 07, 2015 at 01:41:37PM +0200, Jarkko Sakkinen wrote: > > > > 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, > > > > > > > [...] > > > > I pushed a fix over here: > > > > > > > > https://github.com/jsakkine/linux-tpmdd/tree/fixes > > > > > > > This fix does work since it effectively avoids the call to > > > __compat_only_sysfs_link_entry_to_kobj(). > > > > > > Have you tested cases where __compat_only_sysfs_link_entry_to_kobj() > > > is actually used? > > > > Yes, of course I have. And I checked your DSDT and my assumption was > > correct. There was no DSM in the ACPI object. > > > > However, there is probably another regression but it is caused by some > > patch that was added earlier. I strongly believe it is not caused by any > > of my 4.4 patches. > > > > I think what was happening with you was that > > __compat_only_sysfs_link_entry_to_kobj() was returning -ENOENT, which it > > should do when target is not found. This was propagated to tpm_tis and > > it probably messes up clean up somehow. > > > > I have to test my hypothesis as soon as possible. The fix that I pushed > > is still valid no matter which way the things are. > > > > > [...] > > > > > > -- > > > - Jeremiah Mahler > > > > /Jarkko > > It sounds like you have the problem figured out and have a good fix. > If I can do anything else to help let me know :-) Turns out that after all the second issue that I described was also because of this patch. Clean up was not done properly when that function. I revised my fix. If you want to help, check that the revised patch works I'll add Tested-by to the patch. Thank you! > -- > - Jeremiah Mahler /Jarkko ------------------------------------------------------------------------------
Jarkko, On Sun, Nov 08, 2015 at 02:49:06AM +0200, Jarkko Sakkinen wrote: > On Sat, Nov 07, 2015 at 03:11:47PM -0800, Jeremiah Mahler wrote: > > Jarkko, > > [...] > > > > It sounds like you have the problem figured out and have a good fix. > > If I can do anything else to help let me know :-) > > Turns out that after all the second issue that I described was also > because of this patch. > > Clean up was not done properly when that function. I revised my fix. > > If you want to help, check that the revised patch works I'll add > Tested-by to the patch. Thank you! > > > -- > > - Jeremiah Mahler > > /Jarkko I tested the patch (link below) and it works with one small caveat. The patch would not apply because line 231 uses list_add_tail_rcu instead of list_add_rcu. I am working from commit 9b774d5cf2d where the problem started. Your repo likely has other changes involved. https://github.com/jsakkine/linux-tpmdd/commit/73ea7e0b8045f9610c3274bcefaf89b7a05ee781
On Sat, Nov 07, 2015 at 07:04:04PM -0800, Jeremiah Mahler wrote: > Jarkko, > > On Sun, Nov 08, 2015 at 02:49:06AM +0200, Jarkko Sakkinen wrote: > > On Sat, Nov 07, 2015 at 03:11:47PM -0800, Jeremiah Mahler wrote: > > > Jarkko, > > > > [...] > > > > > > It sounds like you have the problem figured out and have a good fix. > > > If I can do anything else to help let me know :-) > > > > Turns out that after all the second issue that I described was also > > because of this patch. > > > > Clean up was not done properly when that function. I revised my fix. > > > > If you want to help, check that the revised patch works I'll add > > Tested-by to the patch. Thank you! > > > > > -- > > > - Jeremiah Mahler > > > > /Jarkko > > I tested the patch (link below) and it works with one small caveat. > The patch would not apply because line 231 uses list_add_tail_rcu > instead of list_add_rcu. I am working from commit 9b774d5cf2d where > the problem started. Your repo likely has other changes involved. > > https://github.com/jsakkine/linux-tpmdd/commit/73ea7e0b8045f9610c3274bcefaf89b7a05ee781 Yes, that branch also fix that changes one list_add_rcu() to list_add_tail_rcu(). I'll add Tested-by to the commit message. Thanks again for the good work. > -- > - Jeremiah Mahler /Jarkko ------------------------------------------------------------------------------
On Fri, Nov 06, 2015 at 04:23:56PM -0800, Jeremiah Mahler wrote: > 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. > One problem with this patch is that it adds a bunch of code but none of > it is used. So if there is a bug, git bisect will point to a later > patch instead of this one. It is fairly standard practice to split cross-subsystem patches like this, it aids review. git bisect is not a perfect tool. Jason ------------------------------------------------------------------------------ Presto, an open source distributed SQL query engine for big data, initially developed by Facebook, enables you to easily query your data on Hadoop in a more interactive manner. Teradata is also now providing full enterprise support for Presto. Download a free open source copy now. http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140
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; + } + + link = kernfs_create_link(kobj->sd, target_name, entry); + if (IS_ERR(link) && PTR_ERR(link) == -EEXIST) + sysfs_warn_dup(kobj->sd, target_name); + + kernfs_put(entry); + kernfs_put(target); + return IS_ERR(link) ? PTR_ERR(link) : 0; +} +EXPORT_SYMBOL_GPL(__compat_only_sysfs_link_entry_to_kobj); diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 9f65758..ea090ea 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -268,6 +268,9 @@ int sysfs_add_link_to_group(struct kobject *kobj, const char *group_name, struct kobject *target, const char *link_name); void sysfs_remove_link_from_group(struct kobject *kobj, const char *group_name, const char *link_name); +int __compat_only_sysfs_link_entry_to_kobj(struct kobject *kobj, + struct kobject *target_kobj, + const char *target_name); void sysfs_notify(struct kobject *kobj, const char *dir, const char *attr); @@ -451,6 +454,14 @@ static inline void sysfs_remove_link_from_group(struct kobject *kobj, { } +static inline int __compat_only_sysfs_link_entry_to_kobj( + struct kobject *kobj, + struct kobject *target_kobj, + const char *target_name) +{ + return 0; +} + static inline void sysfs_notify(struct kobject *kobj, const char *dir, const char *attr) {
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(+)