Eric,
responses below:
Eric Sandeen wrote:
On Tue, 25 Mar 2003, Steven Dake wrote:
The function write_full_page will unlock the page if delalloc_convert
returns a negative value. I don't understand why delalloc_convert
returns a negative value, and if the page should be unlocked, but it
seems that it shouldn't. Could you give me some background on why the
page is unlocked at this point? If I comment out this unlock, bonnie++
delalloc_convert can fail on some error, and we have to do something
with the buffer at that point. This case was put in because
if we ignored the error, we'd write out a buffer with block 0 (delalloc)
and clobber the superblock. Perhaps locking has changed such that
the unlock_page is no longer correct, I'd have to look more closely.
Are you hitting this case?
No, I'm not. I am hitting the case where the file size is on a page
boundary (causing offset to be zero), causing an error to be returned
and the page to be unlocked in the out goto (ret is negative). Then the
page is unlocked _again_ in the function generic_file_write_nolock,
causing an Oops. Whatever should be done with the page after an error,
I don't know, but it shouldn't be unlocked twice.
Even though I am running into only one of the cases, I believe the other
case to be in error as well, because it would cause an oops if this
function operates on the same page as generic_file_write_nolock (double
unlock_page == Oops).
Should the size of the file be checked to ensure it is NOT on a page
boundary? I don't understand the need for this check... Could you
explain it ?
STATIC int
write_full_page(
struct page *page,
int delalloc)
{
struct inode *inode = (struct inode*)page->mapping->host;
unsigned long end_index = inode->i_size >> PAGE_CACHE_SHIFT;
int ret;
/* Are we off the end of the file ? */
if (page->index >= end_index) {
unsigned offset = inode->i_size & (PAGE_CACHE_SIZE-1);
if ((page->index >= end_index+1) || !offset) {
ret = -EIO;
printk ("Off the end of the file %d %d %d %d\n",
page->index, end_index, offset, inode-
>i_size);
goto out;
}
}
It would be very helpful if you could run this on a "stock" xfs
1.2 kernel to see if you still hit this problem.
I'll give it a try, but unfortunately I only have FibreChannel disks so
will have to add in the Qlogic driver...
-Eric
Thanks
-steve
|