Hi!
On Thu, 29 Sep 2011 11:44:40 +0200, Sergio López <sl...@sinrega.org> wrote:
> Clearing MAY_CACHE flag on a pager initiates a memory object
> termination if this one is not referenced anymore. If the object has a
> significant number of dirty pages (i.e. a file recently created was
> unlinked before diskfs periodical sync) this operation generates a lot
> of stress on the translator. This is one of the most common sources
> for thread storms.
>
> Sync'ing the pager before clearing that flag ensures that there aren't
> dirty pages in the object before its termination.
>
>
> * ext2fs/pager.c (drop_pager_softrefs): Sync pager before clearing
> MAY_CACHE flag.
>
> ---
> ext2fs/pager.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/ext2fs/pager.c b/ext2fs/pager.c
> index 082537c..89a1b71 100644
> --- a/ext2fs/pager.c
> +++ b/ext2fs/pager.c
> @@ -851,7 +851,10 @@ drop_pager_softrefs (struct node *node)
> spin_unlock (&node_to_page_lock);
>
> if (MAY_CACHE && pager)
> - pager_change_attributes (pager, 0, MEMORY_OBJECT_COPY_DELAY, 0);
> + {
> + pager_sync (pager, 1);
> + pager_change_attributes (pager, 0, MEMORY_OBJECT_COPY_DELAY, 0);
> + }
> if (pager)
> ports_port_deref (pager);
> }First, thanks for this! (But I guess you don't happen to have hard data on the effectiveness?) Shouldn't the same change be made in the other libdiskfs translators, too: fatfs, isofs, ufs? (Of course, fatfs and isofs are read-only at the moment, and nobody, really nobody uses ufs, but it's still good to keep the sources in sync.) (Feel free to commit such a change for yourself.) And, shouldn't the commit message in fact be a comment before the pager_sync invocation, for the next human reading this code? (My rationale still is is that source code comments should describe the existing code where it helps for understanding. Here I as a reader would wonder why there is an explicit synchronization, and searching for that information in the commit log is one step too much.) (Feel free to commit such a change for yourself.) Grüße, Thomas
pgp60JtkCQPHI.pgp
Description: PGP signature
