ssd vs ssd_spread with sdcard or emmc
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?
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?
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
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?
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?
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
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
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?
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
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
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
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
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
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
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
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