On 8/12/22 13:20, Christian Brauner wrote:
> So iiuc, for tmpfs this is effectively a per-inode limit of 128 xattrs
> and 128 user xattrs; nr_inodes * 256. I honestly have no idea if there
> are legimitate use-cases to want more. But there's at least a remote
> chance that this might break someone.
>
> Apart from
>
>> Currently it's possible to create a huge number of xattr per inode,
>
> what exactly is this limit protecting against? In other words, the
> commit message misses the motivation for the patch.
This should prevent softlockup and hung_task_panic caused by slow search
in xattrs->list in simple_xattr_set() and _get()
Adding new xattr checks all present entries in the list,
so execution time linearly depends on the number of such entries.
To avoid this problem I decided to limit somehow the number of entries in the list.
As an alternative Tejun advises to switch this list to something like rb-tree,
now I think he is right.
> I'd also prefer to see a summary of what filesystems are affected by
> this change. Afaict, the patchset doesn't change anything for kernfs
> users such as cgroup{1,2} so it should only be tmpfs and potential
> future users of the simple_xattr_* api.
This affect all file systems used simple_xattr_* API: sysfs and tmpfs.
Now I'll try to follow Tejun's advice and switch the xatrrs list to rb-tree.
Unfortunately, I doubt that I will have enough time to finish before the merge
window closes.
Thank you,
Vasily Averin
