On Mon, Apr 21, 2014 at 11:47:37PM -0700, Christoph Hellwig wrote: > On Tue, Apr 22, 2014 at 09:35:12AM +1000, Dave Chinner wrote: > > > Shouldn't we apply the same scheme as for directories here, that is if > > > it fails with a verifier error re-read without the verifier and then > > > still do the full check as well? > > > > The directory code is the special case - it uses xfs_trans_read_buf* > > interfaces, which return either a good buffer with no error or an > > error with no buffer. Hence for the directory code, we have to > > re-read the buffer without the verifier to grab the unchecked buffer > > from the cache when the verifier detects an error. > > How about just having a variant of xfs_da_read_buf that doesn't use > xfs_trans_read_buf *? xfs_da_read_buf is pretty simple, especially > when removing the magic test that has been superceeded by the verifiers. Right now I ijust want to keep the change as small as possible and get a release out. Yes, I agree there's much more cleanup that is needed in this code, but at this point is seems unnecessary and doesn't matter for the purpose of providing this functionality initially. We have to provide the same functionality for the kernel code for it to be able to handle CRC errors sanely, and so we're going to need to restructure xfs_trans_read_buf() to handle it. in doing this, we solve the libxfs problem, too, because what we do to the kernel code will flow on to userspace... So really, I'd prefer to leave the userspace code doing this retry for this one piece of infrastructure and do all the major restructing of the directory buffer read code in the kernel code first. That way we can pick up all the changes for userspace when the libxfs code is updated. Keep in mind that right now we've got a *lot* of updates from the kernel to the libxfs code pending that are stalled waiting for a release to be done... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
