Re: ext2 on a CD-RW

[Jens Axboe <axboe@suse.de>]

On Fri, Jan 02 2004, Peter Osterlund wrote:
> Andrew Morton <akpm@osdl.org> writes:
> 
> > Peter Osterlund <petero2@telia.com> wrote:
> > >
> > > If I create an ext2 filesystem
> > >  with 2kb blocksize, I hit a bug when I try to write some large files to
> > >  the filesystem. The problem is that the code in mpage_writepage() fails if
> > >  a page is mapped to disk across a packet boundary. In that case, the
> > >  bio_add_page() call at line 543 in mpage.c can fail even if the bio was
> > >  previously empty. The code then passes an empty bio to submit_bio(), which
> > >  triggers a bug at line 2303 in ll_rw_blk.c. This patch seems to fix the
> > >  problem.
> > > 
> > >  --- linux/fs/mpage.c.old	2004-01-02 00:26:19.000000000 +0100
> > >  +++ linux/fs/mpage.c	2004-01-02 00:26:50.000000000 +0100
> > >  @@ -541,6 +541,11 @@
> > >   
> > >   	length = first_unmapped << blkbits;
> > >   	if (bio_add_page(bio, page, length, 0) < length) {
> > >  +		if (!bio->bi_size) {
> > >  +			bio_put(bio);
> > >  +			bio = NULL;
> > >  +			goto confused;
> > >  +		}
> > >   		bio = mpage_bio_submit(WRITE, bio);
> > >   		goto alloc_new;
> > >   	}
> > 
> > Confused.  We initially have an empty BIO, and we run bio_add_page()
> > against it, adding one page.
> > 
> > How can that bio_add_page() fail to add the page?
> > 
> > Cold you describe the failure a little more please?
> 
> In bio_add_page(), there is this check:
> 
> 	/*
> 	 * if queue has other restrictions (eg varying max sector size
> 	 * depending on offset), it can specify a merge_bvec_fn in the
> 	 * queue to get further control
> 	 */
> 	if (q->merge_bvec_fn) {
> 		/*
> 		 * merge_bvec_fn() returns number of bytes it can accept
> 		 * at this offset
> 		 */
> 		if (q->merge_bvec_fn(q, bio, bvec) < len) {
> 			bvec->bv_page = NULL;
> 			bvec->bv_len = 0;
> 			bvec->bv_offset = 0;
> 			return 0;
> 		}
> 	}
> 
> The packet writing code has the restriction that a bio must not span a
> packet boundary. (A packet is 32*2048 bytes.) If the page when mapped
> to disk starts 2kb before a packet boundary, merge_bvec_fn therefore
> returns 2048, which is less than len, which is 4096 if the whole page
> is mapped, so the bio_add_page() call fails.

The packet writing code is buggy then, you must always allow a page to
be added to an empty bio. You'll have to deal with the pieces there, I'm
afraid.

So it's not a bug in mpage. If that was the case, there are other buggy
places in the kernel.

-- 
Jens Axboe