public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Daniel Phillips <phillips@bonn-fries.net>
To: Andreas Dilger <adilger@turbolinux.com>
Cc: Andreas Dilger <adilger@turbolinux.com>,
	Alexander Viro <viro@math.psu.edu>,
	Linux kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH][CFT] (updated) ext2 directories in pagecache
Date: Mon, 14 May 2001 21:29:34 +0200	[thread overview]
Message-ID: <01051421293401.14979@starship> (raw)
In-Reply-To: <200105141833.f4EIXrQs001765@webber.adilger.int>
In-Reply-To: <200105141833.f4EIXrQs001765@webber.adilger.int>

On Monday 14 May 2001 20:33, Andreas Dilger wrote:
> Danie, you write:
> > This can go in ext2_bread, which already has dir-specific code in
> > it (readahead), and ext2_getblk remains generic, for what it's
> > worth.
>
> Note that the dir-specific code in ext2_bread() is not readahead, but
> rather directory block pre-allocation,

oops, yes that's what I meant.

> which would totally break
> indexed directory code (because the empty blocks would not be put
> into the index and would remain unreferenced thereafter).

The preallocation just does ext2_getblk, it doesn't increase i_size.  
The indexing code relies on i_size to append a new block, so this 
should work ok.

> For that matter, I'm not sure whether the dir-prealloc feature even
> works. The blocks are created/allocated, but are not initialized with
> the empty dirent data (i.e. set rec_len = blocksize), so it would
> just create a corrupt directory block.

I guess the lifetime of these blocks is the same as the lifetime of the 
directory's dcache entry, but I have to go spelunking to be sure.  If 
that's right then it's probably a good feature.  Having speculated 
about it, I should test this and see what happens.   

> > > ...(later)... I had a look at pulling out the ext2_check_page()
> > > code so that it can be used for both ext2_get_page() and
> > > ext2_bread(), but one thing concerns me - if we do checking on
> > > the whole page/buffer at once, then any error in the page/buffer
> > > will discard all of the dir entries in that page.  In the old
> > > code, we would at least return all of the entries up to the bad
> > > dir entry.  Comments on this?
> >
> > For a completely empty page that's the right thing to do, much
> > better than hanging as it does now.  We don't want to try to repair
> > damage on the fly do we?
>
> What do you mean by hanging?  You refer to new (indexed) code or old
> code?

I mean the new code, which just loops forever if it gets a zero 
rec_len.  It would be nice to get rid of this fragility without putting 
an extra check in the inner loop.  I think it's worth the effort, we 
are still cycling linearly through an average of roughly 128 directory 
entires on every directory operation.  I'll try fixing this by method 
related to what Al does, except instead of using the page flag, the 
place to do it is where the block gets mapped.

[...] No repair on the fly: Ack [...]

> > Now, if the check routine tells us how much good data it found we
> > could use that to set a limit for the dirent scan, thus keeping the
> > same robustness as the old code but without having all the checks
> > in the inner loop.  Or.  We could have separate loops for good
> > blocks and bad blocks, it's just a very small amount of code.
>
> Yes, I was thinking about both of those as well.  I think the latter
> would be easiest, because we only need to keep a single error bit per
> buffer.

Heh.  I was going to try the other one first.

By the way, all the code for the directory link limit enhancement was 
in fact in your last patch, and is included in the patch I uploaded 
yesterday.  I haven't tested it.  It would be nice to know how fast we 
can create 1,000,000 empty directories :-)

I noticed that Postfix creates an elaborate directory bucket structure 
that will be completely obsolete when the directory indexing becomes 
generally available.

--
Daniel

  reply	other threads:[~2001-05-14 19:34 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-05-11  7:10 [PATCH][CFT] (updated) ext2 directories in pagecache Andreas Dilger
2001-05-11  7:19 ` Alexander Viro
2001-05-11 16:34   ` Andreas Dilger
2001-05-11 20:20     ` Daniel Phillips
2001-05-12 21:41       ` Andreas Dilger
2001-05-12 22:18         ` Alexander Viro
2001-05-13  2:13           ` Daniel Phillips
2001-05-13  2:34         ` Daniel Phillips
2001-05-14 18:33           ` Andreas Dilger
2001-05-14 19:29             ` Daniel Phillips [this message]
2001-05-14 21:50             ` Daniel Phillips
2001-05-16  3:11               ` Daniel Phillips
2001-05-11 13:02 ` Daniel Phillips
  -- strict thread matches above, loose matches on Subject: below --
2001-05-10 20:53 Andreas Dilger
2001-05-11  1:10 ` Daniel Phillips
2001-05-10  7:21 Andreas Dilger
2001-05-13 22:15 ` Daniel Phillips
2001-05-14 20:04   ` Andreas Dilger
2001-05-14 22:18     ` Daniel Phillips
2001-05-14 22:23     ` Daniel Phillips
2001-05-06 23:16 Daniel Phillips
2001-05-09 21:22 ` Andreas Dilger
2001-05-11  1:04   ` Daniel Phillips
2001-05-03 21:10 Daniel Phillips
2001-05-03 22:59 ` Albert Cranford
     [not found] <01050303150500.00633@starship>
2001-05-03  1:43 ` Daniel Phillips
2001-04-29 20:35 Daniel Phillips
2001-05-02  3:03 ` Albert Cranford
2001-04-12 16:33 [PATCH][CFT] " Alexander Viro
2001-04-23 22:21 ` [PATCH][CFT] (updated) " Alexander Viro
2001-04-28 18:16   ` Alexander Viro

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=01051421293401.14979@starship \
    --to=phillips@bonn-fries.net \
    --cc=adilger@turbolinux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@math.psu.edu \
    /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