Re: [patch 1/3] add the fsblock layer

2007-06-30 Thread Christoph Hellwig
On Tue, Jun 26, 2007 at 08:26:50AM -0400, Chris Mason wrote: > Since we're testing new code, I would just leave the blkdev address > space alone. If a filesystem wants to use fsblocks, they allocate a new > inode during mount, stuff it into their private super block (or in the > generic super), an

Re: [patch 1/3] add the fsblock layer

2007-06-30 Thread Christoph Hellwig
On Tue, Jun 26, 2007 at 12:34:26PM +1000, Nick Piggin wrote: > That would require a new inode and address_space for the fsblock > type blockdev pagecache, wouldn't it? Yes. That's easily possible, XFS already does it for it's own buffer cache. - To unsubscribe from this list: send the line "unsu

Re: [patch 1/3] add the fsblock layer

2007-06-26 Thread Chris Mason
On Tue, Jun 26, 2007 at 01:07:43PM +1000, Nick Piggin wrote: > Neil Brown wrote: > >On Tuesday June 26, [EMAIL PROTECTED] wrote: > > > >>Chris Mason wrote: > >> > >>>The block device pagecache isn't special, and certainly isn't that much > >>>code. I would suggest keeping it buffer head specific a

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Nick Piggin
Neil Brown wrote: On Tuesday June 26, [EMAIL PROTECTED] wrote: Chris Mason wrote: The block device pagecache isn't special, and certainly isn't that much code. I would suggest keeping it buffer head specific and making a second variant that does only fsblocks. This is mostly to keep the sem

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Neil Brown
On Tuesday June 26, [EMAIL PROTECTED] wrote: > Chris Mason wrote: > > > > The block device pagecache isn't special, and certainly isn't that much > > code. I would suggest keeping it buffer head specific and making a > > second variant that does only fsblocks. This is mostly to keep the > > sema

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Nick Piggin
Chris Mason wrote: On Sun, Jun 24, 2007 at 03:46:13AM +0200, Nick Piggin wrote: Rewrite the buffer layer. Overall, I like the basic concepts, but it is hard to track the locking rules. Could you please write them up? Yeah I will do that. Thanks for taking a look. One thing I am thinking

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Nick Piggin
Chris Mason wrote: On Mon, Jun 25, 2007 at 05:41:58PM +1000, Nick Piggin wrote: Neil Brown wrote: Why do you think you need PG_blocks? Block device pagecache (buffer cache) has to be able to accept attachment of either buffers or blocks for filesystem metadata, and call into either buffer.

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Chris Mason
On Sun, Jun 24, 2007 at 03:46:13AM +0200, Nick Piggin wrote: > Rewrite the buffer layer. Overall, I like the basic concepts, but it is hard to track the locking rules. Could you please write them up? I like the way you split out the assoc_buffers from the main fsblock code, but the list setup is

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Chris Mason
On Mon, Jun 25, 2007 at 05:41:58PM +1000, Nick Piggin wrote: > Neil Brown wrote: > >On Sunday June 24, [EMAIL PROTECTED] wrote: > > > >> > >>+#define PG_blocks 20 /* Page has block mappings */ > >>+ > > > > > >I've only had a very quick look, but this line looks *very* wrong. > >You s

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Andi Kleen
On Sun, Jun 24, 2007 at 01:18:42PM -0700, Arjan van de Ven wrote: > > > Hmm, could define a macro DECLARE_ATOMIC_BITMAP(maxbit) that expands to the > > smallest > > possible type for each architecture. And a couple of ugly casts for set_bit > > et.al. > > but those could be also hidden in macros

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Nick Piggin
Neil Brown wrote: On Sunday June 24, [EMAIL PROTECTED] wrote: +#define PG_blocks 20 /* Page has block mappings */ + I've only had a very quick look, but this line looks *very* wrong. You should be using PG_private. There should never be any confusion about whether ->priva

Re: [patch 1/3] add the fsblock layer

2007-06-25 Thread Nick Piggin
Andi Kleen wrote: Nick Piggin <[EMAIL PROTECTED]> writes: [haven't read everything, just commenting on something that caught my eye] +struct fsblock { + atomic_tcount; + union { + struct { + unsigned long flags; /* XXX: flags could be

Re: [patch 1/3] add the fsblock layer

2007-06-24 Thread Neil Brown
On Sunday June 24, [EMAIL PROTECTED] wrote: > > +#define PG_blocks20 /* Page has block mappings */ > + I've only had a very quick look, but this line looks *very* wrong. You should be using PG_private. There should never be any confusion about whether ->private has buffers or b

Re: [patch 1/3] add the fsblock layer

2007-06-24 Thread Arjan van de Ven
> Hmm, could define a macro DECLARE_ATOMIC_BITMAP(maxbit) that expands to the > smallest > possible type for each architecture. And a couple of ugly casts for set_bit > et.al. > but those could be also hidden in macros. Should be relatively easy to do. or make a "smallbit" type that is small/su

Re: [patch 1/3] add the fsblock layer

2007-06-24 Thread Andi Kleen
Nick Piggin <[EMAIL PROTECTED]> writes: [haven't read everything, just commenting on something that caught my eye] > +struct fsblock { > + atomic_tcount; > + union { > + struct { > + unsigned long flags; /* XXX: flags could be int for > better p