Hello, On Fri, Jun 01, 2018 at 09:25:32AM +0000, Hatayama, Daisuke wrote: > kernfs_dir_pos() checks if a kernfs_node object given as one of its > arguments is still active and if so returns it, or returns a > kernfs_node object that is most equal (possibly smaller and larger) to > the given object.
Sometimes they're duplicate operations tho, which is exactly the bug the posted patch is trying to fix. What I'm suggesting is instead of leaving both instances and skipping one conditionally, put them in one place and trigger only when necessary. The sequence of operations would be exactly the same. The only difference is how the code is organized. > kernfs_dir_next_pos() returns a kernfs_node object that is next to the > object given by kernfs_dir_pos(). > > Two functions does different things and both need to skip inactive > nodes. I don't think it natural to remove the skip only from > kernfs_dir_pos(). > > OTOH, throughout getdents(), there is no case that the kernfs_node > object given to kernfs_dir_pos() is used afterwards in the > processing. So, is it enough to provide kernfs_dir_next_pos() only? > Then, the skip code is now not duplicated. > > The patch below is my thought. How do you think? > > But note that this patch has some bug so that system boot get hang > without detecting root filesystem disk :) I'm debugging this now. I haven't looked into the code that closely but given that we had cases where both skippings were fine and not, the condition is likely gonna be a bit tricker? Thanks. -- tejun
