[PATCH] btrfs-progs: Eliminate remaining uses of strerror(errno)
%m allows a smaller filesize. Useful on embedded systems. Signed-off-by: Rosen Penev --- build/Documentation/Makefile | 142 + build/Makefile.inc | 42 ++ build/config.h | 139 + build/config.log | 822 ++ build/config.status | 1059 ++ build/version.h | 14 + cmds-qgroup.c|3 +- messages.h |4 +- mkfs/rootdir.c | 47 +- qgroup.c |3 +- 10 files changed, 2243 insertions(+), 32 deletions(-) create mode 100644 build/Documentation/Makefile create mode 100644 build/Makefile.inc create mode 100644 build/config.h create mode 100644 build/config.log create mode 100755 build/config.status create mode 100644 build/version.h diff --git a/build/Documentation/Makefile b/build/Documentation/Makefile new file mode 100644 index ..82ee005a --- /dev/null +++ b/build/Documentation/Makefile @@ -0,0 +1,142 @@ +# Guard against environment variables +MAN8_TXT = + +# Top level commands +MAN8_TXT += btrfs.asciidoc +MAN8_TXT += btrfs-convert.asciidoc +MAN8_TXT += btrfs-find-root.asciidoc +MAN8_TXT += btrfs-image.asciidoc +MAN8_TXT += btrfs-map-logical.asciidoc +MAN8_TXT += btrfs-select-super.asciidoc +MAN8_TXT += btrfstune.asciidoc +MAN8_TXT += fsck.btrfs.asciidoc +MAN8_TXT += mkfs.btrfs.asciidoc + +# Sub commands for btrfs +MAN8_TXT += btrfs-subvolume.asciidoc +MAN8_TXT += btrfs-filesystem.asciidoc +MAN8_TXT += btrfs-balance.asciidoc +MAN8_TXT += btrfs-device.asciidoc +MAN8_TXT += btrfs-scrub.asciidoc +MAN8_TXT += btrfs-check.asciidoc +MAN8_TXT += btrfs-rescue.asciidoc +MAN8_TXT += btrfs-inspect-internal.asciidoc +MAN8_TXT += btrfs-send.asciidoc +MAN8_TXT += btrfs-receive.asciidoc +MAN8_TXT += btrfs-quota.asciidoc +MAN8_TXT += btrfs-qgroup.asciidoc +MAN8_TXT += btrfs-replace.asciidoc +MAN8_TXT += btrfs-restore.asciidoc +MAN8_TXT += btrfs-property.asciidoc + +# Category 5 manual page +MAN5_TXT += btrfs-man5.asciidoc + +MAN3_TXT += btrfs-ioctl.asciidoc + +MAN_TXT = $(MAN3_TXT) $(MAN8_TXT) $(MAN5_TXT) +MAN_XML = $(patsubst %.asciidoc,%.xml,$(MAN_TXT)) +MAN_HTML = $(patsubst %.asciidoc,%.html,$(MAN_TXT)) +GZ_MAN3 = $(patsubst %.asciidoc,%.3.gz,$(MAN3_TXT)) +GZ_MAN5 = $(patsubst %.asciidoc,%.5.gz,$(MAN5_TXT)) +GZ_MAN8 = $(patsubst %.asciidoc,%.8.gz,$(MAN8_TXT)) + +mandir ?= $(prefix)/share/man +man3dir = $(mandir)/man3 +man5dir = $(mandir)/man5 +man8dir = $(mandir)/man8 + +ifeq (none,asciidoc) +ASCIIDOC = +ASCIIDOC_ARGS = -abtrfs_version=$(BTRFS_VERSION) -f asciidoc.conf +ASCIIDOC_HTML = html +ASCIIDOC_DOCBOOK = docbook +ASCIIDOC_DEPS = asciidoc.conf +endif +ifeq (none,asciidoctor) +ASCIIDOC = +ASCIIDOC_ARGS = -abtrfs_version=$(BTRFS_VERSION) +ASCIIDOC_HTML = xhtml5 +ASCIIDOC_DOCBOOK = docbook45 +ASCIIDOC_DEPS = +endif + +MANPAGE_XSL = manpage-normal.xsl +XMLTO = +XMLTO_EXTRA = +XMLTO_EXTRA = -m manpage-bold-literal.xsl +GZIPCMD = +INSTALL = /usr/bin/install -c +RM = /usr/bin/rm +RMDIR = /usr/bin/rmdir +LN_S = ln -s +MV = +SED = +BTRFS_VERSION = $(shell $(SED) -n 's/.*PACKAGE_VERSION "\(.*\)"/\1/p'\ + ../config.h) + +ifneq ($(findstring $(MAKEFLAGS),s),s) +ifndef V + QUIET_RM= @ + QUIET_ASCIIDOC = @echo "[ASCII] $@"; + QUIET_XMLTO = @echo "[XMLTO] $@"; + QUIET_GZIP = @echo "[GZ] $@"; +endif +endif + +all: man +man: man3 man5 man8 +man3: $(GZ_MAN3) +man5: $(GZ_MAN5) +man8: $(GZ_MAN8) +html: $(MAN_HTML) + +install: install-man + +install-man: man + $(INSTALL) -d -m 755 $(DESTDIR)$(man5dir) + $(INSTALL) -d -m 755 $(DESTDIR)$(man8dir) + $(INSTALL) -m 644 $(GZ_MAN5) $(DESTDIR)$(man5dir) + # the source file name of btrfs.5 clashes with section 8 page, but we + # want to keep the code generic + $(MV) $(DESTDIR)$(man5dir)/btrfs-man5.5.gz $(DESTDIR)$(man5dir)/btrfs.5.gz + $(INSTALL) -m 644 $(GZ_MAN8) $(DESTDIR)$(man8dir) + $(LN_S) -f btrfs-check.8.gz $(DESTDIR)$(man8dir)/btrfsck.8.gz + +uninstall: + cd $(DESTDIR)$(man8dir); rm -f btrfs-check.8.gz $(GZ_MAN8) + $(RMDIR) -p --ignore-fail-on-non-empty $(DESTDIR)$(man8dir) + +clean: + $(QUIET_RM)$(RM) -f *.xml *.xml+ *.3 *.3.gz *.5 *.5.gz *.8 *.8.gz *.html + +%.3.gz : %.3 + $(QUIET_GZIP)$(GZIPCMD) -n -c $< > $@ + +%.5.gz : %.5 + $(QUIET_GZIP)$(GZIPCMD) -n -c $< > $@ + +%.8.gz : %.8 + $(QUIET_GZIP)$(GZIPCMD) -n -c $< > $@ + +%.3 : %.xml + $(QUIET_XMLTO)$(RM) -f $@ && \ + $(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< + +%.5 : %.xml + $(QUIET_XMLTO)$(RM) -f $@ && \ + $(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< + +%.8 : %.xml + $(QUIET_XMLTO)$(RM) -f $@ && \ + $(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< + +%.xml : %.asciidoc $(ASCIIDOC_DEPS) + $(QUIET_ASCIIDOC)$(RM) -f $@+ $@ && \ + $(ASCIIDOC) $(ASCIIDOC_ARGS) -b $(ASCIIDOC_DOCBOOK) -d manpage -o $@+
Re: [PATCH] Btrfs: remove level==0 check in balance_level
On Wed, Sep 12, 2018 at 02:52:38PM +0200, David Sterba wrote: > On Wed, Sep 12, 2018 at 06:06:23AM +0800, Liu Bo wrote: > > btrfs_search_slot() > >if (level != 0) > > setup_nodes_for_search() > > balance_level() > > > > It is just impossible to have level=0 in balance_level. > > While this is true, what do you think about adding ASSERT(level > 0) ? > This is to catch accidentally passing level 0. Sounds good, will update it. thanks, -liubo
Re: [PATCH] Btrfs: assert page dirty bit
On Wed, Sep 12, 2018 at 09:38:49AM +0300, Nikolay Borisov wrote: > > > On 12.09.2018 01:06, Liu Bo wrote: > > Just in case that someone breaks the rule that pages are dirty as long > > as eb is dirty. > > > > Signed-off-by: Liu Bo > > --- > > fs/btrfs/extent_io.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index fb2bf50134a1..99895f196ecb 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -5184,6 +5184,11 @@ bool set_extent_buffer_dirty(struct extent_buffer > > *eb) > > set_page_dirty(eb->pages[i]); > > } > > > > +#ifdef BTRFS_DEBUG > > And this will never be compiled since the actual ifdef name is > "CONFIG_BTRFS_DEBUG" > Oops, will fix it. thanks, -liubo > > + for (i = 0; i < num_pages; i++) > > + ASSERT(PageDirty(eb->pages[i])); > > +#endif > > + > > return was_dirty; > > } > > > >
Re: [PATCH] Btrfs: skip set_page_dirty if eb is dirty
On Wed, Sep 12, 2018 at 09:37:20AM +0300, Nikolay Borisov wrote:
>
>
> On 12.09.2018 01:06, Liu Bo wrote:
> > As long as @eb is marked with EXTENT_BUFFER_DIRTY, all of its pages
> > are dirty, so no need to set pages dirty again.
> >
> > Signed-off-by: Liu Bo
>
> Does make it any performance difference, numbers?
>
To be honest, the performance difference would be trivial in a normal
big test round. But I just looked into the difference from my ftrace,
removing the loop can reduce the time spent by 10us in my box.
> Reviewed-by: Nikolay Borisov
Thanks a lot for reviewing.
thanks,
-liubo
> > ---
> > fs/btrfs/extent_io.c | 11 +++
> > fs/btrfs/extent_io.h | 2 +-
> > 2 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> > index 628f1aef34b0..fb2bf50134a1 100644
> > --- a/fs/btrfs/extent_io.c
> > +++ b/fs/btrfs/extent_io.c
> > @@ -5165,11 +5165,11 @@ void clear_extent_buffer_dirty(struct extent_buffer
> > *eb)
> > WARN_ON(atomic_read(>refs) == 0);
> > }
> >
> > -int set_extent_buffer_dirty(struct extent_buffer *eb)
> > +bool set_extent_buffer_dirty(struct extent_buffer *eb)
> > {
> > int i;
> > int num_pages;
> > - int was_dirty = 0;
> > + bool was_dirty;
> >
> > check_buffer_tree_ref(eb);
> >
> > @@ -5179,8 +5179,11 @@ int set_extent_buffer_dirty(struct extent_buffer *eb)
> > WARN_ON(atomic_read(>refs) == 0);
> > WARN_ON(!test_bit(EXTENT_BUFFER_TREE_REF, >bflags));
> >
> > - for (i = 0; i < num_pages; i++)
> > - set_page_dirty(eb->pages[i]);
> > + if (!was_dirty) {
> > + for (i = 0; i < num_pages; i++)
> > + set_page_dirty(eb->pages[i]);
> > + }
> > +
> > return was_dirty;
> > }
> >
> > diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> > index b4d03e677e1d..f2ab42d57f02 100644
> > --- a/fs/btrfs/extent_io.h
> > +++ b/fs/btrfs/extent_io.h
> > @@ -479,7 +479,7 @@ void extent_buffer_bitmap_set(struct extent_buffer *eb,
> > unsigned long start,
> > void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long
> > start,
> > unsigned long pos, unsigned long len);
> > void clear_extent_buffer_dirty(struct extent_buffer *eb);
> > -int set_extent_buffer_dirty(struct extent_buffer *eb);
> > +bool set_extent_buffer_dirty(struct extent_buffer *eb);
> > void set_extent_buffer_uptodate(struct extent_buffer *eb);
> > void clear_extent_buffer_uptodate(struct extent_buffer *eb);
> > int extent_buffer_under_io(struct extent_buffer *eb);
> >
Re: [PATCH] btrfs: wait on caching when putting the bg cache
On Wed, Sep 12, 2018 at 10:45:45AM -0400, Josef Bacik wrote: > While testing my backport I noticed there was a panic if I ran > generic/416 generic/417 generic/418 all in a row. This just happened to > uncover a race where we had outstanding IO after we destroy all of our > workqueues, and then we'd go to queue the endio work on those free'd > workqueues. This is because we aren't waiting for the caching threads > to be done before freeing everything up, so to fix this make sure we > wait on any outstanding caching that's being done before we free up the > block group, so we're sure to be done with all IO by the time we get to > btrfs_stop_all_workers(). This fixes the panic I was seeing > consistently in testing. Can you please attach the stacktrace(s)? I think I've seen similar error once or twice but not able to reproduce.
Re: [PATCH] btrfs: wait on caching when putting the bg cache
On Wed, Sep 12, 2018 at 10:45:45AM -0400, Josef Bacik wrote:
> While testing my backport I noticed there was a panic if I ran
> generic/416 generic/417 generic/418 all in a row. This just happened to
> uncover a race where we had outstanding IO after we destroy all of our
> workqueues, and then we'd go to queue the endio work on those free'd
> workqueues. This is because we aren't waiting for the caching threads
> to be done before freeing everything up, so to fix this make sure we
> wait on any outstanding caching that's being done before we free up the
> block group, so we're sure to be done with all IO by the time we get to
> btrfs_stop_all_workers(). This fixes the panic I was seeing
> consistently in testing.
Reviewed-by: Omar Sandoval
> Signed-off-by: Josef Bacik
> ---
> fs/btrfs/extent-tree.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 414492a18f1e..2eb2e37f2354 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -9889,6 +9889,7 @@ void btrfs_put_block_group_cache(struct btrfs_fs_info
> *info)
>
> block_group = btrfs_lookup_first_block_group(info, last);
> while (block_group) {
> + wait_block_group_cache_done(block_group);
> spin_lock(_group->lock);
> if (block_group->iref)
> break;
> --
> 2.14.3
>
Re: [PATCH 00/15] Add delayed-refs support to btrfs-progs
On Wed, Sep 12, 2018 at 07:51:39PM +0800, Su Yue wrote: > Actually, now kdave/devel still fails at fsck-tests/020 due to > version 1st of this patchset. See the thread please > https://www.spinics.net/lists/linux-btrfs/msg81675.html > > Nikolay's V2 patchset should slove the problem. > You may have known the situation, this mail is just a gentle reminder :). I'll replace the patches today, thanks.
Re: [PATCH 05/36] btrfs: only count ref heads run in __btrfs_run_delayed_refs
On Tue, Sep 11, 2018 at 04:07:30PM -0700, Omar Sandoval wrote: > On Tue, Sep 11, 2018 at 01:57:36PM -0400, Josef Bacik wrote: > > We pick the number of ref's to run based on the number of ref heads, and > > only make the decision to stop once we've processed entire ref heads, so > > only count the ref heads we've run and bail once we've hit the number of > > ref heads we wanted to process. > > Despite Nikolay's comment, it seems wrong to me to split this patch up > from the previous one. After the first one, you have this nonsensical > middle ground where the counter is number of heads but this counter is > number of refs. If the changes now split in two are really just one, then this needs to be explained in the changelog. Nikolay's comment can be read in two ways, split the unrelated change, or add the reason why. https://patchwork.kernel.org/patch/10534671/ As v1 od the patch says "We use this number ..." and the patch changes two variables, I find it ambiguous. Though the last sentence looks like it refers to the 'count', it's not very clear and I got lost there too.
Re: [PATCH 08/36] btrfs: dump block_rsv whe dumping space info
On Tue, Sep 11, 2018 at 04:11:45PM -0700, Omar Sandoval wrote:
> On Tue, Sep 11, 2018 at 01:57:39PM -0400, Josef Bacik wrote:
> > For enospc_debug having the block rsvs is super helpful to see if we've
> > done something wrong.
> >
> > Signed-off-by: Josef Bacik
> > ---
> > fs/btrfs/extent-tree.c | 16
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index a3baa16d456f..1cf66a92829b 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -7918,6 +7918,16 @@ static noinline int find_free_extent(struct
> > btrfs_fs_info *fs_info,
> > return ret;
> > }
> >
> > +static void dump_block_rsv(struct btrfs_fs_info *fs_info,
> > + struct btrfs_block_rsv *rsv)
> > +{
> > + spin_lock(>lock);
> > + btrfs_info(fs_info, "%d: size %llu reserved %llu\n",
no "\n" for the message helpers
> > + rsv->type, (unsigned long long)rsv->size,
> > + (unsigned long long)rsv->reserved);
u64 does not need the (unsigned long long) typecast since long.
> How about passing a string name for each of these instead of an ID which
> we have to cross-reference with the source?
Agreed, this would be better. As this is a debugging stuff, we can use
macros to simplify it like:
#define DUMP_BLOCK_RSV(fs_info, rsv_name) \
do {\
struct btrfs_block_rsv *__rsv = &(fs_info)->rsv_name; \
spin_lock(&__rsv->lock);\
btrfs_info((fs_info), #rsv_name ": size ..."); \
spin_unlock(&__rsv->lock); \
} while(0)
and use as
DUMP_BLOCK_RSV(fs_info, global_block_rsv);
The name of the block reserve name remains greppable.
Re: [PATCH] btrfs: wait on caching when putting the bg cache
On Wed, Sep 12, 2018 at 06:15:41PM +0300, Nikolay Borisov wrote: > > > On 12.09.2018 17:45, Josef Bacik wrote: > > While testing my backport I noticed there was a panic if I ran > > generic/416 generic/417 generic/418 all in a row. This just happened to > > uncover a race where we had outstanding IO after we destroy all of our > > workqueues, and then we'd go to queue the endio work on those free'd > > workqueues. This is because we aren't waiting for the caching threads > > to be done before freeing everything up, so to fix this make sure we > > wait on any outstanding caching that's being done before we free up the > > block group, so we're sure to be done with all IO by the time we get to > > btrfs_stop_all_workers(). This fixes the panic I was seeing > > consistently in testing. > > It's not clear whether this is caused by one of the patches in your > latest patchbomb or has the issue been there all along? Been here always, I noticed this on the backport of linus/master before I even got to pulling my shit ontop of that. Thanks, Josef
Re: [PATCH] btrfs: wait on caching when putting the bg cache
On 12.09.2018 17:45, Josef Bacik wrote:
> While testing my backport I noticed there was a panic if I ran
> generic/416 generic/417 generic/418 all in a row. This just happened to
> uncover a race where we had outstanding IO after we destroy all of our
> workqueues, and then we'd go to queue the endio work on those free'd
> workqueues. This is because we aren't waiting for the caching threads
> to be done before freeing everything up, so to fix this make sure we
> wait on any outstanding caching that's being done before we free up the
> block group, so we're sure to be done with all IO by the time we get to
> btrfs_stop_all_workers(). This fixes the panic I was seeing
> consistently in testing.
It's not clear whether this is caused by one of the patches in your
latest patchbomb or has the issue been there all along?
>
> Signed-off-by: Josef Bacik
> ---
> fs/btrfs/extent-tree.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 414492a18f1e..2eb2e37f2354 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -9889,6 +9889,7 @@ void btrfs_put_block_group_cache(struct btrfs_fs_info
> *info)
>
> block_group = btrfs_lookup_first_block_group(info, last);
> while (block_group) {
> + wait_block_group_cache_done(block_group);
> spin_lock(_group->lock);
> if (block_group->iref)
> break;
>
Re: [RFC PATCH 0/6] btrfs-progs: build distinct binaries for specific btrfs subcommands
On 30/08/2018 19.23, Austin S. Hemmelgarn wrote: > On 2018-08-30 13:13, Axel Burri wrote: >> On 29/08/2018 21.02, Austin S. Hemmelgarn wrote: >>> On 2018-08-29 13:24, Axel Burri wrote: This patch allows to build distinct binaries for specific btrfs subcommands, e.g. "btrfs-subvolume-show" which would be identical to "btrfs subvolume show". Motivation: While btrfs-progs offer the all-inclusive "btrfs" command, it gets pretty cumbersome to restrict privileges to the subcommands [1]. Common approaches are to either setuid root for "/sbin/btrfs" (which is not recommended at all), or to write sudo rules for each subcommand. Separating the subcommands into distinct binaries makes it easy to set elevated privileges using capabilities(7) or setuid. A typical use case where this is needed is when it comes to automated scripts, e.g. btrbk [2] [3] creating snapshots and send/receive them via ssh. >>> Let me start by saying I think this is a great idea to have as an >>> option, and that the motivation is a particularly good one. >>> >>> I've posted my opinions on your two open questions below, but there's >>> two other comments I'd like to make: >>> >>> * Is there some particular reason that this only includes the commands >>> it does, and _hard codes_ which ones it works with? if we just do >>> everything instead of only the stuff we think needs certain >>> capabilities, then we can auto-generate the list of commands to be >>> processed based on function names in the C files, and it will >>> automatically pick up any newly added commands. At the very least, it >>> could still parse through the C files and look for tags in the comments >>> for the functions to indicate which ones need to be processed this way. >>> Either case will make it significantly easier to add new commands, and >>> would also better justify the overhead of shipping all the files >>> pre-generated (because there would be much more involved in >>> pre-generating them). >> >> It includes the commands that are required by btrbk. It was quite >> painful to figure out the required capabilities (reading kernel code and >> some trial and error involved), and I did not get around to include >> other commands yet. > Yeah, I can imagine that it was not an easy task. I've actually been > thinking of writing a script to scan the kernel sources and assemble a > summary of the permissions checks performed by each system call and > ioctl so that stuff like this is a bit easier, but that's unfortunately > way beyond my abilities right now (parsing C and building call graphs is > not easy no matter what language you're doing it with). >> >> I like your idea of adding some tags in the C files, I'll try to >> implement this, and we'll see what it gets to. > Something embedded in the comments is likely to be the easiest option in > terms of making sure it doesn't break the regular build. Just the > tagging in general would be useful as documentation though. > > It would be kind of neat to have the list of capabilities needed for > each one auto-generated from what it calls, but that's getting into some > particularly complex territory that would likely require call graphs to > properly implement. After some more iterations I came up with a generic "Makefile only" version of this patchset. The new version does not need generated c-files, and matches entry point declarations as well as additional tags (for fscaps declaration or future enhancements) in all "cmds-*.c" files. See new thread: "[RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands" >> >>> * While not essential, it would be really neat to have the `btrfs` >>> command detect if an associated binary exists for whatever command was >>> just invoked, and automatically exec that (possibly with some >>> verification) instead of calling the command directly so that desired >>> permissions are enforced. This would mitigate the need for users to >>> remember different command names depending on execution context. >> >> Hmm this sounds a bit too magic for me, and would probably be more >> confusing than useful. It would mean than running "btrfs" as user would >> work when splitted commands are available, and would not work if not. > It would also mean scripts would not have to add special handling for > the case of running as a non-root user and seeing if the split commands > actually exist or not (and, for that matter, would not have to directly > depend on having the split commands at all), and that users would not > need to worry about how to call BTRFS based on who they were running as. > Realistically, I'd expect the same error to show if the binary isn't > available as if it's not executable, so that it just becomes a case of > 'if you see this error, re-run the same thing as root and it should work'. >> Description: Patch 1 adds a template as well as a
[RFC PATCH v2 3/4] btrfs-progs: Makefile: add extra objects definitions for separated binaries
Some separated binaries have references to specific command objects ($cmds_objects). Add these dependencies in the Makefile, and use them in the linker target (as in target "btrfs-%:"). Fixes linkage errors for these subcommands. The "make separated" target now builds without errors. Signed-off-by: Axel Burri --- Makefile | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 95db571b..362550c9 100644 --- a/Makefile +++ b/Makefile @@ -249,6 +249,15 @@ sc_cmds_all := $(sort $(foreach val,$(sc_map),$(firstword $(subst @, ,$(val) sc_cmds := $(filter-out $(sc_exclude),$(sc_cmds_all)) sc_cmds_fscaps := $(sort $(subst @fscaps,,$(filter %@fscaps,$(subst :, ,$(sc_map) +# Additional cmds_objects required for separated binaries +btrfs_device_add_objects = mkfs/common.o +btrfs_device_usage_objects = cmds-fi-usage.o +btrfs_filesystem_show_objects = cmds-fi-usage.o +btrfs_replace_start_objects = mkfs/common.o +btrfs_rescue_super_recover_objects = super-recover.o +btrfs_rescue_chunk_recover_objects = check/mode-lowmem.o check/mode-common.o check/main.o chunk-recover.o +btrfs_restore_objects = $(LIBS_COMP) + # Using suffix allows strict distinction in targets below (btrfs-%.separated[.o]) progs_separated = $(addsuffix .separated,$(sc_cmds)) progs_separated_fscaps = $(addsuffix .separated,$(sc_cmds_fscaps)) @@ -504,6 +513,7 @@ btrfs-%.static: btrfs-%.static.o $(static_objects) $(patsubst %.o,%.static.o,$(s btrfs-%.separated: btrfs-%.separated.o $(objects) $(cmds_objects) $(libs_static) @echo "[LD] $@" $(Q)$(CC) -o $@ $@.o $(objects) $(libs_static) \ + $($(subst -,_,$(@:%.separated=%)-objects)) \ $(LDFLAGS) $(LIBS) btrfs-%: btrfs-%.o $(objects) $(standalone_deps) $(libs_static) -- 2.16.4
[RFC PATCH v2 2/4] btrfs-progs: remove unneeded dependencies on separated build (-DBTRFS_SEPARATED_BUILD)
Remove references to unneeded symbols when building separated
subcommands: "btrfs-*.separated.o".
Note that this patch still leaves unreferenced static symbols, which
in turn makes the compiler warn about "unused function/variable".
Stripping all symbols would imply adaptions in the source code
(especially by moving "int handle_command_group" functionality into a
separate compile unit).
This is not a problem, ignore these warnings when building object:
$(CC) -Wno-unused-function -Wno-unused-const-variable
Signed-off-by: Axel Burri
---
Makefile | 2 +-
cmds-balance.c| 2 ++
cmds-device.c | 2 ++
cmds-filesystem.c | 2 ++
cmds-inspect.c| 2 ++
cmds-property.c | 2 ++
cmds-qgroup.c | 2 ++
cmds-quota.c | 2 ++
cmds-replace.c| 2 ++
cmds-rescue.c | 2 ++
cmds-scrub.c | 2 ++
cmds-subvolume.c | 2 ++
commands.h| 8
13 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 35666ec9..95db571b 100644
--- a/Makefile
+++ b/Makefile
@@ -338,7 +338,7 @@ endif
# BTRFS_SEPARATED_BUILD), using "cfile" from sc_map (cmd-xxx.c) as gcc infile.
btrfs-%.separated.o: $(call sc_get,$(@:%.separated.o=%),cfile)
@echo "[CC] $@"
- $(Q)$(CC) $(CFLAGS) \
+ $(Q)$(CC) $(CFLAGS) -Wno-unused-function -Wno-unused-const-variable \
-DBTRFS_SEPARATED_BUILD \
-DBTRFS_SEPARATED_ENTRY=$(call
sc_get,$(@:%.separated.o=%),entry) \
-DBTRFS_SEPARATED_USAGE=$(call
sc_get,$(@:%.separated.o=%),entry)_usage \
diff --git a/cmds-balance.c b/cmds-balance.c
index 6cc26c35..72292bc8 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -914,6 +914,7 @@ static int cmd_balance_full(int argc, char **argv)
static const char balance_cmd_group_info[] =
"balance data across devices, or change block groups using filters";
+#ifndef BTRFS_SEPARATED_BUILD
const struct cmd_group balance_cmd_group = {
balance_cmd_group_usage, balance_cmd_group_info, {
{ "start", cmd_balance_start, cmd_balance_start_usage, NULL, 0
},
@@ -940,3 +941,4 @@ int cmd_balance(int argc, char **argv)
return handle_command_group(_cmd_group, argc, argv);
}
+#endif
diff --git a/cmds-device.c b/cmds-device.c
index 2a05f70a..719afc01 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -597,6 +597,7 @@ static int cmd_device_usage(int argc, char **argv)
static const char device_cmd_group_info[] =
"manage and query devices in the filesystem";
+#ifndef BTRFS_SEPARATED_BUILD
const struct cmd_group device_cmd_group = {
device_cmd_group_usage, device_cmd_group_info, {
{ "add", cmd_device_add, cmd_device_add_usage, NULL, 0 },
@@ -616,3 +617,4 @@ int cmd_device(int argc, char **argv)
{
return handle_command_group(_cmd_group, argc, argv);
}
+#endif
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 06c8311b..be5d50f2 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -1182,6 +1182,7 @@ static int cmd_filesystem_label(int argc, char **argv)
static const char filesystem_cmd_group_info[] =
"overall filesystem tasks and information";
+#ifndef BTRFS_SEPARATED_BUILD
const struct cmd_group filesystem_cmd_group = {
filesystem_cmd_group_usage, filesystem_cmd_group_info, {
{ "df", cmd_filesystem_df, cmd_filesystem_df_usage, NULL, 0 },
@@ -1209,3 +1210,4 @@ int cmd_filesystem(int argc, char **argv)
{
return handle_command_group(_cmd_group, argc, argv);
}
+#endif
diff --git a/cmds-inspect.c b/cmds-inspect.c
index ac77a5ee..1ad48548 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -634,6 +634,7 @@ out:
static const char inspect_cmd_group_info[] =
"query various internal information";
+#ifndef BTRFS_SEPARATED_BUILD
const struct cmd_group inspect_cmd_group = {
inspect_cmd_group_usage, inspect_cmd_group_info, {
{ "inode-resolve", cmd_inspect_inode_resolve,
@@ -660,3 +661,4 @@ int cmd_inspect(int argc, char **argv)
{
return handle_command_group(_cmd_group, argc, argv);
}
+#endif
diff --git a/cmds-property.c b/cmds-property.c
index 03bafa05..c98e8ebb 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -408,6 +408,7 @@ static int cmd_property_list(int argc, char **argv)
static const char property_cmd_group_info[] =
"modify properties of filesystem objects";
+#ifndef BTRFS_SEPARATED_BUILD
const struct cmd_group property_cmd_group = {
property_cmd_group_usage, property_cmd_group_info, {
{ "get", cmd_property_get,
@@ -424,3 +425,4 @@ int cmd_property(int argc, char **argv)
{
return handle_command_group(_cmd_group, argc, argv);
}
+#endif
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 554f2bf2..5f5a3563 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -503,6 +503,7 @@ static int cmd_qgroup_limit(int argc, char **argv)
static const char qgroup_cmd_group_info[] =
"manage quota groups";
+#ifndef BTRFS_SEPARATED_BUILD
const struct cmd_group
[RFC PATCH v2 0/4] btrfs-progs: build distinct binaries for specific btrfs subcommands
This patch allows to build distinct binaries for specific btrfs
subcommands, e.g. "btrfs-subvolume-show" which would be identical to
"btrfs subvolume show".
Changes from v1 [1]:
- No more need of generated c-file for each separated commands (all
functionality has moved to Makefile).
- More generic approach: match entry point declarations as well as
additional tage in all "cmds-*.c" files.
- Change naming: use "separated" instead of "splitcmd".
Motivation:
While btrfs-progs offer the all-inclusive "btrfs" command, it gets
pretty cumbersome to restrict privileges to the subcommands [2].
Common approaches are to either setuid root for "/sbin/btrfs" (which
is not recommended at all), or to write sudo rules for each
subcommand.
Separating the subcommands into distinct binaries makes it easy to set
elevated privileges using capabilities(7) or setuid. A typical use
case where this is needed is when it comes to automated scripts,
e.g. btrbk [3] [4] creating snapshots and send/receive them via ssh.
Description:
Patch 1 adds a minimal, non-invasive framework for building separated
binaries. Note that some subcommands fail to build ("make -k separated").
Patches 2,3 fix build dependencies: make all subcommands build
correctly, with smaller binary size. Probably to be squashed into
patch 1 for final commit.
Patch 4 adds configuration options -enable-setcap-install,
--enable-setuid-install, --enable-btrfs-separated.
Notes:
- This patchset is available on github [5].
- A gentoo ebuild "sys-fs/btrfs-progs-separated" is available on
github [6], as well as in the digint-overlay [7]:
USE=filecaps emerge -av sys-fs/btrfs-progs-separated
References:
[1] https://www.spinics.net/lists/linux-btrfs/msg81451.html
[2] https://www.spinics.net/lists/linux-btrfs/msg75736.html
[3] https://github.com/digint/btrbk
[4] https://github.com/digint/btrfs-progs-btrbk
[5] https://github.com/digint/btrfs-progs/tree/cmds-separated-fscaps-v2
[6]
https://github.com/digint/gentoo/tree/btrfs-progs-separated/sys-fs/btrfs-progs-separated
[7] https://dev.tty0.ch/portage/digint-overlay.git
Axel Burri (4):
btrfs-progs: Makefile: create separated binaries for "btrfs"
subcommands; add fscaps declarations
btrfs-progs: remove unneeded dependencies on separated build
(-DBTRFS_SEPARATED_BUILD)
btrfs-progs: Makefile: add extra objects definitions for separated
binaries
btrfs-progs: build: add --enable-setcap-install,
--enable-setuid-install, --enable-btrfs-separated
.gitignore| 1 +
Makefile | 99 ++-
Makefile.inc.in | 6
cmds-balance.c| 2 ++
cmds-device.c | 2 ++
cmds-fi-usage.c | 1 +
cmds-filesystem.c | 2 ++
cmds-inspect.c| 2 ++
cmds-property.c | 2 ++
cmds-qgroup.c | 3 ++
cmds-quota.c | 2 ++
cmds-receive.c| 1 +
cmds-replace.c| 2 ++
cmds-rescue.c | 2 ++
cmds-scrub.c | 2 ++
cmds-send.c | 1 +
cmds-subvolume.c | 6
commands.h| 37 +
configure.ac | 40 ++
19 files changed, 212 insertions(+), 1 deletion(-)
--
2.16.4
[RFC PATCH v2 4/4] btrfs-progs: build: add --enable-setcap-install, --enable-setuid-install, --enable-btrfs-separated
Adds Makefile target "install-separated": install all "btrfs-*.separated" binaries and rename to "btrfs-*". If configured with --enable-setcap-install, also sets linux capabilities(7) using setcap(8). If configured with "--enable-setuid-install", sets setuid bit while installing. Use --enable-btrfs-separated if you want to build (but not install) all "btrfs-*.separated" binaries. Signed-off-by: Axel Burri --- Makefile| 36 +++- Makefile.inc.in | 6 ++ configure.ac| 40 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 362550c9..e0edcb75 100644 --- a/Makefile +++ b/Makefile @@ -232,6 +232,25 @@ progs_install = progs_build = endif +ifeq ($(BUILD_BTRFS_SEPARATED),1) +# Note: intentionally not addded to progs_install: +# use -enable-setcap-install, --enable-setuid-install instead. +progs_build += $(progs_separated) +endif + +INSTALL_SEPARATED_OPTIONS = -m755 +ifeq ($(ENABLE_INSTALL_SETCAP),1) +INSTALL_SEPARATED_OPTIONS = -m710 +progs_install_separated += $(progs_separated_fscaps) +endif +ifeq ($(ENABLE_INSTALL_SETUID),1) +INSTALL_SEPARATED_OPTIONS = -m4710 +progs_install_separated += $(progs_separated_fscaps) +endif +ifdef SETCAP_GROUP +INSTALL_SEPARATED_OPTIONS += -g$(SETCAP_GROUP) +endif + # Parse "int cmd_xxx_yyy(int argc, char **argv)" lines in cfiles, and # create whitespace separated map of form: "btrfs-xxx-yyy@key:value". sc_cfiles := $(wildcard cmds-*.c) @@ -704,7 +723,7 @@ $(CLEANDIRS): @echo "Cleaning $(patsubst clean-%,%,$@)" $(Q)$(MAKE) $(MAKEOPTS) -C $(patsubst clean-%,%,$@) clean -install: $(libs_build) $(progs_install) $(INSTALLDIRS) +install: $(libs_build) $(progs_install) $(progs_install_separated) $(INSTALLDIRS) ifeq ($(BUILD_PROGRAMS),1) $(INSTALL) -m755 -d $(DESTDIR)$(bindir) $(INSTALL) $(progs_install) $(DESTDIR)$(bindir) @@ -726,6 +745,9 @@ endif $(INSTALL) -m644 $(libbtrfs_headers) $(DESTDIR)$(incdir)/btrfs $(INSTALL) -m644 libbtrfsutil/btrfsutil.h $(DESTDIR)$(incdir) endif +ifeq ($(BUILD_BTRFS_SEPARATED),1) + $(Q)$(MAKE) $(MAKEOPTS) install-separated +endif ifeq ($(PYTHON_BINDINGS),1) install_python: libbtrfsutil_python @@ -741,6 +763,18 @@ install-static: $(progs_static) $(INSTALLDIRS) # btrfsck is a link to btrfs in the src tree, make it so for installed file as well $(LN_S) -f btrfs.static $(DESTDIR)$(bindir)/btrfsck.static +# install separated btrfs binaries, set linux capabilities(7) defined +# in "@SEPARATED" lines using setcap(8), remove ".separated" postfix +install-btrfs-%.separated: btrfs-%.separated + $(INSTALL) -m755 -d $(DESTDIR)$(bindir) + $(INSTALL) $(INSTALL_SEPARATED_OPTIONS) $< $(DESTDIR)$(bindir) +ifeq ($(ENABLE_INSTALL_SETCAP),1) + $(SETCAP) $(call sc_get,$(<:%.separated=%),fscaps)+ep $(DESTDIR)$(bindir)/$< +endif + $(MV) $(DESTDIR)$(bindir)/$< $(DESTDIR)$(bindir)/$(<:%.separated=%) + +install-separated: $(progs_install_separated) $(patsubst %,install-%,$(progs_install_separated)) + $(INSTALLDIRS): @echo "Making install in $(patsubst install-%,%,$@)" $(Q)$(MAKE) $(MAKEOPTS) -C $(patsubst install-%,%,$@) install diff --git a/Makefile.inc.in b/Makefile.inc.in index a86c528e..93df2edf 100644 --- a/Makefile.inc.in +++ b/Makefile.inc.in @@ -10,6 +10,8 @@ AR = @AR@ RM = @RM@ RMDIR = @RMDIR@ INSTALL = @INSTALL@ +MV = @MV@ +SETCAP = @SETCAP@ DISABLE_DOCUMENTATION = @DISABLE_DOCUMENTATION@ DISABLE_BTRFSCONVERT = @DISABLE_BTRFSCONVERT@ BUILD_PROGRAMS = @BUILD_PROGRAMS@ @@ -22,6 +24,10 @@ PYTHON_BINDINGS = @PYTHON_BINDINGS@ PYTHON = @PYTHON@ PYTHON_CFLAGS = @PYTHON_CFLAGS@ +BUILD_BTRFS_SEPARATED = @BUILD_BTRFS_SEPARATED@ +ENABLE_INSTALL_SETCAP = @ENABLE_INSTALL_SETCAP@ +ENABLE_INSTALL_SETUID = @ENABLE_INSTALL_SETUID@ + SUBST_CFLAGS = @CFLAGS@ SUBST_LDFLAGS = @LDFLAGS@ diff --git a/configure.ac b/configure.ac index df02f206..981250bc 100644 --- a/configure.ac +++ b/configure.ac @@ -39,6 +39,8 @@ AC_PROG_LN_S AC_CHECK_TOOL([AR], [ar]) AC_PATH_PROG([RM], [rm], [rm]) AC_PATH_PROG([RMDIR], [rmdir], [rmdir]) +AC_PATH_PROG([MV], [mv], [mv]) +AC_PATH_PROG([SETCAP], [setcap], [setcap]) AC_CHECK_FUNCS([openat], [], @@ -248,6 +250,40 @@ AS_IF([test "x$enable_python" = xyes], [PYTHON_BINDINGS=1], [PYTHON_BINDINGS=0]) AC_SUBST(PYTHON_BINDINGS) AC_SUBST(PYTHON) +# check whether to build/install separated btrfs binaries +AC_ARG_ENABLE([setcap-install], + AS_HELP_STRING([--enable-setcap-install], [install separated binaries with capabilities]), + [], [enable_setcap_install=no] +) +AS_IF([test "x$enable_setcap_install" = xyes], [ENABLE_INSTALL_SETCAP=1], [ENABLE_INSTALL_SETCAP=0]) +AC_SUBST([ENABLE_INSTALL_SETCAP]) +AM_CONDITIONAL(SETCAP_INSTALL, test x$enable_setcap_install = xyes) + +AC_ARG_ENABLE([setuid-install], + AS_HELP_STRING([--enable-setuid-install], [install separated binaries
[RFC PATCH v2 1/4] btrfs-progs: Makefile: create separated binaries for "btrfs" subcommands; add fscaps declarations
Create separate binaries "btrfs-xxx-yyy.separated" for each subcommand "btrfs xxx yyy". Also declares fscaps for (supported) subcommands. This is useful for admins to provide specific subcommand binaries with elevated privileges (capabilities(7), suid). Example: # make separated-fscaps # make list-fscaps # ./btrfs-subvolume-list.separated # ./btrfs-send.separated <...> # make -k separated A list of subcommands is assembled in the makefile by matching main entry points and generic "@SEPARATED" tags in "cmds-*.c", e.g.: // @SEPARATED btrfs-subvolume-delete fscaps: cap_sys_admin,cap_dac_override static int cmd_subvol_delete(int argc, char **argv) A patch in "commands.h" adds a "int main()" symbol and tweaks for building separated binaries if BTRFS_SEPARATED_BUILD and BTRFS_SEPARATED_ENTRY are defined. These defines are set generically for each subcommand in the "btrfs-%.separated.o" makefile target. Makefile targets: - "btrfs-%.separated": builds a separated binary for a specific btrfs subcommand (see "make list-separated"). - `make -k separated`: builds all separated (btrfs-xxx-yyy.separated) binaries. Note: some targets fail with "ld: undefined reference" errors (will be fixed in later commit). - `make separated-fscaps`: build separated binaries for subcommands declaring "@SEPARATED btrfs-xxx fscaps" (comments) within the c-file (for sysadmins / package maintainer). - `make list-separated`: print a list of supported subcommands. - `make list-fscaps`: print fscaps required for setcap(8). Note that this list only covers some basic subcommands (the ones used by btrbk-0.26.1 "backend btrfs-progs-btrbk") and needs to be improved. Signed-off-by: Axel Burri --- .gitignore | 1 + Makefile | 53 + cmds-fi-usage.c | 1 + cmds-qgroup.c| 1 + cmds-receive.c | 1 + cmds-send.c | 1 + cmds-subvolume.c | 4 commands.h | 45 + 8 files changed, 107 insertions(+) diff --git a/.gitignore b/.gitignore index 144ebb3b..e1eb1341 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ Documentation/*.gz Documentation/*.html btrfs btrfs.static +btrfs-*.separated btrfs-map-logical btrfs-fragments btrfsck diff --git a/Makefile b/Makefile index fcfc815a..35666ec9 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ # Basic build targets: # allall main tools and the shared library # static build static bnaries, requires static version of the libraries +# separated build separated binary for each "btrfs" subcommand # testrun the full testsuite # install install to default location (/usr/local) # clean clean built binaries (not the documentation) @@ -231,6 +232,27 @@ progs_install = progs_build = endif +# Parse "int cmd_xxx_yyy(int argc, char **argv)" lines in cfiles, and +# create whitespace separated map of form: "btrfs-xxx-yyy@key:value". +sc_cfiles := $(wildcard cmds-*.c) +sc_map := $(foreach file,$(sc_cfiles),$(shell sed -rn 's/^(static )?int cmd_([a-z_]+)\(int argc, char.*argv.*\)$$/btrfs_\2@cfile:$(file) btrfs_\2@entry:cmd_\2 btrfs_\2@static_entry:\1/p' $(file))) +sc_map := $(foreach val,$(sc_map),$(subst _,-,$(firstword $(subst @, ,$(val@$(word 2,$(subst @, ,$(val +sc_map := $(subst btrfs-subvol-,btrfs-subvolume-,$(sc_map)) + +# Parse generic "@SEPARATED btrfs-xxx-yyy key: value" in cfiles, and add to map. +sc_map += $(shell sed -rn 's/^.*@SEPARATED\s+(btrfs-[a-z-]+)\s+([a-z_]+):\s*(.*)$$/\1@\2:\3/p' $(sc_cfiles)) +sc_get = $(word 2,$(subst :, ,$(filter $(1)@$(2):%,$(sc_map + +# Exclude first-level commands relying on "int handle_command_group()" or buggy +sc_exclude = btrfs-balance btrfs-balance-full btrfs-device btrfs-filesystem btrfs-inspect btrfs-property btrfs-qgroup btrfs-quota btrfs-replace btrfs-rescue btrfs-scrub btrfs-subvolume +sc_cmds_all := $(sort $(foreach val,$(sc_map),$(firstword $(subst @, ,$(val) +sc_cmds := $(filter-out $(sc_exclude),$(sc_cmds_all)) +sc_cmds_fscaps := $(sort $(subst @fscaps,,$(filter %@fscaps,$(subst :, ,$(sc_map) + +# Using suffix allows strict distinction in targets below (btrfs-%.separated[.o]) +progs_separated = $(addsuffix .separated,$(sc_cmds)) +progs_separated_fscaps = $(addsuffix .separated,$(sc_cmds_fscaps)) + # external libs required by various binaries; for btrfs-foo, # specify btrfs_foo_libs = ; see $($(subst...)) rules below btrfs_convert_cflags = -DBTRFSCONVERT_EXT2=$(BTRFSCONVERT_EXT2) @@ -312,6 +334,17 @@ endif $(Q)$(CC) $(CFLAGS) -c $< -o $@ $($(subst -,_,$(@:%.o=%)-cflags)) \ $($(subst -,_,btrfs-$(@:%/$(notdir $@)=%)-cflags)) +# Compile target objects providing main() symbol (see commands.h: +# BTRFS_SEPARATED_BUILD), using "cfile" from sc_map (cmd-xxx.c) as gcc infile. +btrfs-%.separated.o: $(call sc_get,$(@:%.separated.o=%),cfile) +
[PATCH] btrfs: wait on caching when putting the bg cache
While testing my backport I noticed there was a panic if I ran
generic/416 generic/417 generic/418 all in a row. This just happened to
uncover a race where we had outstanding IO after we destroy all of our
workqueues, and then we'd go to queue the endio work on those free'd
workqueues. This is because we aren't waiting for the caching threads
to be done before freeing everything up, so to fix this make sure we
wait on any outstanding caching that's being done before we free up the
block group, so we're sure to be done with all IO by the time we get to
btrfs_stop_all_workers(). This fixes the panic I was seeing
consistently in testing.
Signed-off-by: Josef Bacik
---
fs/btrfs/extent-tree.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 414492a18f1e..2eb2e37f2354 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9889,6 +9889,7 @@ void btrfs_put_block_group_cache(struct btrfs_fs_info
*info)
block_group = btrfs_lookup_first_block_group(info, last);
while (block_group) {
+ wait_block_group_cache_done(block_group);
spin_lock(_group->lock);
if (block_group->iref)
break;
--
2.14.3
Re: [PATCH] Btrfs: do not wait after queue async work for delaye refs
On Wed, Sep 12, 2018 at 10:19:23AM +0300, Nikolay Borisov wrote: > [Adding Josef to CC] > > On 12.09.2018 01:06, Liu Bo wrote: > > If metadata space is hungry, how fast flush_space() can run determines > > the latency we spend in reserve_metadata_space(). > > > > flush_space() > >case FLUSH_DELAYED_ITEMS: > > ... > > btrfs_end_transaction() > >case ALLOC_CHUNK: > > ... > > btrfs_end_transaction() > > > > btrfs_end_transaction() > >btrfs_async_run_delayed_refs() > >// queue a work to process delayed refs > >... > >if (wait) > >wait_for_completion() > > > > Although processing delayed refs can add to pinned bytes, pinned bytes > > can only be used after committing transaction, so waiting async in > > flush_space() doesn't help. Nack, but just because I get rid of all of this code in my delayed refs rsv patchset. Thanks, Josef
Re: [PATCH] Btrfs: remove level==0 check in balance_level
On Wed, Sep 12, 2018 at 06:06:23AM +0800, Liu Bo wrote: > btrfs_search_slot() >if (level != 0) > setup_nodes_for_search() > balance_level() > > It is just impossible to have level=0 in balance_level. While this is true, what do you think about adding ASSERT(level > 0) ? This is to catch accidentally passing level 0.
[PATCH v2 7/7] btrfs-progs: fsck-tests: add test case inode_extref without dir_item and dir_index
From: Su Yue
This case contains an inode_extref:
==
...
item 1 key (257 INODE_EXTREF 3460996356) itemoff 3947 itemsize 24
index 257 parent 256 namelen 6 name: foo255
...
==
The related dir_item and dir_index are missing.
Add the case to ensure both original and lowmem mode check can handle
the case of inode_extref.
Lowmem part is supported since patch named
'btrfs-progs: lowmem: improve check_inode_extref()'.
And rename default_case.img to inode_ref_without_dir_item_index.img.
Signed-off-by: Su Yue
---
.../inode_extref_without_dir_item_index.img | Bin 0 -> 10240 bytes
...=> inode_ref_without_dir_item_and_index.img} | Bin
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644
tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
rename tests/fsck-tests/009-no-dir-item-or-index/{default_case.img =>
inode_ref_without_dir_item_and_index.img} (100%)
diff --git
a/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
b/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
new file mode 100644
index
..85f7a8211fe66dfa9b2b1f3a98e72f54cd402616
GIT binary patch
literal 10240
zcmeHscQo8j8?UlhEOymZqDGCfdWp7rf~Y~1#Of{4q9>NMSe=L(Wkn}~5JZVy61@b`
zOY|NsBwmBv=mszSgNy!H|F1
z$NlwH{s)Ks?ejn7KN0v(1pZ$}z<*}eU$kky0Zga5{R>1!Q|jBj!Kk_IdwP^`75
zM#+ojWKDf@k7f}D9aT8qYmSi4F2VupuXr{D%*M11tY*yA57Z39C|MYjWMVhzVK5j1
zieWerkiPZjcdzsBS22#CPt1DVI!`Y9h*_LYFoYww=`DnvA?wF8(}YD85$
zG7$zaGL3dVC{eyOEl3W_-qsf~#zJSUyqf<-HM2v|!&%~jSR%lz|08~jx;7^e3~;sY
zT^phzaQf-A9pzJu_AnE#I*l;xF8RnBNniHgj!g0JPRgY|`OkhQSmPV3fQT{G?g$ZX6e{@Ov^aKMAG
zr|O5JwU)}O00QwdZZ<+yyfQ#~Ka9r6hA@3Oik}0|;rd7T)~)3|9Uli=;g
zh~{}Qn(6qP?bLNIenC5
zLO3=6UuM9&&4o2fB&4lmozz;AiUWx^ndhJ)O`@bM4QWxL!mC>|7xSNK`Sw)H$N)-m
zSXz2)pA0>PYZPT59_=l(%tNBo?D|v)$%|6$`GKhK<>+y93Fiy
zTiWZzmC*aeUfXv1K)7a?^YiuO;WXH%#MegmjkD8b)YP2MzV{*|{#*5|R9Ov&-Q^d%
z_IMBdg|z+-kf1lZG%Y^>3;_8eTVDY0gLuVuK^iH*1%Z5W_7lF*~uM$6u+hgOC_
z4rWyxVvQeo1hp}4g+fK$4!+0}!ov|G-z-7PF$8c+7m_drD7D1!U-DzS0t`X
z>Y@fV(K_9a{<$w^me4NJyvG7Sf}U~z9#&01Q3o5cr>bn8t5a2O%6)BWf6SW`JBx#V08j00K~k%COtKaNNL64F+f;tgKG=lY(fg3Y1(No%L^e>S=)3E(
z&!A-jV%H0qu$NE*8j|3Wl=2@yv!W(Y57SB519_C
z6)+t9-_I4%1fb5Z{cpNOM?t%BnXTyFy@$oS?=g{%z#FR7*F40BkJ{o-r
zeg|!2s2At!us$=cRn=-Ad2{xrofs7EHO~dqhBX`nd|fIgA*e6HJ^;GIet=Fzs|Guf
[PATCH v2 6/7] btrfs-progs: lowmem: improve check_inode_extref()
From: Su Yue
inode_extref is much similar with inode_ref, most codes are reused in
check_inode_extref().
Exception:
There is no need to check root directory, so remove it.
Make check_inode_extref() verify hash value with key offset now.
And lowmem check can detect errors about inode_extref and try to
repair errors.
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 119 ++--
1 file changed, 92 insertions(+), 27 deletions(-)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index b6b33786d02b..bec2ee185cc8 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1182,37 +1182,79 @@ out:
*
* Return 0 if no error occurred.
*/
-static int check_inode_extref(struct btrfs_root *root,
- struct btrfs_key *ref_key,
- struct extent_buffer *node, int slot, u64 *refs,
- int mode)
+static int check_inode_extref(struct btrfs_root *root, struct btrfs_key
*ref_key,
+ struct btrfs_path *path, char *name_ret,
+ u32 *namelen_ret, u64 *refs_ret, int mode)
{
struct btrfs_key key;
struct btrfs_key location;
struct btrfs_inode_extref *extref;
+ struct extent_buffer *node;
char namebuf[BTRFS_NAME_LEN] = {0};
u32 total;
- u32 cur = 0;
+ u32 cur;
u32 len;
u32 name_len;
u64 index;
u64 parent;
+ int err;
+ int tmp_err;
int ret;
- int err = 0;
+ int slot;
+ u64 refs;
+ bool search_again = false;
location.objectid = ref_key->objectid;
location.type = BTRFS_INODE_ITEM_KEY;
location.offset = 0;
+begin:
+ cur = 0;
+ err = 0;
+ refs = *refs_ret;
+ *namelen_ret = 0;
+
+ /* since after repair, path and the dir item may be changed */
+ if (search_again) {
+ search_again = false;
+ btrfs_release_path(path);
+ ret = btrfs_search_slot(NULL, root, ref_key, path, 0, 0);
+ /*
+* The item was deleted, let the path point to the last checked
+* item.
+*/
+ if (ret > 0) {
+ if (path->slots[0] == 0) {
+ ret = btrfs_prev_leaf(root, path);
+ /*
+* we are checking the inode item, there must
+* be some items before, the case shall never
+* happen.
+*/
+ BUG_ON(ret);
+ } else {
+ path->slots[0]--;
+ }
+ goto out;
+ } else if (ret < 0) {
+ err |= ret;
+ goto out;
+ }
+ }
+
+ node = path->nodes[0];
+ slot = path->slots[0];
extref = btrfs_item_ptr(node, slot, struct btrfs_inode_extref);
total = btrfs_item_size_nr(node, slot);
-next:
- /* update inode ref count */
- (*refs)++;
- name_len = btrfs_inode_extref_name_len(node, extref);
- index = btrfs_inode_extref_index(node, extref);
+loop:
+ /* Update inode ref count */
+ refs++;
+ tmp_err = 0;
parent = btrfs_inode_extref_parent(node, extref);
+ index = btrfs_inode_extref_index(node, extref);
+ name_len = btrfs_inode_extref_name_len(node, extref);
+
if (name_len <= BTRFS_NAME_LEN) {
len = name_len;
} else {
@@ -1220,37 +1262,60 @@ next:
warning("root %llu INODE_EXTREF[%llu %llu] name too long",
root->objectid, ref_key->objectid, ref_key->offset);
}
+
read_extent_buffer(node, namebuf, (unsigned long)(extref + 1), len);
- /* Check root dir ref name */
- if (index == 0 && strncmp(namebuf, "..", name_len)) {
- error("root %llu INODE_EXTREF[%llu %llu] ROOT_DIR name
shouldn't be %s",
+ /* verify hash value */
+ if (ref_key->offset != btrfs_extref_hash(parent, namebuf, len)) {
+ err |= -EIO;
+ error("root %llu INODE_EXTREF[%llu %llu] name %s namelen %u
mode %d mismatch with its hash, wanted %llu have %llu",
root->objectid, ref_key->objectid, ref_key->offset,
- namebuf);
- err |= ROOT_DIR_ERROR;
+ namebuf, len, mode, ref_key->offset,
+ btrfs_extref_hash(parent, namebuf, len));
+ goto out;
}
- /* find related dir_index */
+ /* copy the first name found to name_ret */
+ if (refs == 1 && name_ret) {
+ memcpy(name_ret, namebuf, len);
+ *namelen_ret = len;
+ }
+
+ /* Find related DIR_INDEX
[PATCH v2 5/7] btrfs-progs: lowmem: continue to check item in last slot while checking inodes
From: Su Yue
After call of check_inode_item(), path may point to the last unchecked
slot of the leaf. The outer walk_up_tree() always treats the position
as checked item then skips to next item.
If the last item was an inode item, yes, it was unchecked.
Then walk_up_tree() will think the leaf is checked and walk up to
upper node, process_one_leaf() in walk_down_tree() would skip to
check next inode item. Which means, the inode item won't be checked.
Solution:
After check_inode_item returns, if found path point to the last item
of a leaf, decrease path slot manually, so walk_up_tree() will stay
on the leaf.
Fixes: 5e2dc770471b ("btrfs-progs: check: skip shared node or leaf check for
low_memory mode")
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 12
1 file changed, 12 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 8fc9edab1d66..b6b33786d02b 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2612,6 +2612,18 @@ again:
if (cur->start == cur_bytenr)
goto again;
+ /*
+* path may point at the last item(a inode item maybe) in a leaf.
+* Without below lines, walk_up_tree() will skip the item which
+* means all items related to the inode will never be checked.
+* Decrease the slot manually, walk_up_tree won't skip to next node
+* if it occurs.
+*/
+ if (path->slots[0] + 1 >= btrfs_header_nritems(path->nodes[0])) {
+ if (path->slots[0])
+ path->slots[0]--;
+ }
+
/*
* we have switched to another leaf, above nodes may
* have changed, here walk down the path, if a node
--
2.18.0
[PATCH v2 0/7] btrfs-progs: lowmem: bug fixes and inode_extref repair
From: Su Yue
This patchset can be fetched from my github(based on v4.17.1):
https://github.com/Damenly/btrfs-progs/tree/lowmem_extref
The patchset aims to support check and repair errors about
inode_extref in lowmem mode.
patch[1-2] let btrfs_unlink() detect inode_extref.
patch[3] fixes a minor bug in check_dir_item due to my careless
long ago.
patch[4] fixes a bug about inconsistent path in check_fs_roots()
under repair.
patch[5] fixes a corner case about traversal of inode items.
patch[6] enable inode_extref repair support and remove unnecessary
checks.
patch[7] add a test image, it can verify above patches except
patch[3].
Changelog:
v2:
Resend with patches in right order.
Su Yue (7):
btrfs-progs: adjust arguments of btrfs_lookup_inode_extref()
btrfs-progs: make btrfs_unlink() lookup inode_extref
btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()
btrfs-progs: lowmem: search key of root again after check_fs_root()
under repair
btrfs-progs: lowmem: continue to check item in last slot while
checking inodes
btrfs-progs: lowmem: improve check_inode_extref()
btrfs-progs: fsck-tests: add test case inode_extref without dir_item
and dir_index
check/mode-lowmem.c | 148 ++
ctree.h | 6 +-
inode-item.c | 6 +-
inode.c | 14 +-
.../inode_extref_without_dir_item_index.img | Bin 0 -> 10240 bytes
... inode_ref_without_dir_item_and_index.img} | Bin
6 files changed, 139 insertions(+), 35 deletions(-)
create mode 100644
tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
rename tests/fsck-tests/009-no-dir-item-or-index/{default_case.img =>
inode_ref_without_dir_item_and_index.img} (100%)
--
2.18.0
[PATCH v2 3/7] btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()
From: Su Yue
In check_dir_item, we are going to search corresponding
dir_item/index.
Commit 564901eac7a4 ("btrfs-progs: check: introduce
print_dir_item_err()") Changed argument name from key to di_key but
forgot to change the key name for dir_item search.
So @key shouldn't be used here. It should be @di_key.
Fixes: 564901eac7a4 ("btrfs-progs: check: introduce print_dir_item_err()")
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1bce44f5658a..89a304bbdd69 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1658,7 +1658,7 @@ begin:
/* check relative INDEX/ITEM */
key.objectid = di_key->objectid;
- if (key.type == BTRFS_DIR_ITEM_KEY) {
+ if (di_key->type == BTRFS_DIR_ITEM_KEY) {
key.type = BTRFS_DIR_INDEX_KEY;
key.offset = index;
} else {
--
2.18.0
[PATCH v2 2/7] btrfs-progs: make btrfs_unlink() lookup inode_extref
From: Su Yue
btrfs_unlink() uses btrfs_lookup_inode_ref() to look up inode_ref
but forget inode_extref case.
Let btrfs_unlink() call btrfs_lookup_inode_extref() if inode_ref is
found and EXTENDED_IREF feature is enabled.
Fixes: 0cc75eddd093 ("btrfs-progs: Add btrfs_unlink() and btrfs_add_link()
functions.")
Signed-off-by: Su Yue
---
inode.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/inode.c b/inode.c
index 2398bca4a109..598ad0ab6b4c 100644
--- a/inode.c
+++ b/inode.c
@@ -277,6 +277,7 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct
btrfs_root *root,
struct btrfs_key key;
struct btrfs_inode_item *inode_item;
struct btrfs_inode_ref *inode_ref;
+ struct btrfs_inode_extref *inode_extref = NULL;
struct btrfs_dir_item *dir_item;
u64 inode_size;
u32 nlinks;
@@ -296,7 +297,18 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct
btrfs_root *root,
ret = PTR_ERR(inode_ref);
goto out;
}
- if (inode_ref)
+
+ if (!inode_ref && btrfs_fs_incompat(root->fs_info, EXTENDED_IREF)) {
+ btrfs_release_path(path);
+ inode_extref = btrfs_lookup_inode_extref(trans, root, path,
+name, namelen, ino, parent_ino, 0);
+ if (IS_ERR(inode_extref)) {
+ ret = PTR_ERR(inode_extref);
+ goto out;
+ }
+ }
+
+ if (inode_ref || inode_extref)
del_inode_ref = 1;
btrfs_release_path(path);
--
2.18.0
[PATCH v2 4/7] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair
From: Su Yue
In check_fs_roots_lowmem(), we do search and follow the resulted path
to call check_fs_root(), then call btrfs_next_item() to check next
root.
However, if repair is enabled, the root tree can be cowed, the
existed path can cause strange errors.
Solution:
If repair, save the key before calling check_fs_root,
search the saved key again before checking next root.
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 89a304bbdd69..8fc9edab1d66 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
}
while (1) {
+ struct btrfs_key saved_key;
+
node = path.nodes[0];
slot = path.slots[0];
btrfs_item_key_to_cpu(node, , slot);
+ if (repair)
+ saved_key = key;
if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
goto out;
if (key.type == BTRFS_ROOT_ITEM_KEY &&
@@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
err |= ret;
}
next:
+ /*
+* Since root tree can be cowed during repair,
+* here search the saved key again.
+*/
+ if (repair) {
+ btrfs_release_path();
+ ret = btrfs_search_slot(NULL, fs_info->tree_root,
+ _key, , 0, 0);
+ /* Repair never deletes trees, search must succeed. */
+ BUG_ON(ret);
+ }
ret = btrfs_next_item(tree_root, );
if (ret > 0)
goto out;
--
2.18.0
[PATCH v2 1/7] btrfs-progs: adjust arguments of btrfs_lookup_inode_extref()
From: Su Yue
The argument index is not used in btrfs_lookup_inode_extref(),
so remove it.
And adjust positions its arguments to make it consistent with
kernel part.
No functional change.
Fixes: 260675657767 ("btrfs-progs: Import btrfs_insert/del/lookup_extref()
functions.")
Signed-off-by: Su Yue
---
ctree.h | 6 +++---
inode-item.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/ctree.h b/ctree.h
index 4719962df67d..e7f6c5df95f1 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2708,9 +2708,9 @@ int btrfs_lookup_inode(struct btrfs_trans_handle *trans,
struct btrfs_root
*root, struct btrfs_path *path,
struct btrfs_key *location, int mod);
struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
- *trans, struct btrfs_path *path, struct btrfs_root *root,
- u64 ino, u64 parent_ino, u64 index, const char *name,
- int namelen, int ins_len);
+ *trans, struct btrfs_root *root, struct btrfs_path *path,
+ const char *name, int namelen, u64 ino, u64 parent_ino,
+ int ins_len);
int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
const char *name, int name_len,
diff --git a/inode-item.c b/inode-item.c
index 1cc106670cd4..461557cb83d6 100644
--- a/inode-item.c
+++ b/inode-item.c
@@ -228,9 +228,9 @@ static int btrfs_find_name_in_ext_backref(struct btrfs_path
*path,
}
struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
- *trans, struct btrfs_path *path, struct btrfs_root *root,
- u64 ino, u64 parent_ino, u64 index, const char *name,
- int namelen, int ins_len)
+ *trans, struct btrfs_root *root, struct btrfs_path *path,
+ const char *name, int namelen, u64 ino, u64 parent_ino,
+ int ins_len)
{
struct btrfs_key key;
struct btrfs_inode_extref *extref;
--
2.18.0
Re: [PATCH] Btrfs: unify error handling of btrfs_lookup_dir_item
On Wed, Sep 12, 2018 at 06:06:26AM +0800, Liu Bo wrote: > Unify the error handling part with IS_ERR_OR_NULL. > > No functional changes. > > Signed-off-by: Liu Bo Reviewed-by: David Sterba
Re: [PATCH 0/6] btrfs-progs: lowmem: bug fixes and inode_extref repair
Sorry for the noise, Please ignore this patchset. Will send v2. On 2018/9/13 3:20 AM, damenly...@gmail.com wrote: From: Su Yue This patchset can be fetched from my github(based on v4.17.1): https://github.com/Damenly/btrfs-progs/tree/lowmem_extref The patchset aims to support check and repair errors about inode_extref in lowmem mode. patch[1-2] let btrfs_unlink() detect inode_extref. patch[3] fixes a minor bug in check_dir_item due to my careless long ago. patch[4] fixes a bug about inconsistent path in check_fs_roots() under repair. patch[5] fixes a corner case about traversal of inode items. patch[6] enable inode_extref repair support and remove unnecessary checks. patch[7] add a test image, it can verify above patches except patch[3]. Su Yue (6): btrfs-progs: adjust arguments of btrfs_lookup_inode_extref() btrfs-progs: make btrfs_unlink() lookup inode_extref btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item() btrfs-progs: lowmem: search key of root again after check_fs_root() under repair btrfs-progs: lowmem: continue to check item in last slot while checking inodes btrfs-progs: lowmem: optimization and repair for check_inode_extref() check/mode-lowmem.c | 148 +++- ctree.h | 6 +- inode-item.c| 6 +- inode.c | 14 - 4 files changed, 139 insertions(+), 35 deletions(-)
Re: [PATCH 00/15] Add delayed-refs support to btrfs-progs
On 2018/7/16 11:39 PM, David Sterba wrote: On Fri, Jun 08, 2018 at 03:47:43PM +0300, Nikolay Borisov wrote: Hello, Here is a series which adds support for delayed refs. This is needed to enable later work on adding freespace tree repair code. Additionally, it results in more code sharing between kernel/user space. Patches 1-9 are simple prep patches removing some arguments, causing problems later. They can go independently of the delayed refs work. They don't introduce any functional changes. Next, patches 10-13 introduce the needed infrastructure to for delayed refs without actually activating it. Patch 14 finally wires it up by adding the necessary call outs to btrfs_run_delayed refs and reworking the extent addition/freeing functions. With all of this done, patch 15 finally removes the old code. This series passes all btrfs progs fsck and misc tests + fuzz tests apart from fuzz-003/007/009 - but those fail without this series so it's unlikely it's caused by it. Nikolay Borisov (15): btrfs-progs: Remove root argument from pin_down_bytes btrfs-progs: Remove root argument from btrfs_del_csums btrfs-progs: Add functions to modify the used space by a root btrfs-progs: Refactor the root used bytes are updated btrfs-progs: Make update_block_group take fs_info instead of root btrfs-progs: check: Drop trans/root arguments from free_extent_hook btrfs-progs: Remove root argument from __free_extent btrfs-progs: Remove root argument from alloc_reserved_tree_block btrfs-progs: Always pass 0 for offset when calling btrfs_free_extent for btree blocks. btrfs-progs: Add boolean to signal whether we are re-initing extent tree btrfs-progs: Add delayed refs infrastructure btrfs-progs: Add __free_extent2 function btrfs-progs: Add alloc_reserved_tree_block2 function btrfs-progs: Wire up delayed refs btrfs-progs: Remove old delayed refs infrastructure Added to devel. There were some patch-to-patch compilation issues, function alloc_reserved_tree_block2 used earlier than defined so I reordered the patches to fix that. The CI fails at test check/020-extent-ref-cases but it works on my machine so it's not caused by the patchset. Hi, David Actually, now kdave/devel still fails at fsck-tests/020 due to version 1st of this patchset. See the thread please https://www.spinics.net/lists/linux-btrfs/msg81675.html Nikolay's V2 patchset should slove the problem. You may have known the situation, this mail is just a gentle reminder :). Thanks, Su -- 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 5/7] btrfs-progs: lowmem: improve check_inode_extref()
From: Su Yue
inode_extref is much similar with inode_ref, most codes are reused in
check_inode_extref().
Exception:
There is no need to check root directory, so remove it.
Make check_inode_extref() verify hash value with key offset now.
And lowmem check can detect errors about inode_extref and try to
repair errors.
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 119 ++--
1 file changed, 92 insertions(+), 27 deletions(-)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 8fc9edab1d66..b665c700328c 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1182,37 +1182,79 @@ out:
*
* Return 0 if no error occurred.
*/
-static int check_inode_extref(struct btrfs_root *root,
- struct btrfs_key *ref_key,
- struct extent_buffer *node, int slot, u64 *refs,
- int mode)
+static int check_inode_extref(struct btrfs_root *root, struct btrfs_key
*ref_key,
+ struct btrfs_path *path, char *name_ret,
+ u32 *namelen_ret, u64 *refs_ret, int mode)
{
struct btrfs_key key;
struct btrfs_key location;
struct btrfs_inode_extref *extref;
+ struct extent_buffer *node;
char namebuf[BTRFS_NAME_LEN] = {0};
u32 total;
- u32 cur = 0;
+ u32 cur;
u32 len;
u32 name_len;
u64 index;
u64 parent;
+ int err;
+ int tmp_err;
int ret;
- int err = 0;
+ int slot;
+ u64 refs;
+ bool search_again = false;
location.objectid = ref_key->objectid;
location.type = BTRFS_INODE_ITEM_KEY;
location.offset = 0;
+begin:
+ cur = 0;
+ err = 0;
+ refs = *refs_ret;
+ *namelen_ret = 0;
+
+ /* since after repair, path and the dir item may be changed */
+ if (search_again) {
+ search_again = false;
+ btrfs_release_path(path);
+ ret = btrfs_search_slot(NULL, root, ref_key, path, 0, 0);
+ /*
+* The item was deleted, let the path point to the last checked
+* item.
+*/
+ if (ret > 0) {
+ if (path->slots[0] == 0) {
+ ret = btrfs_prev_leaf(root, path);
+ /*
+* we are checking the inode item, there must
+* be some items before, the case shall never
+* happen.
+*/
+ BUG_ON(ret);
+ } else {
+ path->slots[0]--;
+ }
+ goto out;
+ } else if (ret < 0) {
+ err |= ret;
+ goto out;
+ }
+ }
+
+ node = path->nodes[0];
+ slot = path->slots[0];
extref = btrfs_item_ptr(node, slot, struct btrfs_inode_extref);
total = btrfs_item_size_nr(node, slot);
-next:
- /* update inode ref count */
- (*refs)++;
- name_len = btrfs_inode_extref_name_len(node, extref);
- index = btrfs_inode_extref_index(node, extref);
+loop:
+ /* Update inode ref count */
+ refs++;
+ tmp_err = 0;
parent = btrfs_inode_extref_parent(node, extref);
+ index = btrfs_inode_extref_index(node, extref);
+ name_len = btrfs_inode_extref_name_len(node, extref);
+
if (name_len <= BTRFS_NAME_LEN) {
len = name_len;
} else {
@@ -1220,37 +1262,60 @@ next:
warning("root %llu INODE_EXTREF[%llu %llu] name too long",
root->objectid, ref_key->objectid, ref_key->offset);
}
+
read_extent_buffer(node, namebuf, (unsigned long)(extref + 1), len);
- /* Check root dir ref name */
- if (index == 0 && strncmp(namebuf, "..", name_len)) {
- error("root %llu INODE_EXTREF[%llu %llu] ROOT_DIR name
shouldn't be %s",
+ /* verify hash value */
+ if (ref_key->offset != btrfs_extref_hash(parent, namebuf, len)) {
+ err |= -EIO;
+ error("root %llu INODE_EXTREF[%llu %llu] name %s namelen %u
mode %d mismatch with its hash, wanted %llu have %llu",
root->objectid, ref_key->objectid, ref_key->offset,
- namebuf);
- err |= ROOT_DIR_ERROR;
+ namebuf, len, mode, ref_key->offset,
+ btrfs_extref_hash(parent, namebuf, len));
+ goto out;
}
- /* find related dir_index */
+ /* copy the first name found to name_ret */
+ if (refs == 1 && name_ret) {
+ memcpy(name_ret, namebuf, len);
+ *namelen_ret = len;
+ }
+
+ /* Find related DIR_INDEX
[PATCH 5/6] btrfs-progs: lowmem: continue to check item in last slot while checking inodes
From: Su Yue
After call of check_inode_item(), path may point to the last unchecked
slot of the leaf. The outer walk_up_tree() always treats the position
as checked item then skips to next item.
If the last item was an inode item, yes, it was unchecked.
Then walk_up_tree() will think the leaf is checked and walk up to
upper node, process_one_leaf() in walk_down_tree() would skip to
check next inode item. Which means, the inode item won't be checked.
Solution:
After check_inode_item returns, if found path point to the last item
of a leaf, decrease path slot manually, so walk_up_tree() will stay
on the leaf.
Fixes: 5e2dc770471b ("btrfs-progs: check: skip shared node or leaf check for
low_memory mode")
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 12
1 file changed, 12 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 8fc9edab1d66..b6b33786d02b 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2612,6 +2612,18 @@ again:
if (cur->start == cur_bytenr)
goto again;
+ /*
+* path may point at the last item(a inode item maybe) in a leaf.
+* Without below lines, walk_up_tree() will skip the item which
+* means all items related to the inode will never be checked.
+* Decrease the slot manually, walk_up_tree won't skip to next node
+* if it occurs.
+*/
+ if (path->slots[0] + 1 >= btrfs_header_nritems(path->nodes[0])) {
+ if (path->slots[0])
+ path->slots[0]--;
+ }
+
/*
* we have switched to another leaf, above nodes may
* have changed, here walk down the path, if a node
--
2.18.0
[PATCH 6/7] btrfs-progs: lowmem: continue to check item in last slot while checking inodes
From: Su Yue
After call of check_inode_item(), path may point to the last unchecked
slot of the leaf. The outer walk_up_tree() always treats the position
as checked item then skips to next item.
If the last item was an inode item, yes, it was unchecked.
Then walk_up_tree() will think the leaf is checked and walk up to
upper node, process_one_leaf() in walk_down_tree() would skip to
check next inode item. Which means, the inode item won't be checked.
Solution:
After check_inode_item returns, if found path point to the last item
of a leaf, decrease path slot manually, so walk_up_tree() will stay
on the leaf.
Fixes: 5e2dc770471b ("btrfs-progs: check: skip shared node or leaf check for
low_memory mode")
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 12
1 file changed, 12 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index b665c700328c..bec2ee185cc8 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2677,6 +2677,18 @@ again:
if (cur->start == cur_bytenr)
goto again;
+ /*
+* path may point at the last item(a inode item maybe) in a leaf.
+* Without below lines, walk_up_tree() will skip the item which
+* means all items related to the inode will never be checked.
+* Decrease the slot manually, walk_up_tree won't skip to next node
+* if it occurs.
+*/
+ if (path->slots[0] + 1 >= btrfs_header_nritems(path->nodes[0])) {
+ if (path->slots[0])
+ path->slots[0]--;
+ }
+
/*
* we have switched to another leaf, above nodes may
* have changed, here walk down the path, if a node
--
2.18.0
[PATCH 7/7] btrfs-progs: fsck-tests: add test case inode_extref without dir_item and dir_index
From: Su Yue
This case contains an inode_extref:
==
...
item 1 key (257 INODE_EXTREF 3460996356) itemoff 3947 itemsize 24
index 257 parent 256 namelen 6 name: foo255
...
==
The related dir_item and dir_index are missing.
Add the case to ensure both original and lowmem mode check can handle
the case of inode_extref.
Lowmem part is supported since patch named
'btrfs-progs: lowmem: improve check_inode_extref()'.
And rename default_case.img to inode_ref_without_dir_item_index.img.
Signed-off-by: Su Yue
---
.../inode_extref_without_dir_item_index.img | Bin 0 -> 10240 bytes
...=> inode_ref_without_dir_item_and_index.img} | Bin
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644
tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
rename tests/fsck-tests/009-no-dir-item-or-index/{default_case.img =>
inode_ref_without_dir_item_and_index.img} (100%)
diff --git
a/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
b/tests/fsck-tests/009-no-dir-item-or-index/inode_extref_without_dir_item_index.img
new file mode 100644
index
..85f7a8211fe66dfa9b2b1f3a98e72f54cd402616
GIT binary patch
literal 10240
zcmeHscQo8j8?UlhEOymZqDGCfdWp7rf~Y~1#Of{4q9>NMSe=L(Wkn}~5JZVy61@b`
zOY|NsBwmBv=mszSgNy!H|F1
z$NlwH{s)Ks?ejn7KN0v(1pZ$}z<*}eU$kky0Zga5{R>1!Q|jBj!Kk_IdwP^`75
zM#+ojWKDf@k7f}D9aT8qYmSi4F2VupuXr{D%*M11tY*yA57Z39C|MYjWMVhzVK5j1
zieWerkiPZjcdzsBS22#CPt1DVI!`Y9h*_LYFoYww=`DnvA?wF8(}YD85$
zG7$zaGL3dVC{eyOEl3W_-qsf~#zJSUyqf<-HM2v|!&%~jSR%lz|08~jx;7^e3~;sY
zT^phzaQf-A9pzJu_AnE#I*l;xF8RnBNniHgj!g0JPRgY|`OkhQSmPV3fQT{G?g$ZX6e{@Ov^aKMAG
zr|O5JwU)}O00QwdZZ<+yyfQ#~Ka9r6hA@3Oik}0|;rd7T)~)3|9Uli=;g
zh~{}Qn(6qP?bLNIenC5
zLO3=6UuM9&&4o2fB&4lmozz;AiUWx^ndhJ)O`@bM4QWxL!mC>|7xSNK`Sw)H$N)-m
zSXz2)pA0>PYZPT59_=l(%tNBo?D|v)$%|6$`GKhK<>+y93Fiy
zTiWZzmC*aeUfXv1K)7a?^YiuO;WXH%#MegmjkD8b)YP2MzV{*|{#*5|R9Ov&-Q^d%
z_IMBdg|z+-kf1lZG%Y^>3;_8eTVDY0gLuVuK^iH*1%Z5W_7lF*~uM$6u+hgOC_
z4rWyxVvQeo1hp}4g+fK$4!+0}!ov|G-z-7PF$8c+7m_drD7D1!U-DzS0t`X
z>Y@fV(K_9a{<$w^me4NJyvG7Sf}U~z9#&01Q3o5cr>bn8t5a2O%6)BWf6SW`JBx#V08j00K~k%COtKaNNL64F+f;tgKG=lY(fg3Y1(No%L^e>S=)3E(
z&!A-jV%H0qu$NE*8j|3Wl=2@yv!W(Y57SB519_C
z6)+t9-_I4%1fb5Z{cpNOM?t%BnXTyFy@$oS?=g{%z#FR7*F40BkJ{o-r
zeg|!2s2At!us$=cRn=-Ad2{xrofs7EHO~dqhBX`nd|fIgA*e6HJ^;GIet=Fzs|Guf
[PATCH 6/6] btrfs-progs: lowmem: optimization and repair for check_inode_extref()
From: Su Yue
inode_extref is much similar with inode_ref, most codes are reused in
check_inode_extref().
Exception:
There is no need to check root directory, so remove it.
Make check_inode_extref() verify hash value with key offset now.
And lowmem check can detect errors about inode_extref and try to
repair errors.
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 119 ++--
1 file changed, 92 insertions(+), 27 deletions(-)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index b6b33786d02b..bec2ee185cc8 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1182,37 +1182,79 @@ out:
*
* Return 0 if no error occurred.
*/
-static int check_inode_extref(struct btrfs_root *root,
- struct btrfs_key *ref_key,
- struct extent_buffer *node, int slot, u64 *refs,
- int mode)
+static int check_inode_extref(struct btrfs_root *root, struct btrfs_key
*ref_key,
+ struct btrfs_path *path, char *name_ret,
+ u32 *namelen_ret, u64 *refs_ret, int mode)
{
struct btrfs_key key;
struct btrfs_key location;
struct btrfs_inode_extref *extref;
+ struct extent_buffer *node;
char namebuf[BTRFS_NAME_LEN] = {0};
u32 total;
- u32 cur = 0;
+ u32 cur;
u32 len;
u32 name_len;
u64 index;
u64 parent;
+ int err;
+ int tmp_err;
int ret;
- int err = 0;
+ int slot;
+ u64 refs;
+ bool search_again = false;
location.objectid = ref_key->objectid;
location.type = BTRFS_INODE_ITEM_KEY;
location.offset = 0;
+begin:
+ cur = 0;
+ err = 0;
+ refs = *refs_ret;
+ *namelen_ret = 0;
+
+ /* since after repair, path and the dir item may be changed */
+ if (search_again) {
+ search_again = false;
+ btrfs_release_path(path);
+ ret = btrfs_search_slot(NULL, root, ref_key, path, 0, 0);
+ /*
+* The item was deleted, let the path point to the last checked
+* item.
+*/
+ if (ret > 0) {
+ if (path->slots[0] == 0) {
+ ret = btrfs_prev_leaf(root, path);
+ /*
+* we are checking the inode item, there must
+* be some items before, the case shall never
+* happen.
+*/
+ BUG_ON(ret);
+ } else {
+ path->slots[0]--;
+ }
+ goto out;
+ } else if (ret < 0) {
+ err |= ret;
+ goto out;
+ }
+ }
+
+ node = path->nodes[0];
+ slot = path->slots[0];
extref = btrfs_item_ptr(node, slot, struct btrfs_inode_extref);
total = btrfs_item_size_nr(node, slot);
-next:
- /* update inode ref count */
- (*refs)++;
- name_len = btrfs_inode_extref_name_len(node, extref);
- index = btrfs_inode_extref_index(node, extref);
+loop:
+ /* Update inode ref count */
+ refs++;
+ tmp_err = 0;
parent = btrfs_inode_extref_parent(node, extref);
+ index = btrfs_inode_extref_index(node, extref);
+ name_len = btrfs_inode_extref_name_len(node, extref);
+
if (name_len <= BTRFS_NAME_LEN) {
len = name_len;
} else {
@@ -1220,37 +1262,60 @@ next:
warning("root %llu INODE_EXTREF[%llu %llu] name too long",
root->objectid, ref_key->objectid, ref_key->offset);
}
+
read_extent_buffer(node, namebuf, (unsigned long)(extref + 1), len);
- /* Check root dir ref name */
- if (index == 0 && strncmp(namebuf, "..", name_len)) {
- error("root %llu INODE_EXTREF[%llu %llu] ROOT_DIR name
shouldn't be %s",
+ /* verify hash value */
+ if (ref_key->offset != btrfs_extref_hash(parent, namebuf, len)) {
+ err |= -EIO;
+ error("root %llu INODE_EXTREF[%llu %llu] name %s namelen %u
mode %d mismatch with its hash, wanted %llu have %llu",
root->objectid, ref_key->objectid, ref_key->offset,
- namebuf);
- err |= ROOT_DIR_ERROR;
+ namebuf, len, mode, ref_key->offset,
+ btrfs_extref_hash(parent, namebuf, len));
+ goto out;
}
- /* find related dir_index */
+ /* copy the first name found to name_ret */
+ if (refs == 1 && name_ret) {
+ memcpy(name_ret, namebuf, len);
+ *namelen_ret = len;
+ }
+
+ /* Find related DIR_INDEX
[PATCH 4/6] btrfs-progs: lowmem: search key of root again after check_fs_root() under repair
From: Su Yue
In check_fs_roots_lowmem(), we do search and follow the resulted path
to call check_fs_root(), then call btrfs_next_item() to check next
root.
However, if repair is enabled, the root tree can be cowed, the
existed path can cause strange errors.
Solution:
If repair, save the key before calling check_fs_root,
search the saved key again before checking next root.
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 89a304bbdd69..8fc9edab1d66 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4967,9 +4967,13 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
}
while (1) {
+ struct btrfs_key saved_key;
+
node = path.nodes[0];
slot = path.slots[0];
btrfs_item_key_to_cpu(node, , slot);
+ if (repair)
+ saved_key = key;
if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
goto out;
if (key.type == BTRFS_ROOT_ITEM_KEY &&
@@ -5000,6 +5004,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info)
err |= ret;
}
next:
+ /*
+* Since root tree can be cowed during repair,
+* here search the saved key again.
+*/
+ if (repair) {
+ btrfs_release_path();
+ ret = btrfs_search_slot(NULL, fs_info->tree_root,
+ _key, , 0, 0);
+ /* Repair never deletes trees, search must succeed. */
+ BUG_ON(ret);
+ }
ret = btrfs_next_item(tree_root, );
if (ret > 0)
goto out;
--
2.18.0
[PATCH 2/6] btrfs-progs: make btrfs_unlink() lookup inode_extref
From: Su Yue
btrfs_unlink() uses btrfs_lookup_inode_ref() to look up inode_ref
but forget inode_extref case.
Let btrfs_unlink() call btrfs_lookup_inode_extref() if inode_ref is
found and EXTENDED_IREF feature is enabled.
Fixes: 0cc75eddd093 ("btrfs-progs: Add btrfs_unlink() and btrfs_add_link()
functions.")
Signed-off-by: Su Yue
---
inode.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/inode.c b/inode.c
index 2398bca4a109..598ad0ab6b4c 100644
--- a/inode.c
+++ b/inode.c
@@ -277,6 +277,7 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct
btrfs_root *root,
struct btrfs_key key;
struct btrfs_inode_item *inode_item;
struct btrfs_inode_ref *inode_ref;
+ struct btrfs_inode_extref *inode_extref = NULL;
struct btrfs_dir_item *dir_item;
u64 inode_size;
u32 nlinks;
@@ -296,7 +297,18 @@ int btrfs_unlink(struct btrfs_trans_handle *trans, struct
btrfs_root *root,
ret = PTR_ERR(inode_ref);
goto out;
}
- if (inode_ref)
+
+ if (!inode_ref && btrfs_fs_incompat(root->fs_info, EXTENDED_IREF)) {
+ btrfs_release_path(path);
+ inode_extref = btrfs_lookup_inode_extref(trans, root, path,
+name, namelen, ino, parent_ino, 0);
+ if (IS_ERR(inode_extref)) {
+ ret = PTR_ERR(inode_extref);
+ goto out;
+ }
+ }
+
+ if (inode_ref || inode_extref)
del_inode_ref = 1;
btrfs_release_path(path);
--
2.18.0
[PATCH 1/6] btrfs-progs: adjust arguments of btrfs_lookup_inode_extref()
From: Su Yue
The argument index is not used in btrfs_lookup_inode_extref(),
so remove it.
And adjust positions its arguments to make it consistent with
kernel part.
No functional change.
Fixes: 260675657767 ("btrfs-progs: Import btrfs_insert/del/lookup_extref()
functions.")
Signed-off-by: Su Yue
---
ctree.h | 6 +++---
inode-item.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/ctree.h b/ctree.h
index 4719962df67d..e7f6c5df95f1 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2708,9 +2708,9 @@ int btrfs_lookup_inode(struct btrfs_trans_handle *trans,
struct btrfs_root
*root, struct btrfs_path *path,
struct btrfs_key *location, int mod);
struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
- *trans, struct btrfs_path *path, struct btrfs_root *root,
- u64 ino, u64 parent_ino, u64 index, const char *name,
- int namelen, int ins_len);
+ *trans, struct btrfs_root *root, struct btrfs_path *path,
+ const char *name, int namelen, u64 ino, u64 parent_ino,
+ int ins_len);
int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
const char *name, int name_len,
diff --git a/inode-item.c b/inode-item.c
index 1cc106670cd4..461557cb83d6 100644
--- a/inode-item.c
+++ b/inode-item.c
@@ -228,9 +228,9 @@ static int btrfs_find_name_in_ext_backref(struct btrfs_path
*path,
}
struct btrfs_inode_extref *btrfs_lookup_inode_extref(struct btrfs_trans_handle
- *trans, struct btrfs_path *path, struct btrfs_root *root,
- u64 ino, u64 parent_ino, u64 index, const char *name,
- int namelen, int ins_len)
+ *trans, struct btrfs_root *root, struct btrfs_path *path,
+ const char *name, int namelen, u64 ino, u64 parent_ino,
+ int ins_len)
{
struct btrfs_key key;
struct btrfs_inode_extref *extref;
--
2.18.0
[PATCH 3/6] btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item()
From: Su Yue
In check_dir_item, we are going to search corresponding
dir_item/index.
Commit 564901eac7a4 ("btrfs-progs: check: introduce
print_dir_item_err()") Changed argument name from key to di_key but
forgot to change the key name for dir_item search.
So @key shouldn't be used here. It should be @di_key.
Fixes: 564901eac7a4 ("btrfs-progs: check: introduce print_dir_item_err()")
Signed-off-by: Su Yue
---
check/mode-lowmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1bce44f5658a..89a304bbdd69 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1658,7 +1658,7 @@ begin:
/* check relative INDEX/ITEM */
key.objectid = di_key->objectid;
- if (key.type == BTRFS_DIR_ITEM_KEY) {
+ if (di_key->type == BTRFS_DIR_ITEM_KEY) {
key.type = BTRFS_DIR_INDEX_KEY;
key.offset = index;
} else {
--
2.18.0
[PATCH 0/6] btrfs-progs: lowmem: bug fixes and inode_extref repair
From: Su Yue This patchset can be fetched from my github(based on v4.17.1): https://github.com/Damenly/btrfs-progs/tree/lowmem_extref The patchset aims to support check and repair errors about inode_extref in lowmem mode. patch[1-2] let btrfs_unlink() detect inode_extref. patch[3] fixes a minor bug in check_dir_item due to my careless long ago. patch[4] fixes a bug about inconsistent path in check_fs_roots() under repair. patch[5] fixes a corner case about traversal of inode items. patch[6] enable inode_extref repair support and remove unnecessary checks. patch[7] add a test image, it can verify above patches except patch[3]. Su Yue (6): btrfs-progs: adjust arguments of btrfs_lookup_inode_extref() btrfs-progs: make btrfs_unlink() lookup inode_extref btrfs-progs: lowmem check: find dir_item by di_key in check_dir_item() btrfs-progs: lowmem: search key of root again after check_fs_root() under repair btrfs-progs: lowmem: continue to check item in last slot while checking inodes btrfs-progs: lowmem: optimization and repair for check_inode_extref() check/mode-lowmem.c | 148 +++- ctree.h | 6 +- inode-item.c| 6 +- inode.c | 14 - 4 files changed, 139 insertions(+), 35 deletions(-) -- 2.18.0
[PATCH] btrfs-progs: change filename limit to 255 when creating subvolume
Modify the file name length limit to meet the Linux naming convention.
In addition, the file name length is always bigger than 0, no need to
compare with 0 again.
Issue: #145
Signed-off-by: Su Yanjun
---
cmds-subvolume.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e7a884af..fe97fca3 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -715,7 +715,7 @@ static int cmd_subvol_snapshot(int argc, char **argv)
}
len = strlen(newname);
- if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
+ if (len > BTRFS_VOL_NAME_MAX) {
error("snapshot name too long '%s'", newname);
goto out;
}
--
2.18.0
Re: [PATCH] Btrfs: do not wait after queue async work for delaye refs
[Adding Josef to CC]
On 12.09.2018 01:06, Liu Bo wrote:
> If metadata space is hungry, how fast flush_space() can run determines
> the latency we spend in reserve_metadata_space().
>
> flush_space()
>case FLUSH_DELAYED_ITEMS:
> ...
> btrfs_end_transaction()
>case ALLOC_CHUNK:
> ...
> btrfs_end_transaction()
>
> btrfs_end_transaction()
>btrfs_async_run_delayed_refs()
>// queue a work to process delayed refs
>...
>if (wait)
>wait_for_completion()
>
> Although processing delayed refs can add to pinned bytes, pinned bytes
> can only be used after committing transaction, so waiting async in
> flush_space() doesn't help.
But not waiting for the async delayed refs to complete don't we
introduce a race where checking the pinned bytes might indicate there
are not enough bytes to commit at the same time we will have delayed
refs which haven't yet run but might add enough pinned bytes to make
sense to commit?
>
> In fact we don't have to wait for asynchronous delayed refs processing
> as delayed refs are not blocking the subsequent operations(unless within
> committing transaction we need to make sure filesystem is consistent).
>
> Signed-off-by: Liu Bo
> ---
> fs/btrfs/ctree.h | 2 +-
> fs/btrfs/extent-tree.c | 18 ++
> fs/btrfs/inode.c | 2 +-
> fs/btrfs/transaction.c | 3 +--
> 4 files changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 00e506de70ba..4c3a733ee4cf 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2620,7 +2620,7 @@ void btrfs_dec_block_group_reservations(struct
> btrfs_fs_info *fs_info,
> int btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
> unsigned long count);
> int btrfs_async_run_delayed_refs(struct btrfs_fs_info *fs_info,
> - unsigned long count, u64 transid, int wait);
> + unsigned long count, u64 transid);
> int btrfs_lookup_data_extent(struct btrfs_fs_info *fs_info, u64 start, u64
> len);
> int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
>struct btrfs_fs_info *fs_info, u64 bytenr,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index b6767f9031b5..cac9a9d04d0c 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2852,14 +2852,11 @@ static void delayed_ref_async_start(struct btrfs_work
> *work)
> if (ret && !async->error)
> async->error = ret;
> done:
> - if (async->sync)
> - complete(>wait);
> - else
> - kfree(async);
> + kfree(async);
> }
>
> int btrfs_async_run_delayed_refs(struct btrfs_fs_info *fs_info,
> - unsigned long count, u64 transid, int wait)
> + unsigned long count, u64 transid)
> {
> struct async_delayed_refs *async;
> int ret;
> @@ -2872,23 +2869,12 @@ int btrfs_async_run_delayed_refs(struct btrfs_fs_info
> *fs_info,
> async->count = count;
> async->error = 0;
> async->transid = transid;
> - if (wait)
> - async->sync = 1;
> - else
> - async->sync = 0;
> - init_completion(>wait);
>
> btrfs_init_work(>work, btrfs_extent_refs_helper,
> delayed_ref_async_start, NULL, NULL);
>
> btrfs_queue_work(fs_info->extent_workers, >work);
>
> - if (wait) {
> - wait_for_completion(>wait);
> - ret = async->error;
> - kfree(async);
> - return ret;
> - }
> return 0;
> }
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 99ab0203b701..fd4af54f0d3b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4736,7 +4736,7 @@ int btrfs_truncate_inode_items(struct
> btrfs_trans_handle *trans,
> if (btrfs_should_throttle_delayed_refs(trans, fs_info))
> btrfs_async_run_delayed_refs(fs_info,
> trans->delayed_ref_updates * 2,
> - trans->transid, 0);
> + trans->transid);
> if (be_nice) {
> if (truncate_space_check(trans, root,
>extent_num_bytes)) {
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 772963a61072..a816c0999fb2 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -889,8 +889,7 @@ static int __btrfs_end_transaction(struct
> btrfs_trans_handle *trans,
>
> kmem_cache_free(btrfs_trans_handle_cachep, trans);
> if (must_run_delayed_refs) {
> - btrfs_async_run_delayed_refs(info, cur, transid,
> - must_run_delayed_refs == 1);
> +
Re: [PATCH] Btrfs: unify error handling of btrfs_lookup_dir_item
On 12.09.2018 01:06, Liu Bo wrote:
> Unify the error handling part with IS_ERR_OR_NULL.
>
> No functional changes.
>
> Signed-off-by: Liu Bo
Reviewed-by: Nikolay Borisov
> ---
> fs/btrfs/inode.c | 21 +
> fs/btrfs/send.c | 8 ++--
> 2 files changed, 7 insertions(+), 22 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index fd64d7ac76f9..99ab0203b701 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3925,12 +3925,8 @@ static int __btrfs_unlink_inode(struct
> btrfs_trans_handle *trans,
> path->leave_spinning = 1;
> di = btrfs_lookup_dir_item(trans, root, path, dir_ino,
> name, name_len, -1);
> - if (IS_ERR(di)) {
> - ret = PTR_ERR(di);
> - goto err;
> - }
> - if (!di) {
> - ret = -ENOENT;
> + if (IS_ERR_OR_NULL(di)) {
> + ret = di ? PTR_ERR(di) : -ENOENT;
> goto err;
> }
> ret = btrfs_delete_one_dir_name(trans, root, path, di);
> @@ -4088,10 +4084,7 @@ static int btrfs_unlink_subvol(struct
> btrfs_trans_handle *trans,
> di = btrfs_lookup_dir_item(trans, root, path, dir_ino,
> name, name_len, -1);
> if (IS_ERR_OR_NULL(di)) {
> - if (!di)
> - ret = -ENOENT;
> - else
> - ret = PTR_ERR(di);
> + ret = di ? PTR_ERR(di) : -ENOENT;
> goto out;
> }
>
> @@ -5481,12 +5474,8 @@ static int btrfs_inode_by_name(struct inode *dir,
> struct dentry *dentry,
>
> di = btrfs_lookup_dir_item(NULL, root, path, btrfs_ino(BTRFS_I(dir)),
> name, namelen, 0);
> - if (!di) {
> - ret = -ENOENT;
> - goto out;
> - }
> - if (IS_ERR(di)) {
> - ret = PTR_ERR(di);
> + if (IS_ERR_OR_NULL(di)) {
> + ret = di ? PTR_ERR(di) : -ENOENT;
> goto out;
> }
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index ba8950bfd9c7..b8c83778f0f7 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -1693,12 +1693,8 @@ static int lookup_dir_item_inode(struct btrfs_root
> *root,
>
> di = btrfs_lookup_dir_item(NULL, root, path,
> dir, name, name_len, 0);
> - if (!di) {
> - ret = -ENOENT;
> - goto out;
> - }
> - if (IS_ERR(di)) {
> - ret = PTR_ERR(di);
> + if (IS_ERR_OR_NULL(di)) {
> + ret = di ? PTR_ERR(di) : -ENOENT;
> goto out;
> }
> btrfs_dir_item_key_to_cpu(path->nodes[0], di, );
>
Re: [PATCH] Btrfs: remove unused code in __btrfs_unlink_inode
On 12.09.2018 01:06, Liu Bo wrote:
> It might get @leaf and @key in order to do some sanity checks on key's
> fields, but since we don't check it any more, it's fine to remove the
> code.
This is actually false, leaf and key weren't used for sanity checks.
Instead this code was first introduced in 5f39d397dfbe ("Btrfs: Create
extent_buffer interface for large blocksizes") and the function was
named btrfs_unlink_trans. It later got renamed to __btrfs_unlink_inode
and finally commit 16cdcec736cd ("btrfs: implement delayed inode items
operation") changed the way inodes are deleted and obviated the need for
those two members.
Generally when unused variables are deleted you need to do a bit of
archeology and explain how they got to be unused in the changelog.
>
> Signed-off-by: Liu Bo
> ---
> fs/btrfs/inode.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index d3febc3a6bc0..fd64d7ac76f9 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3911,9 +3911,7 @@ static int __btrfs_unlink_inode(struct
> btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info = root->fs_info;
> struct btrfs_path *path;
> int ret = 0;
> - struct extent_buffer *leaf;
> struct btrfs_dir_item *di;
> - struct btrfs_key key;
> u64 index;
> u64 ino = btrfs_ino(inode);
> u64 dir_ino = btrfs_ino(dir);
> @@ -3935,8 +3933,6 @@ static int __btrfs_unlink_inode(struct
> btrfs_trans_handle *trans,
> ret = -ENOENT;
> goto err;
> }
> - leaf = path->nodes[0];
> - btrfs_dir_item_key_to_cpu(leaf, di, );
> ret = btrfs_delete_one_dir_name(trans, root, path, di);
> if (ret)
> goto err;
>
Re: [PATCH] Btrfs: skip setting path to blocking mode if balance is not needed
On 12.09.2018 01:06, Liu Bo wrote:
> balance_level() may return early in some cases, but these checks don't
> have to be done with blocking write lock.
>
> This puts together these checks into a helper and the benefit is to
> avoid switching spinning locks to blocking locks (in these paticular
> cases) which slows down btrfs overall.
Performance patches without numbers are frowned upon. You need to
substantiate your claims.
>
> Signed-off-by: Liu Bo
> ---
> fs/btrfs/ctree.c | 41 ++---
> 1 file changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 858085490e23..ba267a069ca1 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1758,6 +1758,29 @@ static void root_sub_used(struct btrfs_root *root, u32
> size)
> return eb;
> }
>
> +static bool need_balance_level(struct btrfs_fs_info *fs_info,
nit: I think should_balance_level seems more readable, but it could be
just me so won't insist on that.
> + struct btrfs_trans_handle *trans,
> + struct btrfs_path *path, int level)
> +{
> + struct extent_buffer *mid;
> +
> + mid = path->nodes[level];
> +
> + WARN_ON(path->locks[level] != BTRFS_WRITE_LOCK &&
> + path->locks[level] != BTRFS_WRITE_LOCK_BLOCKING);
> + WARN_ON(btrfs_header_generation(mid) != trans->transid);
> +
> + /* If mid is the root node. */
> + if (level < BTRFS_MAX_LEVEL - 1 && path->nodes[level + 1] == NULL)
> + if (btrfs_header_nritems(mid) != 1)
> + return false;
> +
> + if (btrfs_header_nritems(mid) > BTRFS_NODEPTRS_PER_BLOCK(fs_info) / 4)
> + return false;
> +
> + return true;
> +}
> +
> /*
> * node level balancing, used to make sure nodes are in proper order for
> * item deletion. We balance from the top down, so we have to make sure
> @@ -1780,10 +1803,6 @@ static noinline int balance_level(struct
> btrfs_trans_handle *trans,
>
> mid = path->nodes[level];
>
> - WARN_ON(path->locks[level] != BTRFS_WRITE_LOCK &&
> - path->locks[level] != BTRFS_WRITE_LOCK_BLOCKING);
> - WARN_ON(btrfs_header_generation(mid) != trans->transid);
> -
> orig_ptr = btrfs_node_blockptr(mid, orig_slot);
>
> if (level < BTRFS_MAX_LEVEL - 1) {
> @@ -1798,9 +1817,6 @@ static noinline int balance_level(struct
> btrfs_trans_handle *trans,
> if (!parent) {
> struct extent_buffer *child;
>
> - if (btrfs_header_nritems(mid) != 1)
> - return 0;
> -
> /* promote the child to a root */
> child = read_node_slot(fs_info, mid, 0);
> if (IS_ERR(child)) {
> @@ -1838,9 +1854,6 @@ static noinline int balance_level(struct
> btrfs_trans_handle *trans,
> free_extent_buffer_stale(mid);
> return 0;
> }
> - if (btrfs_header_nritems(mid) >
> - BTRFS_NODEPTRS_PER_BLOCK(fs_info) / 4)
> - return 0;
>
> left = read_node_slot(fs_info, parent, pslot - 1);
> if (IS_ERR(left))
> @@ -2460,14 +2473,20 @@ noinline void btrfs_unlock_up_safe(struct btrfs_path
> *path, int level)
> goto again;
> }
>
> + /* Skip setting path to blocking if balance is not needed. */
> + if (!need_balance_level(fs_info, trans, p, level)) {
> + ret = 0;
> + goto done;
> + }
> +
> btrfs_set_path_blocking(p);
> reada_for_balance(fs_info, p, level);
> sret = balance_level(trans, root, p, level);
> -
> if (sret) {
> ret = sret;
> goto done;
> }
> +
> b = p->nodes[level];
> if (!b) {
> btrfs_release_path(p);
>
Re: [PATCH] Btrfs: remove level==0 check in balance_level
On 12.09.2018 01:06, Liu Bo wrote: > btrfs_search_slot() >if (level != 0) > setup_nodes_for_search() > balance_level() > > It is just impossible to have level=0 in balance_level. > > Signed-off-by: Liu Bo I concur with the analysis: Reviewed-by: Nikolay Borisov > --- > fs/btrfs/ctree.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 8b31caa60b0a..858085490e23 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1778,9 +1778,6 @@ static noinline int balance_level(struct > btrfs_trans_handle *trans, > int orig_slot = path->slots[level]; > u64 orig_ptr; > > - if (level == 0) > - return 0; > - > mid = path->nodes[level]; > > WARN_ON(path->locks[level] != BTRFS_WRITE_LOCK && >
Re: [PATCH] Btrfs: assert page dirty bit
On 12.09.2018 01:06, Liu Bo wrote: > Just in case that someone breaks the rule that pages are dirty as long > as eb is dirty. > > Signed-off-by: Liu Bo > --- > fs/btrfs/extent_io.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index fb2bf50134a1..99895f196ecb 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -5184,6 +5184,11 @@ bool set_extent_buffer_dirty(struct extent_buffer *eb) > set_page_dirty(eb->pages[i]); > } > > +#ifdef BTRFS_DEBUG And this will never be compiled since the actual ifdef name is "CONFIG_BTRFS_DEBUG" > + for (i = 0; i < num_pages; i++) > + ASSERT(PageDirty(eb->pages[i])); > +#endif > + > return was_dirty; > } > >
Re: [PATCH] Btrfs: skip set_page_dirty if eb is dirty
On 12.09.2018 01:06, Liu Bo wrote:
> As long as @eb is marked with EXTENT_BUFFER_DIRTY, all of its pages
> are dirty, so no need to set pages dirty again.
>
> Signed-off-by: Liu Bo
Does make it any performance difference, numbers?
Reviewed-by: Nikolay Borisov
> ---
> fs/btrfs/extent_io.c | 11 +++
> fs/btrfs/extent_io.h | 2 +-
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 628f1aef34b0..fb2bf50134a1 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -5165,11 +5165,11 @@ void clear_extent_buffer_dirty(struct extent_buffer
> *eb)
> WARN_ON(atomic_read(>refs) == 0);
> }
>
> -int set_extent_buffer_dirty(struct extent_buffer *eb)
> +bool set_extent_buffer_dirty(struct extent_buffer *eb)
> {
> int i;
> int num_pages;
> - int was_dirty = 0;
> + bool was_dirty;
>
> check_buffer_tree_ref(eb);
>
> @@ -5179,8 +5179,11 @@ int set_extent_buffer_dirty(struct extent_buffer *eb)
> WARN_ON(atomic_read(>refs) == 0);
> WARN_ON(!test_bit(EXTENT_BUFFER_TREE_REF, >bflags));
>
> - for (i = 0; i < num_pages; i++)
> - set_page_dirty(eb->pages[i]);
> + if (!was_dirty) {
> + for (i = 0; i < num_pages; i++)
> + set_page_dirty(eb->pages[i]);
> + }
> +
> return was_dirty;
> }
>
> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
> index b4d03e677e1d..f2ab42d57f02 100644
> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -479,7 +479,7 @@ void extent_buffer_bitmap_set(struct extent_buffer *eb,
> unsigned long start,
> void extent_buffer_bitmap_clear(struct extent_buffer *eb, unsigned long
> start,
> unsigned long pos, unsigned long len);
> void clear_extent_buffer_dirty(struct extent_buffer *eb);
> -int set_extent_buffer_dirty(struct extent_buffer *eb);
> +bool set_extent_buffer_dirty(struct extent_buffer *eb);
> void set_extent_buffer_uptodate(struct extent_buffer *eb);
> void clear_extent_buffer_uptodate(struct extent_buffer *eb);
> int extent_buffer_under_io(struct extent_buffer *eb);
>
