Re: [Scst-devel] VDISK error with 2.6.32.15 and SCST trunk r1787

[Vladislav B. <vs...@vl...>]

Hi Bryan,

Bryan Mesich, on 07/23/2010 11:18 PM wrote:
> On Thu, Jul 15, 2010 at 03:40:29PM +0400, Vladislav Bolkhovitin wrote:
>
> Hi Vlad,
>
> [snip...]
>>> As a side note, would it be possible to patch a 2.6.32 kernel
>>> with a patch from the TRUNK, but compile a release version of
>>> SCST against the trunk patched kernel?
>>
>> Yes, sure. You can use scripts/generate-kernel-patch from the trunk for
>> that.
>
> Thanks for the clarification.
>
> [snip...]
>> It is very important to localize if update of the kernel or SCST is
>> responsible for the behavior change. So, I'd suggest you try the latest
>> SCST trunk (please update) with 2.6.27.42, which is known to work well.
>
> Well...I've localized to the problem to be kernel related.  My
> testing puts the break point between 2.6.31.14 and 2.6.32.  I
> attempted to bisect the kernel with git, but I'm running into
> problems getting SCST to compile against the bisected tree.
>
> In particular, I'm getting implicit declaration warnings for
> sync_page_range() in scst_vdisk.c (line 1927).
>
> #if LINUX_VERSION_CODE<  KERNEL_VERSION(2, 6, 32)
>          res = sync_page_range(file->f_dentry->d_inode, file->f_mapping,
>                  loff, len);
> #else
> #if 0   /* For sparse files we might need to sync metadata as well */
>          res = generic_write_sync(file, loff, len);
> #else
>          res = filemap_write_and_wait_range(file->f_mapping, loff, len);
> #endif
> #endif
>
> (BTW, I've been using SCST r1849 for my recent testing)
>
> I'm pushing my technical abilities here, but It would seem that
> sync_page_range() has been replaced for kernels>  2.6.32.  I
> tried hacking around the compile warnings by commenting out
> sync_page_range() and forcing the use of
> filemap_write_and_wait_range(), but that resulted in a stack
> trace and kernel panic when trying to load SCST configuration
> (Modules loaded, opening VDISK failed).
>
>> Additionally, you should add in scst_vdisk.c::blockio_endio() just after
>> "PRINT_ERROR("cmd %p returned error %d", blockio_work->cmd, error);"
>> dump_stack() call. It will give us more info about the failed call stack.
>
> I put the dump_stack() where you suggested, but the resulting
> dump didn't seem helpful.  I instead put the dump_stack() below
> the spinlock just after the above referenced "PRINT_ERROR".  This
> resulted in a cleaner stack dump (see below).
>
>
> static DEFINE_SPINLOCK(blockio_endio_lock);
> static int error_count = 0;
>
>   PRINT_ERROR("cmd %p returned error %d", blockio_work->cmd,
>            error);
>
> /* To protect from several bios finishing simultaneously */
> spin_lock_bh(&blockio_endio_lock);
> if ( error_count++<  5 ) {
> 	printk("Error %d stack trace", error_count + 1);
> 	dump_stack();
> }
>
> See attached for stack dump.  Kernel version is 2.6.34.1 w/SCST
> r1849
>
> I think I've reached the end of my skill set for this particular
> problem.  If there is anything else I can do that would be
> helpful, please let me know.

You have made a very good progress! Now we know that (1) this is 
regression in the kernel and (2), most likey, raid5.c::make_request() 
for some reason sometimes calls bio_endio() with not BIO_UPTODATE bios. 
Now we need to find out that reason.

I'd suggest you to finish the bisecting. You can simply comment out all 
the sync_page_range() related code, for this investigation it doesn't 
matter.

Also please in bio.c::bio_endio() change:

	else if (!test_bit(BIO_UPTODATE, &bio->bi_flags))
		error = -EIO;

to

	else if (!test_bit(BIO_UPTODATE, &bio->bi_flags)) {
		printk("%s: not BIO_UPTODATE bio %p!\n", __func__, bio);
		error = -EIO;
	}

This will allow us to see if I correct in my analyze.

Vlad