All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.