Re: Patch??: linux-2.5.20/fs/bio.c - ll_rw_kio could generate bio's

[Andrew Morton]

"Adam J. Richter" wrote:
> 
> ...
> +int bio_append(struct bio **bio_p, struct page *page, int len, int offset)
> +{
> +       struct bio *bio = *bio_p;
> +       struct bio_vec *vec;
> +       if (bio->bi_idx == bio->bi_vcnt) {
> +               struct bio *old = bio;
> +               *bio_p = bio = bio_alloc(GFP_KERNEL, old->bi_vcnt);

GFP_KERKEL is OK on the readpages() path, but on the writepages()
path it can go infinitely recursive - it needs to be GFP_NOFS.
So another arg is needed here.  I suspect this function ends up adding
more complexity than it takes away, frankly.

> +               if (bio != NULL) {
> +                       bio->bi_sector =
> +                               old->bi_sector + (old->bi_size >> 9);
> +
> +#define COPY(field)    bio->bi_ ## field = old->bi_ ## field
> +                       COPY(bdev);
> +                       COPY(flags);
> +                       COPY(idx);
> +                       COPY(rw);
> +                       COPY(end_io);
> +                       COPY(private);
> +#undef COPY
> +               }
> +               old->bi_idx = 0;
> +               submit_bio(old->bi_rw, old);

Here we're allocating a new BIO before submitting the old one.
This increases the risk of oom deadlocks on the vm_writeback()
path.

I'd prefer that the bio alloc/submit code paths be left as-is...

> +       struct block_device *bdev =
> +               S_ISBLK(inode->i_mode) ? inode->i_bdev : inode->i_sb->s_bdev;

I believe it's cleaner to allow get_block() to decide which
blockdevice belongs to this mapping, really.

-