lkml.org 
[lkml]   [2008]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: AZFS file system proposal
Thank you very much Christoph for your comments. Some of them I have already found useful and fixed my code accordinly. Some other your comments I find interesting but would like to investigate them a little bit more.
I will send an updated version of my patch soon.

> truncate and seeking are done by the VFS, not need to do it.

I think VFS is doing it not exactly the same way which I need. But I will look at it once more again to make sure.

> Also no need to stuff the inode in file->private_data because
> it's always available through file->f_path.dentry->inode.

Fixed.

> You need to check for an NULL pointer from kmem_cache_alloc
> here.

Fixed.

> > + if (!disk || !disk->queue) {
>
> This won't ever be zero, no need to check.

Agree 50%. I have removed a printk from here but have left the following line;
BUG_ON(!disk || !disk->queue);

> > + if (!get_device(disk->driverfs_dev)) {
> > + printk(KERN_ERR "%s cannot get reference to device driver\n",
> > + AZFS_FILESYSTEM_NAME);
> > + return -EFAULT;
> > + }
>
> You don't need another reference, the disk won't go away while the
> block device is open.

Not agree. I leave get_device here but remove the printk.

> > + spin_lock(&super_list.lock);
> > + list_for_each_entry(knoten, &super_list.head, list)
> > + if (knoten->blkdev == sb->s_bdev) {
> > + super = knoten;
> > + break;
> > + }
> > + spin_unlock(&super_list.lock);
>
> This can't happen. get_sb_bdev already searches for the same superblock
> already existing and doesn't even call into fill_super in that case.

I will check it.

> > +static void
> > +azfs_kill_sb(struct super_block *sb)
> > +{
> > + sb->s_root = NULL;
>
> Very bad idea, if you set sb->s_root to zero before calling
> generic_shutdown_super it will miss a lot of the taerdown activity.

I need it because I want to keep all super blocks and inodes to make it possible to mount the same AZFS partition later and to let user see all his files again. Don't forget - AZFS keeps all the inode data in RAM.

> > + spin_lock(&super_list.lock);
> > + list_for_each_entry_safe(super, SUPER, &super_list.head, list) {
> > + disk = super->blkdev->bd_disk;
> > + list_del(&super->list);
> > + iounmap((void*) super->io_addr);
> > + write_lock(&super->lock);
> > + for_each_block_safe(block, knoten, &super->block_list)
> > + azfs_block_free(block);
> > + write_unlock(&super->lock);
> > + disk->driverfs_dev->driver_data = NULL;
> > + disk->driverfs_dev->platform_data = NULL;
> > + kfree(super);
> > + put_device(disk->driverfs_dev);
>
> All this teardown should happen in ->put_super, and with this and
> the above comment there should be need for a list of all superblocks.

Same thing - super blocks and inodes of unmounted file systems are still in RAM.

--
Mit freundlichen Grüßen / met vriendelijke groeten / avec regards

Maxim V. Shchetynin
Linux Kernel Entwicklung
IBM Deutschland Entwicklung GmbH
Linux für Cell, Abteilung 3250
Schönaicher Straße 220
71032 Böblingen

Vorsitzender des Aufsichtsrats: Johann Weihen
Geschäftsführung: Herbert Kircher
Sitz der Gesellschaft: Böblingen
Registriergericht: Amtsgericht Stuttgart, HRB 243294

Fahr nur so schnell wie dein Schutzengel fliegen kann!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2008-06-18 16:07    [W:0.101 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site