ssd vs ssd_spread with sdcard or emmc

2018-07-24 Thread Chris Murphy
Hi,

I'm not finding any recent advice for sdcard or eMMC media, both of
which trigger the ssd mount option automatically. I seem to recall ssd
has had some optimizations recently, but haven't heard much about
ssd_spread.

While sdcard and eMMC are rather different, it seems they have two
things in common: they don't have the wear durability of even consumer
SATA SSD let alone NVMe, and also they both suffer from dog slow
writes. I'm unable to tell that ssd_spread does any better writes wise
on a Samsung EVO+ sdcard. So that leaves wear and in particular if
wandering trees is at all affected by ssd_spread?

-- 
Chris Murphy
--
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: File permissions lost during send/receive?

2018-07-24 Thread Duncan
Marc Joliet posted on Tue, 24 Jul 2018 22:42:06 +0200 as excerpted:

> On my system I get:
> 
> % sudo getcap /bin/ping /sbin/unix_chkpwd
> /bin/ping = cap_net_raw+ep
> /sbin/unix_chkpwd = cap_dac_override+ep
> 
>> (getcap on unix_chkpwd returns nothing, but while I use kde/plasma I
>> don't normally use the lockscreen at all, so for all I know that's
>> broken here too.)

OK, after remerging pam, I get the same for unix_chkpwd (tho here I have 
sbin merge so it's /bin/unix_chkpwd with sbin -> bin), so indeed, it must 
have been the same problem for you with it, that I've simply not run into 
since whatever killed the filecaps here, because I don't use the 
lockscreen.

But if I start using the lockscreen again and it fails, I know one not-so-
intuitive thing to check, now. =:^)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

--
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: File permissions lost during send/receive?

2018-07-24 Thread Marc Joliet
Am Dienstag, 24. Juli 2018, 21:46:14 CEST schrieb Duncan:
> Andrei Borzenkov posted on Tue, 24 Jul 2018 20:53:15 +0300 as excerpted:
> > 24.07.2018 15:16, Marc Joliet пишет:
> >> Hi list,
> >> 
> >> (Preemptive note: this was with btrfs-progs 4.15.1, I have since
> >> upgraded to 4.17.  My kernel version is 4.14.52-gentoo.)
> >> 
> >> I recently had to restore the root FS of my desktop from backup (extent
> >> tree corruption; not sure how, possibly a loose SATA cable?).
> >> Everything was fine,
> >> even if restoring was slower than expected.  However, I encountered two
> >> files with permission problems, namely:
> >> 
> >> - /bin/ping, which caused running ping as a normal user to fail due to
> >> missing permissions, and
> >> 
> >> - /sbin/unix_chkpwd (part of PAM), which prevented me from unlocking
> >> the KDE Plasma lock screen; I needed to log into a TTY and run
> >> "loginctl unlock- session".
> >> 
> >> Both were easily fixed by reinstalling the affected packages (iputils
> >> and pam), but I wonder why this happened after restoring from backup.
> >> 
> >> I originally thought it was related to the SUID bit not being set,
> >> because of the explanation in the ping(8) man page (section
> >> "SECURITY"), but cannot find evidence of that -- that is, after
> >> reinstallation, "ls -lh" does not show the sticky bit being set, or any
> >> other special permission bits, for that matter:
> >> 
> >> % ls -lh /bin/ping /sbin/unix_chkpwd
> >> -rwx--x--x 1 root root 60K 22. Jul 14:47 /bin/ping*
> >> -rwx--x--x 1 root root 31K 23. Jul 00:21 /sbin/unix_chkpwd*
> >> 
> >> (Note: no ACLs are set, either.)
> > 
> > What "getcap /bin/ping" says? You may need to install package providing
> > getcap (libcap-progs here on openSUSE).
> 
> sys-libs/libcap on gentoo.  Here's what I get:
> 
> $ getcap /bin/ping
> /bin/ping = cap_net_raw+ep

On my system I get:

% sudo getcap /bin/ping /sbin/unix_chkpwd 
/bin/ping = cap_net_raw+ep
/sbin/unix_chkpwd = cap_dac_override+ep

> (getcap on unix_chkpwd returns nothing, but while I use kde/plasma I
> don't normally use the lockscreen at all, so for all I know that's broken
> here too.)
> 
> As hinted, it's almost certainly a problem with filecaps.  While I'll
> freely admit to not fully understanding how file-caps work, and my use-
> case doesn't use send/receive, I do recall filecaps are what ping uses
> these days instead of SUID/SGID (on gentoo it'd be iputils' filecaps and
> possibly caps USE flags controlling this for ping), and also that btrfs
> send/receive did have a recent bugfix related to the extended-attributes
> normally used to record filecaps, so the symptoms match the bug and
> that's probably what you were seeing.

Ah, thanks, that looks like it was it!  I didn't think about extended 
attributes, but including "xattr" in my search yielded the following patches 
from April this year (this turns out to be that vaguely remembered patch/
discussion that I mentioned):

[PATCH] btrfs: add chattr support for send/receive
[PATCH] btrfs: add verify chattr support for send/receive test

However, IIUC those changes are going to be merged along with other changes 
into a v2 of the send protocoll, so until that gets finalized this is 
something to be aware of for those like me that use send/receive for backups.

Anyway, thanks for pointing me in the right direction!  At least now I 
understand what happened.

Greetings
-- 
Marc Joliet
--
"People who think they know everything really annoy those of us who know we
don't" - Bjarne Stroustrup


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH resend 1/2] btrfs: allow defrag on a file opened ro that has rw permissions

2018-07-24 Thread Adam Borowski
On Mon, Jul 23, 2018 at 05:26:24PM +0200, David Sterba wrote:
> On Wed, Jul 18, 2018 at 12:08:59AM +0200, Adam Borowski wrote:
(Combined with as-folded)

| | btrfs: allow defrag on a file opened read-only that has rw permissions
| |
> > Requiring a rw descriptor conflicts both ways with exec, returning ETXTBSY
> > whenever you try to defrag a program that's currently being run, or
> > causing intermittent exec failures on a live system being defragged.
> > 
> > As defrag doesn't change the file's contents in any way, there's no reason
> > to consider it a rw operation.  Thus, let's check only whether the file
> > could have been opened rw.  Such access control is still needed as
> > currently defrag can use extra disk space, and might trigger bugs.
<-
| | We give EINVAL when the request is invalid; here it's ok but merely the
| | user has insufficient privileges.  Thus, this return value reflects the
| | error better -- as discussed in the identical case for dedupe.
| |
| | According to codesearch.debian.net, no userspace program distinguishes
| | these values beyond strerror().
| |
| | Signed-off-by: Adam Borowski 
| | Reviewed-by: David Sterba 
| | [ fold the EPERM patch from Adam ]
| | Signed-off-by: David Sterba 

[...]
> So, I'll add the patch to 4.19 queue. It's small and isolated change so
> a revert would be easy in case we find something bad. The 2nd patch
> should be IMHO part of this change as it's logical to return the error
> code in the patch that modifies the user visible behaviour.

A nitpick: the new commit message has a dangling pointer "this" to the title
of the commit that was squashed.  It was:

| btrfs: defrag: return EPERM not EINVAL when only permissions fail

It'd be nice if it could be inserted in some form in the place I marked with
an arrow.

But then, commit messages are not vital.  The actual functionality patch has
been applied correctly.  And thanks for adding the comment.


Meow!
-- 
// If you believe in so-called "intellectual property", please immediately
// cease using counterfeit alphabets.  Instead, contact the nearest temple
// of Amon, whose priests will provide you with scribal services for all
// your writing needs, for Reasonable And Non-Discriminatory prices.
--
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: File permissions lost during send/receive?

2018-07-24 Thread Duncan
Andrei Borzenkov posted on Tue, 24 Jul 2018 20:53:15 +0300 as excerpted:

> 24.07.2018 15:16, Marc Joliet пишет:
>> Hi list,
>> 
>> (Preemptive note: this was with btrfs-progs 4.15.1, I have since
>> upgraded to 4.17.  My kernel version is 4.14.52-gentoo.)
>> 
>> I recently had to restore the root FS of my desktop from backup (extent
>> tree corruption; not sure how, possibly a loose SATA cable?). 
>> Everything was fine,
>> even if restoring was slower than expected.  However, I encountered two
>> files with permission problems, namely:
>> 
>> - /bin/ping, which caused running ping as a normal user to fail due to
>> missing permissions, and
>> 
>> - /sbin/unix_chkpwd (part of PAM), which prevented me from unlocking
>> the KDE Plasma lock screen; I needed to log into a TTY and run
>> "loginctl unlock- session".
>> 
>> Both were easily fixed by reinstalling the affected packages (iputils
>> and pam), but I wonder why this happened after restoring from backup.
>> 
>> I originally thought it was related to the SUID bit not being set,
>> because of the explanation in the ping(8) man page (section
>> "SECURITY"), but cannot find evidence of that -- that is, after
>> reinstallation, "ls -lh" does not show the sticky bit being set, or any
>> other special permission bits, for that matter:
>> 
>> % ls -lh /bin/ping /sbin/unix_chkpwd
>> -rwx--x--x 1 root root 60K 22. Jul 14:47 /bin/ping*
>> -rwx--x--x 1 root root 31K 23. Jul 00:21 /sbin/unix_chkpwd*
>> 
>> (Note: no ACLs are set, either.)
>> 
>> 
> What "getcap /bin/ping" says? You may need to install package providing
> getcap (libcap-progs here on openSUSE).

sys-libs/libcap on gentoo.  Here's what I get:

$ getcap /bin/ping
/bin/ping = cap_net_raw+ep

(getcap on unix_chkpwd returns nothing, but while I use kde/plasma I 
don't normally use the lockscreen at all, so for all I know that's broken 
here too.)

As hinted, it's almost certainly a problem with filecaps.  While I'll 
freely admit to not fully understanding how file-caps work, and my use-
case doesn't use send/receive, I do recall filecaps are what ping uses 
these days instead of SUID/SGID (on gentoo it'd be iputils' filecaps and 
possibly caps USE flags controlling this for ping), and also that btrfs 
send/receive did have a recent bugfix related to the extended-attributes 
normally used to record filecaps, so the symptoms match the bug and 
that's probably what you were seeing.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

--
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: File permissions lost during send/receive?

2018-07-24 Thread Andrei Borzenkov
24.07.2018 15:16, Marc Joliet пишет:
> Hi list,
> 
> (Preemptive note: this was with btrfs-progs 4.15.1, I have since upgraded to 
> 4.17.  My kernel version is 4.14.52-gentoo.)
> 
> I recently had to restore the root FS of my desktop from backup (extent tree 
> corruption; not sure how, possibly a loose SATA cable?).  Everything was 
> fine, 
> even if restoring was slower than expected.  However, I encountered two files 
> with permission problems, namely:
> 
> - /bin/ping, which caused running ping as a normal user to fail due to 
> missing 
> permissions, and
> 
> - /sbin/unix_chkpwd (part of PAM), which prevented me from unlocking the KDE 
> Plasma lock screen; I needed to log into a TTY and run "loginctl unlock-
> session".
> 
> Both were easily fixed by reinstalling the affected packages (iputils and 
> pam), but I wonder why this happened after restoring from backup.
> 
> I originally thought it was related to the SUID bit not being set, because of 
> the explanation in the ping(8) man page (section "SECURITY"), but cannot find 
> evidence of that -- that is, after reinstallation, "ls -lh" does not show the 
> sticky bit being set, or any other special permission bits, for that matter:
> 
> % ls -lh /bin/ping /sbin/unix_chkpwd 
> -rwx--x--x 1 root root 60K 22. Jul 14:47 /bin/ping*   
>   
>   
>  
> -rwx--x--x 1 root root 31K 23. Jul 00:21 /sbin/unix_chkpwd*
> 
> (Note: no ACLs are set, either.)
> 

What "getcap /bin/ping" says? You may need to install package providing
getcap (libcap-progs here on openSUSE).

> I do remember the qcheck program (a Gentoo-specific program that checks the 
> integrity of installed packages) complaining about wrong file permissions, 
> but 
> I didn't save its output, and there's a chance it *might* have been because I 
> ran qcheck without root permissions :-/ .
> 
> I vaguely remember some patches and/or discussion regarding permission 
> transfer issues with send/receive on this ML, but didn't find anything after 
> searching through my Email archive, so I might be misremembering.
> 
> Does anybody have any idea what possibly went wrong, or any similar 
> experience 
> to speak of?
> 
> Greetings
> 




signature.asc
Description: OpenPGP digital signature


Next btrfs development cycle open - 4.20

2018-07-24 Thread David Sterba
From: David Sterba 

Hi,

a friendly reminder of the timetable and what's expected at this phase.

Besides the recurring stuff, there's an update about development git repos.

4.17 - current
4.18 - upcoming, urgent regression fixes only
4.19 - development closed, pull request in prep, fixes or regressions only
4.20 - development open, until 4.19-rc5 (at least)

(https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#Development_schedule)


Current status
--

What's in misc-next is going to be merged to 4.19, unless something really
serious pops out.

Branches that have been part of for-next are eligible but may need some
last reviews and testing, because the for-next is mainly meant for early
warnings and fstests report way more failures compared to master or
misc-next.


Git development repos udpate


I'm going to phase out the git mirror at repo.or.cz. It served me well over the
years, but with recent changes to github, I'm going to use gitlab.com more
extensively and it's a good opportunity to switch the mirror. I'm now managing
2 devel repos plus the k.org and want to keep it sane.

  k.org: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git
  devel1: https://gitlab.com/kdave/btrfs-devel
  devel2: https://github.com/kdave/btrfs-devel

The devel repos should be identical regarding the main development branches
like misc-next or for-next-MMDD. The mirror at repo.or.cz will be still
updated for a few weeks.


List of things with earlier ack for 4.19


* patchsets sent until this week that have been reviewed

I want to focus on stabilization of misc-next in the upcoming weeks so
patches that are not going make things better may be postponed. We can
do some cleanup rounds, update messages or do other obviously low-risk
changes.


List of things pushed to 4.20 or later
--

* ioctl CLEAR_FREE
* send v2 updates
* in-band dedupe
* the rest not mentioned above


Usual points


* the current patch queue (as is in misc-next) looks stable, so no big
  changes are going to be applied at this time. The usual exceptions are
  bugfixes or obvious cleanups.

* the base of the patches should be the last announced pull request,
  which is going to be named 'for-4.19' in my k.org tree.  Reviewed
  patches will be collected in a branch that's usually named 'misc-next'
  in my devel git repos and is part of the for-next at k.org git repo.

* merging of new patches to misc-next will be slow during the
  merge window, also because there's a btrfs-progs release scheduled and it's
  summer

* everybody is encouraged to review or test other's patches
--
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] Btrfs: fix send failure when root has deleted files still open

2018-07-24 Thread David Sterba
On Tue, Jul 24, 2018 at 11:54:04AM +0100, fdman...@kernel.org wrote:
> From: Filipe Manana 
> 
> The more common use case of send involves creating a RO snapshot and then
> use it for a send operation. In this case it's not possible to have inodes
> in the snapshot that have a link count of zero (inode with an orphan item)
> since during snapshot creation we do the orphan cleanup. However, other
> less common use cases for send can end up seeing inodes with a link count
> of zero and in this case the send operation fails with a ENOENT error
> because any attempt to generate a path for the inode, with the purpose
> of creating it or updating it at the receiver, fails since there are no
> inode reference items. One use case it to use a regular subvolume for
> a send operation after turning it to RO mode or turning a RW snapshot
> into RO mode and then using it for a send operation. In both cases, if a
> file gets all its hard links deleted while there is an open file
> descriptor before turning the subvolume/snapshot into RO mode, the send
> operation will encounter an inode with a link count of zero and then
> fail with errno ENOENT.
> 
> Example using a full send with a subvolume:
> 
>   $ mkfs.btrfs -f /dev/sdb
>   $ mount /dev/sdb /mnt
> 
>   $ btrfs subvolume create /mnt/sv1
>   $ touch /mnt/sv1/foo
>   $ touch /mnt/sv1/bar
> 
>   # keep an open file descriptor on file bar
>   $ exec 73   $ unlink /mnt/sv1/bar
> 
>   # Turn the subvolume to RO mode and use it for a full send, while
>   # holding the open file descriptor.
>   $ btrfs property set /mnt/sv1 ro true
> 
>   $ btrfs send -f /tmp/full.send /mnt/sv1
>   At subvol /mnt/sv1
>   ERROR: send ioctl failed with -2: No such file or directory
> 
> Example using an incremental send with snapshots:
> 
>   $ mkfs.btrfs -f /dev/sdb
>   $ mount /dev/sdb /mnt
> 
>   $ btrfs subvolume create /mnt/sv1
>   $ touch /mnt/sv1/foo
>   $ touch /mnt/sv1/bar
> 
>   $ btrfs subvolume snapshot -r /mnt/sv1 /mnt/snap1
> 
>   $ echo "hello world" >> /mnt/sv1/bar
> 
>   $ btrfs subvolume snapshot -r /mnt/sv1 /mnt/snap2
> 
>   # Turn the second snapshot to RW mode and delete file foo while
>   # holding an open file descriptor on it.
>   $ btrfs property set /mnt/snap2 ro false
>   $ exec 73   $ unlink /mnt/snap2/foo
> 
>   # Set the second snapshot back to RO mode and do an incremental send.
>   $ btrfs property set /mnt/snap2 ro true
> 
>   $ btrfs send -f /tmp/inc.send -p /mnt/snap1 /mnt/snap2
>   At subvol /mnt/snap2
>   ERROR: send ioctl failed with -2: No such file or directory
> 
> So fix this by ignoring inodes with a link count of zero if we are either
> doing a full send or if they do not exist in the parent snapshot (they
> are new in the send snapshot), and unlink all paths found in the parent
> snapshot when doing an incremental send (and ignoring all other inode
> items, such as xattrs and extents).
> 
> A test case for fstests follows soon.
> 
> Reported-by: Martin Wilck 
> Signed-off-by: Filipe Manana 

Added to misc-next, thanks. I did a light review of the overall logic
how the ignore_cur_inode is passed around, skipping the orphans and
replacing with send_unlink sounds ok to me.
--
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


File permissions lost during send/receive?

2018-07-24 Thread Marc Joliet
Hi list,

(Preemptive note: this was with btrfs-progs 4.15.1, I have since upgraded to 
4.17.  My kernel version is 4.14.52-gentoo.)

I recently had to restore the root FS of my desktop from backup (extent tree 
corruption; not sure how, possibly a loose SATA cable?).  Everything was fine, 
even if restoring was slower than expected.  However, I encountered two files 
with permission problems, namely:

- /bin/ping, which caused running ping as a normal user to fail due to missing 
permissions, and

- /sbin/unix_chkpwd (part of PAM), which prevented me from unlocking the KDE 
Plasma lock screen; I needed to log into a TTY and run "loginctl unlock-
session".

Both were easily fixed by reinstalling the affected packages (iputils and 
pam), but I wonder why this happened after restoring from backup.

I originally thought it was related to the SUID bit not being set, because of 
the explanation in the ping(8) man page (section "SECURITY"), but cannot find 
evidence of that -- that is, after reinstallation, "ls -lh" does not show the 
sticky bit being set, or any other special permission bits, for that matter:

% ls -lh /bin/ping /sbin/unix_chkpwd 
-rwx--x--x 1 root root 60K 22. Jul 14:47 /bin/ping* 


   
-rwx--x--x 1 root root 31K 23. Jul 00:21 /sbin/unix_chkpwd*

(Note: no ACLs are set, either.)

I do remember the qcheck program (a Gentoo-specific program that checks the 
integrity of installed packages) complaining about wrong file permissions, but 
I didn't save its output, and there's a chance it *might* have been because I 
ran qcheck without root permissions :-/ .

I vaguely remember some patches and/or discussion regarding permission 
transfer issues with send/receive on this ML, but didn't find anything after 
searching through my Email archive, so I might be misremembering.

Does anybody have any idea what possibly went wrong, or any similar experience 
to speak of?

Greetings
-- 
Marc Joliet
--
"People who think they know everything really annoy those of us who know we
don't" - Bjarne Stroustrup


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH 07/22] btrfs: don't leak ret from do_chunk_alloc

2018-07-24 Thread David Sterba
On Thu, Jul 19, 2018 at 06:43:39PM +0300, Nikolay Borisov wrote:
> 
> 
> On 19.07.2018 17:49, Josef Bacik wrote:
> > If we're trying to make a data reservation and we have to allocate a
> > data chunk we could leak ret == 1, as do_chunk_alloc() will return 1 if
> > it allocated a chunk.  Since the end of the function is the success path
> > just return 0.
> > 
> > Signed-off-by: Josef Bacik 
> 
> Reviewed-by: Nikolay Borisov 
> 
> The logic flow in this function is a steaming pile of turd and is in
> dire need of refactoring...

Agreed, fixing the default return code seems like the right fix now,
compared to untangling the gotos. Patch added to misc-next.
--
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 2/2] btrfs: fix missing superblock update in the device delete commit transaction

2018-07-24 Thread David Sterba
On Tue, Jul 24, 2018 at 05:28:03PM +0800, Lu Fengqi wrote:
> >I can't reproduce the issue. Do you reproduce consistently? and I
> 
> Sorry I've taken so long to reply.
> 
> There are four virtual machines with the different storage backend here.
> And I can reproduce this issue consistently on the virtual machines (with
> HDD or qcow2 file in HDD), however, I can't reproduce on the virtual machine
> (with SSD or qcow2 file in SSD). So this may depend on the disk IO speed.
> 
> >wonder if your workspace contains the latest changes from
> >kdave/misc-next? Because last weeks kdave/misc had some issues.
> >Can you please give a try?
> 
> I used the latest kdave/misc-next branch on July 21, 2018, when I send
> the previous mail. Just as David replied to Nikolay's thread, the current
> latest kdave/misc-next still has the same problem.

I hit that on a physical machine with a mix of HDD and SSD drives under
fstests. The VM tests did not hit the problem.
--
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 v2] Btrfs: fix send failure when root has deleted files still open

2018-07-24 Thread fdmanana
From: Filipe Manana 

The more common use case of send involves creating a RO snapshot and then
use it for a send operation. In this case it's not possible to have inodes
in the snapshot that have a link count of zero (inode with an orphan item)
since during snapshot creation we do the orphan cleanup. However, other
less common use cases for send can end up seeing inodes with a link count
of zero and in this case the send operation fails with a ENOENT error
because any attempt to generate a path for the inode, with the purpose
of creating it or updating it at the receiver, fails since there are no
inode reference items. One use case it to use a regular subvolume for
a send operation after turning it to RO mode or turning a RW snapshot
into RO mode and then using it for a send operation. In both cases, if a
file gets all its hard links deleted while there is an open file
descriptor before turning the subvolume/snapshot into RO mode, the send
operation will encounter an inode with a link count of zero and then
fail with errno ENOENT.

Example using a full send with a subvolume:

  $ mkfs.btrfs -f /dev/sdb
  $ mount /dev/sdb /mnt

  $ btrfs subvolume create /mnt/sv1
  $ touch /mnt/sv1/foo
  $ touch /mnt/sv1/bar

  # keep an open file descriptor on file bar
  $ exec 73> /mnt/sv1/bar

  $ btrfs subvolume snapshot -r /mnt/sv1 /mnt/snap2

  # Turn the second snapshot to RW mode and delete file foo while
  # holding an open file descriptor on it.
  $ btrfs property set /mnt/snap2 ro false
  $ exec 73
Signed-off-by: Filipe Manana 
---

V2: Fixed a null pointer dereference for non-incremental send on
sctx->left_path.

 fs/btrfs/send.c | 138 
 1 file changed, 130 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 30be18be0036..0f31760f875f 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -100,6 +100,7 @@ struct send_ctx {
u64 cur_inode_rdev;
u64 cur_inode_last_extent;
u64 cur_inode_next_write_offset;
+   bool ignore_cur_inode;
 
u64 send_progress;
 
@@ -5799,6 +5800,9 @@ static int finish_inode_if_needed(struct send_ctx *sctx, 
int at_end)
int pending_move = 0;
int refs_processed = 0;
 
+   if (sctx->ignore_cur_inode)
+   return 0;
+
ret = process_recorded_refs_if_needed(sctx, at_end, _move,
  _processed);
if (ret < 0)
@@ -5917,6 +5921,94 @@ static int finish_inode_if_needed(struct send_ctx *sctx, 
int at_end)
return ret;
 }
 
+struct parent_paths_ctx {
+   struct list_head *refs;
+   struct send_ctx *sctx;
+};
+
+static int record_parent_ref(int num, u64 dir, int index, struct fs_path *name,
+void *ctx)
+{
+   struct parent_paths_ctx *ppctx = ctx;
+
+   return record_ref(ppctx->sctx->parent_root, dir, name, ppctx->sctx,
+ ppctx->refs);
+}
+
+/*
+ * Issue unlink operations for all paths of the current inode found in the
+ * parent snapshot.
+ */
+static int btrfs_unlink_all_paths(struct send_ctx *sctx)
+{
+   LIST_HEAD(deleted_refs);
+   struct btrfs_path *path;
+   struct btrfs_key key;
+   struct parent_paths_ctx ctx;
+   int ret;
+
+   path = alloc_path_for_send();
+   if (!path)
+   return -ENOMEM;
+
+   key.objectid = sctx->cur_ino;
+   key.type = BTRFS_INODE_REF_KEY;
+   key.offset = 0;
+   ret = btrfs_search_slot(NULL, sctx->parent_root, , path, 0, 0);
+   if (ret < 0)
+   goto out;
+
+   ctx.refs = _refs;
+   ctx.sctx = sctx;
+
+   while (true) {
+   struct extent_buffer *eb = path->nodes[0];
+   int slot = path->slots[0];
+
+   if (slot >= btrfs_header_nritems(eb)) {
+   ret = btrfs_next_leaf(sctx->parent_root, path);
+   if (ret < 0)
+   goto out;
+   else if (ret > 0)
+   break;
+   continue;
+   }
+
+   btrfs_item_key_to_cpu(eb, , slot);
+   if (key.objectid != sctx->cur_ino)
+   break;
+   if (key.type != BTRFS_INODE_REF_KEY &&
+   key.type != BTRFS_INODE_EXTREF_KEY)
+   break;
+
+   ret = iterate_inode_ref(sctx->parent_root, path, , 1,
+   record_parent_ref, );
+   if (ret < 0)
+   goto out;
+
+   path->slots[0]++;
+   }
+
+   while (!list_empty(_refs)) {
+   struct recorded_ref *ref;
+
+   ref = list_first_entry(_refs, struct recorded_ref,
+  list);
+   ret = send_unlink(sctx, ref->full_path);
+   if (ret < 0)
+   goto out;
+   

Re: [PATCH 0/7] fs_info cleanups for volume.c

2018-07-24 Thread David Sterba
On Tue, Jul 24, 2018 at 04:59:00PM +0800, Lu Fengqi wrote:
> After I applied this patchset, my test also encountered this crash.
> However, the result of git bisect shows the cause may be the
> "[PATCH 2/2] btrfs: fix missing superblock update in the device delete commit 
> transaction".
> Moreover, I have reported this to Anand Jain. I will send him the updated
> log(kasan enabled), and tell him why he can't reproduce before. Please
> see the thread I will cc you.

Thanks for bisecting it, I'll remove the patch until it's resolved and
re-add Nikolai's patches.
--
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 2/2] btrfs: fix missing superblock update in the device delete commit transaction

2018-07-24 Thread Lu Fengqi
On Mon, Jul 23, 2018 at 03:52:59PM +0800, Anand Jain wrote:
>
>
>On 07/21/2018 02:01 PM, Lu Fengqi wrote:
>> On Tue, Jul 03, 2018 at 05:07:24PM +0800, Anand Jain wrote:
>> > When a device is deleted, the btrfs_super_block::number_devices is
>> > reduced by 1, but we do that after the commit transaction, so this
>> > change did not made it to the disk and waits for the next commit
>> > transaction whenever it happens.
>> > 
>> > This can be easily demonstrated using the following test case where I
>> > use the btrfs device ready cli to read the disk and report.
>> > 
>> >   mkfs.btrfs -fq -dsingle -msingle $dev1 $dev2
>> >   mount $dev1 /btrfs
>> >   btrfs dev del $dev2 /btrfs
>> >   btrfs dev ready $dev1; echo RESULT=$? <-- 1
>> >Without this patch RESULT returns 1, indicating not ready!
>> > 
>> >   Testing with a seed device:
>> > 
>> >   mkfs.btrfs -fq $dev1
>> >   btrfstune -S1 $dev1
>> >   mount $dev1 /btrfs
>> >   btrfs dev add -f $dev2 /btrfs
>> >   umount /btrfs
>> >   mount $dev2 /btrfs
>> >   btrfs dev del $dev1 /btrfs
>> >   btrfs dev ready $dev2; echo RESULT=$? <-- 1
>> > 
>> >   Fix this by bringing in the num_devices update with in the
>> >   current commit transaction.
>> >   Also align the local variable declarations in the function
>> >   btrfs_rm_dev_item()
>> >   Delete a todo comment about transient inconsistent state
>> 
>> Hi, Anand
>> 
>> The btrfs/164 failed when I run xfstests with kdave/misc-next.
>
> And the
>> result of git bisect shows this patch may be the cause. Please see the
>> following log and dmesg.
>> 
>> xfstests log:
>> # sudo ./check btrfs/164
>> FSTYP -- btrfs
>> PLATFORM  -- Linux/x86_64 larch 4.18.0-rc5+
>> MKFS_OPTIONS  -- /dev/vdb2
>> MOUNT_OPTIONS -- /dev/vdb2 /mnt/scratch
>> 
>> btrfs/164 14s ... [failed, exit status 1]
>> 
>> dmesg:
>> [  212.009967] general protection fault:  [#1] SMP PTI
>> [  212.015834] CPU: 1 PID: 5665 Comm: btrfs Tainted: G   O  
>> 4.18.0-rc5+ #2
>> [  212.021985] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
>> 0.0.0 02/06/2015
>> [  212.031137] RIP: 0010:btrfs_update_commit_device_size+0x74/0xd0 [btrfs]
>
>
>Thanks for the report.
>
>
>--
>void btrfs_update_commit_device_size(struct btrfs_fs_info *fs_info)
>{
>struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>struct btrfs_device *curr, *next;
>
>if (list_empty(_devices->resized_devices))
>return;
>
>mutex_lock(_devices->device_list_mutex);
>mutex_lock(_info->chunk_mutex);
>list_for_each_entry_safe(curr, next, _devices->resized_devices,
> resized_list) {
>list_del_init(>resized_list); <  GPF
>curr->commit_total_bytes = curr->disk_total_bytes;
>}
>mutex_unlock(_info->chunk_mutex);
>mutex_unlock(_devices->device_list_mutex);
>}
>--
>
>
>I can't reproduce the issue. Do you reproduce consistently? and I

Sorry I've taken so long to reply.

There are four virtual machines with the different storage backend here.
And I can reproduce this issue consistently on the virtual machines (with
HDD or qcow2 file in HDD), however, I can't reproduce on the virtual machine
(with SSD or qcow2 file in SSD). So this may depend on the disk IO speed.

>wonder if your workspace contains the latest changes from
>kdave/misc-next? Because last weeks kdave/misc had some issues.
>Can you please give a try?

I used the latest kdave/misc-next branch on July 21, 2018, when I send
the previous mail. Just as David replied to Nikolay's thread, the current
latest kdave/misc-next still has the same problem.

In addition, this case will not fail when I enable KASAN. But the something
found in dmesg may help you to investigate. I attached them below.

-- 
Thanks,
Lu

log:
# sudo ./check btrfs/164
FSTYP -- btrfs
PLATFORM  -- Linux/x86_64 localhost 4.18.0-rc6+
MKFS_OPTIONS  -- /dev/vdc1
MOUNT_OPTIONS -- /dev/vdc1 /mnt/scratch

btrfs/164 15s ... _check_dmesg: something found in dmesg (see 
/home/luke/workspace/xfstests-dev/results//btrfs/164.dmesg)

Ran: btrfs/164
Failures: btrfs/164
Failed 1 of 1 tests

dmesg(KASAN enabled):
  444.636617] ==
[  444.639206] BUG: KASAN: use-after-free in 
btrfs_update_commit_device_size+0x124/0x2c0 [btrfs]
[  444.641234] Read of size 8 at addr 8801542ad1d0 by task btrfs/4575

[  444.645210] CPU: 0 PID: 4575 Comm: btrfs Not tainted 4.18.0-rc6+ #4
[  444.647150] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 
02/06/2015
[  444.649351] Call Trace:
[  444.650715]  dump_stack+0xd1/0x16c
[  444.652201]  ? dump_stack_print_info.cold.1+0x2f/0x2f
[  444.653924]  ? kmsg_dump_rewind_nolock+0x59/0x59
[  444.655637]  ? btrfs_update_commit_device_size+0x124/0x2c0 [btrfs]
[  444.657492]  print_address_description+0x6c/0x23c
[  444.659257]  ? 

Re: [PATCH 0/7] fs_info cleanups for volume.c

2018-07-24 Thread Lu Fengqi
On Tue, Jul 24, 2018 at 10:28:04AM +0200, David Sterba wrote:
>On Mon, Jul 23, 2018 at 03:25:01PM +0200, David Sterba wrote:
>> On Fri, Jul 20, 2018 at 07:37:46PM +0300, Nikolay Borisov wrote:
>> > Here are a bunch of patches which cleanup extraneous fs_info parameters to 
>> > function which already take a structure that holds a reference to the 
>> > fs_info. 
>> > 
>> > Except for patches 4 and 5, everything else is correct - due to those 
>> > functions
>> > always taking a transaction. 4 and 5 in turn reference the fs_info from 
>> > struct btrfs_device. Inspecting the callers I managed to convince myself 
>> > that 
>> > those function are always called with well-formed btrfs_device i.e one 
>> > which 
>> > has its fs_info member initialised. Reviewers might want to pay extra 
>> > attention to that but otherwise they are trivial. 
>> 
>> 4 and 5 look good to me, a device without a valid fs_info has a short
>> timespan and should not appear anywhere besides the helpers that set up
>> fs_devices etc. Series added to misc-next, thanks.
>
>btrfs/164 crashed in device rm that could be related to the patches, I
>haven't analyzed that but this series looks most suspicious since the
>last round of tests that did not see that crash:
>
>[ 6712.084324] general protection fault:  [#1] PREEMPT SMP
>[ 6712.090096] CPU: 0 PID: 2694 Comm: btrfs Not 
>tainted4.18.0-rc6-1.ge195904-vanilla+ #279
>[ 6712.098398] Hardware name: empty empty/S3993, BIOS PAQEX0-302/24/2008
>[ 6712.105072] RIP: 0010:__list_del_entry_valid+0x25/0xc0
>[ 6712.129603] RSP: 0018:b01281e2bbd0 EFLAGS: 00010a83
>[ 6712.134969] RAX: 6b6b6b6b6b6b6b6b RBX: 9972756dafd8 RCX:dead0200
>[ 6712.142246] RDX: 6b6b6b6b6b6b6b6b RSI: c10252cf RDI:9972756db100
>[ 6712.149507] RBP: 9972756db100 R08:  R09:0001
>[ 6712.156788] R10: b01281e2bbd8 R11:  R12:99728a2d7500
>[ 6712.164059] R13: 9972756c0910 R14: 99728a2d7450 R15:6b6b6b6b6b6b6a43
>[ 6712.171332] FS:  7f3100bb58c0() 
>GS:9972a660()knlGS:
>[ 6712.179615] CS:  0010 DS:  ES:  CR0: 80050033
>[ 6712.185507] CR2: 7f336c989000 CR3: 0001f6868000 CR4:06f0
>[ 6712.192779] Call Trace:
>[ 6712.195423]  btrfs_update_commit_device_size+0x75/0xf0 [btrfs]
>[ 6712.201424]  btrfs_commit_transaction+0x57d/0xa90 [btrfs]
>[ 6712.206999]  btrfs_rm_device+0x627/0x850 [btrfs]
>[ 6712.211800]  btrfs_ioctl+0x2b03/0x3120 [btrfs]
>[ 6712.216404]  ? __might_fault+0x3e/0x90
>[ 6712.220277]  ? lock_acquire+0x9f/0x270
>[ 6712.224156]  ? __audit_syscall_entry+0xd6/0x170
>[ 6712.228835]  ? ktime_get_coarse_real_ts64+0x67/0x100
>[ 6712.233940]  ? do_vfs_ioctl+0xa5/0x6f0
>[ 6712.237836]  do_vfs_ioctl+0xa5/0x6f0
>[ 6712.241554]  ? syscall_trace_enter+0x1e8/0x3e0
>[ 6712.246130]  ksys_ioctl+0x70/0x80
>[ 6712.249593]  __x64_sys_ioctl+0x16/0x20
>[ 6712.253484]  do_syscall_64+0x62/0x1c0
>[ 6712.257291]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>[ 6712.262482] RIP: 0033:0x7f30ffc52417
>[ 6712.285413] RSP: 002b:7ffd636f32c8 EFLAGS: 0202 
>ORIG_RAX:0010
>[ 6712.293191] RAX: ffda RBX: 7ffd636f5460 RCX:7f30ffc52417
>[ 6712.300462] RDX: 7ffd636f4300 RSI: 5000943a RDI:0003
>[ 6712.307742] RBP: 7ffd636f4300 R08:  R09:0010
>[ 6712.315005] R10: 0fa99fa0 R11: 0202 R12:
>[ 6712.322286] R13:  R14: 0003 R15:7ffd636f5468
>[ 6712.391596] ---[ end trace f05bf71894e4ee4d ]---
>
>

Hi, David

After I applied this patchset, my test also encountered this crash.
However, the result of git bisect shows the cause may be the
"[PATCH 2/2] btrfs: fix missing superblock update in the device delete commit 
transaction".
Moreover, I have reported this to Anand Jain. I will send him the updated
log(kasan enabled), and tell him why he can't reproduce before. Please
see the thread I will cc you.

-- 
Thanks,
Lu


--
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 0/7] fs_info cleanups for volume.c

2018-07-24 Thread David Sterba
On Mon, Jul 23, 2018 at 03:25:01PM +0200, David Sterba wrote:
> On Fri, Jul 20, 2018 at 07:37:46PM +0300, Nikolay Borisov wrote:
> > Here are a bunch of patches which cleanup extraneous fs_info parameters to 
> > function which already take a structure that holds a reference to the 
> > fs_info. 
> > 
> > Except for patches 4 and 5, everything else is correct - due to those 
> > functions
> > always taking a transaction. 4 and 5 in turn reference the fs_info from 
> > struct btrfs_device. Inspecting the callers I managed to convince myself 
> > that 
> > those function are always called with well-formed btrfs_device i.e one 
> > which 
> > has its fs_info member initialised. Reviewers might want to pay extra 
> > attention to that but otherwise they are trivial. 
> 
> 4 and 5 look good to me, a device without a valid fs_info has a short
> timespan and should not appear anywhere besides the helpers that set up
> fs_devices etc. Series added to misc-next, thanks.

btrfs/164 crashed in device rm that could be related to the patches, I
haven't analyzed that but this series looks most suspicious since the
last round of tests that did not see that crash:

[ 6712.084324] general protection fault:  [#1] PREEMPT SMP
[ 6712.090096] CPU: 0 PID: 2694 Comm: btrfs Not 
tainted4.18.0-rc6-1.ge195904-vanilla+ #279
[ 6712.098398] Hardware name: empty empty/S3993, BIOS PAQEX0-302/24/2008
[ 6712.105072] RIP: 0010:__list_del_entry_valid+0x25/0xc0
[ 6712.129603] RSP: 0018:b01281e2bbd0 EFLAGS: 00010a83
[ 6712.134969] RAX: 6b6b6b6b6b6b6b6b RBX: 9972756dafd8 RCX:dead0200
[ 6712.142246] RDX: 6b6b6b6b6b6b6b6b RSI: c10252cf RDI:9972756db100
[ 6712.149507] RBP: 9972756db100 R08:  R09:0001
[ 6712.156788] R10: b01281e2bbd8 R11:  R12:99728a2d7500
[ 6712.164059] R13: 9972756c0910 R14: 99728a2d7450 R15:6b6b6b6b6b6b6a43
[ 6712.171332] FS:  7f3100bb58c0() 
GS:9972a660()knlGS:
[ 6712.179615] CS:  0010 DS:  ES:  CR0: 80050033
[ 6712.185507] CR2: 7f336c989000 CR3: 0001f6868000 CR4:06f0
[ 6712.192779] Call Trace:
[ 6712.195423]  btrfs_update_commit_device_size+0x75/0xf0 [btrfs]
[ 6712.201424]  btrfs_commit_transaction+0x57d/0xa90 [btrfs]
[ 6712.206999]  btrfs_rm_device+0x627/0x850 [btrfs]
[ 6712.211800]  btrfs_ioctl+0x2b03/0x3120 [btrfs]
[ 6712.216404]  ? __might_fault+0x3e/0x90
[ 6712.220277]  ? lock_acquire+0x9f/0x270
[ 6712.224156]  ? __audit_syscall_entry+0xd6/0x170
[ 6712.228835]  ? ktime_get_coarse_real_ts64+0x67/0x100
[ 6712.233940]  ? do_vfs_ioctl+0xa5/0x6f0
[ 6712.237836]  do_vfs_ioctl+0xa5/0x6f0
[ 6712.241554]  ? syscall_trace_enter+0x1e8/0x3e0
[ 6712.246130]  ksys_ioctl+0x70/0x80
[ 6712.249593]  __x64_sys_ioctl+0x16/0x20
[ 6712.253484]  do_syscall_64+0x62/0x1c0
[ 6712.257291]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 6712.262482] RIP: 0033:0x7f30ffc52417
[ 6712.285413] RSP: 002b:7ffd636f32c8 EFLAGS: 0202 
ORIG_RAX:0010
[ 6712.293191] RAX: ffda RBX: 7ffd636f5460 RCX:7f30ffc52417
[ 6712.300462] RDX: 7ffd636f4300 RSI: 5000943a RDI:0003
[ 6712.307742] RBP: 7ffd636f4300 R08:  R09:0010
[ 6712.315005] R10: 0fa99fa0 R11: 0202 R12:
[ 6712.322286] R13:  R14: 0003 R15:7ffd636f5468
[ 6712.391596] ---[ end trace f05bf71894e4ee4d ]---
--
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