On 9/8/23 10:22, Christoph Hellwig wrote:
On Wed, Sep 06, 2023 at 05:59:03PM +0200, Bernd Schubert wrote:
file_remove_privs might call into notify_change(), which requires to hold an exclusive lock.
Looks good:
Reviewed-by: Christoph Hellwig hch@lst.de
FYI, I'd be really curious about benchmarking this against you version that checks xattrs for shared locked writes on files that have xattrs but not security ones or setuid bits. On the one hand being able to do the shared lock sounds nice, on the other hand even just looking up the xattrs will probably make it slower at least for smaller I/O.
I had checked the history of S_NOSEC and I guess that already tells that the xattr lookup is too slow (commit 69b4573296469fd3f70cf7044693074980517067) I don't promise that I benchmark it today, but I can try to find some time in the next week or the week after. Although I guess there won't be any difference with my initial patch, as dentry_needs_remove_privs() also checks for IS_NOSEC(inode) - overhead was just the additional non inlined function call to file_needs_remove_privs(). And if the flag was not set, overhead was looking up xattr two times.
Bernd