Re: [PATCH 2/3] Sysfs: Allow directories to be populated dynamically

From: Tejun Heo
Date: Thu Oct 29 2009 - 12:27:16 EST


Matthew Wilcox wrote:
> On Thu, Oct 29, 2009 at 05:20:00PM +0100, Tejun Heo wrote:
>> This will increase the size of struct sysfs_dirent by three pointers
>> which is considerable. Bloating the size of sysfs_dirent can waste
>> large amount of memory on machines with a lot of disks.
>
> No it won't. It's in a union with sysfs_inode_attrs which contains a
> struct iattr, which is at least 52 bytes.

struct sysfs_dirent {
atomic_t s_count;
atomic_t s_active;
struct sysfs_dirent *s_parent;
struct sysfs_dirent *s_sibling;
const char *s_name;

union {
struct sysfs_elem_dir s_dir;
struct sysfs_elem_symlink s_symlink;
struct sysfs_elem_attr s_attr;
struct sysfs_elem_bin_attr s_bin_attr;
};

unsigned int s_flags;
ino_t s_ino;
umode_t s_mode;
struct sysfs_inode_attrs *s_iattr;
^^
};

>> The implementation looks quite scary to me. Is this the only way to
>> do this? It it because trying to create individual entries for msix
>> will end up creating too many sysfs entries? If so, how many are we
>> talking about?
>
> While that's the original motivation, shrinking the amount of memory
> taken by sysfs overall is a worthwhile achievement, don't you think?

It feels a bit too convoluted to me. sysfs is already pretty
convoluted and adding yet more convolution would require pretty good
justification, so I'm curious about the numbers.

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/