reiserfs-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Edward Shishkin <edward.shishkin@gmail.com>
To: Ivan Shapovalov <intelfx100@gmail.com>, reiserfs-devel@vger.kernel.org
Subject: Re: [PATCH 2/2] reiser4: block_alloc: get rid of discard-related hack in reiser4_dealloc_blocks().
Date: Sun, 31 Aug 2014 23:19:10 +0200	[thread overview]
Message-ID: <5403914E.2050605@gmail.com> (raw)
In-Reply-To: <1408361471-1488-3-git-send-email-intelfx100@gmail.com>


On 08/18/2014 01:31 PM, Ivan Shapovalov wrote:
> When discard was enabled, immediate deallocations were made deferred in order
> to record these extents in the atom's delete set and, consequently, allow
> their discarding.
>
> However, this is wrong in the following way. Immediate deallocations make use
> of target allocation stage and ancillary flags like BA_PERMANENT and BA_FORMATTED.
> By converting immediate deallocations to deferred, these flags are essentially
> dropped, and application of deferred deallocations in reiser4_post_write_back_hook()
> uses an equivalent of BLOCK_NOT_COUNTED stage and BA_FORMATTED flag.
>
> Dropping this hack does not hinder efficiency of the discard procedure, because
> immediate deallocations are now used only to deallocate "just allocated" and not
> yet written blocks, which actually do not need to be discarded.


The idea to defer every "successful" deallocation regardless of discard
support status looks OK, but I don't like the first patch (1/2), so 
let's try
to improve it..

So, we want to defer also deallocations in the following 2 places:
1) in  dealloc_tx_list();
2) in dealloc_wmap_actor().

Here we should take care about block stages.
Let's take a look where and how they were allocated. There are exactly
2 places:

a) in get_more_wandered_blocks() with block_stage = BLOCK_GRABBED;
b) in alloc_tx() with block_stage = BLOCK_GRABBED.

Note, that alloc_blocks() with the stage BLOCK_GRABBED calls
grabbed2used(). This is exactly what we want in apply_dset() called by
post_write_back_hook.

That is, just adding BA_DEFER to (1) and (2) with the patch 2/2 should work.
Please, make sure..

P.S. In (1)-(2) we allocate with stage BLOCK_GRABBED, but in (a)-(b) we
deallocate with stage BLOCK_NOT_COUNTED. It can confuse. Let's allocate
with stage BLOCK_NOT_COUNTED: from the standpoint of alloc_blocks() it
doesn't matter.

Thanks.

> Signed-off-by: Ivan Shapovalov <intelfx100@gmail.com>
> ---
>   fs/reiser4/block_alloc.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/reiser4/block_alloc.c b/fs/reiser4/block_alloc.c
> index 324b11c..33ca86c 100644
> --- a/fs/reiser4/block_alloc.c
> +++ b/fs/reiser4/block_alloc.c
> @@ -1008,8 +1008,7 @@ reiser4_dealloc_blocks(const reiser4_block_nr * start,
>   		spin_unlock_reiser4_super(sbinfo);
>   	}
>   
> -	if ((flags & BA_DEFER) ||
> -	    reiser4_is_set(reiser4_get_current_sb(), REISER4_DISCARD)) {
> +	if (flags & BA_DEFER) {
>   		/* store deleted block numbers in the atom's deferred delete set
>   		   for further actual deletion */
>   		do {


  reply	other threads:[~2014-08-31 21:19 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-18 11:31 [PATCH 0/2] reiser4: block deallocation fixes Ivan Shapovalov
2014-08-18 11:31 ` [PATCH 1/2] reiser4: sanitize deallocations throughout the code Ivan Shapovalov
2014-08-31 11:24   ` Edward Shishkin
2014-09-01 13:24     ` Ivan Shapovalov
2014-08-18 11:31 ` [PATCH 2/2] reiser4: block_alloc: get rid of discard-related hack in reiser4_dealloc_blocks() Ivan Shapovalov
2014-08-31 21:19   ` Edward Shishkin [this message]
2014-09-06 10:33     ` [PATCH 2/2] reiser4: block_alloc: get rid of discard-related hack in reiser4_dealloc_blocks()., Ivan Shapovalov
2014-09-07 12:06       ` Edward Shishkin

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=5403914E.2050605@gmail.com \
    --to=edward.shishkin@gmail.com \
    --cc=intelfx100@gmail.com \
    --cc=reiserfs-devel@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).