linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 8/9] btrfs: qgroup: Introduce extent changeset for qgroup reserve functions
Date: Mon, 10 Apr 2017 16:00:25 +0200	[thread overview]
Message-ID: <20170410140025.GZ4781@suse.cz> (raw)
In-Reply-To: <e97cbe28-1333-22bc-357b-570b4e865ec5@cn.fujitsu.com>

On Mon, Apr 10, 2017 at 09:25:18AM +0800, Qu Wenruo wrote:
> 
> 
> At 04/07/2017 08:00 PM, David Sterba wrote:
> > On Mon, Mar 13, 2017 at 03:52:15PM +0800, Qu Wenruo wrote:
> >> @@ -3355,12 +3355,14 @@ static int cache_save_setup(struct btrfs_block_group_cache *block_group,
> >>  	struct btrfs_fs_info *fs_info = block_group->fs_info;
> >>  	struct btrfs_root *root = fs_info->tree_root;
> >>  	struct inode *inode = NULL;
> >> +	struct extent_changeset data_reserved;
> >
> > Size of this structure is 40 bytes, and it's being added to many
> > functions. This will be noticeable on the stack consumption.
> >
> > -extent-tree.c:btrfs_check_data_free_space              40 static
> > -extent-tree.c:cache_save_setup                         96 static
> > +extent-tree.c:btrfs_check_data_free_space              48 static
> > +extent-tree.c:cache_save_setup                         136 static
> > -file.c:__btrfs_buffered_write                          192 static
> > +file.c:__btrfs_buffered_write                          232 static
> > -file.c:btrfs_fallocate                                 208 static
> > +file.c:btrfs_fallocate                                 248 static
> > -inode-map.c:btrfs_save_ino_cache                       112 static
> > +inode-map.c:btrfs_save_ino_cache                       152 static
> > -inode.c:btrfs_direct_IO                                128 static
> > +inode.c:btrfs_direct_IO                                176 static
> > -inode.c:btrfs_writepage_fixup_worker                   88 static
> > +inode.c:btrfs_writepage_fixup_worker                   128 static
> > -inode.c:btrfs_truncate_block                           136 static
> > +inode.c:btrfs_truncate_block                           176 static
> > -inode.c:btrfs_page_mkwrite                             112 static
> > +inode.c:btrfs_page_mkwrite                             152 static
> > +ioctl.c:cluster_pages_for_defrag                       200 static
> > -ioctl.c:btrfs_defrag_file                              312 static
> > +ioctl.c:btrfs_defrag_file                              232 static
> > -qgroup.c:btrfs_qgroup_reserve_data                     136 static
> > +qgroup.c:btrfs_qgroup_reserve_data                     128 static
> > -relocation.c:prealloc_file_extent_cluster              152 static
> > +relocation.c:prealloc_file_extent_cluster              192 static
> >
> > There are generic functions so this will affect non-qgroup workloads as
> > well. So there need to be a dynamic allocation (which would add another
> > point of failure), or reworked in another way.
> 
> Well, I'll try to rework this to allocate extent_changeset inside 
> btrfs_qgroup_reserve_data(), so that callers only need extra 8 bytes 
> stack space, and no need to introduce extra error handlers.

So this still requires the dynamic allocation, on various call paths,
that's what I object against most at the moment.

  reply	other threads:[~2017-04-10 14:00 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-13  7:52 [PATCH v2 0/9] Qgroup fixes Qu Wenruo
2017-03-13  7:52 ` [PATCH v2 1/9] btrfs: qgroup: Add trace point for qgroup reserved space Qu Wenruo
2017-03-30 12:49   ` David Sterba
2017-03-13  7:52 ` [PATCH v2 2/9] btrfs: qgroup: Re-arrange tracepoint timing to co-operate with reserved space tracepoint Qu Wenruo
2017-03-30 13:11   ` David Sterba
2017-03-13  7:52 ` [PATCH v2 3/9] btrfs: qgroup: Fix qgroup corruption caused by inode_cache mount option Qu Wenruo
2017-04-07 11:05   ` David Sterba
2017-03-13  7:52 ` [PATCH v2 4/9] btrfs: qgroup: Add quick exit for non-fs extents Qu Wenruo
2017-03-13  7:52 ` [PATCH v2 5/9] btrfs: qgroup: Cleanup btrfs_qgroup_prepare_account_extents function Qu Wenruo
2017-03-13  7:52 ` [PATCH v2 6/9] btrfs: qgroup: Return actually freed bytes for qgroup release or free data Qu Wenruo
2017-03-30 13:08   ` David Sterba
2017-03-13  7:52 ` [PATCH v2 7/9] btrfs: qgroup: Fix qgroup reserved space underflow caused by buffered write and quota enable Qu Wenruo
2017-03-13  7:52 ` [PATCH v2 8/9] btrfs: qgroup: Introduce extent changeset for qgroup reserve functions Qu Wenruo
2017-04-07 12:00   ` David Sterba
2017-04-10  1:25     ` Qu Wenruo
2017-04-10 14:00       ` David Sterba [this message]
2017-04-10 14:14         ` David Sterba
2017-04-11  1:44           ` Qu Wenruo
2017-05-05 17:24             ` David Sterba
2017-03-13  7:52 ` [PATCH v2 9/9] btrfs: qgroup: Fix qgroup reserved space underflow by only freeing reserved ranges Qu Wenruo
2017-04-07 12:05   ` David Sterba

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=20170410140025.GZ4781@suse.cz \
    --to=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo@cn.fujitsu.com \
    /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).