[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Fwd: [patch #2508] ext2fs support for large store (> 1.5G)]

From: Ognyan Kulev
Subject: Re: [Fwd: [patch #2508] ext2fs support for large store (> 1.5G)]
Date: Wed, 11 Feb 2004 22:22:00 +0200
User-agent: Mozilla Thunderbird 0.5 (X11/20040209)

Marco Gerards wrote:
Something I have noticed is that there is a disk_cache_blocks. I do
not understand why you have that. You can easily map in every page and
let GNUMach decide to evict the page (like Neal proposed). But before
I can say more about it I can better wait for your reply.

Do you mean to map (all metadata at fixed location) and (all indirect blocks), and to not care about reusing pages for different blocks?

The same is true for pokels. They are there to keep track of all th
metadata in memory so you can write it easely to disk when syncing. I
assume you have datastructures for your cache that hold this
information as well, do you still need the pokel stuff? Well, perhaps
I am misunderstanding something about pokels or your patch (because I
do not fully understand it yet, give me a bit more time for that :)).

I'm writing description of the patch, but I'm still at introduction yet. Anyway, you can see part of the style of the description in http://debian.fmi.uni-sofia.bg/~ogi/hurd/ext3fs/ext2fs_20040211.txt . (This link is temporary.) I hope to finish it in couple of days. And again: currently there isn't much information this text.

Some things I have noticed so far:

      /* XXX: Touch all pages.  It seems that sometimes GNU Mach
         "forgets" to notify us about evicted pages.  */
      for (vm_offset_t i = 0; i < disk_cache_size; i += vm_page_size)
        *(volatile char *)(disk_cache + i);
Why do you need that? What happens when it isn't used?

It's just that: GNU Mach is instructed to notify us about evicted pages, but it doesn't. This is not rare case. The code above is work-around. It first touches all pages and then synces them all. Fortunately this seems to always work.

And doesn't this mean al pages get dirty??

With small cache, after some time, yes. Some pages are evicted, but Mach doesn't tell us. And the count of these pages increases slowly.

In disk_cache_map:

  bptr = ihash_find (disk_cache_bptr, block);
  if (bptr)
      return bptr;

Personally I would use "return 0" here.

"if (bptr)" means that bptr != 0, so the code is correct.

In the same function I see this code:

      if (pending_end >= 0)
        pager_return_some (diskfs_disk_pager,
                           pending_begin * vm_page_size,
                           (pending_end - pending_begin) * vm_page_size,
        printf ("ext2fs: disk cache is starving\n");

      /* Give it some time.  This should happen rarely.  */
      sleep (1);

I'm not that happy with this.  Can you explain when it happens (I
guess when you run out of cache?) and how sleeping fixes it.

The "starving" happens when all blocks are in use. That is, each block has reference count > 0 and it can't be replaced. So the best we can hope of is to wait for some other thread to release block.

The non-starving case is when there are some blocks with reference count == 0, but none of them is evicted. So the "for" is to sync and flush all these potential candidates for replacement. Although pager_return_some (..., 1) (sync+flush with waiting) is used, sleep (1) is executed again. In theory, this is not needed. I'll remove it (for the non-starving case) in RC2 and test it.

I see some function calls that is enabled in the debugging
code. Please make sure the code works the same when debugging is of
like it works when it is on. (So, have you tested with debugging off?)

I don't remember if I try it with debugging off. So this is one more note before releasing RC2 :-)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]