Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

sysfs: make attr namespace interface less convoluted

sysfs ns (namespace) implementation became more convoluted than
necessary while trying to hide ns information from visible interface.
The relatively recent attr ns support is a good example.

* attr ns tag is determined by sysfs_ops->namespace() callback while
dir tag is determined by kobj_type->namespace(). The placement is
arbitrary.

* Instead of performing operations with explicit ns tag, the namespace
callback is routed through sysfs_attr_ns(), sysfs_ops->namespace(),
class_attr_namespace(), class_attr->namespace(). It's not simpler
in any sense. The only thing this convolution does is traversing
the whole stack backwards.

The namespace callbacks are unncessary because the operations involved
are inherently synchronous. The information can be provided in in
straight-forward top-down direction and reversing that direction is
unnecessary and against basic design principles.

This backward interface is unnecessarily convoluted and hinders
properly separating out sysfs from driver model / kobject for proper
layering. This patch updates attr ns support such that

* sysfs_ops->namespace() and class_attr->namespace() are dropped.

* sysfs_{create|remove}_file_ns(), which take explicit @ns param, are
added and sysfs_{create|remove}_file() are now simple wrappers
around the ns aware functions.

* ns handling is dropped from sysfs_chmod_file(). Nobody uses it at
this point. sysfs_chmod_file_ns() can be added later if necessary.

* Explicit @ns is propagated through class_{create|remove}_file_ns()
and netdev_class_{create|remove}_file_ns().

* driver/net/bonding which is currently the only user of attr
namespace is updated to use netdev_class_{create|remove}_file_ns()
with @bh->net as the ns tag instead of using the namespace callback.

This patch should be an equivalent conversion without any functional
difference. It makes the code easier to follow, reduces lines of code
a bit and helps proper separation and layering.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Kay Sievers <kay@vrfy.org>
Acked-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Tejun Heo and committed by
Greg Kroah-Hartman
58292cbe bcac3769

+106 -129
+9 -20
drivers/base/class.c
··· 47 47 return ret; 48 48 } 49 49 50 - static const void *class_attr_namespace(struct kobject *kobj, 51 - const struct attribute *attr) 52 - { 53 - struct class_attribute *class_attr = to_class_attr(attr); 54 - struct subsys_private *cp = to_subsys_private(kobj); 55 - const void *ns = NULL; 56 - 57 - if (class_attr->namespace) 58 - ns = class_attr->namespace(cp->class, class_attr); 59 - return ns; 60 - } 61 - 62 50 static void class_release(struct kobject *kobj) 63 51 { 64 52 struct subsys_private *cp = to_subsys_private(kobj); ··· 74 86 static const struct sysfs_ops class_sysfs_ops = { 75 87 .show = class_attr_show, 76 88 .store = class_attr_store, 77 - .namespace = class_attr_namespace, 78 89 }; 79 90 80 91 static struct kobj_type class_ktype = { ··· 86 99 static struct kset *class_kset; 87 100 88 101 89 - int class_create_file(struct class *cls, const struct class_attribute *attr) 102 + int class_create_file_ns(struct class *cls, const struct class_attribute *attr, 103 + const void *ns) 90 104 { 91 105 int error; 92 106 if (cls) 93 - error = sysfs_create_file(&cls->p->subsys.kobj, 94 - &attr->attr); 107 + error = sysfs_create_file_ns(&cls->p->subsys.kobj, 108 + &attr->attr, ns); 95 109 else 96 110 error = -EINVAL; 97 111 return error; 98 112 } 99 113 100 - void class_remove_file(struct class *cls, const struct class_attribute *attr) 114 + void class_remove_file_ns(struct class *cls, const struct class_attribute *attr, 115 + const void *ns) 101 116 { 102 117 if (cls) 103 - sysfs_remove_file(&cls->p->subsys.kobj, &attr->attr); 118 + sysfs_remove_file_ns(&cls->p->subsys.kobj, &attr->attr, ns); 104 119 } 105 120 106 121 static struct class *class_get(struct class *cls) ··· 589 600 return 0; 590 601 } 591 602 592 - EXPORT_SYMBOL_GPL(class_create_file); 593 - EXPORT_SYMBOL_GPL(class_remove_file); 603 + EXPORT_SYMBOL_GPL(class_create_file_ns); 604 + EXPORT_SYMBOL_GPL(class_remove_file_ns); 594 605 EXPORT_SYMBOL_GPL(class_unregister); 595 606 EXPORT_SYMBOL_GPL(class_destroy); 596 607
+3 -11
drivers/net/bonding/bond_sysfs.c
··· 149 149 return -EPERM; 150 150 } 151 151 152 - static const void *bonding_namespace(struct class *cls, 153 - const struct class_attribute *attr) 154 - { 155 - const struct bond_net *bn = 156 - container_of(attr, struct bond_net, class_attr_bonding_masters); 157 - return bn->net; 158 - } 159 - 160 152 /* class attribute for bond_masters file. This ends up in /sys/class/net */ 161 153 static const struct class_attribute class_attr_bonding_masters = { 162 154 .attr = { ··· 157 165 }, 158 166 .show = bonding_show_bonds, 159 167 .store = bonding_store_bonds, 160 - .namespace = bonding_namespace, 161 168 }; 162 169 163 170 int bond_create_slave_symlinks(struct net_device *master, ··· 1778 1787 bn->class_attr_bonding_masters = class_attr_bonding_masters; 1779 1788 sysfs_attr_init(&bn->class_attr_bonding_masters.attr); 1780 1789 1781 - ret = netdev_class_create_file(&bn->class_attr_bonding_masters); 1790 + ret = netdev_class_create_file_ns(&bn->class_attr_bonding_masters, 1791 + bn->net); 1782 1792 /* 1783 1793 * Permit multiple loads of the module by ignoring failures to 1784 1794 * create the bonding_masters sysfs file. Bonding devices ··· 1809 1817 */ 1810 1818 void bond_destroy_sysfs(struct bond_net *bn) 1811 1819 { 1812 - netdev_class_remove_file(&bn->class_attr_bonding_masters); 1820 + netdev_class_remove_file_ns(&bn->class_attr_bonding_masters, bn->net); 1813 1821 } 1814 1822 1815 1823 /*
+24 -71
fs/sysfs/file.c
··· 485 485 .poll = sysfs_poll, 486 486 }; 487 487 488 - static int sysfs_attr_ns(struct kobject *kobj, const struct attribute *attr, 489 - const void **pns) 490 - { 491 - struct sysfs_dirent *dir_sd = kobj->sd; 492 - const struct sysfs_ops *ops; 493 - const void *ns = NULL; 494 - int err; 495 - 496 - if (!dir_sd) { 497 - WARN(1, KERN_ERR "sysfs: kobject %s without dirent\n", 498 - kobject_name(kobj)); 499 - return -ENOENT; 500 - } 501 - 502 - err = 0; 503 - if (!sysfs_ns_type(dir_sd)) 504 - goto out; 505 - 506 - err = -EINVAL; 507 - if (!kobj->ktype) 508 - goto out; 509 - ops = kobj->ktype->sysfs_ops; 510 - if (!ops) 511 - goto out; 512 - if (!ops->namespace) 513 - goto out; 514 - 515 - err = 0; 516 - ns = ops->namespace(kobj, attr); 517 - out: 518 - if (err) { 519 - WARN(1, KERN_ERR 520 - "missing sysfs namespace attribute operation for kobject: %s\n", 521 - kobject_name(kobj)); 522 - } 523 - *pns = ns; 524 - return err; 525 - } 526 - 527 - int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, 528 - const struct attribute *attr, int type, umode_t amode) 488 + int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd, 489 + const struct attribute *attr, int type, 490 + umode_t amode, const void *ns) 529 491 { 530 492 umode_t mode = (amode & S_IALLUGO) | S_IFREG; 531 493 struct sysfs_addrm_cxt acxt; 532 494 struct sysfs_dirent *sd; 533 - const void *ns; 534 495 int rc; 535 - 536 - rc = sysfs_attr_ns(dir_sd->s_dir.kobj, attr, &ns); 537 - if (rc) 538 - return rc; 539 496 540 497 sd = sysfs_new_dirent(attr->name, mode, type); 541 498 if (!sd) ··· 516 559 int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr, 517 560 int type) 518 561 { 519 - return sysfs_add_file_mode(dir_sd, attr, type, attr->mode); 562 + return sysfs_add_file_mode_ns(dir_sd, attr, type, attr->mode, NULL); 520 563 } 521 564 522 - 523 565 /** 524 - * sysfs_create_file - create an attribute file for an object. 525 - * @kobj: object we're creating for. 526 - * @attr: attribute descriptor. 566 + * sysfs_create_file_ns - create an attribute file for an object with custom ns 567 + * @kobj: object we're creating for 568 + * @attr: attribute descriptor 569 + * @ns: namespace the new file should belong to 527 570 */ 528 - int sysfs_create_file(struct kobject *kobj, const struct attribute *attr) 571 + int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr, 572 + const void *ns) 529 573 { 530 574 BUG_ON(!kobj || !kobj->sd || !attr); 531 575 532 - return sysfs_add_file(kobj->sd, attr, SYSFS_KOBJ_ATTR); 576 + return sysfs_add_file_mode_ns(kobj->sd, attr, SYSFS_KOBJ_ATTR, 577 + attr->mode, ns); 533 578 534 579 } 535 - EXPORT_SYMBOL_GPL(sysfs_create_file); 580 + EXPORT_SYMBOL_GPL(sysfs_create_file_ns); 536 581 537 582 int sysfs_create_files(struct kobject *kobj, const struct attribute **ptr) 538 583 { ··· 589 630 { 590 631 struct sysfs_dirent *sd; 591 632 struct iattr newattrs; 592 - const void *ns; 593 633 int rc; 594 - 595 - rc = sysfs_attr_ns(kobj, attr, &ns); 596 - if (rc) 597 - return rc; 598 634 599 635 mutex_lock(&sysfs_mutex); 600 636 601 637 rc = -ENOENT; 602 - sd = sysfs_find_dirent(kobj->sd, ns, attr->name); 638 + sd = sysfs_find_dirent(kobj->sd, NULL, attr->name); 603 639 if (!sd) 604 640 goto out; 605 641 ··· 609 655 EXPORT_SYMBOL_GPL(sysfs_chmod_file); 610 656 611 657 /** 612 - * sysfs_remove_file - remove an object attribute. 613 - * @kobj: object we're acting for. 614 - * @attr: attribute descriptor. 658 + * sysfs_remove_file_ns - remove an object attribute with a custom ns tag 659 + * @kobj: object we're acting for 660 + * @attr: attribute descriptor 661 + * @ns: namespace tag of the file to remove 615 662 * 616 - * Hash the attribute name and kill the victim. 663 + * Hash the attribute name and namespace tag and kill the victim. 617 664 */ 618 - void sysfs_remove_file(struct kobject *kobj, const struct attribute *attr) 665 + void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr, 666 + const void *ns) 619 667 { 620 - const void *ns; 668 + struct sysfs_dirent *dir_sd = kobj->sd; 621 669 622 - if (sysfs_attr_ns(kobj, attr, &ns)) 623 - return; 624 - 625 - sysfs_hash_and_remove(kobj->sd, ns, attr->name); 670 + sysfs_hash_and_remove(dir_sd, ns, attr->name); 626 671 } 627 - EXPORT_SYMBOL_GPL(sysfs_remove_file); 672 + EXPORT_SYMBOL_GPL(sysfs_remove_file_ns); 628 673 629 674 void sysfs_remove_files(struct kobject *kobj, const struct attribute **ptr) 630 675 {
+4 -3
fs/sysfs/group.c
··· 56 56 if (!mode) 57 57 continue; 58 58 } 59 - error = sysfs_add_file_mode(dir_sd, *attr, 60 - SYSFS_KOBJ_ATTR, 61 - (*attr)->mode | mode); 59 + error = sysfs_add_file_mode_ns(dir_sd, *attr, 60 + SYSFS_KOBJ_ATTR, 61 + (*attr)->mode | mode, 62 + NULL); 62 63 if (unlikely(error)) 63 64 break; 64 65 }
+3 -2
fs/sysfs/sysfs.h
··· 230 230 int sysfs_add_file(struct sysfs_dirent *dir_sd, 231 231 const struct attribute *attr, int type); 232 232 233 - int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, 234 - const struct attribute *attr, int type, umode_t amode); 233 + int sysfs_add_file_mode_ns(struct sysfs_dirent *dir_sd, 234 + const struct attribute *attr, int type, 235 + umode_t amode, const void *ns); 235 236 /* 236 237 * bin.c 237 238 */
+18 -6
include/linux/device.h
··· 427 427 char *buf); 428 428 ssize_t (*store)(struct class *class, struct class_attribute *attr, 429 429 const char *buf, size_t count); 430 - const void *(*namespace)(struct class *class, 431 - const struct class_attribute *attr); 432 430 }; 433 431 434 432 #define CLASS_ATTR(_name, _mode, _show, _store) \ ··· 436 438 #define CLASS_ATTR_RO(_name) \ 437 439 struct class_attribute class_attr_##_name = __ATTR_RO(_name) 438 440 439 - extern int __must_check class_create_file(struct class *class, 440 - const struct class_attribute *attr); 441 - extern void class_remove_file(struct class *class, 442 - const struct class_attribute *attr); 441 + extern int __must_check class_create_file_ns(struct class *class, 442 + const struct class_attribute *attr, 443 + const void *ns); 444 + extern void class_remove_file_ns(struct class *class, 445 + const struct class_attribute *attr, 446 + const void *ns); 447 + 448 + static inline int __must_check class_create_file(struct class *class, 449 + const struct class_attribute *attr) 450 + { 451 + return class_create_file_ns(class, attr, NULL); 452 + } 453 + 454 + static inline void class_remove_file(struct class *class, 455 + const struct class_attribute *attr) 456 + { 457 + return class_remove_file_ns(class, attr, NULL); 458 + } 443 459 444 460 /* Simple class attribute that is just a static string */ 445 461 struct class_attribute_string {
+14 -2
include/linux/netdevice.h
··· 2873 2873 #define dev_proc_init() 0 2874 2874 #endif 2875 2875 2876 - extern int netdev_class_create_file(struct class_attribute *class_attr); 2877 - extern void netdev_class_remove_file(struct class_attribute *class_attr); 2876 + extern int netdev_class_create_file_ns(struct class_attribute *class_attr, 2877 + const void *ns); 2878 + extern void netdev_class_remove_file_ns(struct class_attribute *class_attr, 2879 + const void *ns); 2880 + 2881 + static inline int netdev_class_create_file(struct class_attribute *class_attr) 2882 + { 2883 + return netdev_class_create_file_ns(class_attr, NULL); 2884 + } 2885 + 2886 + static inline void netdev_class_remove_file(struct class_attribute *class_attr) 2887 + { 2888 + netdev_class_remove_file_ns(class_attr, NULL); 2889 + } 2878 2890 2879 2891 extern struct kobj_ns_type_operations net_ns_type_operations; 2880 2892
+23 -8
include/linux/sysfs.h
··· 173 173 struct sysfs_ops { 174 174 ssize_t (*show)(struct kobject *, struct attribute *, char *); 175 175 ssize_t (*store)(struct kobject *, struct attribute *, const char *, size_t); 176 - const void *(*namespace)(struct kobject *, const struct attribute *); 177 176 }; 178 177 179 178 struct sysfs_dirent; ··· 188 189 int __must_check sysfs_move_dir(struct kobject *kobj, 189 190 struct kobject *new_parent_kobj); 190 191 191 - int __must_check sysfs_create_file(struct kobject *kobj, 192 - const struct attribute *attr); 192 + int __must_check sysfs_create_file_ns(struct kobject *kobj, 193 + const struct attribute *attr, 194 + const void *ns); 193 195 int __must_check sysfs_create_files(struct kobject *kobj, 194 196 const struct attribute **attr); 195 197 int __must_check sysfs_chmod_file(struct kobject *kobj, 196 198 const struct attribute *attr, umode_t mode); 197 - void sysfs_remove_file(struct kobject *kobj, const struct attribute *attr); 199 + void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr, 200 + const void *ns); 198 201 void sysfs_remove_files(struct kobject *kobj, const struct attribute **attr); 199 202 200 203 int __must_check sysfs_create_bin_file(struct kobject *kobj, ··· 278 277 return 0; 279 278 } 280 279 281 - static inline int sysfs_create_file(struct kobject *kobj, 282 - const struct attribute *attr) 280 + static inline int sysfs_create_file_ns(struct kobject *kobj, 281 + const struct attribute *attr, 282 + const void *ns) 283 283 { 284 284 return 0; 285 285 } ··· 297 295 return 0; 298 296 } 299 297 300 - static inline void sysfs_remove_file(struct kobject *kobj, 301 - const struct attribute *attr) 298 + static inline void sysfs_remove_file_ns(struct kobject *kobj, 299 + const struct attribute *attr, 300 + const void *ns) 302 301 { 303 302 } 304 303 ··· 437 434 } 438 435 439 436 #endif /* CONFIG_SYSFS */ 437 + 438 + static inline int __must_check sysfs_create_file(struct kobject *kobj, 439 + const struct attribute *attr) 440 + { 441 + return sysfs_create_file_ns(kobj, attr, NULL); 442 + } 443 + 444 + static inline void sysfs_remove_file(struct kobject *kobj, 445 + const struct attribute *attr) 446 + { 447 + return sysfs_remove_file_ns(kobj, attr, NULL); 448 + } 440 449 441 450 #endif /* _SYSFS_H_ */
+8 -6
net/core/net-sysfs.c
··· 1344 1344 return error; 1345 1345 } 1346 1346 1347 - int netdev_class_create_file(struct class_attribute *class_attr) 1347 + int netdev_class_create_file_ns(struct class_attribute *class_attr, 1348 + const void *ns) 1348 1349 { 1349 - return class_create_file(&net_class, class_attr); 1350 + return class_create_file_ns(&net_class, class_attr, ns); 1350 1351 } 1351 - EXPORT_SYMBOL(netdev_class_create_file); 1352 + EXPORT_SYMBOL(netdev_class_create_file_ns); 1352 1353 1353 - void netdev_class_remove_file(struct class_attribute *class_attr) 1354 + void netdev_class_remove_file_ns(struct class_attribute *class_attr, 1355 + const void *ns) 1354 1356 { 1355 - class_remove_file(&net_class, class_attr); 1357 + class_remove_file_ns(&net_class, class_attr, ns); 1356 1358 } 1357 - EXPORT_SYMBOL(netdev_class_remove_file); 1359 + EXPORT_SYMBOL(netdev_class_remove_file_ns); 1358 1360 1359 1361 int netdev_kobject_init(void) 1360 1362 {