Timothy Shimmin wrote:
Shailendra Tripathi wrote:
Hi,
Can someone explain how not checking the flushiter can losse
filesize updates.
Let me the take the case mentioned here in the fix statement:
a. Clustered inode create - flush iter - X( 0)
b. size update --> flush iter --> Y
X and Y will always hold as: X <= Y, that is, it is not possible to
have X >Y (unless size update is non -transactional. As far as I know,
size update is always transactional.)
There are 2 cases here:
a) log of Y reached to the disk --> 1) inode with flush iter was
reached 2) inode didn't make.
b) log of Y didn't reach the disk --> flush_iter Y should have never
reached disk
In none of cases, I can see the possibility that size update can be
lost becuase of replaying of the logs in the sequential order. If Log
of Y didn't reach, does it not make sense to have the flush_iter and
size correctly set to the last known transaction on the disk. To me,
it appears unsafe to do as the inode state will not match the state of
the last known transaction after recovery.
Regards,
Shailendra
Dave answered this but yes this is a case where we are breaking
the transaction model IMO. And my understanding is that we are doing
this for performance reasons.
One of Lachlan's proposals (IIRC) was to log the size change before the
ondisk size change in xfs_aops.c/xfs_setfilesize()
and thus follow the model but questions were raised about introducing
performance overhead of log traffic in the write path.
It would make life easier to do it that way - we wouldn't have to check
the flushiter field of the ondisk inode because we know we will end up
with the same thing by just replaying the log. But since the addition
of the flushiter stuff pre-dates the NULL files changes there must be
another reason we need it. Previously the size change would get logged
with an extent allocation transaction but extending a file within the
same last block would not send the new file size to the log. I think
that may have been the reason for needing the flushiter stuff. If we
log the file size change in xfs_setfilesize() we may not need the
flushiter stuff at all, hmmm maybe for timestamp updates?
|