bdi: Use parent filesystem BDI for inodes not capable of writeback
in [Kernel]
|
Prev: ipmi: Run a dummy command before submitting a new command
Next: PCI / PCIe: Ask BIOS for control of all native services simultaneously
From: Christoph Hellwig on 27 Jul 2010 16:30 On Tue, Jul 27, 2010 at 04:12:07PM -0400, Christoph Hellwig wrote: > On Tue, Jul 27, 2010 at 08:01:31PM +0200, Jan Kara wrote: > > > It shouldn't. Block device nodes are on the bdev filesystems, and > > Ok, so inode->i_sb->s_bdi will actually point to noop_backing_dev_info > > as set by set_anon_super(). Or am I completely out? > > I think you're right. This seems rather bad if it's indeed true. I'll > quickly verify it using Dave's new tracing once I've built a block > tree kernel. Indeed it does. So using ->s_bdi actually is wrong for the block device node, given that it does set up the bdevfs inode's backing_dev_info to the proper one, but can't actually do it for the per-sb one. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jan Kara on 27 Jul 2010 16:50 On Tue 27-07-10 16:21:29, Christoph Hellwig wrote: > On Tue, Jul 27, 2010 at 04:12:07PM -0400, Christoph Hellwig wrote: > > On Tue, Jul 27, 2010 at 08:01:31PM +0200, Jan Kara wrote: > > > > It shouldn't. Block device nodes are on the bdev filesystems, and > > > Ok, so inode->i_sb->s_bdi will actually point to noop_backing_dev_info > > > as set by set_anon_super(). Or am I completely out? > > > > I think you're right. This seems rather bad if it's indeed true. I'll > > quickly verify it using Dave's new tracing once I've built a block > > tree kernel. > > Indeed it does. So using ->s_bdi actually is wrong for the block > device node, given that it does set up the bdevfs inode's > backing_dev_info to the proper one, but can't actually do it for > the per-sb one. Yes. So what I do in inode_to_bdi() is necessary (although I agree it looks a bit ugly). Honza -- Jan Kara <jack(a)suse.cz> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on 27 Jul 2010 16:50 On Tue, Jul 27, 2010 at 10:44:07PM +0200, Jan Kara wrote: > Yes. So what I do in inode_to_bdi() is necessary (although I agree it > looks a bit ugly). Yes, seem like it. Damn, so much for my plan of never looking at mapping->backing_dev_info from writeback code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on 29 Jul 2010 04:20 Given that we only need the per-mapping one for the bdev fs what about doing: /* * Return the writeback-relevant backing device for this inode. * * For a normal filesystem this must always be the bdi hanging off the * superblock, given that we only expect one bdi per filesystems in * the per-superblock sync functions. But the block device special * filesystem requires a quick given that it contains the internal * I/O inodes for block devices on a single superblock. This works * because the block deevice filesystem inodes are never user visible * and we will never do a per-superblock sync on it. */ static struct backing_dev_info *inode_to_bdi(struct inode *inode) { if (inode->i_sb == blockdev_superblock) return inode->i_mapping->backing_dev_info; return inode->i_sb->s_bdi; } that also avoids the need for doing the bdi capabilities audit ASAP. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jan Kara on 29 Jul 2010 08:20
On Thu 29-07-10 04:12:10, Christoph Hellwig wrote: > Given that we only need the per-mapping one for the bdev fs what about > doing: > > /* > * Return the writeback-relevant backing device for this inode. > * > * For a normal filesystem this must always be the bdi hanging off the > * superblock, given that we only expect one bdi per filesystems in > * the per-superblock sync functions. But the block device special > * filesystem requires a quick given that it contains the internal > * I/O inodes for block devices on a single superblock. This works > * because the block deevice filesystem inodes are never user visible > * and we will never do a per-superblock sync on it. > */ > static struct backing_dev_info *inode_to_bdi(struct inode *inode) > { > if (inode->i_sb == blockdev_superblock) > return inode->i_mapping->backing_dev_info; > return inode->i_sb->s_bdi; > } > > that also avoids the need for doing the bdi capabilities audit ASAP. Well, but I'm not sure we'll help ourselves with having to do audit this way. Some other pseudofilesystems can play tricks with backing_dev_info as well and thus inode->i_mapping->backing_dev_info != inode->i_sb->s_bdi. In particular MTD seems to do such things. As I'm looking at the code, it looks like a similar case as blockdev filesystem. Honza -- Jan Kara <jack(a)suse.cz> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |