Re: [PATCH 3/3] btrfs: document extent mapping assumptions in checksum

2018-11-27 Thread Noah Massey 
On Tue, Nov 27, 2018 at 2:32 PM Nikolay Borisov  wrote:
>
> On 27.11.18 г. 21:08 ч., Noah Massey wrote:
> > On Tue, Nov 27, 2018 at 11:43 AM Nikolay Borisov  wrote:
> >>
> >> On 27.11.18 г. 18:00 ч., Johannes Thumshirn wrote:
> >>> Document why map_private_extent_buffer() cannot return '1' (i.e. the map
> >>> spans two pages) for the csum_tree_block() case.
> >>>
> >>> The current algorithm for detecting a page boundary crossing in
> >>> map_private_extent_buffer() will return a '1' *IFF* the product of the
> >>
> >> I think the word product must be replaced with 'sum', since product
> >> implies multiplication :)
> >>
> >
> > doesn't 'sum' imply addition? How about 'output'?
>
> It does and the code indeed sums the value and not multiply them hence
> my suggestion.
>

I'm sorry, I didn't phrase that well.

Since 'sum' already implies addition, it gets confusing with the
mathematical operators used later in the description. So, if a
objective noun is required, a generic term such as 'output' or
'result' reads more cleanly for me. OTOH, dropping that and creating
an actual expression

*IFF* the extent buffer's offset in the page + the offset passed in by
csum_tree_block() + the minimal length passed in by csum_tree_block()
- 1 > PAGE_SIZE.

is also straightforward.

Re: [PATCH 3/3] btrfs: document extent mapping assumptions in checksum

2018-11-27 Thread Noah Massey 
On Tue, Nov 27, 2018 at 11:43 AM Nikolay Borisov  wrote:
>
> On 27.11.18 г. 18:00 ч., Johannes Thumshirn wrote:
> > Document why map_private_extent_buffer() cannot return '1' (i.e. the map
> > spans two pages) for the csum_tree_block() case.
> >
> > The current algorithm for detecting a page boundary crossing in
> > map_private_extent_buffer() will return a '1' *IFF* the product of the
>
> I think the word product must be replaced with 'sum', since product
> implies multiplication :)
>

doesn't 'sum' imply addition? How about 'output'?

> > extent buffer's offset in the page + the offset passed in by
> > csum_tree_block() and the minimal length passed in by csum_tree_block() - 1
> > are bigger than PAGE_SIZE.
> >
> > We always pass BTRFS_CSUM_SIZE (32) as offset and a minimal length of 32
> > and the current extent buffer allocator always guarantees page aligned
> > extends, so the above condition can't be true.
> >
> > Signed-off-by: Johannes Thumshirn 
>
> With that wording changed:
>
> Reviewed-by: Nikolay Borisov 
>
> > ---
> >  fs/btrfs/disk-io.c | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 4bc270ef29b4..14d355d0cb7a 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -279,6 +279,12 @@ static int csum_tree_block(struct btrfs_fs_info 
> > *fs_info,
> >
> >   len = buf->len - offset;
> >   while (len > 0) {
> > + /*
> > +  * Note: we don't need to check for the err == 1 case here, as
> > +  * with the given combination of 'start = BTRFS_CSUM_SIZE 
> > (32)'
> > +  * and 'min_len = 32' and the currently implemented mapping
> > +  * algorithm we cannot cross a page boundary.
> > +  */
> >   err = map_private_extent_buffer(buf, offset, 32,
> >   , _start, _len);
> >   if (err)
> >

Re: 14Gb of space lost after distro upgrade on BTFS root partition (long thread with logs)

2018-08-28 Thread Noah Massey 
On Tue, Aug 28, 2018 at 1:25 PM Menion  wrote:
>
> Ok, I have removed the snapshot and the free expected space is here, thank 
> you!
> As a side note: apt-btrfs-snapshot was not installed, but it is
> present in Ubuntu repository and I have used it (and I like the idea
> of automatic snapshot during upgrade)
> This means that the do-release-upgrade does it's own job on BTRFS,
> silently which I believe is not good from the usability perspective,

You are correct. DistUpgradeController.py from python3-distupgrade
imports 'apt_btrfs_snapshot', which I read as coming from
/usr/lib/python3/dist-packages/apt_btrfs_snapshot.py, supplied by
apt-btrfs-snapshot, but I missed the fact that python3-distupgrade
ships its own /usr/lib/python3/dist-packages/DistUpgrade/apt_btrfs_snapshot.py

So now it looks like that cannot be easily disabled, and without the
apt-btrfs-snapshot package scheduling cleanups it's not ever
automatically removed?

> just google it, there is no mention of this behaviour
> Il giorno mar 28 ago 2018 alle ore 19:07 Austin S. Hemmelgarn
>  ha scritto:
> >
> > On 2018-08-28 12:05, Noah Massey wrote:
> > > On Tue, Aug 28, 2018 at 11:47 AM Austin S. Hemmelgarn
> > >  wrote:
> > >>
> > >> On 2018-08-28 11:27, Noah Massey wrote:
> > >>> On Tue, Aug 28, 2018 at 10:59 AM Menion  wrote:
> > >>>>
> > >>>> [sudo] password for menion:
> > >>>> ID  gen top level   path
> > >>>> --  --- -   
> > >>>> 257 600627  5   /@
> > >>>> 258 600626  5   /@home
> > >>>> 296 599489  5
> > >>>> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:29:55
> > >>>> 297 599489  5
> > >>>> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:30:08
> > >>>> 298 599489  5
> > >>>> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:33:30
> > >>>>
> > >>>> So, there are snapshots, right? The time stamp is when I have launched
> > >>>> do-release-upgrade, but it didn't ask anything about snapshot, neither
> > >>>> I asked for it.
> > >>>
> > >>> This is an Ubuntu thing
> > >>> `apt show apt-btrfs-snapshot`
> > >>> which "will create a btrfs snapshot of the root filesystem each time
> > >>> that apt installs/removes/upgrades a software package."
> > >> Not Ubuntu, Debian.  It's just that Ubuntu installs and configures the
> > >> package by default, while Debian does not.
> > >
> > > Ubuntu also maintains the package, and I did not find it in Debian 
> > > repositories.
> > > I think it's also worth mentioning that these snapshots were created
> > > by the do-release-upgrade script using the package directly, not as a
> > > result of the apt configuration. Meaning if you do not want a snapshot
> > > taken prior to upgrade, you have to remove the apt-btrfs-snapshot
> > > package prior to running the upgrade script. You cannot just update
> > > /etc/apt/apt.conf.d/80-btrfs-snapshot
> > Hmm... I could have sworn that it was in the Debian repositories.
> >
> > That said, it's kind of stupid that the snapshot is not trivially
> > optional for a release upgrade.  Yes, that's where it's arguably the
> > most important, but it's still kind of stupid to have to remove a
> > package to get rid of that behavior and then reinstall it again afterwards.

Re: 14Gb of space lost after distro upgrade on BTFS root partition (long thread with logs)

2018-08-28 Thread Noah Massey 
On Tue, Aug 28, 2018 at 11:47 AM Austin S. Hemmelgarn
 wrote:
>
> On 2018-08-28 11:27, Noah Massey wrote:
> > On Tue, Aug 28, 2018 at 10:59 AM Menion  wrote:
> >>
> >> [sudo] password for menion:
> >> ID  gen top level   path
> >> --  --- -   
> >> 257 600627  5   /@
> >> 258 600626  5   /@home
> >> 296 599489  5
> >> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:29:55
> >> 297 599489  5
> >> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:30:08
> >> 298 599489  5
> >> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:33:30
> >>
> >> So, there are snapshots, right? The time stamp is when I have launched
> >> do-release-upgrade, but it didn't ask anything about snapshot, neither
> >> I asked for it.
> >
> > This is an Ubuntu thing
> > `apt show apt-btrfs-snapshot`
> > which "will create a btrfs snapshot of the root filesystem each time
> > that apt installs/removes/upgrades a software package."
> Not Ubuntu, Debian.  It's just that Ubuntu installs and configures the
> package by default, while Debian does not.

Ubuntu also maintains the package, and I did not find it in Debian repositories.
I think it's also worth mentioning that these snapshots were created
by the do-release-upgrade script using the package directly, not as a
result of the apt configuration. Meaning if you do not want a snapshot
taken prior to upgrade, you have to remove the apt-btrfs-snapshot
package prior to running the upgrade script. You cannot just update
/etc/apt/apt.conf.d/80-btrfs-snapshot

>
> This behavior in general is not specific to Debian either, a lot of
> distributions are either working on or already have this type of
> functionality, because it's the only sane and correct way to handle
> updates short of rebuilding the entire system from scratch.

Yup. Everyone in their own way, plus all the home-brews.

Re: 14Gb of space lost after distro upgrade on BTFS root partition (long thread with logs)

2018-08-28 Thread Noah Massey 
On Tue, Aug 28, 2018 at 10:59 AM Menion  wrote:
>
> [sudo] password for menion:
> ID  gen top level   path
> --  --- -   
> 257 600627  5   /@
> 258 600626  5   /@home
> 296 599489  5
> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:29:55
> 297 599489  5
> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:30:08
> 298 599489  5
> /@apt-snapshot-release-upgrade-bionic-2018-08-27_15:33:30
>
> So, there are snapshots, right? The time stamp is when I have launched
> do-release-upgrade, but it didn't ask anything about snapshot, neither
> I asked for it.

This is an Ubuntu thing
`apt show apt-btrfs-snapshot`
which "will create a btrfs snapshot of the root filesystem each time
that apt installs/removes/upgrades a software package."

> During the do-release-upgrade I got some issues due to the (very) bad
> behaviour of the script in remote terminal, then I have fixed
> everything manually and now the filesystem is operational in bionic
> version
> If it is confirmed, how can I remove the unwanted snapshot, keeping
> the current "visible" filesystem contents

By default, the package runs a weekly cron job to cleanup old
snapshots. (Defaults to 90d, but you can configure that in
APT::Snapshots::MaxAge) Alternatively, you can cleanup with the
command yourself. Run `sudo apt-btrfs-snapshot list`, and then `sudo
apt-btrfs-snapshot delete `

~ Noah

Re: [PATCH v2] btrfs: Add graceful handling of V0 extents

2018-06-27 Thread Noah Massey 
On Tue, Jun 26, 2018 at 12:02 PM David Sterba  wrote:
>
> On Tue, Jun 26, 2018 at 04:57:36PM +0300, Nikolay Borisov wrote:
> > Following the removal of the v0 handling code let's be courteous and
> > print an error message when such extents are handled. In the cases
> > where we have a transaction just abort it, otherwise just call
> > btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
> >
> > Signed-off-by: Nikolay Borisov 
>
> > - ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
> > - if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> > + if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> > + err = -EINVAL;
> > + btrfs_print_v0_err(rc->extent_root->fs_info);
> > + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> > +   NULL);
> > + goto out;
> > + } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
>
> The v0 check should be made last as it's not expected to happen. I'm
> commiting with this diff
>
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -787,13 +787,7 @@ struct backref_node *build_backref_tree(struct 
> reloc_control *rc,
> goto next;
> }
>
> -   if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> -   err = -EINVAL;
> -   btrfs_print_v0_err(rc->extent_root->fs_info);
> -   btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> - NULL);
> -   goto out;
> -   } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> +   if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> if (key.objectid == key.offset) {
> /*
>  * only root blocks of reloc trees use
> @@ -838,6 +832,12 @@ struct backref_node *build_backref_tree(struct 
> reloc_control *rc,
> goto next;
> } else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
> goto next;

The V0 check needs to be before this one

> +   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> +   err = -EINVAL;
> +   btrfs_print_v0_err(rc->extent_root->fs_info);
> +   btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> + NULL);
> +   goto out;
> }
>
> /* key.type == BTRFS_TREE_BLOCK_REF_KEY */
> @@ -3734,11 +3734,7 @@ int add_data_references(struct reloc_control *rc,
> if (key.objectid != extent_key->objectid)
> break;
>
> -   if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> -   btrfs_print_v0_err(eb->fs_info);
> -   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> -   ret = -EINVAL;
> -   } else if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
> +   if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
> ret = __add_tree_block(rc, key.offset, blocksize,
>blocks);
> } else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
> @@ -3746,6 +3742,10 @@ int add_data_references(struct reloc_control *rc,
>   struct btrfs_extent_data_ref);
> ret = find_data_references(rc, extent_key,
>eb, dref, blocks);
> +   } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> +   btrfs_print_v0_err(eb->fs_info);
> +   btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> +   ret = -EINVAL;
> } else {
> ret = 0;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: Add more details while checking tree block

2018-06-22 Thread Noah Massey 
On Fri, Jun 22, 2018 at 12:32 PM Hans van Kranenburg
 wrote:
>
> On 06/22/2018 06:25 PM, Nikolay Borisov wrote:
> >
> >
> > On 22.06.2018 19:17, Su Yue wrote:
> >>
> >>
> >>
> >>> Sent: Friday, June 22, 2018 at 11:26 PM
> >>> From: "Hans van Kranenburg" 
> >>> To: "Nikolay Borisov" , "Su Yue" 
> >>> , linux-btrfs@vger.kernel.org
> >>> Subject: Re: [PATCH] btrfs: Add more details while checking tree block
> >>>
> >>> On 06/22/2018 01:48 PM, Nikolay Borisov wrote:
> 
> 
>  On 22.06.2018 04:52, Su Yue wrote:
> > For easier debug, print eb->start if level is invalid.
> > Also make print clear if bytenr found is not expected.
> >
> > Signed-off-by: Su Yue 
> > ---
> >  fs/btrfs/disk-io.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index c3504b4d281b..a90dab84f41b 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -615,8 +615,8 @@ static int btree_readpage_end_io_hook(struct 
> > btrfs_io_bio *io_bio,
> >
> >   found_start = btrfs_header_bytenr(eb);
> >   if (found_start != eb->start) {
> > - btrfs_err_rl(fs_info, "bad tree block start %llu %llu",
> > -  found_start, eb->start);
> > + btrfs_err_rl(fs_info, "bad tree block start want %llu have 
> > %llu",
> 
>  nit: I'd rather have the want/have in brackets (want %llu have% llu)
> >>>
> >>> From a user support point of view, this text should really be improved.
> >>> There are a few places where 'want' and 'have' are reported in error
> >>> strings, and it's totally unclear what they mean.
> >>>
> >> Yes. The strings are too concise for users to understand errors.
> >> Developers always prefer to avoid "unnecessary" words in logs.
> >>
> >>> Intuitively I'd say when checking a csum, the "want" would be what's on
> >>> disk now, since you want that to be correct, and the "have" would be
> >>> what you have calculated, but it's actually the other way round, or
> >>> wasn't it? Or was it?
> >> For csum, you are right at all.
> >>
> >> In this situation, IIRC bytenr of "have" is read from disk.
> >> Bytenr of "want" is assigned while constructing eb, from parent ptr
> >> , root item and other things which are also read from disk.
> >>>
> >>> Every time someone pastes such a message when we help on IRC for
> >>> example, there's confusion, and I have to look up the source again,
> >>> because I always forget.
> >>
> >> Me too.
> >>>
> >>> What about (%llu stored on disk, %llu calculated now) or something 
> >>> similar?
> >
> > Wha tabout expected got
>
> No, that's just as horrible as want and found.
>
> Did you 'expect' the same checksum to be on disk disk as what you 'get'
> when calculating one? Or did you 'expect' the same thing after
> calculation as what you 'got' when reading from disk?
>
> /o\
>

So it's "read | archived | recorded" versus "calculated | generated |
computed" ?

If we want to be succinct, it's "then" versus "now", but that's not as
descriptive.

~ Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: send ioctl failed with -2

2018-05-08 Thread Noah Massey 
On Tue, May 8, 2018 at 1:44 PM, Nicolas Porcel
 wrote:
> Hello,
>
> When I do a btrfs send, I get the following error code:
>
> # btrfs send mysnapshot > /dev/null
> ERROR: send ioctl failed with -2: No such file or directory
>

Silly question, but is 'mysnapshot' actually accessible at './mysnapshot' ?
Because the output from btrfs send should immediately output
'At subvol mysnapshot'

> I get it after 70-100% of the bytes are sent.
>

If 'btrfs send' is actually generating output, 'btrfs receive -vv' may
help "parse" the send stream enough to figure out where it is
terminating.

~ Noah

ps - I was going to suggest 'btrfs send -v'. According the the
manpage, that would "enable verbose output, print generated commands
in a readable form". But it does not seem to be working for me, and
after a quick glance at the code I'm not seeing how the ioctl call is
setting up any kind of verbose feedback. So that may be out-of-date
documentation.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] btrfs: qgroups, fix rescan worker running races

2018-04-27 Thread Noah Massey 
On Fri, Apr 27, 2018 at 11:56 AM, David Sterba  wrote:
> On Thu, Apr 26, 2018 at 03:23:49PM -0400, je...@suse.com wrote:
>> From: Jeff Mahoney 
>>
>> Commit d2c609b834d6 (Btrfs: fix qgroup rescan worker initialization)
>> fixed the issue with BTRFS_IOC_QUOTA_RESCAN_WAIT being racy, but
>> ended up reintroducing the hang-on-unmount bug that the commit it
>> intended to fix addressed.
>>
>> The race this time is between qgroup_rescan_init setting
>> ->qgroup_rescan_running = true and the worker starting.  There are
>> many scenarios where we initialize the worker and never start it.  The
>> completion btrfs_ioctl_quota_rescan_wait waits for will never come.
>> This can happen even without involving error handling, since mounting
>> the file system read-only returns between initializing the worker and
>> queueing it.
>>
>> The right place to do it is when we're queuing the worker.  The flag
>> really just means that btrfs_ioctl_quota_rescan_wait should wait for
>> a completion.
>>
>> This patch introduces a new helper, queue_rescan_worker, that handles
>> the ->qgroup_rescan_running flag, including any races with umount.
>>
>> While we're at it, ->qgroup_rescan_running is protected only by the
>> ->qgroup_rescan_mutex.  btrfs_ioctl_quota_rescan_wait doesn't need
>> to take the spinlock too.
>>
>> Fixes: d2c609b834d6 (Btrfs: fix qgroup rescan worker initialization)
>> Signed-off-by: Jeff Mahoney 
>
> I've added this to misc-next as I'd like to push it to the next rc. The
> Fixes is fixed.
>

I don't see it pushed to misc-next yet, but based on f89fbcd776, could
you update the reference in the first line of the commit to match the
Fixes line?

Thanks,
Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] btrfs: Separate space_info create/update

2017-05-17 Thread Noah Massey 
On Wed, May 17, 2017 at 4:55 PM, Jeff Mahoney <je...@suse.com> wrote:
> On 5/17/17 4:52 PM, Noah Massey wrote:
>> On Wed, May 17, 2017 at 4:34 PM, Nikolay Borisov <nbori...@suse.com> wrote:
>>>
>>>
>>> On 17.05.2017 21:57, Noah Massey wrote:
>>>> On Wed, May 17, 2017 at 11:07 AM, Nikolay Borisov <nbori...@suse.com> 
>>>> wrote:
>>>>> Currently the struct space_info creation code is intermixed in the
>>>>> udpate_space_info function. There are well-defined points at which the we
>>>>
>>>> ^^^ update_space_info
>>>>
>>>>> actually want to create brand-new space_info structs (e.g. during mount of
>>>>> the filesystem as well as sometimes when adding/initialising new chunks). 
>>>>> In
>>>>> such cases udpate_space_info is called with 0 as the bytes parameter. All 
>>>>> of
>>>>> this makes for spaghetti code.
>>>>>
>>>>> Fix it by factoring out the creation code in a separate create_space_info
>>>>> structure. This also allows to simplify the internals. Furthermore it will
>>>>> make the update_space_info function not fail, allowing to remove error
>>>>> handling in callers. This will come in a follow up patch.
>>>>>
>>>>> This bears no functional changes
>>>>>
>>>>> Signed-off-by: Nikolay Borisov <nbori...@suse.com>
>>>>> Reviewed-by: Jeff Mahoney <je...@suse.com>
>>>>> ---
>>>>>  fs/btrfs/extent-tree.c | 127 
>>>>> -
>>>>>  1 file changed, 62 insertions(+), 65 deletions(-)
>>>>>
>>>>> Change since v1:
>>>>>
>>>>>  Incorporated Jeff Mahoney's feedback and added his reviewed-by
>>>>>
>>>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>>>> index be5477676cc8..28848e45b018 100644
>>>>> --- a/fs/btrfs/extent-tree.c
>>>>> +++ b/fs/btrfs/extent-tree.c
>>>>> @@ -3914,15 +3914,58 @@ static const char *alloc_name(u64 flags)
>>>>> };
>>>>>  }
>>>>>
>>>>> +static int create_space_info(struct btrfs_fs_info *info, u64 flags,
>>>>> +struct btrfs_space_info **new) {
>>>>> +
>>>>> +   struct btrfs_space_info *space_info;
>>>>> +   int i;
>>>>> +   int ret;
>>>>> +
>>>>> +   space_info = kzalloc(sizeof(*space_info), GFP_NOFS);
>>>>> +   if (!space_info)
>>>>> +   return -ENOMEM;
>>>>> +
>>>>> +   ret = percpu_counter_init(_info->total_bytes_pinned, 0, 
>>>>> GFP_KERNEL);
>>>>> +   if (ret) {
>>>>> +   kfree(space_info);
>>>>> +   return ret;
>>>>> +   }
>>>>> +
>>>>> +   for (i = 0; i < BTRFS_NR_RAID_TYPES; i++)
>>>>> +   INIT_LIST_HEAD(_info->block_groups[i]);
>>>>> +   init_rwsem(_info->groups_sem);
>>>>> +   spin_lock_init(_info->lock);
>>>>> +   space_info->flags = flags & BTRFS_BLOCK_GROUP_TYPE_MASK;
>>>>> +   space_info->force_alloc = CHUNK_ALLOC_NO_FORCE;
>>>>> +   init_waitqueue_head(_info->wait);
>>>>> +   INIT_LIST_HEAD(_info->ro_bgs);
>>>>> +   INIT_LIST_HEAD(_info->tickets);
>>>>> +   INIT_LIST_HEAD(_info->priority_tickets);
>>>>> +
>>>>> +   ret = kobject_init_and_add(_info->kobj, _info_ktype,
>>>>> +   info->space_info_kobj, "%s",
>>>>> +   alloc_name(space_info->flags));
>>>>> +   if (ret) {
>>>>> +   percpu_counter_destroy(_info->total_bytes_pinned);
>>>>> +   kfree(space_info);
>>>>> +   return ret;
>>>>> +   }
>>>>> +
>>>>> +   *new = space_info;
>>>>> +   list_add_rcu(_info->list, >space_info);
>>>>> +   if (flags & BTRFS_BLOCK_GROUP_DATA)
>>>>> +   info->data_sinfo = space_info;
>>>>> +
>>>>>

Re: [PATCH v2 1/2] btrfs: Separate space_info create/update

2017-05-17 Thread Noah Massey 
On Wed, May 17, 2017 at 4:34 PM, Nikolay Borisov <nbori...@suse.com> wrote:
>
>
> On 17.05.2017 21:57, Noah Massey wrote:
>> On Wed, May 17, 2017 at 11:07 AM, Nikolay Borisov <nbori...@suse.com> wrote:
>>> Currently the struct space_info creation code is intermixed in the
>>> udpate_space_info function. There are well-defined points at which the we
>>
>> ^^^ update_space_info
>>
>>> actually want to create brand-new space_info structs (e.g. during mount of
>>> the filesystem as well as sometimes when adding/initialising new chunks). In
>>> such cases udpate_space_info is called with 0 as the bytes parameter. All of
>>> this makes for spaghetti code.
>>>
>>> Fix it by factoring out the creation code in a separate create_space_info
>>> structure. This also allows to simplify the internals. Furthermore it will
>>> make the update_space_info function not fail, allowing to remove error
>>> handling in callers. This will come in a follow up patch.
>>>
>>> This bears no functional changes
>>>
>>> Signed-off-by: Nikolay Borisov <nbori...@suse.com>
>>> Reviewed-by: Jeff Mahoney <je...@suse.com>
>>> ---
>>>  fs/btrfs/extent-tree.c | 127 
>>> -
>>>  1 file changed, 62 insertions(+), 65 deletions(-)
>>>
>>> Change since v1:
>>>
>>>  Incorporated Jeff Mahoney's feedback and added his reviewed-by
>>>
>>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>>> index be5477676cc8..28848e45b018 100644
>>> --- a/fs/btrfs/extent-tree.c
>>> +++ b/fs/btrfs/extent-tree.c
>>> @@ -3914,15 +3914,58 @@ static const char *alloc_name(u64 flags)
>>> };
>>>  }
>>>
>>> +static int create_space_info(struct btrfs_fs_info *info, u64 flags,
>>> +struct btrfs_space_info **new) {
>>> +
>>> +   struct btrfs_space_info *space_info;
>>> +   int i;
>>> +   int ret;
>>> +
>>> +   space_info = kzalloc(sizeof(*space_info), GFP_NOFS);
>>> +   if (!space_info)
>>> +   return -ENOMEM;
>>> +
>>> +   ret = percpu_counter_init(_info->total_bytes_pinned, 0, 
>>> GFP_KERNEL);
>>> +   if (ret) {
>>> +   kfree(space_info);
>>> +   return ret;
>>> +   }
>>> +
>>> +   for (i = 0; i < BTRFS_NR_RAID_TYPES; i++)
>>> +   INIT_LIST_HEAD(_info->block_groups[i]);
>>> +   init_rwsem(_info->groups_sem);
>>> +   spin_lock_init(_info->lock);
>>> +   space_info->flags = flags & BTRFS_BLOCK_GROUP_TYPE_MASK;
>>> +   space_info->force_alloc = CHUNK_ALLOC_NO_FORCE;
>>> +   init_waitqueue_head(_info->wait);
>>> +   INIT_LIST_HEAD(_info->ro_bgs);
>>> +   INIT_LIST_HEAD(_info->tickets);
>>> +   INIT_LIST_HEAD(_info->priority_tickets);
>>> +
>>> +   ret = kobject_init_and_add(_info->kobj, _info_ktype,
>>> +   info->space_info_kobj, "%s",
>>> +   alloc_name(space_info->flags));
>>> +   if (ret) {
>>> +   percpu_counter_destroy(_info->total_bytes_pinned);
>>> +   kfree(space_info);
>>> +   return ret;
>>> +   }
>>> +
>>> +   *new = space_info;
>>> +   list_add_rcu(_info->list, >space_info);
>>> +   if (flags & BTRFS_BLOCK_GROUP_DATA)
>>> +   info->data_sinfo = space_info;
>>> +
>>> +   return ret;
>>> +}
>>> +
>>>  static int update_space_info(struct btrfs_fs_info *info, u64 flags,
>>>  u64 total_bytes, u64 bytes_used,
>>>  u64 bytes_readonly,
>>>  struct btrfs_space_info **space_info)
>>>  {
>>> struct btrfs_space_info *found;
>>> -   int i;
>>> int factor;
>>> -   int ret;
>>>
>>> if (flags & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1 |
>>>  BTRFS_BLOCK_GROUP_RAID10))
>>> @@ -3946,53 +3989,6 @@ static int update_space_info(struct btrfs_fs_info 
>>> *info, u64 flags,
>>> *space_info = found;
>>>

Re: [PATCH v2 1/2] btrfs: Separate space_info create/update

2017-05-17 Thread Noah Massey 
On Wed, May 17, 2017 at 11:07 AM, Nikolay Borisov  wrote:
> Currently the struct space_info creation code is intermixed in the
> udpate_space_info function. There are well-defined points at which the we

^^^ update_space_info

> actually want to create brand-new space_info structs (e.g. during mount of
> the filesystem as well as sometimes when adding/initialising new chunks). In
> such cases udpate_space_info is called with 0 as the bytes parameter. All of
> this makes for spaghetti code.
>
> Fix it by factoring out the creation code in a separate create_space_info
> structure. This also allows to simplify the internals. Furthermore it will
> make the update_space_info function not fail, allowing to remove error
> handling in callers. This will come in a follow up patch.
>
> This bears no functional changes
>
> Signed-off-by: Nikolay Borisov 
> Reviewed-by: Jeff Mahoney 
> ---
>  fs/btrfs/extent-tree.c | 127 
> -
>  1 file changed, 62 insertions(+), 65 deletions(-)
>
> Change since v1:
>
>  Incorporated Jeff Mahoney's feedback and added his reviewed-by
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index be5477676cc8..28848e45b018 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3914,15 +3914,58 @@ static const char *alloc_name(u64 flags)
> };
>  }
>
> +static int create_space_info(struct btrfs_fs_info *info, u64 flags,
> +struct btrfs_space_info **new) {
> +
> +   struct btrfs_space_info *space_info;
> +   int i;
> +   int ret;
> +
> +   space_info = kzalloc(sizeof(*space_info), GFP_NOFS);
> +   if (!space_info)
> +   return -ENOMEM;
> +
> +   ret = percpu_counter_init(_info->total_bytes_pinned, 0, 
> GFP_KERNEL);
> +   if (ret) {
> +   kfree(space_info);
> +   return ret;
> +   }
> +
> +   for (i = 0; i < BTRFS_NR_RAID_TYPES; i++)
> +   INIT_LIST_HEAD(_info->block_groups[i]);
> +   init_rwsem(_info->groups_sem);
> +   spin_lock_init(_info->lock);
> +   space_info->flags = flags & BTRFS_BLOCK_GROUP_TYPE_MASK;
> +   space_info->force_alloc = CHUNK_ALLOC_NO_FORCE;
> +   init_waitqueue_head(_info->wait);
> +   INIT_LIST_HEAD(_info->ro_bgs);
> +   INIT_LIST_HEAD(_info->tickets);
> +   INIT_LIST_HEAD(_info->priority_tickets);
> +
> +   ret = kobject_init_and_add(_info->kobj, _info_ktype,
> +   info->space_info_kobj, "%s",
> +   alloc_name(space_info->flags));
> +   if (ret) {
> +   percpu_counter_destroy(_info->total_bytes_pinned);
> +   kfree(space_info);
> +   return ret;
> +   }
> +
> +   *new = space_info;
> +   list_add_rcu(_info->list, >space_info);
> +   if (flags & BTRFS_BLOCK_GROUP_DATA)
> +   info->data_sinfo = space_info;
> +
> +   return ret;
> +}
> +
>  static int update_space_info(struct btrfs_fs_info *info, u64 flags,
>  u64 total_bytes, u64 bytes_used,
>  u64 bytes_readonly,
>  struct btrfs_space_info **space_info)
>  {
> struct btrfs_space_info *found;
> -   int i;
> int factor;
> -   int ret;
>
> if (flags & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1 |
>  BTRFS_BLOCK_GROUP_RAID10))
> @@ -3946,53 +3989,6 @@ static int update_space_info(struct btrfs_fs_info 
> *info, u64 flags,
> *space_info = found;
> return 0;
> }
> -   found = kzalloc(sizeof(*found), GFP_NOFS);
> -   if (!found)
> -   return -ENOMEM;
> -
> -   ret = percpu_counter_init(>total_bytes_pinned, 0, GFP_KERNEL);
> -   if (ret) {
> -   kfree(found);
> -   return ret;
> -   }
> -
> -   for (i = 0; i < BTRFS_NR_RAID_TYPES; i++)
> -   INIT_LIST_HEAD(>block_groups[i]);
> -   init_rwsem(>groups_sem);
> -   spin_lock_init(>lock);
> -   found->flags = flags & BTRFS_BLOCK_GROUP_TYPE_MASK;
> -   found->total_bytes = total_bytes;
> -   found->disk_total = total_bytes * factor;
> -   found->bytes_used = bytes_used;
> -   found->disk_used = bytes_used * factor;
> -   found->bytes_pinned = 0;
> -   found->bytes_reserved = 0;
> -   found->bytes_readonly = bytes_readonly;
> -   found->bytes_may_use = 0;
> -   found->full = 0;
> -   found->max_extent_size = 0;
> -   found->force_alloc = CHUNK_ALLOC_NO_FORCE;
> -   found->chunk_alloc = 0;
> -   found->flush = 0;
> -   init_waitqueue_head(>wait);
> -   INIT_LIST_HEAD(>ro_bgs);
> -   INIT_LIST_HEAD(>tickets);
> -   INIT_LIST_HEAD(>priority_tickets);
> -
> -   ret = kobject_init_and_add(>kobj, _info_ktype,
> -

Re: [PATCH v2 2/2] btrfs: Refactor update_space_info

2017-05-17 Thread Noah Massey 
minor nitpicks in the comment

On Wed, May 17, 2017 at 11:07 AM, Nikolay Borisov  wrote:
> Following the factoring out of the creation code udpate_space_info can only

" code, update_space_info "

> be called for already-existing space_info structs.

", which always succeeds"?

> Remove superfulous error
> handling and use the return value to return a pointer to the found space_info.
>
> Signed-off-by: Nikolay Borisov 
> Reviewed-by: Jeff Mahoney 
> ---
>  fs/btrfs/extent-tree.c | 68 
> ++
>  1 file changed, 24 insertions(+), 44 deletions(-)
>
> Change since v1
>  - Incorporated Jeff Mahoney's feedback and added his reviewed-by tag
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 28848e45b018..3d5bf0b7f719 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3959,10 +3959,10 @@ static int create_space_info(struct btrfs_fs_info 
> *info, u64 flags,
> return ret;
>  }
>
> -static int update_space_info(struct btrfs_fs_info *info, u64 flags,
> -u64 total_bytes, u64 bytes_used,
> -u64 bytes_readonly,
> -struct btrfs_space_info **space_info)
> +static struct btrfs_space_info *update_space_info(struct btrfs_fs_info *info,
> + 
>u64 flags, u64 total_bytes,
> + 
>u64 bytes_used,
> + 
>u64 bytes_readonly)
>  {
> struct btrfs_space_info *found;
> int factor;
> @@ -3974,21 +3974,21 @@ static int update_space_info(struct btrfs_fs_info 
> *info, u64 flags,
> factor = 1;
>
> found = __find_space_info(info, flags);
> -   if (found) {
> -   spin_lock(>lock);
> -   found->total_bytes += total_bytes;
> -   found->disk_total += total_bytes * factor;
> -   found->bytes_used += bytes_used;
> -   found->disk_used += bytes_used * factor;
> -   found->bytes_readonly += bytes_readonly;
> -   if (total_bytes > 0)
> -   found->full = 0;
> -   space_info_add_new_bytes(info, found, total_bytes -
> -bytes_used - bytes_readonly);
> -   spin_unlock(>lock);
> -   *space_info = found;
> -   return 0;
> -   }
> +   BUG_ON(!found);
> +
> +   spin_lock(>lock);
> +   found->total_bytes += total_bytes;
> +   found->disk_total += total_bytes * factor;
> +   found->bytes_used += bytes_used;
> +   found->disk_used += bytes_used * factor;
> +   found->bytes_readonly += bytes_readonly;
> +   if (total_bytes > 0)
> +   found->full = 0;
> +   space_info_add_new_bytes(info, found, total_bytes -
> +bytes_used - bytes_readonly);
> +   spin_unlock(>lock);
> +
> +   return found;
>  }
>
>  static void set_avail_alloc_bits(struct btrfs_fs_info *fs_info, u64 flags)
> @@ -10042,19 +10042,9 @@ int btrfs_read_block_groups(struct btrfs_fs_info 
> *info)
> }
>
> trace_btrfs_add_block_group(info, cache, 0);
> -   ret = update_space_info(info, cache->flags, found_key.offset,
> -   btrfs_block_group_used(>item),
> -   cache->bytes_super, _info);
> -   if (ret) {
> -   btrfs_remove_free_space_cache(cache);
> -   spin_lock(>block_group_cache_lock);
> -   rb_erase(>cache_node,
> ->block_group_cache_tree);
> -   RB_CLEAR_NODE(>cache_node);
> -   spin_unlock(>block_group_cache_lock);
> -   btrfs_put_block_group(cache);
> -   goto error;
> -   }
> +   space_info = update_space_info(info, cache->flags, 
> found_key.offset,
> + 
> btrfs_block_group_used(>item),
> + 
> cache->bytes_super);
>
> cache->space_info = space_info;
>
> @@ -10212,18 +10202,8 @@ int btrfs_make_block_group(struct btrfs_trans_handle 
> *trans,
>  * the rbtree, update the space info's counters.
>  */
> trace_btrfs_add_block_group(fs_info, cache, 1);
> -   ret = update_space_info(fs_info, cache->flags, size, bytes_used,
> -   cache->bytes_super, >space_info);
> -   if (ret) {
> -   btrfs_remove_free_space_cache(cache);

Re: [PATCH v2] btrfs-progs: send-dump: always print a space after path

2017-04-13 Thread Noah Massey 
On Wed, Apr 12, 2017 at 7:05 PM, Duncan <1i5t5.dun...@cox.net> wrote:
> Evan Danaher posted on Tue, 11 Apr 2017 12:33:40 -0400 as excerpted:
>
>> I was shocked to discover that 'btrfs receive --dump' doesn't print a
>> space after long filenames, so it runs together into the metadata; for
>> example:
>>
>> truncate./20-00-03/this-name-is-32-characters-longsize=0
>>
>> This is a trivial patch to add a single space unconditionally, so the
>> result is the following:
>>
>> truncate./20-00-03/this-name-is-32-characters-long size=0
>>
>> I suppose this is technically a breaking change, but it seems unlikely
>> to me that anyone would depend on the existing behavior given how
>> unfriendly it is.
>>
>> Signed-off-by: Evan Danaher 
>> ---
>
> I'm not a dev so won't attempt to comment on the patch itself, but it's
> worth noting that according to kernel patch submission guidelines (which
> btrfs-progs use as well) on V2+ patch postings, there should be a short,
> often one-line per version, summary of what changed between versions.
> This helps both reviewers and would-be patch-using admins such as myself
> understand how a patch is evolving, as well as for reviewers preventing
> unnecessary work when re-reviewing a new version of a patch previously
> reviewed in an earlier version.
>
> On patch series this summary is generally found in the 0/N post, while on
> individual patches without a 0/N, it's normally found below the first ---
> delimiter, so as to avoid including the patch history in the final merged
> version comment.

To be specific, something like
> ---
>
>v2: fixed an off-by-one error which caused padding to be 33 characters for 
>short paths
> ---

instead of the email tail you currently appended (for future reference).
FWIW, I'm fine with you adding my 'Reviewed-by', but I don't think it
carries much weight yet. :-)

--
Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs-progs: send-dump: always print a space after path

2017-04-11 Thread Noah Massey 
On Mon, Apr 10, 2017 at 8:09 PM, Evan Danaher  wrote:
> I was shocked to discover that 'btrfs receive --dump' doesn't print a
> space after long filenames, so it runs together into the metadata; for
> example:
>
> truncate./20-00-03/this-name-is-32-characters-longsize=0
>
> This is a trivial patch to add a single space unconditionally, so the
> result is the following:
>
> truncate./20-00-03/this-name-is-32-characters-long size=0
>
> I suppose this is technically a breaking change, but it seems unlikely
> to me that anyone would depend on the existing behavior given how
> unfriendly it is.
>
>
> Signed-off-by: Evan Danaher 
> ---
> diff --git a/send-dump.c b/send-dump.c
> index 67f7977..493389f 100644
> --- a/send-dump.c
> +++ b/send-dump.c
> @@ -116,9 +116,10 @@ static int __print_dump(int subvol, void *user, const 
> char *path,
> putchar('\n');
> return 0;
> }
> -   /* Short paths ale aligned to 32 chars */
> -   while (ret++ < 32)
> +   /* Short paths are aligned to 32 chars; longer paths get a single 
> space */
> +   do {
> putchar(' ');
> +   } while (ret++ < 32);

while (++ret < 32);

Since we're performing the check after the put, we need to
pre-increment to count the space already added.

> va_start(args, fmt);
> /* Operation specified ones */
> vprintf(fmt, args);
> ---
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Cross subvolume "cp --reflink"?

2016-10-19 Thread Noah Massey 
On Wed, Oct 19, 2016 at 12:52 PM, Andrei Borzenkov  wrote:
> I get "Failed to clone: Invalid cross-device link". Is it expected?

Yes, you cannot cross a MOUNTPOINT boundary with reflink.
Or, for that matter, btrfs subvolume snapshot.

> Basically this is (on openSUSE TW which has root on subvolume)
>
> mount -o subvol=/ /dev/vda1 /mnt
> btrfs sub create /mnt/var/cache
> cp -a --reflink=always /var/cache/* /mnt/var/cache
>

You *can* however cross subvolume boundaries within the same mountpoint.
Try (assuming your root subvolume is @root)

cp -a --reflink /mnt/@root/var/cache/* /mnt/var/cache

~ Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Issue: errno:28 (No space left on device)

2016-08-09 Thread Noah Massey 
On Tue, Aug 9, 2016 at 8:56 AM, Austin S. Hemmelgarn
 wrote:
> On 2016-08-09 07:50, Thomas wrote:
>>
>> Hello!
>
> First things first:
> Mailing lists are asynchronous.  You will almost _never_ get an immediate
> response, and will quite often not get a response for a few hours at least.
> Sending a message more than once when you don't get a response does not make
> it more likely you get a response quickly, and in fact is a good way to
> seriously annoy people and make them less willing to help you.
>>
>>
>> I'm facing a severe issue with Debian installation using BTRFS:
>> errno:28 (No space left on device)
>>
>> After a few minutes of system usage I get this error message for any
>> action, means there's really no disk space left.
>>
>> However, the output of btrfs fi usage / reports that ~10% of disk space
>> should be free.
>>
>> I have 2 questions:
>> 1. Why is the OS reporting an error "Issue: errno:28 (No space left on
>> device)" whereas "btrfs fi usage" reports free space?
>
> BTRFS allocates disk space in two stages.  In the first stage, it allocates
> large chunks that then get used only for metadata or data. Within those
> chunks, it then allocates blocks for the given type of data on-demand.  In
> your case, the filesystem has no room to allocate new chunks, but has some
> free space in existing chunks.  This shouldn't result in allocation failures
> for new files, but for currently unknown reasons it sometimes does, and this
> is a known issue that we have been unable to completely fix so far (we've
> technically 'fixed' this about 5 or 6 times already, but there appear to be
> lots of odd corner cases that haven't been found yet).
>>
>> 2. How can I solve this issue w/o the possibility of extending the
>> partition?
>
> Right now, you have three options:
> 1. Extend the partition, run a full balance, and then shrink the partition
> again.
> 2. Temporarily add another device (make sure it's at least 4GB), run a full
> balance, and then remove the device.
> 3. Recreate the filesystem from scratch and restore from a backup.
>

A quick note recreating the filesystem:
Since your data and metadata profiles are both 'single', you may want
to consider creating the filesystem with the --mixed option, which
allows / forces BTRFS to use a single collection of data blocks for
both data and metadata.
This can help with ENOMEM errors when all space is allocated, but the
allocation does not match the currently required type.

~ Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: "No space left on device" and balance doesn't work

2016-08-09 Thread Noah Massey 
On Tue, Aug 9, 2016 at 7:16 AM, Austin S. Hemmelgarn
 wrote:
> On 2016-08-09 05:50, MegaBrutal wrote:
>>
>> 2016-06-03 14:43 GMT+02:00 Austin S. Hemmelgarn :
>>>
>>>
>>> Also, since you're on a new enough kernel, try 'lazytime' in the mount
>>> options as well, this defers all on-disk timestamp updates for up to 24
>>> hours or until the inode gets written out anyway, but keeps the updated info
>>> in memory.  The only downside to this is that mtimes might not be correct
>>> after an unclean shutdown, but most software will have no issues with this.
>>>
>>
>> Hi all,
>>
>> Sorry for reviving this old thread, and probably it's not the best
>> place to ask about this... but I added the "noatime" option in fstab,
>> restarted the system, and now I think I should try "lazytime" too (as
>> I like the idea to have proper atimes with delayed writing to disk).
>> So now I'd just like to test the "lazytime" mount option without
>> restart.
>>
>> I remounted the file system like this:
>>
>> mount -o remount,lazytime /
>>
>> But now the FS still has the "noatime" mount option, which I guess
>> renders "lazytime" ineffective. I thought they are supposed to be
>> mutually exclusive, so I'm actually surprised that I can have both
>> mount options at the same time.
>
> No, lazytime also affects mtime handling, not just atime, so they aren't
> mutually exclusive, and it does make sense to have both.
>>
>>
>> Now my mount looks like this:
>>
>> /dev/mapper/centrevg-rootlv on / type btrfs
>> (rw,noatime,lazytime,space_cache,subvolid=257,subvol=/@)
>>
>> I also tried to explicitly add "atime" to negate "noatime" (man mount
>> says "atime" is the option to disable "noatime"), like this:
>>
>> mount -o remount,atime,lazytime /
>>
>> But the "noatime" option still stays. Why? Is it a BTRFS specific
>> issue, or does it reside in another layer?
>>
>> By the way, is it valid to mount BTRFS subvolumes with different atime
>> policies? Then how do child subvolumes behave?
>
> I'm not entirely certain (I have my kernel patched so noatime is the
> default, and rarely if ever use anything else, so I don't have much in the
> way of experience with this), but my guess would be that it can't be done,
> and that changing atime handling on remount isn't really handled. I do know
> that adding lazytime on remount doesn't always work, but that kind of makes
> sense, since it causes significant changes in how mtimes and atimes are
> handled internally.
>

TLDR:

try 'mount -o remount,strictatime / && mount -o remount,relatime /'

This seems strange, but on my unpatched system:

$ uname -r
4.7.0

$ mount -o loop,noatime,lazytime /var/tmp/test.dd /mnt
$ grep ^/dev/loop0 /proc/mounts
/dev/loop0 /mnt btrfs rw,lazytime,noatime,space_cache,subvolid=5,subvol=/ 0 0

$ mount -o remount,relatime /mnt
$ grep ^/dev/loop0 /proc/mounts
/dev/loop0 /mnt btrfs rw,lazytime,noatime,space_cache,subvolid=5,subvol=/ 0 0

^^^ No change to noatime option

$ mount -o remount,strictatime /mnt
$ grep ^/dev/loop0 /proc/mounts
/dev/loop0 /mnt btrfs rw,lazytime,space_cache,subvolid=5,subvol=/ 0 0

^^^ that updated it...

$ mount -o remount,relatime /mnt
$ grep ^/dev/loop0 /proc/mounts

/dev/loop0 /mnt btrfs rw,lazytime,relatime,space_cache,subvolid=5,subvol=/ 0 0

^^^ now it changes

~ Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs-progs: doc: add missing newline in btrfs-convert

2016-06-10 Thread Noah Massey 
Signed-off-by: Noah Massey <noah.mas...@gmail.com>
---
 Documentation/btrfs-convert.asciidoc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/btrfs-convert.asciidoc 
b/Documentation/btrfs-convert.asciidoc
index 28f9a39..ab3577d 100644
--- a/Documentation/btrfs-convert.asciidoc
+++ b/Documentation/btrfs-convert.asciidoc
@@ -90,6 +90,7 @@ are supported by old kernels. To disable a feature, prefix it 
with '^'.
 To see all available features that btrfs-convert supports run:
 +
 +btrfs-convert -O list-all+
++
 -p|--progress::
 show progress of conversion, on by default
 --no-progress::
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: WARNING: at /home/kernel/COD/linux/fs/btrfs/inode.c:9261 btrfs_destroy_inode+0x247/0x2c0 [btrfs]

2016-06-09 Thread Noah Massey 
On Thu, Jun 9, 2016 at 10:52 AM, Duncan <1i5t5.dun...@cox.net> wrote:
> Fugou Nashi posted on Sun, 05 Jun 2016 10:12:31 +0900 as excerpted:
>
>> Hi,
>>
>> Do I need to worry about this?
>>
>> Thanks.
>>
>> Linux nakku 4.6.0-040600-generic #201605151930 SMP Sun May 15 23:32:59
>> UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
>
> There's a patch for it that didn't quite make 4.6.0, but is in 4.7-rc1
> and should appear in later 4.6.x stable releases (tho I don't track
> stable myself so couldn't tell you if it's there yet or not).
>

It's in 4.6.1

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=4704fa547224fd49041c26f7fca3710a47fc449f
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Can't remove read-only snapshot with immutable flag set

2016-06-08 Thread Noah Massey 
On Wednesday, June 8, 2016, cdlscpmv  wrote:
>
> Hi!
>
> It happened that I set the immutable flag (via `chattr +i`) on a subvolume.
> Then I made a read-only snapshot of that subvolume. Now I can't remove
> this snapshot.
>
> #> btrfs subvolume delete my_snapshot
> Delete subvolume (no-commit): 'my_snapshot'
> ERROR: cannot delete 'my_snapshot': Operation not permitted
> #> chattr -i my_snapshot
> chattr: Read-only file system while setting flags on my_snapshot
> #> btrfs --version
> btrfs-progs v4.4.1
> #> uname -r
> 4.5.2
>
> Is there any other way to do this? Or is it a bug?
>
Try
#> btrfs property set my_snaphot ro false
#> chattr -i my_snapshot
#> btrfs sub del my_snapshot
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] btrfs-progs: utils: Introduce new pseudo random API

2016-05-25 Thread Noah Massey 
On Wed, May 25, 2016 at 12:14 AM, Qu Wenruo  wrote:
> David has reported some quite chaos usage of pseudo random numbers.
> Like using static srand seed, or even calling rand() without setting
> seed correctly.
>
> The new pseudo random API will initialize the random seed on its first
> calling and use uniformly distributed pseudo random number generator as
> backend.
>
> Signed-off-by: Qu Wenruo 
> ---
>  utils.c | 36 
>  utils.h | 36 
>  2 files changed, 72 insertions(+)
>
> diff --git a/utils.c b/utils.c
> index 7761165..c0e860a 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -54,9 +54,11 @@
>  #define BLKDISCARD _IO(0x12,119)
>  #endif
>
> +static int seed_initlized = 0;
>  static int btrfs_scan_done = 0;
>
>  static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
> +static unsigned short seeds[3];
>
>  const char *get_argv0_buf(void)
>  {
> @@ -4051,3 +4053,37 @@ out:
>
> return ret;
>  }
> +
> +void rand_seed(u64 seed)
> +{
> +   int i;
> +   /* only use the last 48 bits */
> +   for (i = 0; i < 3; i++) {
> +   seeds[i] = (unsigned short)(seed ^ (unsigned short)(-1));
> +   seed >>= 16;
> +   }
> +   seed_initlized = 1;
> +}
> +
> +u32 rand_u32(void)
> +{
> +   struct timeval tv;
> +
> +   if (seed_initlized)
> +   return nrand48(seeds);
> +

Missing a (u32) cast.

> +   /*
> +* It's possible to use /dev/random, but we don't need that true
> +* random number nor want to wait for entropy,

Sounds like a good candidate for /dev/urandom, then?

> +* since we're only using random API to do corruption to test.
> +* Time and pid/ppid based seed would be good enough, and won't
> +* cause sleep for entropy pool.
> +*/
> +   gettimeofday(, 0);
> +   seeds[0] = getpid() ^ (tv.tv_sec & 0x);
> +   seeds[1] = getppid() ^ (tv.tv_usec & 0x);
> +   seeds[2] = (tv.tv_sec ^ tv.tv_usec) >> 16;
> +   seed_initlized = 1;
> +
> +   return (u32)nrand48(seeds);
> +}
> diff --git a/utils.h b/utils.h
> index ebe6d61..0977262 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -362,4 +362,40 @@ static inline int error_on(int condition, const char 
> *fmt, ...)
> return 1;
>  }
>
> +/* pseudo random number generator wrappers */
> +u32 rand_u32(void);
> +
> +static inline int rand_int(void)
> +{
> +   return (int)(rand_u32());
> +}
> +
> +static inline u64 rand_u64(void)
> +{
> +   u64 ret = 0;
> +
> +   ret += rand_u32();
> +   ret <<= 32;
> +   ret += rand_u32();
> +   return ret;
> +}
> +
> +static inline u16 rand_u16(void)
> +{
> +   return (u16)(rand_u32());
> +}
> +
> +static inline u8 rand_u8(void)
> +{
> +   return (u8)(rand_u32());
> +}
> +
> +/* Return random number in range [0, limit) */
> +static inline unsigned int rand_range(unsigned int up)
> +{
> +   return (unsigned int)(rand_u32() % up);
> +}

Wouldn't distribution be more uniform if you took the full 48 bits
from nrand64(), and only downcast after the mod?

> +
> +/* Also allow setting seeds manually */
> +void rand_seed(u64 seed);
>  #endif
> --
> 2.8.2
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs-progs: Adjust timing of safety delay countdown

2016-05-04 Thread Noah Massey 
When printing the countdown in the safety delay, the number should
correspond to the number of seconds remaining to wait at the time the
delay is printed.

In other words, there should be a one second sleep after printing '1'.

Signed-off-by: Noah Massey <noah.mas...@gmail.com>
---
 cmds-balance.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index 5f05f60..8f3bf5b 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -445,9 +445,9 @@ static int do_balance(const char *path, struct 
btrfs_ioctl_balance_args *args,
printf("\twarning. The operation will start in %d seconds.\n", 
delay);
printf("\tUse Ctrl-C to stop it.\n");
while (delay) {
-   sleep(1);
printf("%2d", delay--);
fflush(stdout);
+   sleep(1);
}
printf("\nStarting balance without any filters.\n");
}
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs-progs: add safety delay before starting full balance

2016-05-02 Thread Noah Massey 
On Mon, May 2, 2016 at 4:33 AM, David Sterba  wrote:
> A short delay with a warning before starting a full balance should
> improve usability. We have been getting reports from people who run full
> balance after following some random advice and then get surprised by the
> performance impact.
>
> The countdown is done even when run from scripts, but as the whole
> balance takes significanly more time, this shouldn't be an issue.
>
> Signed-off-by: David Sterba 
> ---
>  Documentation/btrfs-balance.asciidoc |  6 
>  cmds-balance.c   | 65 
> ++--
>  2 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/btrfs-balance.asciidoc 
> b/Documentation/btrfs-balance.asciidoc
> index 8b5df96ad41f..7df40b9c6f49 100644
> --- a/Documentation/btrfs-balance.asciidoc
> +++ b/Documentation/btrfs-balance.asciidoc
> @@ -67,6 +67,12 @@ resume interrupted balance
>  start the balance operation according to the specified filters, no filters
>  will rewrite the entire filesystem. The process runs in the foreground.
>  +
> +NOTE: the balance command without filters will basically rewrite everything
> +in the filesystem. The run time is potentially very long, depending on the
> +filesystem size. To prevent starting a full balance by accident, the user is
> +warned and has a few seconds to cancel the operation before it starts. The
> +warning and delay can be skipped with '--full-balance' option.
> ++
>  `Options`
>  +
>  -d[]
> diff --git a/cmds-balance.c b/cmds-balance.c
> index 33f91e41134e..5f05f603d4c8 100644
> --- a/cmds-balance.c
> +++ b/cmds-balance.c
> @@ -417,8 +417,13 @@ static int do_balance_v1(int fd)
> return ret;
>  }
>
> +enum {
> +   BALANCE_START_FILTERS = 1 << 0,
> +   BALANCE_START_NOWARN  = 1 << 1
> +};
> +
>  static int do_balance(const char *path, struct btrfs_ioctl_balance_args 
> *args,
> - int nofilters)
> + unsigned flags)
>  {
> int fd;
> int ret;
> @@ -429,6 +434,24 @@ static int do_balance(const char *path, struct 
> btrfs_ioctl_balance_args *args,
> if (fd < 0)
> return 1;
>
> +   if (!(flags & BALANCE_START_FILTERS) && !(flags & 
> BALANCE_START_NOWARN)) {
> +   int delay = 10;
> +
> +   printf("WARNING:\n\n");
> +   printf("\tFull balance without filters requested. This 
> operation is very\n");
> +   printf("\tintense and takes potentially very long. It is 
> recommended to\n");
> +   printf("\tuse the balance filters to narrow down the balanced 
> data.\n");
> +   printf("\tUse 'btrfs balance start --full-balance' option to 
> skip this\n");
> +   printf("\twarning. The operation will start in %d 
> seconds.\n", delay);
> +   printf("\tUse Ctrl-C to stop it.\n");
> +   while (delay) {
> +   sleep(1);
> +   printf("%2d", delay--);
> +   fflush(stdout);
> +   }

Shouldn't the sleep be after the fflush?

> +   printf("\nStarting balance without any filters.\n");
> +   }
> +
> ret = ioctl(fd, BTRFS_IOC_BALANCE_V2, args);
> e = errno;
>
> @@ -438,7 +461,7 @@ static int do_balance(const char *path, struct 
> btrfs_ioctl_balance_args *args,
>  * old one.  But, the old one doesn't know any filters, so
>  * don't fall back if they tried to use the fancy new things
>  */
> -   if (e == ENOTTY && nofilters) {
> +   if (e == ENOTTY && !(flags & BALANCE_START_FILTERS)) {
> ret = do_balance_v1(fd);
> if (ret == 0)
> goto out;
> @@ -477,13 +500,16 @@ static const char * const cmd_balance_start_usage[] = {
> "passed all filters in a comma-separated list of filters for a",
> "particular chunk type.  If filter list is not given balance all",
> "chunks of that type.  In case none of the -d, -m or -s options is",
> -   "given balance all chunks in a filesystem.",
> +   "given balance all chunks in a filesystem. This is potentially",
> +   "long operation and the user is warned before this start, with",
> +   "a delay to stop it.",
> "",
> "-d[filters]act on data chunks",
> "-m[filters]act on metadata chunks",
> "-s[filters]act on system chunks (only under -f)",
> "-v be verbose",
> "-f force reducing of metadata integrity",
> +   "--full-balance do not print warning and do not delay start",
> NULL
>  };
>
> @@ -494,19 +520,22 @@ static int cmd_balance_start(int argc, char **argv)
> , NULL };
> int force = 0;
> int verbose = 0;
> -   int

[PATCH] btrfs-progs: build: fix static standalone utilities

2016-03-21 Thread Noah Massey 
commit b5e7979 "btrfs-progs: build: extend per-binary objects" allows
the standalone utilities to link against object files shared with the
main binary. However, the btrfs-*.static targets need to be adjusted
to build against the static versions of the common files.

Signed-off-by: Noah Massey <noah.mas...@gmail.com>
---
 Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.in b/Makefile.in
index dd30686..c553c5f 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -254,7 +254,7 @@ $(lib_links):
 btrfs-%.static: $(static_objects) btrfs-%.static.o $(static_libbtrfs_objects)
@echo "[LD] $@"
$(Q)$(CC) $(STATIC_CFLAGS) -o $@ $@.o $(static_objects) \
-   $($(subst -,_,$(subst .static,,$@)-objects)) \
+   $(patsubst %.o, %.static.o, $($(subst -,_,$(subst 
.static,,$@)-objects))) \
$(static_libbtrfs_objects) $(STATIC_LDFLAGS) \
$($(subst -,_,$(subst .static,,$@)-libs)) $(STATIC_LIBS)
 
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs und lvm-cache?

2015-12-23 Thread Noah Massey 
On Wed, Dec 23, 2015 at 6:38 AM, Neuer User  wrote:
> Am 23.12.2015 um 12:21 schrieb Martin Steigerwald:
>> Hi.
>>
>> As far as I understand this way you basically loose the RAID 1 semantics of
>> BTRFS. While the data is redundant on the HDDs, it is not redundant on the
>> SSD. It may work for a pure read cache, but for write-through you definately
>> loose any data integrity protection a RAID 1 gives you.
>>
> Hmm, are you sure? I thought LVM lies underneath btrfs. Btrfs thus
> should not know about the caching SSD at all. It only knows of the two
> LVs on the HDDs, reading and writing data from or to one or both of the
> two LVs.

I believe Martin's concern is two-fold:

The first, major issue, concerns the default writeback cache mode,
which makes the SSD a single point of failure.
(in writeback mode, a write to a block that is cached will go only to
the cache and the block
will be marked dirty in the metadata.) If the SSD fails with dirty
data in the cache which has not been flushed to the backing devices,
the filesystem may be in a unrecoverable state, because writes which
BTRFS was told had succeeded are not present on disk.

The second potential issue is that if the SSD performs internal
deduplication, the two copies of cached data (contents on drive 1,
content on drive 2) may actually be a reference to the same bits of
internal storage, meaning a single corruption will affect both cached
copies. If in writeback, then corrupted data could flush down to both
disks. I'm not sure what would happen in writethrough.

~ Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: cannot move ro-snapshot directly but indirectly works

2015-11-28 Thread Noah Massey 
On Fri, Nov 27, 2015 at 22:06 Christoph Anton Mitterer
 wrote:
>
> Hey.
>
> Not sure whether this is intended or not, but it feels at least
> somewhat strange:
>
> Consider I have a readonly snapshot (the only subvol here):
> /btrfs/snapshots/ro-snapshot
> now I want to move it to the dir:
> /btrfs/snapshots/foo/
> i.e.
> mv /btrfs/snapshots/ro-snapshot
> /btrfs/snapshots/foo/
> but the mv complains about read-only filesystem.
>

Not a developer, but I believe this is intended behavior.
Changing the direct parent directory of a subvolume is not a read only
operation: it involves modifying the '..' directory entry.
Renaming the subvolume, or moving the parent directory, modify the
overall fs-hierachy, but do not modify the subvol.

The same is true for normal directories:
mkdir /tmp/dir1
chmod -w /tmp/dir1
mv /tmp/dir1 /tmp/dir2 ## allowed
mkdir /tmp/dir3
mv /tmp/dir2 /tmp/dir3/dir2 ## Permission denied

~ Noah
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] BTRFS: error in btrfs_run_delayed_refs:2821: errno=-5 IO failure

2015-07-08 Thread Noah Massey 
On Wed, Jul 8, 2015 at 10:01 AM, Chris Murphy li...@colorremedies.com wrote:
 The short version: btrfs convert and subsequent balance will eat an
 ext4 file system. I've reproduced this 6 for 6 times with both kernel
 4.1 and 4.2rc1, but I get different back traces for the two kernels.

 Kernel 4.1.0
 Btrfs-progs 4.1

 New ext4 file system populated with several thousand files (Fedora 22
 installation) and a 1.5GB ISO.


Standard disclaimer: Not a developer, just a user.
The 1.5 GB ISO on a fresh ext4 system made me think of
http://thread.gmane.org/gmane.comp.file-systems.btrfs/36955
(ext4 extents over 1 GB causing ENOSPC).

Did the btrfs filesystem balance specify a -t 900G or similar?

Or has that issue been fixed?
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: btrfs replace seems to corrupt the file system

2015-06-28 Thread Noah Massey 
On Sun, Jun 28, 2015 at 2:02 PM, Mordechay Kaganer mkaga...@gmail.com wrote:
 To recover the old device, that's what i'm trying to do. Asked on IRC
 also, no reply. As stated above, the device passes btrfs check without
 errors but cannot mount because it complains about ongoing replace
 and the replace device is missing.

Standard disclaimer, not a dev, just a user.
The following worked for me to recover the old device after
reproducing your situation:
(where loop0 is my old device)

# mount -t btrfs -o degraded /dev/loop0 /mnt
# btrfs replace cancel /mnt
# btrfs umount /mnt
# mount -t btrfs /dev/loop0 /mnt

mount now succeeds without error.

$ uname -r
4.1.0
$ btrfs version
btrfs-progs v4.1
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] btrfs-progs: read_tree_block() and read_node_slot() cleanup.

2015-01-27 Thread Noah Massey 
On Mon, Jan 26, 2015 at 10:12 PM, Qu Wenruo quwen...@cn.fujitsu.com wrote:
 Allow read_tree_block() and read_node_slot() to return error pointer.
 This should help caller to get more specified error number.

 For existing callers, change (!eb) judgmentt to
 (!extent_buffer_uptodate(eb)) to keep the compatibility, and for caller
 missing the check, use PTR_ERR(eb) if possible.

 Signed-off-by: Qu Wenruo quwen...@cn.fujitsu.com
 ---
  backref.c |  4 ++--
  btrfs-calc-size.c |  2 +-
  btrfs-corrupt-block.c |  6 +++---
  btrfs-debug-tree.c|  7 +--
  btrfs-image.c |  6 +++---
  cmds-check.c  |  6 +++---
  cmds-restore.c|  6 +++---
  ctree.c   | 29 ++---
  disk-io.c | 10 +-
  extent-tree.c |  6 ++
  extent_io.c   |  3 +--
  print-tree.c  |  2 +-
  qgroup-verify.c   |  2 +-
  13 files changed, 56 insertions(+), 33 deletions(-)

 diff --git a/backref.c b/backref.c
 index 593f936..9a2efca 100644
 --- a/backref.c
 +++ b/backref.c
 @@ -451,7 +451,7 @@ static int __add_missing_keys(struct btrfs_fs_info 
 *fs_info,
 BUG_ON(!ref-wanted_disk_byte);
 eb = read_tree_block(fs_info-tree_root, 
 ref-wanted_disk_byte,
  fs_info-tree_root-leafsize, 0);
 -   if (!eb || !extent_buffer_uptodate(eb)) {
 +   if (!extent_buffer_uptodate(eb)) {
 free_extent_buffer(eb);
 return -EIO;
 }
 @@ -808,7 +808,7 @@ static int find_parent_nodes(struct btrfs_trans_handle 
 *trans,
 ref-level);
 eb = read_tree_block(fs_info-extent_root,
ref-parent, bsz, 
 0);
 -   if (!eb || !extent_buffer_uptodate(eb)) {
 +   if (!extent_buffer_uptodate(eb)) {
 free_extent_buffer(eb);
 ret = -EIO;
 goto out;
 diff --git a/btrfs-calc-size.c b/btrfs-calc-size.c
 index 50c..354f70d 100644
 --- a/btrfs-calc-size.c
 +++ b/btrfs-calc-size.c
 @@ -159,7 +159,7 @@ static int walk_nodes(struct btrfs_root *root, struct 
 btrfs_path *path,
 tmp = read_tree_block(root, cur_blocknr,
   btrfs_level_size(root, level - 
 1),
   btrfs_node_ptr_generation(b, 
 i));
 -   if (!tmp) {
 +   if (!extent_buffer_uptodate(tmp)) {
 fprintf(stderr, Failed to read blocknr 
 %Lu\n,
 btrfs_node_blockptr(b, i));
 continue;
 diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
 index b477e87..af03ad1 100644
 --- a/btrfs-corrupt-block.c
 +++ b/btrfs-corrupt-block.c
 @@ -160,7 +160,7 @@ static int corrupt_keys_in_block(struct btrfs_root *root, 
 u64 bytenr)
 struct extent_buffer *eb;

 eb = read_tree_block(root, bytenr, root-leafsize, 0);
 -   if (!eb)
 +   if (!extent_buffer_uptodate(eb))
 return -EIO;;

 corrupt_keys(NULL, root, eb);
 @@ -288,7 +288,7 @@ static void btrfs_corrupt_extent_tree(struct 
 btrfs_trans_handle *trans,
 next = read_tree_block(root, btrfs_node_blockptr(eb, i),
root-leafsize,
btrfs_node_ptr_generation(eb, i));
 -   if (!next)
 +   if (!extent_buffer_uptodate(next))
 continue;
 btrfs_corrupt_extent_tree(trans, root, next);
 free_extent_buffer(next);
 @@ -696,7 +696,7 @@ static int corrupt_metadata_block(struct btrfs_root 
 *root, u64 block,
 }

 eb = read_tree_block(root, block, root-leafsize, 0);
 -   if (!eb) {
 +   if (!extent_buffer_uptodate(eb)) {
 fprintf(stderr, Couldn't read in tree block %s\n, field);
 return -EINVAL;
 }
 diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c
 index 9cdb35f..3efd434 100644
 --- a/btrfs-debug-tree.c
 +++ b/btrfs-debug-tree.c
 @@ -68,6 +68,8 @@ static void print_extents(struct btrfs_root *root, struct 
 extent_buffer *eb)
  btrfs_node_blockptr(eb, i),
  size,
  btrfs_node_ptr_generation(eb, 
 i));
 +   if (!extent_buffer_uptodate(next))
 +   continue;
 if (btrfs_is_leaf(next) 
 btrfs_header_level(eb) != 1)
 BUG();
 @@ -202,7 +204,8 @@ int main(int ac, char **av)

Re: Moving snapshots

2015-01-23 Thread Noah Massey 
On Fri, Jan 23, 2015 at 5:05 AM, Matthias Urlichs matth...@urlichs.de wrote:
 Hello,

 how do I move a (read-only) snapshot?


If you want to move a read-only snapshot to a different directory,
'..' changes, and therefore is not a read-only operation.

 Simply creating another read-only snap from the first one and then deleting
 the source works, except that it destroy's the snapshot's identity -- which
 means that it can't be used as a parent for btrfs receive any more.  :-(

# btrfs property set -ts /path/to/snapshot ro false
# mv /path/to/snapshot /new/path
# btrfs property set -ts /new/path ro true

worked for me


 --
 -- Matthias Urlichs

--
Noah
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

fstrim on BTRFS

2014-10-30 Thread Noah Massey 
Hello,

I am looking for some clarification on TRIM / SSD maintenance.
The wiki [1] suggests periodic fstrim, but fstrim does not discard
unallocated blocks[2].
Which makes sense, given that mkfs issues a device wide trim, so they
shouldn't have data.

But it seems like both a balance, and a pending patch
( 47ab2a6 Btrfs: remove empty block groups automatically )
can deallocate block groups without TRIM, leading to the SSD retaining
data it doesn't need to.

Is there a bitter way to trigger a more thorough discard than
fallocate, rm, fstrim, balance -dusage=0 ?
And are there plans to support trimming unallocated space, or is this
not possible with current FS format?

Thank you,
Noah

[1] 
https://btrfs.wiki.kernel.org/index.php/FAQ#Does_Btrfs_support_TRIM.2Fdiscard.3F
[2] https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg14195.html
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html