[GIT PULL] one writeback regression fix for .36

[GIT PULL] one writeback regression fix for .36

[Jens Axboe]

Hi Linus,

The recent patch series from Jan introduced a WARN_ON() regression.
The real fix for this is adding a super_operations->get_bdi(), which
we'll do for 2.6.37.

Please pull.

are available in the git repository at:
  git://git.kernel.dk/linux-2.6-block.git for-linus

Christoph Hellwig (1):
      writeback: always use sb->s_bdi for writeback purposes

 fs/fs-writeback.c |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 5581122..ab38fef 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -72,22 +72,11 @@ int writeback_in_progress(struct backing_dev_info *bdi)
 static inline struct backing_dev_info *inode_to_bdi(struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
-	struct backing_dev_info *bdi = inode->i_mapping->backing_dev_info;
 
-	/*
-	 * For inodes on standard filesystems, we use superblock's bdi. For
-	 * inodes on virtual filesystems, we want to use inode mapping's bdi
-	 * because they can possibly point to something useful (think about
-	 * block_dev filesystem).
-	 */
-	if (sb->s_bdi && sb->s_bdi != &noop_backing_dev_info) {
-		/* Some device inodes could play dirty tricks. Catch them... */
-		WARN(bdi != sb->s_bdi && bdi_cap_writeback_dirty(bdi),
-			"Dirtiable inode bdi %s != sb bdi %s\n",
-			bdi->name, sb->s_bdi->name);
-		return sb->s_bdi;
-	}
-	return bdi;
+	if (strcmp(sb->s_type->name, "bdev") == 0)
+		return inode->i_mapping->backing_dev_info;
+
+	return sb->s_bdi;
 }
 
 static void bdi_queue_work(struct backing_dev_info *bdi,

-- 
Jens Axboe

Re: [GIT PULL] one writeback regression fix for .36

[Linus Torvalds]

On Wed, Oct 6, 2010 at 9:54 AM, Jens Axboe <jaxboe@fusionio.com> wrote:
>
> The recent patch series from Jan introduced a WARN_ON() regression.
> The real fix for this is adding a super_operations->get_bdi(), which
> we'll do for 2.6.37.
>
> Please pull.
>
> are available in the git repository at:
>  git://git.kernel.dk/linux-2.6-block.git for-linus
>
> Christoph Hellwig (1):
>      writeback: always use sb->s_bdi for writeback purposes

This is a f*cking disgrace. It's now the second patch I see during
this release window that works by parsing random strings in the block
device layer.

This needs to stop. There's something seriously wrong in the whole
subsystem. This kind of hackery is a disease, and it seems to be
endemic.

So let's just make sure that 2.6.37 really does clean these things up.
And dammit, I don't want to see more random crap added. There has been
too much crazyness going on, with too little taste in the whole
writeback and IO scheduler area. Jens, you need to put the brakes on.
No more crap. Really. Make 2.6.37 a stabilization and cleanup release,
without new crazy features or clever tweaking. Ok? Because writeback
was a disaster in 2.6.35, and it's been this kind of crazy ugly in
2.6.36.

So I'm not taking any more writeback changes unless I feel that you
are actively working on cleaning up the crazy tasteless crap without
adding new code.

                     Linus

Re: [GIT PULL] one writeback regression fix for .36

[Al Viro]

On Wed, Oct 06, 2010 at 11:17:54AM -0700, Linus Torvalds wrote:
> > Christoph Hellwig (1):
> > ? ? ?writeback: always use sb->s_bdi for writeback purposes
> 
> This is a f*cking disgrace. It's now the second patch I see during
> this release window that works by parsing random strings in the block
> device layer.
> 
> This needs to stop. There's something seriously wrong in the whole
> subsystem. This kind of hackery is a disease, and it seems to be
> endemic.

Eh...  I'm no fonder of that than you are (and strcmp is fucking stupid),
but... that thing is really sb_is_blkdev_sb() trying to make a comeback
in fs/fs-writeback.c.  IOW, it's sick, but not for the reasons you are
mentioning; strcmp() use is trivially removable.

Re: [GIT PULL] one writeback regression fix for .36

[Jens Axboe]

On 2010-10-06 20:17, Linus Torvalds wrote:
> On Wed, Oct 6, 2010 at 9:54 AM, Jens Axboe <jaxboe@fusionio.com> wrote:
>>
>> The recent patch series from Jan introduced a WARN_ON() regression.
>> The real fix for this is adding a super_operations->get_bdi(), which
>> we'll do for 2.6.37.
>>
>> Please pull.
>>
>> are available in the git repository at:
>>  git://git.kernel.dk/linux-2.6-block.git for-linus
>>
>> Christoph Hellwig (1):
>>      writeback: always use sb->s_bdi for writeback purposes
> 
> This is a f*cking disgrace. It's now the second patch I see during
> this release window that works by parsing random strings in the block
> device layer.
> 
> This needs to stop. There's something seriously wrong in the whole
> subsystem. This kind of hackery is a disease, and it seems to be
> endemic.

I'm not enjoying this patch anymore than you are, trust me. As Al
mentioned, it's not as bad as it appears (which is good, because it
appears like a bastard). We could work around this ugliness, but at this
point it'll just be a prettied up turd anyway. So may as well go for the
simple variant of that at least.

> So let's just make sure that 2.6.37 really does clean these things up.
> And dammit, I don't want to see more random crap added. There has been
> too much crazyness going on, with too little taste in the whole
> writeback and IO scheduler area. Jens, you need to put the brakes on.
> No more crap. Really. Make 2.6.37 a stabilization and cleanup release,
> without new crazy features or clever tweaking. Ok? Because writeback
> was a disaster in 2.6.35, and it's been this kind of crazy ugly in
> 2.6.36.
> 
> So I'm not taking any more writeback changes unless I feel that you
> are actively working on cleaning up the crazy tasteless crap without
> adding new code.

Agree, there's no complex stuff queued up for .37. Brakes are on, we do
need a stability release on that side.

-- 
Jens Axboe

Re: [GIT PULL] one writeback regression fix for .36

[Christoph Hellwig]

On Wed, Oct 06, 2010 at 07:31:24PM +0100, Al Viro wrote:
> Eh...  I'm no fonder of that than you are (and strcmp is fucking stupid),
> but... that thing is really sb_is_blkdev_sb() trying to make a comeback
> in fs/fs-writeback.c.  IOW, it's sick, but not for the reasons you are
> mentioning; strcmp() use is trivially removable.

The alternative basically is exporting bd_type so that it can be used
here and comparing ->s_type.  The real problem is that right now the
internal block device filesystem is the only one having more than one
backing device per filesystem.  I have patches to generalize that, which
is something we'll need for writeback performance on large filesystem
sooner or later.  I was planning to get this ready for 2.6.37, but if
you prefer no big chance in this area I can calm down and move other
things up on my TODO list.