On Wed, 2020-12-23 at 15:30 +0800, Fox Chen wrote: > Hi Ian, > > On Tue, Dec 22, 2020 at 3:47 PM Ian Kent <raven@xxxxxxxxxx> wrote: > > Here is what I currently have for the patch series we were > > discussing > > recently. > > > > I'm interested to see how this goes with the problem you are > > seeing. > > > > The last patch in the series (the attributes update patch) has seen > > no more than compile testing, I hope I haven't messed up on that. > > OK, I just ignored the last patch (kernfs: add a spinlock to kernfs > iattrs for inode updates) > because I can not boot the kernel after applying it. Right, clearly I haven't got that last patch right yet. It looks like the problem there is that the iattrs structure might not be allocated at the time so clearly I can't just use the spin lock inside. It would be simple enough to resolve except for the need to set the inode link count too. Ian > > Applying the first five patches, my benchmark shows no improvement > this time, > one open+read+close cycle spends 500us. :( > > perf suggests that down_write in kernfs_iop_permission drags the most > performance > (full report see attachment) which makes sense to me, since > down_write > has no difference > here compared to the mutex, they all allow only one thread running > simultaneously. > > sigh... It's really hard to fix. > > | > |--51.81%--inode_permission > | | > | --51.36%--kernfs_iop_permission > | | > | |--50.41%--down_write > | | | > | | --50.32% > --rwsem_down_write_slowpath > | | | > | | --49.67% > --rwsem_optimistic_spin > | | | > | | --49.31% > --osq_lock > | | > | --0.74%--up_write > | rwsem_wake.isra.0 > | | > | --0.69%--wake_up_q > | | > | --0.62% > --try_to_wake_up > | | > | > --0.54%--_raw_spin_lock_irqsave > | > | > | > --0.52%--native_queued > | > --29.90%--walk_component > | > --29.72%--lookup_fast > | > --29.51%--kernfs_dop_revalidate > | > |--28.88%--down_read > | | > | > --28.75%--rwsem_down_read_slowpath > | | > | > --28.50%--rwsem_optimistic_spin > | > | > | > --28.38%--osq_lock > | > --0.59%--up_read > rwsem_wake.isra.0 > | > --0.54%--wake_up_q > | > > --0.53%--try_to_wake_up > > > Please keep in mind that Greg's continued silence on the question > > of whether the series might be re-considered indicates to me that > > he has not changed his position on this. > > Since the problem doesn't break the system, it doesn't need immediate > attention. > So I bet it must be labeled with a low priority in Greg's to-do list. > And it's merging > window & holiday season. Let's just give him more time. :) > > > > --- > > > > Ian Kent (6): > > kernfs: move revalidate to be near lookup > > kernfs: use VFS negative dentry caching > > kernfs: use revision to identify directory node changes > > kernfs: switch kernfs to use an rwsem > > kernfs: stay in rcu-walk mode if possible > > kernfs: add a spinlock to kernfs iattrs for inode updates > > > > > > fs/kernfs/dir.c | 285 ++++++++++++++++++++++++++++--- > > ------------ > > fs/kernfs/file.c | 4 - > > fs/kernfs/inode.c | 19 ++- > > fs/kernfs/kernfs-internal.h | 30 ++++- > > fs/kernfs/mount.c | 12 +- > > fs/kernfs/symlink.c | 4 - > > include/linux/kernfs.h | 5 + > > 7 files changed, 238 insertions(+), 121 deletions(-) > > > > -- > > Ian > > > > thanks, > fox
