linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: arnd@arndb.de
Cc: cbe-oss-dev@ozlabs.org, linuxppc-dev@ozlabs.org
Subject: Re: [Cbe-oss-dev] [patch 7/9] azfs: initial submit of azfs, a non-buffered filesystem
Date: Tue, 22 Jul 2008 11:49:50 +0200	[thread overview]
Message-ID: <20080722094950.GA3234@lst.de> (raw)
In-Reply-To: <20080715195739.820365109@arndb.de>

On Tue, Jul 15, 2008 at 09:51:46PM +0200, arnd@arndb.de wrote:
> From: Maxim Shchetynin <maxim@linux.vnet.ibm.com>
> 
> AZFS is a file system which keeps all files on memory mapped random
> access storage. It was designed to work on the axonram device driver
> for IBM QS2x blade servers, but can operate on any block device
> that exports a direct_access method.

I don't thinks it's quite ready yet.  I've had another look through the
code and here's some issues I came up with:

 - first thing is that it's not sparse clean, which is a bit of a red
   flag.  The __iomem and __user annotations are there for a reason.
 - then even with annotations we still have the issue of
   copy_{from,to}_user into mmio regions.  According to benh that's
   still an open issue, but it at least needs very good explanations in
   comments.
 - the aio_read and aio_write methods don't handle actually vectored
   writes.  They need to iterate over all iovecs.  Or just implement
   plain read/write given that it's not actually asynchronous.
 - the persistent superblock hack needs to go away.  Just clean up
   everything in ->put_super.  If we want a fully persistant fs
   we should just be using ext2 + xip
 - azfs_open looks very fishy.  there's never a need to do seeks
   inside open.  if O_APPEND is set the VFS makes sure the read and
   write methods get the right ppos pointer passed.
   And truncation is done by the VFS for O_TRUNC opens through
   ->setattr
 - azfs_znode should not have a size field of it's own, but the
   filesystem should only use the inode one
 - the lists and inode_init_once should be called from the slab
   constructor as in other filesystems
 - I don't think there is any point of having a slab cache
   for the azfs_block structures
 - disk->driverfs_dev is not writeable to the filesystem, but for
   driver use.  The information azfs stores in there is not used
   anyway, so it could easily be removed.
 - lots of duplicated field in azfs_super where the superblock
   ones should be used:

	media_size	-> sb->s_maxbytes
	sector_size	-> not needed at all
	blkdev		-> sb->s_bdev
	root		-> sb->s_root

  parent reply	other threads:[~2008-07-22  9:49 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-15 19:51 [patch 0/9] Cell patches for 2.6.27, version 2 arnd
2008-07-15 19:51 ` [patch 1/9] powerpc/cell/edac: log a syndrome code in case of correctable error arnd
2008-07-17  5:59   ` Benjamin Herrenschmidt
2008-07-17 18:35     ` Doug Thompson
2008-07-15 19:51 ` [patch 2/9] powerpc/axonram: use only one block device major number arnd
2008-07-15 19:51 ` [patch 3/9] powerpc/axonram: enable partitioning of the Axons DDR2 DIMMs arnd
2008-07-15 19:51 ` [patch 4/9] powerpc/cell/cpufreq: add spu aware cpufreq governor arnd
2008-07-15 19:51 ` [patch 5/9] powerpc/cell: cleanup sysreset_hack for IBM cell blades arnd
2008-07-15 19:51 ` [patch 6/9] powerpc/cell: add support for power button of future " arnd
2008-07-15 19:51 ` [patch 7/9] azfs: initial submit of azfs, a non-buffered filesystem arnd
2008-07-17  6:13   ` Benjamin Herrenschmidt
2008-07-22  9:49   ` Christoph Hellwig [this message]
2008-07-15 19:51 ` [patch 8/9] powerpc/dma: use the struct dma_attrs in iommu code arnd
2008-07-15 19:51 ` [patch 9/9] powerpc/cell: Add DMA_ATTR_STRONG_ORDERING dma attribute and use in IOMMU code arnd
2008-07-15 20:34   ` Roland Dreier
2008-07-15 21:27     ` Arnd Bergmann
2008-07-16  2:18       ` Roland Dreier
2008-07-16  7:54         ` Arnd Bergmann
2008-07-17  6:20           ` [Cbe-oss-dev] " Benjamin Herrenschmidt
2008-07-17 14:53             ` Arnd Bergmann
2008-07-17 20:10               ` Benjamin Herrenschmidt
2008-07-17 20:10               ` Benjamin Herrenschmidt
2008-07-18 13:03                 ` [PATCH] Add DMA_ATTR_WEAK_ORDERING dma attribute and use in Cell " Arnd Bergmann
2008-07-19  7:29                   ` [Cbe-oss-dev] " Jeremy Kerr
2008-07-19  8:36                     ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080722094950.GA3234@lst.de \
    --to=hch@lst.de \
    --cc=arnd@arndb.de \
    --cc=cbe-oss-dev@ozlabs.org \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).