[GIT PULL] block fixes for 2.6.32-rc

[GIT PULL] block fixes for 2.6.32-rc

[Jens Axboe]

Hi Linus,

Two important one-liners, both fixing a regression. Please pull.

  git://git.kernel.dk/linux-2.6-block.git for-linus

Mark McLoughlin (1):
      block: silently error unsupported empty barriers too

Neil Brown (1):
      block: use after free bug in __blkdev_get

 block/blk-core.c |    2 +-
 fs/block_dev.c   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index ac0fa10..71da511 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1161,7 +1161,7 @@ static int __make_request(struct request_queue *q, struct bio *bio)
 	const unsigned int ff = bio->bi_rw & REQ_FAILFAST_MASK;
 	int rw_flags;
 
-	if (bio_rw_flagged(bio, BIO_RW_BARRIER) && bio_has_data(bio) &&
+	if (bio_rw_flagged(bio, BIO_RW_BARRIER) &&
 	    (q->next_ordered == QUEUE_ORDERED_NONE)) {
 		bio_endio(bio, -EOPNOTSUPP);
 		return 0;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 9cf4b92..8bed055 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1248,8 +1248,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
 			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
 		}
 	} else {
-		put_disk(disk);
 		module_put(disk->fops->owner);
+		put_disk(disk);
 		disk = NULL;
 		if (bdev->bd_contains == bdev) {
 			if (bdev->bd_disk->fops->open) {

-- 
Jens Axboe

Re: [GIT PULL] block fixes for 2.6.32-rc

[Linus Torvalds]

On Wed, 28 Oct 2009, Jens Axboe wrote:
> 
> Neil Brown (1):
>       block: use after free bug in __blkdev_get
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 9cf4b92..8bed055 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1248,8 +1248,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
>  			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
>  		}
>  	} else {
> -		put_disk(disk);
>  		module_put(disk->fops->owner);
> +		put_disk(disk);
>  		disk = NULL;
>  		if (bdev->bd_contains == bdev) {
>  			if (bdev->bd_disk->fops->open) {

Is this really right? You do the module-put while the disk is still 
available..

I get the feeling that it might have been better to do

	struct module *mod = disk->fops->owner;
	put_disk(disk);
	module_put(mod);

instead, which tries to make sure that the module is put only after we've 
gotten rid of the disk entirely.

But I dunno. Maybe there is some reason why it's safe either way. You're 
sure the kobject_put() in put_disk will never call to the module?

		Linus

Re: [GIT PULL] block fixes for 2.6.32-rc

[Jens Axboe]

On Wed, Oct 28 2009, Linus Torvalds wrote:
> 
> 
> On Wed, 28 Oct 2009, Jens Axboe wrote:
> > 
> > Neil Brown (1):
> >       block: use after free bug in __blkdev_get
> >
> > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > index 9cf4b92..8bed055 100644
> > --- a/fs/block_dev.c
> > +++ b/fs/block_dev.c
> > @@ -1248,8 +1248,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
> >  			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
> >  		}
> >  	} else {
> > -		put_disk(disk);
> >  		module_put(disk->fops->owner);
> > +		put_disk(disk);
> >  		disk = NULL;
> >  		if (bdev->bd_contains == bdev) {
> >  			if (bdev->bd_disk->fops->open) {
> 
> Is this really right? You do the module-put while the disk is still 
> available..
> 
> I get the feeling that it might have been better to do
> 
> 	struct module *mod = disk->fops->owner;
> 	put_disk(disk);
> 	module_put(mod);
> 
> instead, which tries to make sure that the module is put only after we've 
> gotten rid of the disk entirely.
> 
> But I dunno. Maybe there is some reason why it's safe either way. You're 
> sure the kobject_put() in put_disk will never call to the module?

Hmm good point. The general use case in block_dev.c is indeed to put the
module after the disk, which does seem a bit backwards (at least
logically). I'd say pull the patch since it fixes Neil's problem and
follows the general pattern, then I'll investigate whether that use
pattern is indeed safe. It wont make things worse and the current usage
being fixed is definitely wrong.

-- 
Jens Axboe

Re: [GIT PULL] block fixes for 2.6.32-rc

[Jens Axboe]

On Wed, Oct 28 2009, Jens Axboe wrote:
> On Wed, Oct 28 2009, Linus Torvalds wrote:
> > 
> > 
> > On Wed, 28 Oct 2009, Jens Axboe wrote:
> > > 
> > > Neil Brown (1):
> > >       block: use after free bug in __blkdev_get
> > >
> > > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > > index 9cf4b92..8bed055 100644
> > > --- a/fs/block_dev.c
> > > +++ b/fs/block_dev.c
> > > @@ -1248,8 +1248,8 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
> > >  			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
> > >  		}
> > >  	} else {
> > > -		put_disk(disk);
> > >  		module_put(disk->fops->owner);
> > > +		put_disk(disk);
> > >  		disk = NULL;
> > >  		if (bdev->bd_contains == bdev) {
> > >  			if (bdev->bd_disk->fops->open) {
> > 
> > Is this really right? You do the module-put while the disk is still 
> > available..
> > 
> > I get the feeling that it might have been better to do
> > 
> > 	struct module *mod = disk->fops->owner;
> > 	put_disk(disk);
> > 	module_put(mod);
> > 
> > instead, which tries to make sure that the module is put only after we've 
> > gotten rid of the disk entirely.
> > 
> > But I dunno. Maybe there is some reason why it's safe either way. You're 
> > sure the kobject_put() in put_disk will never call to the module?
> 
> Hmm good point. The general use case in block_dev.c is indeed to put the
> module after the disk, which does seem a bit backwards (at least
> logically). I'd say pull the patch since it fixes Neil's problem and
> follows the general pattern, then I'll investigate whether that use
> pattern is indeed safe. It wont make things worse and the current usage
> being fixed is definitely wrong.

So if I'm following the convoluted mazes of the kobjects correctly, the
release is disk_release() and it only does a free+release of the disk,
partition, and related partition table. So doing the module_put() before
the put_disk() is safe, even if it does look odd.

-- 
Jens Axboe

[GIT PULL] block fixes for 2.6.32-rc

[Jens Axboe]

Hi Linus,

Two small comment "regressions" from Alberto, which got screwed up
during some of the early .32 block commits. And then a fixup for the
previous sync hang fix, I had modified it slightly after it had been
tested. You should never do that... The last two fixes are for CFQ, both
fixing regressions introduced recently.

Please pull!

  git://git.kernel.dk/linux-2.6-block.git for-linus

Alberto Bertogli (2):
      bio_put(): add bio_clone() to the list of functions in the comment
      Fix bio_alloc() and bio_kmalloc() documentation

Jens Axboe (2):
      backing-dev: bdi sb prune should be in the unregister path, not destroy
      cfq-iosched: fix bad return value cfq_should_preempt()

Shaohua Li (1):
      cfq-iosched: limit coop preemption

 block/cfq-iosched.c |   19 ++++++++++++++++---
 fs/bio.c            |   28 ++++++++++++++--------------
 mm/backing-dev.c    |    3 ++-
 3 files changed, 32 insertions(+), 18 deletions(-)

-- 
Jens Axboe