Messages in this thread | | | Date | Sat, 31 Jan 2004 10:41:03 -0600 | From | Steve Lord <> | Subject | Re: 2.6.2-rc2 nfsd+xfs spins in i_size_read() |
| |
Christoph Hellwig wrote: > On Sat, Jan 31, 2004 at 09:59:06AM -0600, Steve Lord wrote: > >>The vn_revalidate call is called out of linvfs_setattr, >>which is called with the i_sem held, it is also potentially called out >>of linvfs_getattr, although since the i_size is always maintained >>as it is changed, this call should not actually be updating the size. >>Possibly changing the code in vn_revalidate to do this: >> >> if (i_size_read(inode) < va.va_size)) >> i_size_write(inode, va.va_size); >> >>Would be a good starting point, I suspect those calls from the nfs >>revalidate call are not really going to change the inode size. My >>guess is this will make your problem go away. >> >>Probably some larger code restructure is needed so that revalidate >>knows if the i_sem is held or not at this point. > > > I think the right fix is to update the Linux i_size always shortly after > di_size is updated. There's a lot of updates in the directory code while > should be handled by an i_size_write in the matching linvfs routines, and > there's a few more but we should be able to handle those without > vn_revalidate aswell. >
Changing the validate_fields call to use
if (i_size_read(inode) != va.va_size) i_size_write(inode, va.va_size);
should take care of directories, certainly better than the direct assignment to i_size which is in there now.... This is called from the directory ops which modify the contents of a directory and should already be under the i_sem. The vn_revalidate code should use a != test as well of course...
> >>The O_DIRECT write case is the hard one. In XFS's internal view of >>the world, the inode size is maintained via the XFS_ILOCK, but we >>only hold that across metadata manipulation within the fs code, >>not across I/O such as a call to generic_file_aio_write_nolock. >>Right now the only way I see of dealing with that is to make >>writes which we know will extent the file hold the i_sem for >>the duration in the O_DIRECT case. > > > That's the more difficult case. Any reson why you'd hold i_sem > for the whole O_DIRECT I/O instead of just for updating i_sem? >
We worked hard not to hold it, but that i_size_write in the middle of the async_io code is tough to work around.
> Note that the EOF zeroing code also calls i_size_write. >
But that is called out of an extending setattr or a buffered write which hold the semaphore. Hmm, actually O_DIRECT write can be in there as well, but that is the same problem as the generic I/O path call.
Steve
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |