On Wed, Dec 04, 2013 at 11:10:30AM +1100, Dave Chinner wrote:
> The simple fact is that if we ever want to do concurrent directory
> read operations, we have to take the ilock in readdir() to ensure we
> can serialise correctly against modifications because the i_mutex
> can't be used to do that. So, really, I'm not against the fix you
> proposed - I'm just trying to understand why it is necessary right
Some comments from the person who authered that old comment that removed
- as far as I can tell that was not intentional. I'm usually pretty
good at recording such things in the changelog if I do it
- relying on the open function to read in the extent list seems
potentially dangerous. We generally try to make sure all the
functions using it read it in if needed including the proper
locking. If we want to avoid that for some reason like in the
writeback path we at least comment it and put asserts in.
- like Dave pointed out I think things should just work for mainline
modulo maybe the attr by handle ioctl, but it seems by accident.
Figuring out what issues Ben sees would be useful.
- instead of putting the ilock back at the highest level we might want
to push it down to the dir2 data code and only cover the actual
critical region where we need it.
- xfs_iread_extents really needs an assert to make sure the ilock
- xfs_bmapi_read probably should have an assert as well to make
sure the ilock is held in some way