Re: XFS 1.2 bug?

[Steven Dake <sdake@xxxxxxxxxx>]

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

>