* ✗ patchtest: failure for iputils: upgrade to s20190629 (rev2)
From: Patchwork @ 2018-07-23 14:02 UTC (permalink / raw)
To: Ross Burton; +Cc: openembedded-core
In-Reply-To: <20180723134848.3698-1-ross.burton@intel.com>
== Series Details ==
Series: iputils: upgrade to s20190629 (rev2)
Revision: 2
URL : https://patchwork.openembedded.org/series/13153/
State : failure
== Summary ==
Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:
* Issue Upstream-Status is Inappropriate, but no reason was provided [test_upstream_status_presence_format]
Suggested fix Include a brief reason why install.patch is inappropriate
Current Upstream-Status: Inappropriate
Standard format Upstream-Status: Inappropriate [reason]
If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).
---
Guidelines: https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe
^ permalink raw reply
* [MODERATED] Re: BUG: disabling SMT from BIOS
From: Jiri Kosina @ 2018-07-23 14:03 UTC (permalink / raw)
To: speck
In-Reply-To: <20180723124553.btzfagzsnoxoouhz@treble>
On Mon, 23 Jul 2018, speck for Josh Poimboeuf wrote:
> This permanently disables SMT on my system, regardless of the BIOS
> settings.
I see, we're setting _NOT_SUPPORTED before the sibling map has been
established.
> I suspect it needs to be called later, after the other CPUs have been
> brought online.
Either that, but that will basically mean redefining the CPU_SMT_ENABLED
meaning, and I actually think the current semantics makes more sense.
How about this instead? (still untested)
From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] x86/speculation/l1tf: properly report SMT vulnerable status
Even if SMT has not been disabled from the OS and CPUID indicates
it's supported, we should report as SMT-vulnerable iff there really
is more than one SMT sibling (consider "HT disabled in BIOS" case).
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
arch/x86/kernel/cpu/bugs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index fa6123bdd032..023bcb4b8244 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -760,7 +760,8 @@ static ssize_t l1tf_show_state(char *buf)
return sprintf(buf, "%s\n", L1TF_DEFAULT_MSG);
return sprintf(buf, "%s; VMX: SMT %s, L1D %s\n", L1TF_DEFAULT_MSG,
- cpu_smt_control == CPU_SMT_ENABLED ? "vulnerable" : "disabled",
+ (cpu_smt_control == CPU_SMT_ENABLED && topology_max_smt_threads() > 1)
+ ? "vulnerable" : "disabled",
l1tf_vmx_states[l1tf_vmx_mitigation]);
}
#else
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related
* Re: [PATCH 2/2] scsi: set timed out out mq requests to complete
From: Keith Busch @ 2018-07-23 14:04 UTC (permalink / raw)
To: hch@lst.de
Cc: Bart Van Assche, linux-scsi@vger.kernel.org,
keith.busch@intel.com, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org, axboe@kernel.dk,
jianchao.w.wang@oracle.com
In-Reply-To: <20180723081231.GB19247@lst.de>
On Mon, Jul 23, 2018 at 10:12:31AM +0200, hch@lst.de wrote:
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 8932ae81a15a..902c30d3c0ed 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -296,6 +296,8 @@ enum blk_eh_timer_return scsi_times_out(struct request *req)
> > rtn = host->hostt->eh_timed_out(scmd);
> >
> > if (rtn == BLK_EH_DONE) {
> > + if (req->q->mq_ops && blk_mq_mark_complete(req))
> > + return rtn;
>
> This looks pretty sensible to me as a band-aid. It just needs a very
> detailed comment explaining what is going on here.
Sounds good, v2 will be sent shortly.
^ permalink raw reply
* [PATCH 2/2] scsi: set timed out out mq requests to complete
From: Keith Busch @ 2018-07-23 14:04 UTC (permalink / raw)
In-Reply-To: <20180723081231.GB19247@lst.de>
On Mon, Jul 23, 2018@10:12:31AM +0200, hch@lst.de wrote:
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 8932ae81a15a..902c30d3c0ed 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -296,6 +296,8 @@ enum blk_eh_timer_return scsi_times_out(struct request *req)
> > rtn = host->hostt->eh_timed_out(scmd);
> >
> > if (rtn == BLK_EH_DONE) {
> > + if (req->q->mq_ops && blk_mq_mark_complete(req))
> > + return rtn;
>
> This looks pretty sensible to me as a band-aid. It just needs a very
> detailed comment explaining what is going on here.
Sounds good, v2 will be sent shortly.
^ permalink raw reply
* Re: [PATCH] phy: qcom-qmp: Fix dts bindings to reflect reality
From: Rob Herring @ 2018-07-23 14:04 UTC (permalink / raw)
To: Doug Anderson
Cc: Kishon Vijay Abraham I, Mark Rutland, devicetree,
linux-kernel@vger.kernel.org, Vivek Gautam, Manu Gautam,
Varadarajan Narayanan
In-Reply-To: <CAD=FV=XBCeKUrfMCppfhetWNWLJWzsbiBkQnTugoXDff-0SD5w@mail.gmail.com>
On Fri, Jul 20, 2018 at 11:54 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Fri, Jul 20, 2018 at 10:26 AM, Rob Herring <robh@kernel.org> wrote:
> > On Fri, Jul 20, 2018 at 9:13 AM Doug Anderson <dianders@chromium.org> wrote:
> >>
> >> Rob,
> >>
> >> On Fri, Jul 20, 2018 at 7:10 AM, Rob Herring <robh@kernel.org> wrote:
> >> > On Fri, Jul 06, 2018 at 04:31:42PM -0700, Douglas Anderson wrote:
> >> >> A few patches have landed for the qcom-qmp PHY that affect how you
> >> >> would write a device tree node. ...yet the bindings weren't updated.
> >> >> Let's remedy the situation and make the bindings refelect reality.
> >> >
> >> > "dt-bindings: phy: ..." for the subject.
> >>
> >> Sorry. Every subsystem has different conventions for this so I
> >> usually just do a "git log" on the file and make my best guess. I'll
> >> try to remember this for next time though.
> >
> > NP. I'd like to add this info to MAINTAINERS or maybe a git commit
> > hook could figure this out automagically.
> >
> >> In this case, though, it looks like this already landed. I see this
> >> patch in Kishon's next branch.
> >>
> >>
> >> >> Fixes: efb05a50c956 ("phy: qcom-qmp: Add support for QMP V3 USB3 PHY")
> >> >> Fixes: ac0d239936bd ("phy: qcom-qmp: Add support for runtime PM")
> >> >> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >> >> ---
> >> >>
> >> >> .../devicetree/bindings/phy/qcom-qmp-phy.txt | 14 ++++++++++++--
> >> >> 1 file changed, 12 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> >> >> index 266a1bb8bb6e..0c7629e88bf3 100644
> >> >> --- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> >> >> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> >> >> @@ -12,7 +12,14 @@ Required properties:
> >> >> "qcom,sdm845-qmp-usb3-phy" for USB3 QMP V3 phy on sdm845,
> >> >> "qcom,sdm845-qmp-usb3-uni-phy" for USB3 QMP V3 UNI phy on sdm845.
> >> >>
> >> >> - - reg: offset and length of register set for PHY's common serdes block.
> >> >> + - reg:
> >> >> + - For "qcom,sdm845-qmp-usb3-phy":
> >> >> + - index 0: address and length of register set for PHY's common serdes
> >> >> + block.
> >> >> + - named register "dp_com" (using reg-names): address and length of the
> >> >> + DP_COM control block.
> >> >
> >> > You need to list reg-names and what are the names for the other 2
> >> > regions?
> >>
> >> Here's the code works. You can tell me how you want this expressed in
> >> the bindings:
> >>
> >> * In all cases the driver maps its main memory range (for the common
> >> serdes block) without specifying any name. This is equivalent to
> >> asking for index 0.
> >>
> >> * For "qcom,sdm845-qmp-usb3-phy" the driver requests a second memory
> >> range by name using the name "dp_com".
> >>
> >> ...basically the driver is inconsistent between using names and
> >> indices and I was trying to document that fact.
> >
> > That's fine as long as the indices are fixed.
> >
> >>
> >> I guess options:
> >>
> >> 1. I could reword this so it's clearer (open to suggestions)
> >>
> >> 2. I could add something to the bindings saying that the first reg
> >> name should be "reg-base" or something. Then the question is whether
> >> we should go to the code and start enforcing that. If we do choose to
> >> enforce it then it's technically breaking compatibility (though I
> >> doubt there is any real dts in the wild). If we don't choose to
> >> enforce it then why did we bother saying what it should be named?
> >
> > I think you need to state index 1 is dp_com (and only for
> > "qcom,sdm845-qmp-usb3-phy") and a name for index 0. 'reg-base' doesn't
> > seem great, but I don't have another suggestion.
>
> ...but why do we bother giving "dp_com" a name if we're saying it has
> to be index 1? It feels like the author of the driver was trying to
> transition from specifying to specifying registers by index to
> specifying them by name, but left the first register specified by
> index for compatibility (or code simplicity?). It seems like the
> whole point of referring to things by name is to _not_ force the index
> number.
No. Specifying the order and indexes is how bindings are done.
"-names" is extra information, not a license to change the rules.
> I'm imagining a future compatible string that adds yet another address
> range. Presumably we'd refer to this by name too. In that case both
> of these would be fine:
>
> reg = <0xa x>, <0xb x>, <0xc x>;
> reg-names = "reg-base", "dp_com", "new_range"
>
> reg = <0xa x>, <0xc x>, <0xb x>;
> reg-names = "reg-base", "new_range", "dp_com"
But why. There is absolutely no need to support both of these.
What you could need to support is this:
reg-names = "reg-base", "new_range";
The order is still fixed, but we have optional entries in the middle.
And that is the case when using -names is helpful. Otherwise, "-names"
is just unnecessary bloat to DT.
> > For the driver, it's not the driver's job to enforce any of this.
>
> Sure, but anything that the driver doesn't enforce people will get
> wrong. In other words if we say that the name of the first register
> ought to be "reg-base" but don't enforce it then someone will later
> come in and say it's stupid that one of the names uses a dash and the
> other an underscore. They'll change "reg-base" and it will work.
> Someone else will decide that "reg-base" is a dumb name and will
> change it to "serdes". It will still work. Now we have a bindings
> that claims "reg-base" and lots of different device trees in practice.
You are arguing against making the binding stricter and then worrying
about the driver getting things wrong. Stricter bindings leave less to
interpretation by the driver and less chance to get things wrong.
Unless "anything goes" and then the binding can never be "wrong".
Again, it is not the kernel's job to validate bindings even though we
have little else right now. That will change soon hopefully.
> If the whole "keep things compatible" is important then what matters
> more is what's happening in practice, not what's should happen in
> theory.
>
> IMO if the driver isn't enforcing that the first field be called
> "reg-base" then it shouldn't be in the bindings doc.
>
>
> BTW: I use the name "reg-base" in my example because that's what the
> register was named in downstream device trees that I found. Even if
> the name isn't terribly appealing, if it's not terribly objectionable
> I'd rather pick something that's closer to what people used in
> practice.
I don't really care. People just make shit up anyway.
>
> ---
>
> How about this?
No.
>
> - reg:
> - For "qcom,sdm845-qmp-usb3-phy" (names mapped by reg-names):
> - "reg-base": address and length of register set for PHY's common serdes
> block. MUST BE THE FIRST RANGE LISTED IN THE LIST (AKA index 0). Note
> that the name of "reg-base" is suggested but not enforced.
> - "dp-com": address and length of the DP_COM control block.
> - For all others (only one range--don't use reg-names)
> - offset and length of register set for PHY's common serdes block.
>
>
>
> -Doug
^ permalink raw reply
* Re: [PATCH v2 13/19] mm/migrate: Use xchg instead of spinlock
From: Peter Zijlstra @ 2018-07-23 14:04 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Ingo Molnar, LKML, Mel Gorman, Rik van Riel, Thomas Gleixner
In-Reply-To: <20180723112032.GH30345@linux.vnet.ibm.com>
On Mon, Jul 23, 2018 at 04:20:32AM -0700, Srikar Dronamraju wrote:
> > If you maybe write that like:
> >
> > if (time_after(jiffies, next_window) &&
> > xchg(&pgdat->numabalancing_migrate_nr_pages, 0UL)) {
> >
> > do {
> > next_window += interval;
> > } while (unlikely(time_after(jiffies, next_window)));
> >
> > WRITE_ONCE(pgdat->numabalancing_migrate_next_window, next_window);
> > }
> >
> > Then you avoid an indent level and line-wrap, resulting imo easier to
> > read code.
> >
>
> Okay will do.
FWIW, that code seems to rely on @nr_pages != 0, otherwise you can have
the xchg fail even though time_after is true.
Probably not a problem, but it is a semantic change vs the spinlock.
Also, the spinlock thing could probably have changed to a trylock and
you'd have seens similar 'gains' I suppose.
Another difference vs the lock+unlock is that you lost the release
ordering. Again, probably not a big deal, but it does make the whole
things a little dodgy.
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] hw/intc/exynos4210_gic: Turn instance_init into realize function
From: Peter Maydell @ 2018-07-23 14:04 UTC (permalink / raw)
To: Thomas Huth; +Cc: QEMU Developers, Igor Mitsyanko, qemu-arm
In-Reply-To: <1532337784-334-1-git-send-email-thuth@redhat.com>
On 23 July 2018 at 10:23, Thomas Huth <thuth@redhat.com> wrote:
> The instance_init function of the "exynos4210.gic" device creates a
> new "arm_gic" device and immediately realizes it with qdev_init_nofail().
> This will leave a lot of object in the QOM tree during introspection of
> the "exynos4210.gic" device, e.g. reproducible by starting QEMU like this:
>
> qemu-system-aarch64 -M none -nodefaults -nographic -monitor stdio
>
> And then by running "info qom-tree" at the HMP monitor, followed by
> "device_add exynos4210.gic,help" and finally checking "info qom-tree"
> again.
>
> Also note that qdev_init_nofail() can exit QEMU in case of errors - and
> this must never happen during an instance_init function, otherwise QEMU
> could terminate unexpectedly during introspection of a device.
>
> Since most of the code that follows the qdev_init_nofail() depends on
> the realized "gicbusdev", the easiest solution to the problem is to
> turn the whole instance_init function into a realize function instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply
* Re: etnaviv oops on imx6dl/2G, works fine on imx6s/512M
From: Michal Vokáč @ 2018-07-23 14:05 UTC (permalink / raw)
To: Lucas Stach, dri-devel; +Cc: etnaviv
In-Reply-To: <1532340795.3163.97.camel@pengutronix.de>
On 23.7.2018 12:13, Lucas Stach wrote:
> Hi Michal,
>
> Am Mittwoch, den 18.07.2018, 14:29 +0200 schrieb Michal Vokáč:
>> Ahoj,
>>
>> I encountered an error in etnaviv driver on one of my two very similar SBCs.
>> They have the same board design but slightly different HW configuration.
>> SW configuration/setup is the same for both. Including kernel configuration.
>>
>> 1. is based on i.MX6S, single core, 512MB RAM, parallel LCD.
>> 2. is based on i.MX6DL, dual core, 2GB RAM, HDMI display.
>>
>> System No.1 boots OK. I can run kmscube and HW acceleration works.
>> System No.2 does not boot until I disable etnaviv driver in the kernel.
>>
>> When digging deeper where the problem might be I found those two threads
>> [1],[2]. None of these apply to me. CMA is enabled and I do not specify
>> fixed CMA regions in DT.
>>
>> I get the same results on v4.18-rc2 and on v4.18-rc5.
>>
>> Relevant parts of boot log:
>>
>> [ 0.000000] Memory: 1984044K/2097152K available (11264K kernel code, 738K rwdata, 2672K rodata, 1024K init, 426K bss, 113108K reserved, 0K cma-reserved, 262144K highmem)
>> [ 0.000000] Virtual kernel memory layout:
>> [ 0.000000] vector : 0xffff0000 - 0xffff1000 ( 4 kB)
>> [ 0.000000] fixmap : 0xffc00000 - 0xfff00000 (3072 kB)
>> [ 0.000000] vmalloc : 0xf0800000 - 0xff800000 ( 240 MB)
>> [ 0.000000] lowmem : 0x80000000 - 0xf0000000 (1792 MB)
>> [ 0.000000] pkmap : 0x7fe00000 - 0x80000000 ( 2 MB)
>> [ 0.000000] .text : 0x(ptrval) - 0x(ptrval) (12256 kB)
>> [ 0.000000] .init : 0x(ptrval) - 0x(ptrval) (1024 kB)
>> [ 0.000000] .data : 0x(ptrval) - 0x(ptrval) ( 739 kB)
>> [ 0.000000] .bss : 0x(ptrval) - 0x(ptrval) ( 427 kB)
>>
>> [... snip ...]
>>
>> [ 8.973561] etnaviv etnaviv: bound 130000.gpu (ops gpu_ops)
>> [ 8.979464] etnaviv etnaviv: bound 134000.gpu (ops gpu_ops)
>> [ 8.985105] etnaviv-gpu 130000.gpu: model: GC880, revision: 5106
>> [ 8.997639] etnaviv-gpu 130000.gpu: command buffer outside valid memory window
>> [ 9.005465] etnaviv-gpu 134000.gpu: model: GC320, revision: 5007
>> [ 9.018325] etnaviv-gpu 134000.gpu: command buffer outside valid memory window
>> [ 9.026767] [drm] Initialized etnaviv 1.2.0 20151214 for etnaviv on minor 0
>> [ 9.035510] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
>> [ 9.042138] [drm] No driver support for vblank timestamp query.
>> [ 9.048363] imx-drm display-subsystem: bound imx-ipuv3-crtc.2 (ops ipu_crtc_ops)
>> [ 9.055948] imx-drm display-subsystem: bound imx-ipuv3-crtc.3 (ops ipu_crtc_ops)
>> [ 9.063409] [drm] Cannot find any crtc or sizes
>> [ 9.068354] [drm] Initialized imx-drm 1.0.0 20120507 for display-subsystem on minor 1
>> [ 9.076283] imx-ipuv3 2400000.ipu: IPUv3H probed
> [...]
>> The system does not make it to the login promt but it seems kernel is running
>> as I see a LED with a heart-beat trigger flashing.
>>
>> Any idea what might be wrong? How to further debug this?
>
> I'll look into why things are crashing in the RPM routines, but the
> root-cause of your issue is that your system doesn't provide any CMA
> memory ("0K cma-reserved" in the log above). This is required for
> etnaviv to work, any working configuration without CMA memory is pure
> luck. You will need to tweak your configuration to always have some CMA
> memory, looking at the imx_v6_v7_defconfig should provide a good
> starting point.
Glad you replied Lucas! You nailed it, I totally overlooked that option.
I am able to boot both systems and run kmscube with cma=64M kernel
command line parameter.
Thank you very much for your time,
Michal
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [Qemu-devel] [PATCH] hw/intc/exynos4210_gic: Turn instance_init into realize function
From: Peter Maydell @ 2018-07-23 14:04 UTC (permalink / raw)
To: Thomas Huth; +Cc: Igor Mitsyanko, qemu-arm, QEMU Developers
In-Reply-To: <1532337784-334-1-git-send-email-thuth@redhat.com>
On 23 July 2018 at 10:23, Thomas Huth <thuth@redhat.com> wrote:
> The instance_init function of the "exynos4210.gic" device creates a
> new "arm_gic" device and immediately realizes it with qdev_init_nofail().
> This will leave a lot of object in the QOM tree during introspection of
> the "exynos4210.gic" device, e.g. reproducible by starting QEMU like this:
>
> qemu-system-aarch64 -M none -nodefaults -nographic -monitor stdio
>
> And then by running "info qom-tree" at the HMP monitor, followed by
> "device_add exynos4210.gic,help" and finally checking "info qom-tree"
> again.
>
> Also note that qdev_init_nofail() can exit QEMU in case of errors - and
> this must never happen during an instance_init function, otherwise QEMU
> could terminate unexpectedly during introspection of a device.
>
> Since most of the code that follows the qdev_init_nofail() depends on
> the realized "gicbusdev", the easiest solution to the problem is to
> turn the whole instance_init function into a realize function instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply
* [PATCH v2 02/15] xfs: use ->t_dfops in log recovery intent processing
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
xlog_finish_defer_ops() processes the deferred operations collected
over the entire intent recovery sequence. We can't xfs_defer_init()
here because the dfops is already populated. Attach it manually and
eliminate the last caller of xfs_defer_finish() that doesn't pass
->t_dfops.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_log_recover.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index cbac943896f4..3289811eb076 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4854,6 +4854,8 @@ xlog_finish_defer_ops(
0, XFS_TRANS_RESERVE, &tp);
if (error)
return error;
+ /* dfops is already populated so assign it manually */
+ tp->t_dfops = dfops;
error = xfs_defer_finish(&tp, dfops);
if (error)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 00/15] xfs: embed dfops in the transaction
From: Brian Foster @ 2018-07-23 13:03 UTC (permalink / raw)
To: linux-xfs
Hi all,
Here is v2 of the series to embed dfops in the transaction. This is
rebased onto Darrick's recently posted[1] xfs_defer_finish() patch to
ensure the associated function returns with a clean transaction. A new
patch is appended to the series to avoid that extra roll when
xfs_defer_finish() is called via the transaction commit path. Otherwise
this has minor updates based on feedback to v1.
Note that, as discussed, I'm working on some changes to further condense
xfs_defer_ops with the transaction structure (such as automatic dfops
relogging). Those changes will be part of a separate series.
Thoughts, reviews, flames appreciated.
Brian
[1] https://marc.info/?l=linux-xfs&m=153192905211637&w=2
v2:
- Rebase on djwong xfs_defer_finish() final tx roll patch.
- Added patch to skip extra roll on dfops finish via tx commit.
- Split up assert in xfs_reflink_cancel_cow_blocks().
- Use memset()/memcpy() rather than open-coded reset/copy.
- Relocated struct xfs_defer_ops and xfs_defer_cancel() defs to avoid
header file include mess.
v1: https://marc.info/?l=linux-xfs&m=153200816200883&w=2
Brian Foster (15):
xfs: pull up dfops from xfs_itruncate_extents()
xfs: use ->t_dfops in log recovery intent processing
xfs: fix transaction leak on remote attr set/remove failure
xfs: make deferred processing safe for embedded dfops
xfs: remove unused deferred ops committed field
xfs: reset dfops to initial state after finish
xfs: pack holes in xfs_defer_ops and xfs_trans
xfs: support embedded dfops in transaction
xfs: use internal dfops in cow blocks cancel
xfs: use internal dfops in attr code
xfs: use internal dfops during [b|c]ui recovery
xfs: remove all boilerplate defer init/finish code
xfs: remove unnecessary dfops init calls in xattr code
xfs: drop unnecessary xfs_defer_finish() dfops parameter
xfs: bypass final dfops roll in trans commit path
fs/xfs/libxfs/xfs_attr.c | 49 ++++++-----------
fs/xfs/libxfs/xfs_attr_remote.c | 13 ++---
fs/xfs/libxfs/xfs_bmap.c | 16 +-----
fs/xfs/libxfs/xfs_defer.c | 97 +++++++++++++++++++++++----------
fs/xfs/libxfs/xfs_defer.h | 21 ++-----
fs/xfs/libxfs/xfs_refcount.c | 10 +---
fs/xfs/xfs_attr_inactive.c | 1 +
fs/xfs/xfs_bmap_item.c | 21 +++----
fs/xfs/xfs_bmap_util.c | 45 +++------------
fs/xfs/xfs_dquot.c | 8 +--
fs/xfs/xfs_inode.c | 87 ++++++-----------------------
fs/xfs/xfs_iomap.c | 26 +--------
fs/xfs/xfs_iops.c | 1 +
fs/xfs/xfs_log_recover.c | 12 +---
fs/xfs/xfs_qm_syscalls.c | 1 +
fs/xfs/xfs_refcount_item.c | 30 +++++-----
fs/xfs/xfs_reflink.c | 51 ++++++-----------
fs/xfs/xfs_rtalloc.c | 9 +--
fs/xfs/xfs_symlink.c | 38 +++----------
fs/xfs/xfs_trace.h | 8 +--
fs/xfs/xfs_trans.c | 37 +++++++++++--
fs/xfs/xfs_trans.h | 22 +++++++-
22 files changed, 240 insertions(+), 363 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v2 03/15] xfs: fix transaction leak on remote attr set/remove failure
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
The xattr remote value set/remove handlers both clear args.trans in
the error path without having cancelled the transaction. This leaks
the transaction, causes warnings around returning to userspace with
locks held and leads to system lockups or other general problems.
The higher level xfs_attr_[set|remove]() functions already detect
and cancel args.trans when set in the error path. Drop the NULL
assignments from the rmtval handlers and allow the callers to clean
up the transaction correctly.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_attr_remote.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 7841e6255129..829ab20f0cd7 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -558,7 +558,6 @@ xfs_attr_rmtval_set(
return 0;
out_defer_cancel:
xfs_defer_cancel(args->trans->t_dfops);
- args->trans = NULL;
return error;
}
@@ -646,6 +645,5 @@ xfs_attr_rmtval_remove(
return 0;
out_defer_cancel:
xfs_defer_cancel(args->trans->t_dfops);
- args->trans = NULL;
return error;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 05/15] xfs: remove unused deferred ops committed field
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
dop_committed is set when deferred item processing rolls the
transaction at least once, but is only ever accessed in tracepoints.
The transaction roll/commit events are already available via
independent tracepoints, so remove the otherwise unused field.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_defer.c | 1 -
fs/xfs/libxfs/xfs_defer.h | 1 -
fs/xfs/xfs_trace.h | 8 ++------
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index ee734a8b3fa9..0df09c094e42 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -249,7 +249,6 @@ xfs_defer_trans_roll(
xfs_defer_trans_abort(*tp, dop, error);
return error;
}
- dop->dop_committed = true;
/* Rejoin the joined inodes. */
for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++)
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index c17c9deda995..58c979c9f3fa 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -49,7 +49,6 @@ enum xfs_defer_ops_type {
#define XFS_DEFER_OPS_NR_BUFS 2 /* join up to two buffers */
struct xfs_defer_ops {
- bool dop_committed; /* did any trans commit? */
bool dop_low; /* alloc in low mode */
struct list_head dop_intake; /* unlogged pending work */
struct list_head dop_pending; /* logged pending work */
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index b668fc127aa7..cc6995cfce66 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -2229,14 +2229,12 @@ DECLARE_EVENT_CLASS(xfs_defer_class,
TP_fast_assign(
__entry->dev = mp ? mp->m_super->s_dev : 0;
__entry->dop = dop;
- __entry->committed = dop->dop_committed;
__entry->low = dop->dop_low;
__entry->caller_ip = caller_ip;
),
- TP_printk("dev %d:%d ops %p committed %d low %d, caller %pS",
+ TP_printk("dev %d:%d ops %p low %d, caller %pS",
MAJOR(__entry->dev), MINOR(__entry->dev),
__entry->dop,
- __entry->committed,
__entry->low,
(char *)__entry->caller_ip)
)
@@ -2259,14 +2257,12 @@ DECLARE_EVENT_CLASS(xfs_defer_error_class,
TP_fast_assign(
__entry->dev = mp ? mp->m_super->s_dev : 0;
__entry->dop = dop;
- __entry->committed = dop->dop_committed;
__entry->low = dop->dop_low;
__entry->error = error;
),
- TP_printk("dev %d:%d ops %p committed %d low %d err %d",
+ TP_printk("dev %d:%d ops %p low %d err %d",
MAJOR(__entry->dev), MINOR(__entry->dev),
__entry->dop,
- __entry->committed,
__entry->low,
__entry->error)
)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 04/15] xfs: make deferred processing safe for embedded dfops
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
xfs_defer_finish() has a couple quirks that are not safe with
respect to the upcoming internal dfops functionality. First,
xfs_defer_finish() attaches the passed in dfops structure to
->t_dfops and caches and restores the original value. Second, it
continues to use the initial dfops reference before and after the
transaction roll.
These behaviors assume that dop is an independent memory allocation
from the transaction itself, which may not always be true once
transactions begin to use an embedded dfops structure. In the latter
model, dfops processing creates a new xfs_defer_ops structure with
each transaction and the associated state is migrated across to the
new transaction.
Fix up xfs_defer_finish() to handle the possibility of the current
dfops changing after a transaction roll. Since ->t_dfops is used
unconditionally in this path, it is no longer necessary to
attach/restore ->t_dfops and pass it explicitly down to
xfs_defer_trans_roll(). Update dop in the latter function and the
caller to ensure that it always refers to the current dfops
structure.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_defer.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index c4b0eaeb5190..ee734a8b3fa9 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -225,9 +225,9 @@ xfs_defer_trans_abort(
/* Roll a transaction so we can do some deferred op processing. */
STATIC int
xfs_defer_trans_roll(
- struct xfs_trans **tp,
- struct xfs_defer_ops *dop)
+ struct xfs_trans **tp)
{
+ struct xfs_defer_ops *dop = (*tp)->t_dfops;
int i;
int error;
@@ -243,6 +243,7 @@ xfs_defer_trans_roll(
/* Roll the transaction. */
error = xfs_trans_roll(tp);
+ dop = (*tp)->t_dfops;
if (error) {
trace_xfs_defer_trans_roll_error((*tp)->t_mountp, dop, error);
xfs_defer_trans_abort(*tp, dop, error);
@@ -338,31 +339,25 @@ xfs_defer_finish(
void *state;
int error = 0;
void (*cleanup_fn)(struct xfs_trans *, void *, int);
- struct xfs_defer_ops *orig_dop;
ASSERT((*tp)->t_flags & XFS_TRANS_PERM_LOG_RES);
+ ASSERT((*tp)->t_dfops == dop);
trace_xfs_defer_finish((*tp)->t_mountp, dop, _RET_IP_);
- /*
- * Attach dfops to the transaction during deferred ops processing. This
- * explicitly causes calls into the allocator to defer AGFL block frees.
- * Note that this code can go away once all dfops users attach to the
- * associated tp.
- */
- ASSERT(!(*tp)->t_dfops || ((*tp)->t_dfops == dop));
- orig_dop = (*tp)->t_dfops;
- (*tp)->t_dfops = dop;
-
/* Until we run out of pending work to finish... */
while (xfs_defer_has_unfinished_work(dop)) {
/* Log intents for work items sitting in the intake. */
xfs_defer_intake_work(*tp, dop);
- /* Roll the transaction. */
- error = xfs_defer_trans_roll(tp, dop);
+ /*
+ * Roll the transaction and update dop in case dfops was
+ * embedded in the transaction.
+ */
+ error = xfs_defer_trans_roll(tp);
if (error)
goto out;
+ dop = (*tp)->t_dfops;
/* Log an intent-done item for the first pending item. */
dfp = list_first_entry(&dop->dop_pending,
@@ -428,10 +423,11 @@ xfs_defer_finish(
* Roll the transaction once more to avoid returning to the caller
* with a dirty transaction.
*/
- if ((*tp)->t_flags & XFS_TRANS_DIRTY)
- error = xfs_defer_trans_roll(tp, dop);
+ if ((*tp)->t_flags & XFS_TRANS_DIRTY) {
+ error = xfs_defer_trans_roll(tp);
+ dop = (*tp)->t_dfops;
+ }
out:
- (*tp)->t_dfops = orig_dop;
if (error)
trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
else
--
2.17.1
^ permalink raw reply related
* [PATCH v2 10/15] xfs: use internal dfops in attr code
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
Remove the unnecessary on-stack dfops structure and use the internal
transaction dfops instead. The lower level xattr code already
appropriately accesses ->t_dfops throughout.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 927d4c968f9a..66a22c80a0db 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -202,7 +202,6 @@ xfs_attr_set(
struct xfs_mount *mp = dp->i_mount;
struct xfs_buf *leaf_bp = NULL;
struct xfs_da_args args;
- struct xfs_defer_ops dfops;
struct xfs_trans_res tres;
int rsvd = (flags & ATTR_ROOT) != 0;
int error, err2, local;
@@ -251,7 +250,6 @@ xfs_attr_set(
rsvd ? XFS_TRANS_RESERVE : 0, &args.trans);
if (error)
return error;
- xfs_defer_init(args.trans, &dfops);
xfs_ilock(dp, XFS_ILOCK_EXCL);
error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,
@@ -315,18 +313,18 @@ xfs_attr_set(
*/
error = xfs_attr_shortform_to_leaf(&args, &leaf_bp);
if (error)
- goto out_defer_cancel;
+ goto out;
/*
* Prevent the leaf buffer from being unlocked so that a
* concurrent AIL push cannot grab the half-baked leaf
* buffer and run into problems with the write verifier.
*/
xfs_trans_bhold(args.trans, leaf_bp);
- xfs_defer_bjoin(&dfops, leaf_bp);
- xfs_defer_ijoin(&dfops, dp);
- error = xfs_defer_finish(&args.trans, &dfops);
+ xfs_defer_bjoin(args.trans->t_dfops, leaf_bp);
+ xfs_defer_ijoin(args.trans->t_dfops, dp);
+ error = xfs_defer_finish(&args.trans, args.trans->t_dfops);
if (error)
- goto out_defer_cancel;
+ goto out;
/*
* Commit the leaf transformation. We'll need another (linked)
@@ -366,8 +364,6 @@ xfs_attr_set(
return error;
-out_defer_cancel:
- xfs_defer_cancel(&dfops);
out:
if (leaf_bp)
xfs_trans_brelse(args.trans, leaf_bp);
@@ -389,7 +385,6 @@ xfs_attr_remove(
{
struct xfs_mount *mp = dp->i_mount;
struct xfs_da_args args;
- struct xfs_defer_ops dfops;
int error;
XFS_STATS_INC(mp, xs_attr_remove);
@@ -422,7 +417,6 @@ xfs_attr_remove(
&args.trans);
if (error)
return error;
- xfs_defer_init(args.trans, &dfops);
xfs_ilock(dp, XFS_ILOCK_EXCL);
/*
--
2.17.1
^ permalink raw reply related
* [PATCH v2 07/15] xfs: pack holes in xfs_defer_ops and xfs_trans
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
Both structures have holes due to member alignment. Move dop_low to
the end of xfs_defer ops to sanitize the cache line alignment and
move t_flags to save 8 bytes in xfs_trans.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_defer.h | 3 ++-
fs/xfs/xfs_trans.h | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 58c979c9f3fa..8f58f217fdff 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -49,13 +49,14 @@ enum xfs_defer_ops_type {
#define XFS_DEFER_OPS_NR_BUFS 2 /* join up to two buffers */
struct xfs_defer_ops {
- bool dop_low; /* alloc in low mode */
struct list_head dop_intake; /* unlogged pending work */
struct list_head dop_pending; /* logged pending work */
/* relog these with each roll */
struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES];
struct xfs_buf *dop_bufs[XFS_DEFER_OPS_NR_BUFS];
+
+ bool dop_low; /* alloc in low mode */
};
void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 37fdacc690c7..6f857af61455 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -102,12 +102,12 @@ typedef struct xfs_trans {
unsigned int t_blk_res_used; /* # of resvd blocks used */
unsigned int t_rtx_res; /* # of rt extents resvd */
unsigned int t_rtx_res_used; /* # of resvd rt extents used */
+ unsigned int t_flags; /* misc flags */
xfs_fsblock_t t_firstblock; /* first block allocated */
struct xlog_ticket *t_ticket; /* log mgr ticket */
struct xfs_mount *t_mountp; /* ptr to fs mount struct */
struct xfs_dquot_acct *t_dqinfo; /* acctg info for dquots */
struct xfs_defer_ops *t_dfops; /* dfops reference */
- unsigned int t_flags; /* misc flags */
int64_t t_icount_delta; /* superblock icount change */
int64_t t_ifree_delta; /* superblock ifree change */
int64_t t_fdblocks_delta; /* superblock fdblocks chg */
--
2.17.1
^ permalink raw reply related
* [PATCH v2 01/15] xfs: pull up dfops from xfs_itruncate_extents()
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
xfs_itruncate_extents[_flags]() uses a local dfops with a
transaction provided by the caller. It uses hacky ->t_dfops
replacement logic to avoid stomping over an already populated
->t_dfops.
The latter never occurs for current callers and the logic itself is
not really appropriate. Clean this up by updating all callers to
initialize a dfops and to use that down in xfs_itruncate_extents().
This more closely resembles the upcoming logic where dfops will be
embedded within the transaction. We can also replace the
xfs_defer_init() in the xfs_itruncate_extents_flags() loop with an
assert. Both dfops and firstblock should be in a valid state
after xfs_defer_finish() and the inode joined to the dfops is fixed
throughout the loop.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_attr_inactive.c | 3 +++
fs/xfs/xfs_bmap_util.c | 2 ++
fs/xfs/xfs_inode.c | 8 +++-----
fs/xfs/xfs_iops.c | 3 +++
fs/xfs/xfs_qm_syscalls.c | 3 +++
5 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index 7ce10055f275..d3055972d3a6 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -26,6 +26,7 @@
#include "xfs_quota.h"
#include "xfs_trace.h"
#include "xfs_dir2.h"
+#include "xfs_defer.h"
/*
* Look at all the extents for this logical region,
@@ -381,6 +382,7 @@ xfs_attr_inactive(
{
struct xfs_trans *trans;
struct xfs_mount *mp;
+ struct xfs_defer_ops dfops;
int lock_mode = XFS_ILOCK_SHARED;
int error = 0;
@@ -397,6 +399,7 @@ xfs_attr_inactive(
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_attrinval, 0, 0, 0, &trans);
if (error)
goto out_destroy_fork;
+ xfs_defer_init(trans, &dfops);
lock_mode = XFS_ILOCK_EXCL;
xfs_ilock(dp, lock_mode);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index d3a314fd721f..ee9481b142f0 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -792,6 +792,7 @@ xfs_free_eofblocks(
int nimaps;
struct xfs_bmbt_irec imap;
struct xfs_mount *mp = ip->i_mount;
+ struct xfs_defer_ops dfops;
/*
* Figure out if there are any blocks beyond the end
@@ -831,6 +832,7 @@ xfs_free_eofblocks(
ASSERT(XFS_FORCED_SHUTDOWN(mp));
return error;
}
+ xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 7b2694d3901a..7d7d7e95fa17 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1541,8 +1541,6 @@ xfs_itruncate_extents_flags(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp = *tpp;
- struct xfs_defer_ops *odfops = tp->t_dfops;
- struct xfs_defer_ops dfops;
xfs_fileoff_t first_unmap_block;
xfs_fileoff_t last_block;
xfs_filblks_t unmap_len;
@@ -1579,7 +1577,7 @@ xfs_itruncate_extents_flags(
ASSERT(first_unmap_block < last_block);
unmap_len = last_block - first_unmap_block + 1;
while (!done) {
- xfs_defer_init(tp, &dfops);
+ ASSERT(tp->t_firstblock == NULLFSBLOCK);
error = xfs_bunmapi(tp, ip, first_unmap_block, unmap_len, flags,
XFS_ITRUNC_MAX_EXTENTS, &done);
if (error)
@@ -1618,8 +1616,6 @@ xfs_itruncate_extents_flags(
trace_xfs_itruncate_extents_end(ip, new_size);
out:
- /* ->t_dfops points to local stack, don't leak it! */
- tp->t_dfops = odfops;
*tpp = tp;
return error;
out_bmap_cancel:
@@ -1723,6 +1719,7 @@ xfs_inactive_truncate(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
+ struct xfs_defer_ops dfops;
int error;
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
@@ -1730,6 +1727,7 @@ xfs_inactive_truncate(
ASSERT(XFS_FORCED_SHUTDOWN(mp));
return error;
}
+ xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 0fa29f39d658..704b57a8b99e 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -26,6 +26,7 @@
#include "xfs_dir2.h"
#include "xfs_trans_space.h"
#include "xfs_iomap.h"
+#include "xfs_defer.h"
#include <linux/capability.h>
#include <linux/xattr.h>
@@ -812,6 +813,7 @@ xfs_setattr_size(
struct inode *inode = VFS_I(ip);
xfs_off_t oldsize, newsize;
struct xfs_trans *tp;
+ struct xfs_defer_ops dfops;
int error;
uint lock_flags = 0;
bool did_zeroing = false;
@@ -915,6 +917,7 @@ xfs_setattr_size(
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
if (error)
return error;
+ xfs_defer_init(tp, &dfops);
lock_flags |= XFS_ILOCK_EXCL;
xfs_ilock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index abc8a21e3a82..df0783303887 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -22,6 +22,7 @@
#include "xfs_qm.h"
#include "xfs_trace.h"
#include "xfs_icache.h"
+#include "xfs_defer.h"
STATIC int xfs_qm_log_quotaoff(xfs_mount_t *, xfs_qoff_logitem_t **, uint);
STATIC int xfs_qm_log_quotaoff_end(xfs_mount_t *, xfs_qoff_logitem_t *,
@@ -213,6 +214,7 @@ xfs_qm_scall_trunc_qfile(
{
struct xfs_inode *ip;
struct xfs_trans *tp;
+ struct xfs_defer_ops dfops;
int error;
if (ino == NULLFSINO)
@@ -229,6 +231,7 @@ xfs_qm_scall_trunc_qfile(
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
goto out_put;
}
+ xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 09/15] xfs: use internal dfops in cow blocks cancel
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
All callers either explicitly initialize a dfops or pass a
transaction with an internal dfops. Drop the hacky old dfops
replacement logic and use the one associated with the transaction.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_reflink.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 3143889097f1..c3f2a0d3e6f5 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -483,8 +483,6 @@ xfs_reflink_cancel_cow_blocks(
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
struct xfs_bmbt_irec got, del;
struct xfs_iext_cursor icur;
- struct xfs_defer_ops dfops;
- struct xfs_defer_ops *odfops = (*tpp)->t_dfops;
int error = 0;
if (!xfs_is_reflink_inode(ip))
@@ -511,7 +509,8 @@ xfs_reflink_cancel_cow_blocks(
if (error)
break;
} else if (del.br_state == XFS_EXT_UNWRITTEN || cancel_real) {
- xfs_defer_init(*tpp, &dfops);
+ ASSERT((*tpp)->t_dfops);
+ ASSERT((*tpp)->t_firstblock == NULLFSBLOCK);
/* Free the CoW orphan record. */
error = xfs_refcount_free_cow_extent(ip->i_mount,
@@ -553,7 +552,6 @@ xfs_reflink_cancel_cow_blocks(
/* clear tag if cow fork is emptied */
if (!ifp->if_bytes)
xfs_inode_clear_cowblocks_tag(ip);
- (*tpp)->t_dfops = odfops;
return error;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 11/15] xfs: use internal dfops during [b|c]ui recovery
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
bmap and refcount intent processing associates a dfops from the
caller with a local transaction to collect all deferred items for
post-processing. Use the internal dfops in both of these functions
and move the deferred items to the parent dfops before the
transaction commits.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_bmap_item.c | 21 +++++++++++----------
fs/xfs/xfs_log_recover.c | 6 +++---
fs/xfs/xfs_refcount_item.c | 30 ++++++++++++++++--------------
3 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 478bfc798861..bc5eb2e0ab0c 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -441,7 +441,12 @@ xfs_bui_recover(
XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
if (error)
return error;
- tp->t_dfops = dfops;
+ /*
+ * Recovery stashes all deferred ops during intent processing and
+ * finishes them on completion. Transfer current dfops state to this
+ * transaction and transfer the result back before we return.
+ */
+ xfs_defer_move(tp->t_dfops, dfops);
budp = xfs_trans_get_bud(tp, buip);
/* Grab the inode. */
@@ -470,7 +475,7 @@ xfs_bui_recover(
xfs_trans_ijoin(tp, ip, 0);
count = bmap->me_len;
- error = xfs_trans_log_finish_bmap_update(tp, budp, dfops, type,
+ error = xfs_trans_log_finish_bmap_update(tp, budp, tp->t_dfops, type,
ip, whichfork, bmap->me_startoff,
bmap->me_startblock, &count, state);
if (error)
@@ -482,18 +487,14 @@ xfs_bui_recover(
irec.br_blockcount = count;
irec.br_startoff = bmap->me_startoff;
irec.br_state = state;
- error = xfs_bmap_unmap_extent(tp->t_mountp, dfops, ip, &irec);
+ error = xfs_bmap_unmap_extent(tp->t_mountp, tp->t_dfops, ip,
+ &irec);
if (error)
goto err_inode;
}
set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
- /*
- * Recovery finishes all deferred ops once intent processing is
- * complete. Reset the trans reference because commit expects a finished
- * dfops or none at all.
- */
- tp->t_dfops = NULL;
+ xfs_defer_move(dfops, tp->t_dfops);
error = xfs_trans_commit(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
IRELE(ip);
@@ -501,7 +502,7 @@ xfs_bui_recover(
return error;
err_inode:
- tp->t_dfops = NULL;
+ xfs_defer_move(dfops, tp->t_dfops);
xfs_trans_cancel(tp);
if (ip) {
xfs_iunlock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 3289811eb076..958e9b96dc6a 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4854,10 +4854,10 @@ xlog_finish_defer_ops(
0, XFS_TRANS_RESERVE, &tp);
if (error)
return error;
- /* dfops is already populated so assign it manually */
- tp->t_dfops = dfops;
+ /* transfer all collected dfops to this transaction */
+ xfs_defer_move(tp->t_dfops, dfops);
- error = xfs_defer_finish(&tp, dfops);
+ error = xfs_defer_finish(&tp, tp->t_dfops);
if (error)
goto out_cancel;
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 2064c689bc72..d3582a06626f 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -452,7 +452,12 @@ xfs_cui_recover(
mp->m_refc_maxlevels * 2, 0, XFS_TRANS_RESERVE, &tp);
if (error)
return error;
- tp->t_dfops = dfops;
+ /*
+ * Recovery stashes all deferred ops during intent processing and
+ * finishes them on completion. Transfer current dfops state to this
+ * transaction and transfer the result back before we return.
+ */
+ xfs_defer_move(tp->t_dfops, dfops);
cudp = xfs_trans_get_cud(tp, cuip);
for (i = 0; i < cuip->cui_format.cui_nextents; i++) {
@@ -474,8 +479,8 @@ xfs_cui_recover(
new_len = refc->pe_len;
} else
error = xfs_trans_log_finish_refcount_update(tp, cudp,
- dfops, type, refc->pe_startblock, refc->pe_len,
- &new_fsb, &new_len, &rcur);
+ tp->t_dfops, type, refc->pe_startblock,
+ refc->pe_len, &new_fsb, &new_len, &rcur);
if (error)
goto abort_error;
@@ -486,21 +491,23 @@ xfs_cui_recover(
switch (type) {
case XFS_REFCOUNT_INCREASE:
error = xfs_refcount_increase_extent(
- tp->t_mountp, dfops, &irec);
+ tp->t_mountp, tp->t_dfops,
+ &irec);
break;
case XFS_REFCOUNT_DECREASE:
error = xfs_refcount_decrease_extent(
- tp->t_mountp, dfops, &irec);
+ tp->t_mountp, tp->t_dfops,
+ &irec);
break;
case XFS_REFCOUNT_ALLOC_COW:
error = xfs_refcount_alloc_cow_extent(
- tp->t_mountp, dfops,
+ tp->t_mountp, tp->t_dfops,
irec.br_startblock,
irec.br_blockcount);
break;
case XFS_REFCOUNT_FREE_COW:
error = xfs_refcount_free_cow_extent(
- tp->t_mountp, dfops,
+ tp->t_mountp, tp->t_dfops,
irec.br_startblock,
irec.br_blockcount);
break;
@@ -515,18 +522,13 @@ xfs_cui_recover(
xfs_refcount_finish_one_cleanup(tp, rcur, error);
set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
- /*
- * Recovery finishes all deferred ops once intent processing is
- * complete. Reset the trans reference because commit expects a finished
- * dfops or none at all.
- */
- tp->t_dfops = NULL;
+ xfs_defer_move(dfops, tp->t_dfops);
error = xfs_trans_commit(tp);
return error;
abort_error:
xfs_refcount_finish_one_cleanup(tp, rcur, error);
- tp->t_dfops = NULL;
+ xfs_defer_move(dfops, tp->t_dfops);
xfs_trans_cancel(tp);
return error;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 06/15] xfs: reset dfops to initial state after finish
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
xfs_defer_init() is currently used in two particular situations. The
first and most obvious case is raw initialization of an
xfs_defer_ops struct. The other case is partial reinit of
xfs_defer_ops on reuse due to iteration.
Most instances of the first case will be replaced by a single init
of a dfops embedded in the transaction. Init calls are still
technically required for the second case because the dfops may have
low space mode enabled or have joined items that need to be reset
before the dfops should be reused.
Since the current dfops usage expects either a final transaction
commit after xfs_defer_finish() or xfs_defer_init() if dfops is to
be reused, we can shift some of the init logic into
xfs_defer_finish() such that the latter returns with a reinitialized
dfops. This eliminates the second dependency noted above such that a
dfops is immediately ready for reuse after an xfs_defer_finish()
without the need to change any calling code.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_defer.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 0df09c094e42..23f2a52b088e 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -319,6 +319,19 @@ xfs_defer_bjoin(
return -EFSCORRUPTED;
}
+/*
+ * Reset an already used dfops after finish.
+ */
+static void
+xfs_defer_reset(
+ struct xfs_defer_ops *dop)
+{
+ ASSERT(!xfs_defer_has_unfinished_work(dop));
+ dop->dop_low = false;
+ memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes));
+ memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs));
+}
+
/*
* Finish all the pending work. This involves logging intent items for
* any work items that wandered in since the last transaction roll (if
@@ -427,10 +440,13 @@ xfs_defer_finish(
dop = (*tp)->t_dfops;
}
out:
- if (error)
+ if (error) {
trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
- else
+ } else {
trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_);
+ xfs_defer_reset(dop);
+ }
+
return error;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 14/15] xfs: drop unnecessary xfs_defer_finish() dfops parameter
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
Every caller of xfs_defer_finish() now passes the transaction and
its associated ->t_dfops. The xfs_defer_ops parameter is therefore
no longer necessary and can be removed.
Since most xfs_defer_finish() callers also have to consider
xfs_defer_cancel() on error, update the latter to also receive the
transaction for consistency. The log recovery code contains an
outlier case that cancels a dfops directly without an available
transaction. Retain an internal wrapper to support this outlier case
for the time being.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 27 +++++++++++++--------------
fs/xfs/libxfs/xfs_attr_remote.c | 8 ++++----
fs/xfs/libxfs/xfs_defer.c | 7 +++----
fs/xfs/libxfs/xfs_defer.h | 4 ++--
fs/xfs/xfs_bmap_util.c | 4 ++--
fs/xfs/xfs_dquot.c | 4 ++--
fs/xfs/xfs_inode.c | 4 ++--
fs/xfs/xfs_log_recover.c | 2 +-
fs/xfs/xfs_reflink.c | 8 ++++----
fs/xfs/xfs_trans.c | 13 ++++++++++---
fs/xfs/xfs_trans.h | 3 +++
11 files changed, 46 insertions(+), 38 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 3e98f0af389c..3deb5cdadf08 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -322,7 +322,7 @@ xfs_attr_set(
xfs_trans_bhold(args.trans, leaf_bp);
xfs_defer_bjoin(args.trans->t_dfops, leaf_bp);
xfs_defer_ijoin(args.trans->t_dfops, dp);
- error = xfs_defer_finish(&args.trans, args.trans->t_dfops);
+ error = xfs_defer_finish(&args.trans);
if (error)
goto out;
@@ -591,7 +591,7 @@ xfs_attr_leaf_addname(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
@@ -680,7 +680,7 @@ xfs_attr_leaf_addname(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
}
@@ -698,7 +698,7 @@ xfs_attr_leaf_addname(
}
return error;
out_defer_cancel:
- xfs_defer_cancel(args->trans->t_dfops);
+ xfs_defer_cancel(args->trans);
return error;
}
@@ -744,13 +744,13 @@ xfs_attr_leaf_removename(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
}
return 0;
out_defer_cancel:
- xfs_defer_cancel(args->trans->t_dfops);
+ xfs_defer_cancel(args->trans);
return error;
}
@@ -871,8 +871,7 @@ xfs_attr_node_addname(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans,
- args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
@@ -897,7 +896,7 @@ xfs_attr_node_addname(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
} else {
@@ -994,7 +993,7 @@ xfs_attr_node_addname(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
}
@@ -1023,7 +1022,7 @@ xfs_attr_node_addname(
return error;
return retval;
out_defer_cancel:
- xfs_defer_cancel(args->trans->t_dfops);
+ xfs_defer_cancel(args->trans);
goto out;
}
@@ -1118,7 +1117,7 @@ xfs_attr_node_removename(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
/*
@@ -1150,7 +1149,7 @@ xfs_attr_node_removename(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
} else
@@ -1162,7 +1161,7 @@ xfs_attr_node_removename(
xfs_da_state_free(state);
return error;
out_defer_cancel:
- xfs_defer_cancel(args->trans->t_dfops);
+ xfs_defer_cancel(args->trans);
goto out;
}
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 0fbfb740949e..77ca38586913 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -487,7 +487,7 @@ xfs_attr_rmtval_set(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
@@ -555,7 +555,7 @@ xfs_attr_rmtval_set(
ASSERT(valuelen == 0);
return 0;
out_defer_cancel:
- xfs_defer_cancel(args->trans->t_dfops);
+ xfs_defer_cancel(args->trans);
return error;
}
@@ -628,7 +628,7 @@ xfs_attr_rmtval_remove(
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, args->dp);
- error = xfs_defer_finish(&args->trans, args->trans->t_dfops);
+ error = xfs_defer_finish(&args->trans);
if (error)
goto out_defer_cancel;
@@ -641,6 +641,6 @@ xfs_attr_rmtval_remove(
}
return 0;
out_defer_cancel:
- xfs_defer_cancel(args->trans->t_dfops);
+ xfs_defer_cancel(args->trans);
return error;
}
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index b63cc9e730da..cbee0a86c978 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -342,9 +342,9 @@ xfs_defer_reset(
*/
int
xfs_defer_finish(
- struct xfs_trans **tp,
- struct xfs_defer_ops *dop)
+ struct xfs_trans **tp)
{
+ struct xfs_defer_ops *dop = (*tp)->t_dfops;
struct xfs_defer_pending *dfp;
struct list_head *li;
struct list_head *n;
@@ -353,7 +353,6 @@ xfs_defer_finish(
void (*cleanup_fn)(struct xfs_trans *, void *, int);
ASSERT((*tp)->t_flags & XFS_TRANS_PERM_LOG_RES);
- ASSERT((*tp)->t_dfops == dop);
trace_xfs_defer_finish((*tp)->t_mountp, dop, _RET_IP_);
@@ -454,7 +453,7 @@ xfs_defer_finish(
* Free up any items left in the list.
*/
void
-xfs_defer_cancel(
+__xfs_defer_cancel(
struct xfs_defer_ops *dop)
{
struct xfs_defer_pending *dfp;
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 35507ca9a148..56f927803940 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -48,8 +48,8 @@ enum xfs_defer_ops_type {
void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
struct list_head *h);
-int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop);
-void xfs_defer_cancel(struct xfs_defer_ops *dop);
+int xfs_defer_finish(struct xfs_trans **tp);
+void __xfs_defer_cancel(struct xfs_defer_ops *dop);
void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
int xfs_defer_ijoin(struct xfs_defer_ops *dop, struct xfs_inode *ip);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 6b0f31402a81..7b40e9fef381 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1624,7 +1624,7 @@ xfs_swap_extent_rmap(
goto out_defer;
xfs_defer_ijoin(tp->t_dfops, ip);
- error = xfs_defer_finish(tpp, tp->t_dfops);
+ error = xfs_defer_finish(tpp);
tp = *tpp;
if (error)
goto out_defer;
@@ -1645,7 +1645,7 @@ xfs_swap_extent_rmap(
return 0;
out_defer:
- xfs_defer_cancel(tp->t_dfops);
+ xfs_defer_cancel(tp);
out:
trace_xfs_swap_extent_rmap_error(ip, error, _RET_IP_);
tip->i_d.di_flags2 = tip_flags2;
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index a57d5e8c3118..da5c55cec966 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -368,7 +368,7 @@ xfs_dquot_disk_alloc(
xfs_trans_brelse(tp, bp);
goto error1;
}
- error = xfs_defer_finish(tpp, tp->t_dfops);
+ error = xfs_defer_finish(tpp);
tp = *tpp;
if (error) {
xfs_buf_relse(bp);
@@ -378,7 +378,7 @@ xfs_dquot_disk_alloc(
return 0;
error1:
- xfs_defer_cancel(tp->t_dfops);
+ xfs_defer_cancel(tp);
error0:
return error;
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c47183a2f167..0e4bd559a6a7 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1570,7 +1570,7 @@ xfs_itruncate_extents_flags(
* reservation and commit the old transaction.
*/
xfs_defer_ijoin(tp->t_dfops, ip);
- error = xfs_defer_finish(&tp, tp->t_dfops);
+ error = xfs_defer_finish(&tp);
if (error)
goto out_bmap_cancel;
@@ -1606,7 +1606,7 @@ xfs_itruncate_extents_flags(
* the transaction can be properly aborted. We just need to make sure
* we're not holding any resources that we were not when we came in.
*/
- xfs_defer_cancel(tp->t_dfops);
+ xfs_defer_cancel(tp);
goto out;
}
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 265e1f561157..94908a4019e1 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4946,7 +4946,7 @@ xlog_recover_process_intents(
xfs_trans_ail_cursor_done(&cur);
spin_unlock(&ailp->ail_lock);
if (error)
- xfs_defer_cancel(&dfops);
+ __xfs_defer_cancel(&dfops);
else
error = xlog_finish_defer_ops(log->l_mp, &dfops);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 865ba615dd76..cddde219630a 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -518,9 +518,9 @@ xfs_reflink_cancel_cow_blocks(
/* Roll the transaction */
xfs_defer_ijoin((*tpp)->t_dfops, ip);
- error = xfs_defer_finish(tpp, (*tpp)->t_dfops);
+ error = xfs_defer_finish(tpp);
if (error) {
- xfs_defer_cancel((*tpp)->t_dfops);
+ xfs_defer_cancel(*tpp);
break;
}
@@ -716,7 +716,7 @@ xfs_reflink_end_cow(
xfs_bmap_del_extent_cow(ip, &icur, &got, &del);
xfs_defer_ijoin(tp->t_dfops, ip);
- error = xfs_defer_finish(&tp, tp->t_dfops);
+ error = xfs_defer_finish(&tp);
if (error)
goto out_cancel;
if (!xfs_iext_get_extent(ifp, &icur, &got))
@@ -1077,7 +1077,7 @@ xfs_reflink_remap_extent(
next_extent:
/* Process all the deferred stuff. */
xfs_defer_ijoin(tp->t_dfops, ip);
- error = xfs_defer_finish(&tp, tp->t_dfops);
+ error = xfs_defer_finish(&tp);
if (error)
goto out_cancel;
}
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 412c8d236c71..cd553aa9ecb0 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -933,9 +933,9 @@ __xfs_trans_commit(
/* finish deferred items on final commit */
if (!regrant && tp->t_dfops) {
- error = xfs_defer_finish(&tp, tp->t_dfops);
+ error = xfs_defer_finish(&tp);
if (error) {
- xfs_defer_cancel(tp->t_dfops);
+ xfs_defer_cancel(tp);
goto out_unreserve;
}
}
@@ -1030,7 +1030,7 @@ xfs_trans_cancel(
trace_xfs_trans_cancel(tp, _RET_IP_);
if (tp->t_dfops)
- xfs_defer_cancel(tp->t_dfops);
+ xfs_defer_cancel(tp);
/*
* See if the caller is relying on us to shut down the
@@ -1111,3 +1111,10 @@ xfs_trans_roll(
tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
return xfs_trans_reserve(*tpp, &tres, 0, 0);
}
+
+void
+xfs_defer_cancel(
+ struct xfs_trans *tp)
+{
+ __xfs_defer_cancel(tp->t_dfops);
+}
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index dc79e3c1d3e8..5170e89bec02 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -214,6 +214,9 @@ xfs_trans_read_buf(
flags, bpp, ops);
}
+/* cancel dfops associated with a transaction */
+void xfs_defer_cancel(struct xfs_trans *);
+
struct xfs_buf *xfs_trans_getsb(xfs_trans_t *, struct xfs_mount *, int);
void xfs_trans_brelse(xfs_trans_t *, struct xfs_buf *);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 08/15] xfs: support embedded dfops in transaction
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
The dfops structure used by multi-transaction operations is
typically stored on the stack and carried around by the associated
transaction. The lifecycle of dfops does not quite match that of the
transaction, but they are tightly related in that the former depends
on the latter.
The relationship of these objects is tight enough that we can avoid
the cumbersome boilerplate code required in most cases to manage
them separately by just embedding an xfs_defer_ops in the
transaction itself. This means that a transaction allocation returns
with an initialized dfops, a transaction commit finishes pending
deferred items before the tx commit, a transaction cancel cancels
the dfops before the transaction and a transaction dup operation
transfers the current dfops state to the new transaction.
The dup operation is slightly complicated by the fact that we can no
longer just copy a dfops pointer from the old transaction to the new
transaction. This is solved through a dfops move helper that
transfers the pending items and other dfops state across the
transactions. This also requires that transaction rolling code
always refer to the transaction for the current dfops reference.
Finally, to facilitate incremental conversion to the internal dfops
and continue to support the current external dfops mode of
operation, create the new ->t_dfops_internal field with a layer of
indirection. On allocation, ->t_dfops points to the internal dfops.
This state is overridden by callers who re-init a local dfops on the
transaction. Once ->t_dfops is overridden, the external dfops
reference is maintained as the transaction rolls.
This patch adds the fundamental ability to support an internal
dfops. All codepaths that perform deferred processing continue to
override the internal dfops until they are converted over in
subsequent patches.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_defer.c | 22 ++++++++++++++++++++++
fs/xfs/libxfs/xfs_defer.h | 16 ++--------------
fs/xfs/xfs_trans.c | 30 ++++++++++++++++++++++++++----
fs/xfs/xfs_trans.h | 17 ++++++++++++++++-
4 files changed, 66 insertions(+), 19 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 23f2a52b088e..b63cc9e730da 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -555,3 +555,25 @@ xfs_defer_init(
}
trace_xfs_defer_init(mp, dop, _RET_IP_);
}
+
+/*
+ * Move state from one xfs_defer_ops to another and reset the source to initial
+ * state. This is primarily used to carry state forward across transaction rolls
+ * with internal dfops.
+ */
+void
+xfs_defer_move(
+ struct xfs_defer_ops *dst,
+ struct xfs_defer_ops *src)
+{
+ ASSERT(dst != src);
+
+ list_splice_init(&src->dop_intake, &dst->dop_intake);
+ list_splice_init(&src->dop_pending, &dst->dop_pending);
+
+ memcpy(dst->dop_inodes, src->dop_inodes, sizeof(dst->dop_inodes));
+ memcpy(dst->dop_bufs, src->dop_bufs, sizeof(dst->dop_bufs));
+ dst->dop_low = src->dop_low;
+
+ xfs_defer_reset(src);
+}
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 8f58f217fdff..35507ca9a148 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -7,6 +7,7 @@
#define __XFS_DEFER_H__
struct xfs_defer_op_type;
+struct xfs_defer_ops;
/*
* Save a log intent item and a list of extents, so that we can replay
@@ -45,20 +46,6 @@ enum xfs_defer_ops_type {
XFS_DEFER_OPS_TYPE_MAX,
};
-#define XFS_DEFER_OPS_NR_INODES 2 /* join up to two inodes */
-#define XFS_DEFER_OPS_NR_BUFS 2 /* join up to two buffers */
-
-struct xfs_defer_ops {
- struct list_head dop_intake; /* unlogged pending work */
- struct list_head dop_pending; /* logged pending work */
-
- /* relog these with each roll */
- struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES];
- struct xfs_buf *dop_bufs[XFS_DEFER_OPS_NR_BUFS];
-
- bool dop_low; /* alloc in low mode */
-};
-
void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
struct list_head *h);
int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop);
@@ -67,6 +54,7 @@ void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
int xfs_defer_ijoin(struct xfs_defer_ops *dop, struct xfs_inode *ip);
int xfs_defer_bjoin(struct xfs_defer_ops *dop, struct xfs_buf *bp);
+void xfs_defer_move(struct xfs_defer_ops *dst, struct xfs_defer_ops *src);
/* Description of a deferred type. */
struct xfs_defer_op_type {
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index de00f79ff698..412c8d236c71 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -119,7 +119,13 @@ xfs_trans_dup(
ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
tp->t_rtx_res = tp->t_rtx_res_used;
ntp->t_pflags = tp->t_pflags;
- ntp->t_dfops = tp->t_dfops;
+
+ /* copy the dfops pointer if it's external, otherwise move it */
+ xfs_defer_init(ntp, &ntp->t_dfops_internal);
+ if (tp->t_dfops != &tp->t_dfops_internal)
+ ntp->t_dfops = tp->t_dfops;
+ else
+ xfs_defer_move(ntp->t_dfops, tp->t_dfops);
xfs_trans_dup_dqinfo(tp, ntp);
@@ -275,6 +281,13 @@ xfs_trans_alloc(
INIT_LIST_HEAD(&tp->t_items);
INIT_LIST_HEAD(&tp->t_busy);
tp->t_firstblock = NULLFSBLOCK;
+ /*
+ * We only roll transactions with permanent log reservation. Don't init
+ * ->t_dfops to skip attempts to finish or cancel an empty dfops with a
+ * non-permanent res.
+ */
+ if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
+ xfs_defer_init(tp, &tp->t_dfops_internal);
error = xfs_trans_reserve(tp, resp, blocks, rtextents);
if (error) {
@@ -916,11 +929,17 @@ __xfs_trans_commit(
int error = 0;
int sync = tp->t_flags & XFS_TRANS_SYNC;
- ASSERT(!tp->t_dfops ||
- !xfs_defer_has_unfinished_work(tp->t_dfops) || regrant);
-
trace_xfs_trans_commit(tp, _RET_IP_);
+ /* finish deferred items on final commit */
+ if (!regrant && tp->t_dfops) {
+ error = xfs_defer_finish(&tp, tp->t_dfops);
+ if (error) {
+ xfs_defer_cancel(tp->t_dfops);
+ goto out_unreserve;
+ }
+ }
+
/*
* If there is nothing to be logged by the transaction,
* then unlock all of the items associated with the
@@ -1010,6 +1029,9 @@ xfs_trans_cancel(
trace_xfs_trans_cancel(tp, _RET_IP_);
+ if (tp->t_dfops)
+ xfs_defer_cancel(tp->t_dfops);
+
/*
* See if the caller is relying on us to shut down the
* filesystem. This happens in paths where we detect
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 6f857af61455..dc79e3c1d3e8 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -24,7 +24,6 @@ struct xfs_rui_log_item;
struct xfs_btree_cur;
struct xfs_cui_log_item;
struct xfs_cud_log_item;
-struct xfs_defer_ops;
struct xfs_bui_log_item;
struct xfs_bud_log_item;
@@ -90,6 +89,21 @@ void xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
#define XFS_ITEM_LOCKED 2
#define XFS_ITEM_FLUSHING 3
+/*
+ * Deferred operations tracking structure.
+ */
+#define XFS_DEFER_OPS_NR_INODES 2 /* join up to two inodes */
+#define XFS_DEFER_OPS_NR_BUFS 2 /* join up to two buffers */
+struct xfs_defer_ops {
+ struct list_head dop_intake; /* unlogged pending work */
+ struct list_head dop_pending; /* logged pending work */
+
+ /* relog these with each roll */
+ struct xfs_inode *dop_inodes[XFS_DEFER_OPS_NR_INODES];
+ struct xfs_buf *dop_bufs[XFS_DEFER_OPS_NR_BUFS];
+
+ bool dop_low; /* alloc in low mode */
+};
/*
* This is the structure maintained for every active transaction.
@@ -130,6 +144,7 @@ typedef struct xfs_trans {
struct list_head t_items; /* log item descriptors */
struct list_head t_busy; /* list of busy extents */
unsigned long t_pflags; /* saved process flags state */
+ struct xfs_defer_ops t_dfops_internal;
} xfs_trans_t;
/*
--
2.17.1
^ permalink raw reply related
* [PATCH v2 13/15] xfs: remove unnecessary dfops init calls in xattr code
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
Each xfs_defer_init() call in the xattr code uses the internal dfops
reference. In addition, a successful xfs_defer_finish() always
returns with a reset xfs_defer_ops structure.
Given that along with the fact that every xfs_defer_init() call in
the xattr code is followed up by an xfs_defer_finish(), the former
calls are no longer necessary and can be removed.
Note that the xfs_defer_init() call in the remote value copy loop of
xfs_attr_rmtval_set() is not followed by a finish, but the dfops is
unused in this instance.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 8 --------
fs/xfs/libxfs/xfs_attr_remote.c | 3 ---
2 files changed, 11 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 66a22c80a0db..3e98f0af389c 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -587,7 +587,6 @@ xfs_attr_leaf_addname(
* Commit that transaction so that the node_addname() call
* can manage its own transactions.
*/
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_attr3_leaf_to_node(args);
if (error)
goto out_defer_cancel;
@@ -676,7 +675,6 @@ xfs_attr_leaf_addname(
* If the result is small enough, shrink it all into the inode.
*/
if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
if (error)
@@ -741,7 +739,6 @@ xfs_attr_leaf_removename(
* If the result is small enough, shrink it all into the inode.
*/
if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
if (error)
@@ -870,7 +867,6 @@ xfs_attr_node_addname(
*/
xfs_da_state_free(state);
state = NULL;
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_attr3_leaf_to_node(args);
if (error)
goto out_defer_cancel;
@@ -897,7 +893,6 @@ xfs_attr_node_addname(
* in the index/blkno/rmtblkno/rmtblkcnt fields and
* in the index2/blkno2/rmtblkno2/rmtblkcnt2 fields.
*/
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_da3_split(state);
if (error)
goto out_defer_cancel;
@@ -995,7 +990,6 @@ xfs_attr_node_addname(
* Check to see if the tree needs to be collapsed.
*/
if (retval && (state->path.active > 1)) {
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_da3_join(state);
if (error)
goto out_defer_cancel;
@@ -1120,7 +1114,6 @@ xfs_attr_node_removename(
* Check to see if the tree needs to be collapsed.
*/
if (retval && (state->path.active > 1)) {
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_da3_join(state);
if (error)
goto out_defer_cancel;
@@ -1152,7 +1145,6 @@ xfs_attr_node_removename(
goto out;
if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
if (error)
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 829ab20f0cd7..0fbfb740949e 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -480,7 +480,6 @@ xfs_attr_rmtval_set(
* extent and then crash then the block may not contain the
* correct metadata after log recovery occurs.
*/
- xfs_defer_init(args->trans, args->trans->t_dfops);
nmap = 1;
error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,
blkcnt, XFS_BMAPI_ATTRFORK, args->total, &map,
@@ -522,7 +521,6 @@ xfs_attr_rmtval_set(
ASSERT(blkcnt > 0);
- xfs_defer_init(args->trans, args->trans->t_dfops);
nmap = 1;
error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
blkcnt, &map, &nmap,
@@ -625,7 +623,6 @@ xfs_attr_rmtval_remove(
blkcnt = args->rmtblkcnt;
done = 0;
while (!done) {
- xfs_defer_init(args->trans, args->trans->t_dfops);
error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
XFS_BMAPI_ATTRFORK, 1, &done);
if (error)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 15/15] xfs: bypass final dfops roll in trans commit path
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
Once xfs_defer_finish() has completed all deferred operations, it
checks the dirty state of the transaction and rolls it once more to
return a clean transaction for the caller. This primarily to cover
the case where repeated xfs_defer_finish() calls are made in a loop
and we need to make sure that the caller starts the next iteration
with a clean transaction. Otherwise we risk transaction reservation
overrun.
This final transaction roll is not required in the transaction
commit path, however, because the transaction is immediately
committed and freed after dfops completion. Refactor the final roll
into a separate helper such that we can avoid it in the transaction
commit path. Lift the dfops reset as well so dfops remains valid
until after the last call to xfs_defer_trans_roll(). The reset is
also unnecessary in the transaction commit path because the
transaction is about to complete.
This eliminates unnecessary regrants of transactions where the
associated transaction roll can be replaced by a transaction commit.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_defer.c | 35 ++++++++++++++++++++++-------------
fs/xfs/libxfs/xfs_defer.h | 1 +
fs/xfs/xfs_trans.c | 2 +-
3 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index cbee0a86c978..2d4c4b09977e 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -341,7 +341,7 @@ xfs_defer_reset(
* If an inode is provided, relog it to the new transaction.
*/
int
-xfs_defer_finish(
+xfs_defer_finish_noroll(
struct xfs_trans **tp)
{
struct xfs_defer_ops *dop = (*tp)->t_dfops;
@@ -430,21 +430,30 @@ xfs_defer_finish(
cleanup_fn(*tp, state, error);
}
- /*
- * Roll the transaction once more to avoid returning to the caller
- * with a dirty transaction.
- */
- if ((*tp)->t_flags & XFS_TRANS_DIRTY) {
- error = xfs_defer_trans_roll(tp);
- dop = (*tp)->t_dfops;
- }
out:
- if (error) {
+ if (error)
trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
- } else {
+ else
trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_);
- xfs_defer_reset(dop);
- }
+
+ return error;
+}
+
+int
+xfs_defer_finish(
+ struct xfs_trans **tp)
+{
+ int error;
+
+ /*
+ * Finish and roll the transaction once more to avoid returning to the
+ * caller with a dirty transaction.
+ */
+ error = xfs_defer_finish_noroll(tp);
+ if (!error && ((*tp)->t_flags & XFS_TRANS_DIRTY))
+ error = xfs_defer_trans_roll(tp);
+ if (!error)
+ xfs_defer_reset((*tp)->t_dfops);
return error;
}
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 56f927803940..85c41fe4dbae 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -48,6 +48,7 @@ enum xfs_defer_ops_type {
void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
struct list_head *h);
+int xfs_defer_finish_noroll(struct xfs_trans **tp);
int xfs_defer_finish(struct xfs_trans **tp);
void __xfs_defer_cancel(struct xfs_defer_ops *dop);
void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index cd553aa9ecb0..7bf5c1202719 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -933,7 +933,7 @@ __xfs_trans_commit(
/* finish deferred items on final commit */
if (!regrant && tp->t_dfops) {
- error = xfs_defer_finish(&tp);
+ error = xfs_defer_finish_noroll(&tp);
if (error) {
xfs_defer_cancel(tp);
goto out_unreserve;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 12/15] xfs: remove all boilerplate defer init/finish code
From: Brian Foster @ 2018-07-23 13:04 UTC (permalink / raw)
To: linux-xfs
In-Reply-To: <20180723130414.47980-1-bfoster@redhat.com>
At this point, the transaction subsystem completely manages deferred
items internally such that the common and boilerplate
xfs_trans_alloc() -> xfs_defer_init() -> xfs_defer_finish() ->
xfs_trans_commit() sequence can be replaced with a simple
transaction allocation and commit.
Remove all such boilerplate deferred ops code. In doing so, we
change each case over to use the dfops in the transaction and
specifically eliminate:
- The on-stack dfops and associated xfs_defer_init() call, as the
internal dfops is initialized on transaction allocation.
- xfs_bmap_finish() calls that precede a final xfs_trans_commit() of
a transaction.
- xfs_defer_cancel() calls in error handlers that precede a
transaction cancel.
The only deferred ops calls that remain are those that are
non-deterministic with respect to the final commit of the associated
transaction or are open-coded due to special handling.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 16 +-------
fs/xfs/libxfs/xfs_refcount.c | 10 +----
fs/xfs/xfs_attr_inactive.c | 2 -
fs/xfs/xfs_bmap_util.c | 43 +++-----------------
fs/xfs/xfs_dquot.c | 4 --
fs/xfs/xfs_inode.c | 79 ++++++------------------------------
fs/xfs/xfs_iomap.c | 26 +-----------
fs/xfs/xfs_iops.c | 2 -
fs/xfs/xfs_log_recover.c | 8 ----
fs/xfs/xfs_qm_syscalls.c | 2 -
fs/xfs/xfs_reflink.c | 37 ++++++-----------
fs/xfs/xfs_rtalloc.c | 9 +---
fs/xfs/xfs_symlink.c | 38 ++++-------------
13 files changed, 44 insertions(+), 232 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7b93b1e16ad9..60138514ea86 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1019,7 +1019,6 @@ xfs_bmap_add_attrfork(
int size, /* space new attribute needs */
int rsvd) /* xact may use reserved blks */
{
- struct xfs_defer_ops dfops; /* freed extent records */
xfs_mount_t *mp; /* mount structure */
xfs_trans_t *tp; /* transaction pointer */
int blks; /* space reservation */
@@ -1038,7 +1037,6 @@ xfs_bmap_add_attrfork(
rsvd ? XFS_TRANS_RESERVE : 0, &tp);
if (error)
return error;
- xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
error = xfs_trans_reserve_quota_nblks(tp, ip, blks, 0, rsvd ?
@@ -1103,7 +1101,7 @@ xfs_bmap_add_attrfork(
if (logflags)
xfs_trans_log_inode(tp, ip, logflags);
if (error)
- goto bmap_cancel;
+ goto trans_cancel;
if (!xfs_sb_version_hasattr(&mp->m_sb) ||
(!xfs_sb_version_hasattr2(&mp->m_sb) && version == 2)) {
bool log_sb = false;
@@ -1122,15 +1120,10 @@ xfs_bmap_add_attrfork(
xfs_log_sb(tp);
}
- error = xfs_defer_finish(&tp, &dfops);
- if (error)
- goto bmap_cancel;
error = xfs_trans_commit(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
-bmap_cancel:
- xfs_defer_cancel(&dfops);
trans_cancel:
xfs_trans_cancel(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
@@ -5961,14 +5954,12 @@ xfs_bmap_split_extent(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
int error;
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write,
XFS_DIOSTRAT_SPACE_RES(mp, 0), 0, 0, &tp);
if (error)
return error;
- xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
@@ -5977,14 +5968,9 @@ xfs_bmap_split_extent(
if (error)
goto out;
- error = xfs_defer_finish(&tp, &dfops);
- if (error)
- goto out;
-
return xfs_trans_commit(tp);
out:
- xfs_defer_cancel(&dfops);
xfs_trans_cancel(tp);
return error;
}
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 2ecfb0518580..85e2d7c56c14 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1635,7 +1635,6 @@ xfs_refcount_recover_cow_leftovers(
struct list_head debris;
union xfs_btree_irec low;
union xfs_btree_irec high;
- struct xfs_defer_ops dfops;
xfs_fsblock_t fsb;
xfs_agblock_t agbno;
int error;
@@ -1691,22 +1690,17 @@ xfs_refcount_recover_cow_leftovers(
trace_xfs_refcount_recover_extent(mp, agno, &rr->rr_rrec);
/* Free the orphan record */
- xfs_defer_init(tp, &dfops);
agbno = rr->rr_rrec.rc_startblock - XFS_REFC_COW_START;
fsb = XFS_AGB_TO_FSB(mp, agno, agbno);
error = xfs_refcount_free_cow_extent(mp, tp->t_dfops, fsb,
rr->rr_rrec.rc_blockcount);
if (error)
- goto out_defer;
+ goto out_trans;
/* Free the block. */
xfs_bmap_add_free(mp, tp->t_dfops, fsb,
rr->rr_rrec.rc_blockcount, NULL);
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_defer;
-
error = xfs_trans_commit(tp);
if (error)
goto out_free;
@@ -1716,8 +1710,6 @@ xfs_refcount_recover_cow_leftovers(
}
return error;
-out_defer:
- xfs_defer_cancel(tp->t_dfops);
out_trans:
xfs_trans_cancel(tp);
out_free:
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index d3055972d3a6..228821b2ebe0 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -382,7 +382,6 @@ xfs_attr_inactive(
{
struct xfs_trans *trans;
struct xfs_mount *mp;
- struct xfs_defer_ops dfops;
int lock_mode = XFS_ILOCK_SHARED;
int error = 0;
@@ -399,7 +398,6 @@ xfs_attr_inactive(
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_attrinval, 0, 0, 0, &trans);
if (error)
goto out_destroy_fork;
- xfs_defer_init(trans, &dfops);
lock_mode = XFS_ILOCK_EXCL;
xfs_ilock(dp, lock_mode);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index ee9481b142f0..6b0f31402a81 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -792,7 +792,6 @@ xfs_free_eofblocks(
int nimaps;
struct xfs_bmbt_irec imap;
struct xfs_mount *mp = ip->i_mount;
- struct xfs_defer_ops dfops;
/*
* Figure out if there are any blocks beyond the end
@@ -832,7 +831,6 @@ xfs_free_eofblocks(
ASSERT(XFS_FORCED_SHUTDOWN(mp));
return error;
}
- xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
@@ -880,7 +878,6 @@ xfs_alloc_file_space(
int rt;
xfs_trans_t *tp;
xfs_bmbt_irec_t imaps[1], *imapp;
- struct xfs_defer_ops dfops;
uint qblocks, resblks, resrtextents;
int error;
@@ -973,7 +970,6 @@ xfs_alloc_file_space(
xfs_trans_ijoin(tp, ip, 0);
- xfs_defer_init(tp, &dfops);
error = xfs_bmapi_write(tp, ip, startoffset_fsb,
allocatesize_fsb, alloc_type, resblks,
imapp, &nimaps);
@@ -983,10 +979,6 @@ xfs_alloc_file_space(
/*
* Complete the transaction
*/
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto error0;
-
error = xfs_trans_commit(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
if (error)
@@ -1005,8 +997,7 @@ xfs_alloc_file_space(
return error;
-error0: /* Cancel bmap, unlock inode, unreserve quota blocks, cancel trans */
- xfs_defer_cancel(&dfops);
+error0: /* unlock inode, unreserve quota blocks, cancel trans */
xfs_trans_unreserve_quota_nblks(tp, ip, (long)qblocks, 0, quota_flag);
error1: /* Just cancel transaction */
@@ -1024,7 +1015,6 @@ xfs_unmap_extent(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
int error;
@@ -1042,23 +1032,17 @@ xfs_unmap_extent(
xfs_trans_ijoin(tp, ip, 0);
- xfs_defer_init(tp, &dfops);
error = xfs_bunmapi(tp, ip, startoffset_fsb, len_fsb, 0, 2, done);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
xfs_defer_ijoin(tp->t_dfops, ip);
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
error = xfs_trans_commit(tp);
out_unlock:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
goto out_unlock;
@@ -1310,7 +1294,6 @@ xfs_collapse_file_space(
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
int error;
- struct xfs_defer_ops dfops;
xfs_fileoff_t next_fsb = XFS_B_TO_FSB(mp, offset + len);
xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
@@ -1343,22 +1326,16 @@ xfs_collapse_file_space(
goto out_trans_cancel;
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
- xfs_defer_init(tp, &dfops);
error = xfs_bmap_collapse_extents(tp, ip, &next_fsb, shift_fsb,
&done);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
error = xfs_trans_commit(tp);
}
return error;
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
return error;
@@ -1385,7 +1362,6 @@ xfs_insert_file_space(
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
int error;
- struct xfs_defer_ops dfops;
xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, offset);
xfs_fileoff_t next_fsb = NULLFSBLOCK;
xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
@@ -1421,22 +1397,17 @@ xfs_insert_file_space(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
- xfs_defer_init(tp, &dfops);
error = xfs_bmap_insert_extents(tp, ip, &next_fsb, shift_fsb,
&done, stop_fsb);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
error = xfs_trans_commit(tp);
}
return error;
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
+out_trans_cancel:
xfs_trans_cancel(tp);
return error;
}
@@ -1607,7 +1578,7 @@ xfs_swap_extent_rmap(
/* Unmap the old blocks in the source file. */
while (tirec.br_blockcount) {
- xfs_defer_init(tp, tp->t_dfops);
+ ASSERT(tp->t_firstblock == NULLFSBLOCK);
trace_xfs_swap_extent_rmap_remap_piece(tip, &tirec);
/* Read extent from the source file */
@@ -1841,7 +1812,6 @@ xfs_swap_extents(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
struct xfs_bstat *sbp = &sxp->sx_stat;
int src_log_flags, target_log_flags;
int error = 0;
@@ -1911,7 +1881,6 @@ xfs_swap_extents(
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0, &tp);
if (error)
goto out_unlock;
- xfs_defer_init(tp, &dfops);
/*
* Lock and join the inodes to the tansaction so that transaction commit
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index c53de34c9ae5..a57d5e8c3118 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -295,8 +295,6 @@ xfs_dquot_disk_alloc(
trace_xfs_dqalloc(dqp);
- xfs_defer_init(tp, tp->t_dfops);
-
xfs_ilock(quotip, XFS_ILOCK_EXCL);
if (!xfs_this_quota_on(dqp->q_mount, dqp->dq_flags)) {
/*
@@ -538,7 +536,6 @@ xfs_qm_dqread_alloc(
struct xfs_buf **bpp)
{
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
struct xfs_buf *bp;
int error;
@@ -546,7 +543,6 @@ xfs_qm_dqread_alloc(
XFS_QM_DQALLOC_SPACE_RES(mp), 0, 0, &tp);
if (error)
goto err;
- xfs_defer_init(tp, &dfops);
error = xfs_dquot_disk_alloc(&tp, dqp, &bp);
if (error)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 7d7d7e95fa17..c47183a2f167 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1142,7 +1142,6 @@ xfs_create(
struct xfs_inode *ip = NULL;
struct xfs_trans *tp = NULL;
int error;
- struct xfs_defer_ops dfops;
bool unlock_dp_on_error = false;
prid_t prid;
struct xfs_dquot *udqp = NULL;
@@ -1194,8 +1193,6 @@ xfs_create(
xfs_ilock(dp, XFS_ILOCK_EXCL | XFS_ILOCK_PARENT);
unlock_dp_on_error = true;
- xfs_defer_init(tp, &dfops);
-
/*
* Reserve disk quota and the inode.
*/
@@ -1236,11 +1233,11 @@ xfs_create(
if (is_dir) {
error = xfs_dir_init(tp, ip, dp);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
error = xfs_bumplink(tp, dp);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
/*
@@ -1258,10 +1255,6 @@ xfs_create(
*/
xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp, pdqp);
- error = xfs_defer_finish(&tp, &dfops);
- if (error)
- goto out_bmap_cancel;
-
error = xfs_trans_commit(tp);
if (error)
goto out_release_inode;
@@ -1273,8 +1266,6 @@ xfs_create(
*ipp = ip;
return 0;
- out_bmap_cancel:
- xfs_defer_cancel(&dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
out_release_inode:
@@ -1399,7 +1390,6 @@ xfs_link(
xfs_mount_t *mp = tdp->i_mount;
xfs_trans_t *tp;
int error;
- struct xfs_defer_ops dfops;
int resblks;
trace_xfs_link(tdp, target_name);
@@ -1448,8 +1438,6 @@ xfs_link(
goto error_return;
}
- xfs_defer_init(tp, &dfops);
-
/*
* Handle initial link state of O_TMPFILE inode
*/
@@ -1478,12 +1466,6 @@ xfs_link(
if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
xfs_trans_set_sync(tp);
- error = xfs_defer_finish(&tp, &dfops);
- if (error) {
- xfs_defer_cancel(&dfops);
- goto error_return;
- }
-
return xfs_trans_commit(tp);
error_return:
@@ -1719,7 +1701,6 @@ xfs_inactive_truncate(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
int error;
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
@@ -1727,8 +1708,6 @@ xfs_inactive_truncate(
ASSERT(XFS_FORCED_SHUTDOWN(mp));
return error;
}
- xfs_defer_init(tp, &dfops);
-
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
@@ -1769,7 +1748,6 @@ STATIC int
xfs_inactive_ifree(
struct xfs_inode *ip)
{
- struct xfs_defer_ops dfops;
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
int error;
@@ -1806,7 +1784,6 @@ xfs_inactive_ifree(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- xfs_defer_init(tp, &dfops);
error = xfs_ifree(tp, ip);
if (error) {
/*
@@ -1833,12 +1810,6 @@ xfs_inactive_ifree(
* Just ignore errors at this point. There is nothing we can do except
* to try to keep going. Make sure it's not a silent error.
*/
- error = xfs_defer_finish(&tp, &dfops);
- if (error) {
- xfs_notice(mp, "%s: xfs_defer_finish returned error %d",
- __func__, error);
- xfs_defer_cancel(&dfops);
- }
error = xfs_trans_commit(tp);
if (error)
xfs_notice(mp, "%s: xfs_trans_commit returned error %d",
@@ -2569,7 +2540,6 @@ xfs_remove(
xfs_trans_t *tp = NULL;
int is_dir = S_ISDIR(VFS_I(ip)->i_mode);
int error = 0;
- struct xfs_defer_ops dfops;
uint resblks;
trace_xfs_remove(dp, name);
@@ -2649,11 +2619,10 @@ xfs_remove(
if (error)
goto out_trans_cancel;
- xfs_defer_init(tp, &dfops);
error = xfs_dir_removename(tp, dp, name, ip->i_ino, resblks);
if (error) {
ASSERT(error != -ENOENT);
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
/*
@@ -2664,10 +2633,6 @@ xfs_remove(
if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
xfs_trans_set_sync(tp);
- error = xfs_defer_finish(&tp, &dfops);
- if (error)
- goto out_bmap_cancel;
-
error = xfs_trans_commit(tp);
if (error)
goto std_return;
@@ -2677,8 +2642,6 @@ xfs_remove(
return 0;
- out_bmap_cancel:
- xfs_defer_cancel(&dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
std_return:
@@ -2740,9 +2703,6 @@ static int
xfs_finish_rename(
struct xfs_trans *tp)
{
- struct xfs_defer_ops *dfops = tp->t_dfops;
- int error;
-
/*
* If this is a synchronous mount, make sure that the rename transaction
* goes to disk before returning to the user.
@@ -2750,13 +2710,6 @@ xfs_finish_rename(
if (tp->t_mountp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
xfs_trans_set_sync(tp);
- error = xfs_defer_finish(&tp, dfops);
- if (error) {
- xfs_defer_cancel(dfops);
- xfs_trans_cancel(tp);
- return error;
- }
-
return xfs_trans_commit(tp);
}
@@ -2869,7 +2822,6 @@ xfs_cross_rename(
return xfs_finish_rename(tp);
out_trans_abort:
- xfs_defer_cancel(tp->t_dfops);
xfs_trans_cancel(tp);
return error;
}
@@ -2924,7 +2876,6 @@ xfs_rename(
{
struct xfs_mount *mp = src_dp->i_mount;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
struct xfs_inode *wip = NULL; /* whiteout inode */
struct xfs_inode *inodes[__XFS_SORT_INODES];
int num_inodes = __XFS_SORT_INODES;
@@ -3006,8 +2957,6 @@ xfs_rename(
goto out_trans_cancel;
}
- xfs_defer_init(tp, &dfops);
-
/* RENAME_EXCHANGE is unique from here on. */
if (flags & RENAME_EXCHANGE)
return xfs_cross_rename(tp, src_dp, src_name, src_ip,
@@ -3035,7 +2984,7 @@ xfs_rename(
error = xfs_dir_createname(tp, target_dp, target_name,
src_ip->i_ino, spaceres);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
xfs_trans_ichgtime(tp, target_dp,
XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
@@ -3043,7 +2992,7 @@ xfs_rename(
if (new_parent && src_is_directory) {
error = xfs_bumplink(tp, target_dp);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
} else { /* target_ip != NULL */
/*
@@ -3074,7 +3023,7 @@ xfs_rename(
error = xfs_dir_replace(tp, target_dp, target_name,
src_ip->i_ino, spaceres);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
xfs_trans_ichgtime(tp, target_dp,
XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
@@ -3085,7 +3034,7 @@ xfs_rename(
*/
error = xfs_droplink(tp, target_ip);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
if (src_is_directory) {
/*
@@ -3093,7 +3042,7 @@ xfs_rename(
*/
error = xfs_droplink(tp, target_ip);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
} /* target_ip != NULL */
@@ -3109,7 +3058,7 @@ xfs_rename(
target_dp->i_ino, spaceres);
ASSERT(error != -EEXIST);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
/*
@@ -3135,7 +3084,7 @@ xfs_rename(
*/
error = xfs_droplink(tp, src_dp);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
/*
@@ -3150,7 +3099,7 @@ xfs_rename(
error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
spaceres);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
/*
* For whiteouts, we need to bump the link count on the whiteout inode.
@@ -3164,10 +3113,10 @@ xfs_rename(
ASSERT(VFS_I(wip)->i_nlink == 0);
error = xfs_bumplink(tp, wip);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
error = xfs_iunlink_remove(tp, wip);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
xfs_trans_log_inode(tp, wip, XFS_ILOG_CORE);
/*
@@ -3188,8 +3137,6 @@ xfs_rename(
IRELE(wip);
return error;
-out_bmap_cancel:
- xfs_defer_cancel(&dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
out_release_wip:
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 756694219f77..8e8ca9f03f0e 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -157,7 +157,6 @@ xfs_iomap_write_direct(
int quota_flag;
int rt;
xfs_trans_t *tp;
- struct xfs_defer_ops dfops;
uint qblocks, resblks, resrtextents;
int error;
int lockmode;
@@ -253,20 +252,15 @@ xfs_iomap_write_direct(
* From this point onwards we overwrite the imap pointer that the
* caller gave to us.
*/
- xfs_defer_init(tp, &dfops);
nimaps = 1;
error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb,
bmapi_flags, resblks, imap, &nimaps);
if (error)
- goto out_bmap_cancel;
+ goto out_res_cancel;
/*
* Complete the transaction
*/
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
-
error = xfs_trans_commit(tp);
if (error)
goto out_unlock;
@@ -286,8 +280,7 @@ xfs_iomap_write_direct(
xfs_iunlock(ip, lockmode);
return error;
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
+out_res_cancel:
xfs_trans_unreserve_quota_nblks(tp, ip, (long)qblocks, 0, quota_flag);
out_trans_cancel:
xfs_trans_cancel(tp);
@@ -663,7 +656,6 @@ xfs_iomap_write_allocate(
xfs_mount_t *mp = ip->i_mount;
xfs_fileoff_t offset_fsb, last_block;
xfs_fileoff_t end_fsb, map_start_fsb;
- struct xfs_defer_ops dfops;
xfs_filblks_t count_fsb;
xfs_trans_t *tp;
int nimaps;
@@ -713,8 +705,6 @@ xfs_iomap_write_allocate(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
- xfs_defer_init(tp, &dfops);
-
/*
* it is possible that the extents have changed since
* we did the read call as we dropped the ilock for a
@@ -772,10 +762,6 @@ xfs_iomap_write_allocate(
if (error)
goto trans_cancel;
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto trans_cancel;
-
error = xfs_trans_commit(tp);
if (error)
goto error0;
@@ -806,7 +792,6 @@ xfs_iomap_write_allocate(
}
trans_cancel:
- xfs_defer_cancel(tp->t_dfops);
xfs_trans_cancel(tp);
error0:
xfs_iunlock(ip, XFS_ILOCK_EXCL);
@@ -827,7 +812,6 @@ xfs_iomap_write_unwritten(
int nimaps;
xfs_trans_t *tp;
xfs_bmbt_irec_t imap;
- struct xfs_defer_ops dfops;
struct inode *inode = VFS_I(ip);
xfs_fsize_t i_size;
uint resblks;
@@ -872,7 +856,6 @@ xfs_iomap_write_unwritten(
/*
* Modify the unwritten extent state of the buffer.
*/
- xfs_defer_init(tp, &dfops);
nimaps = 1;
error = xfs_bmapi_write(tp, ip, offset_fsb, count_fsb,
XFS_BMAPI_CONVERT, resblks, &imap,
@@ -896,10 +879,6 @@ xfs_iomap_write_unwritten(
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
}
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto error_on_bmapi_transaction;
-
error = xfs_trans_commit(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
if (error)
@@ -923,7 +902,6 @@ xfs_iomap_write_unwritten(
return 0;
error_on_bmapi_transaction:
- xfs_defer_cancel(tp->t_dfops);
xfs_trans_cancel(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return error;
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 704b57a8b99e..2eac22bfad6a 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -813,7 +813,6 @@ xfs_setattr_size(
struct inode *inode = VFS_I(ip);
xfs_off_t oldsize, newsize;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
int error;
uint lock_flags = 0;
bool did_zeroing = false;
@@ -917,7 +916,6 @@ xfs_setattr_size(
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
if (error)
return error;
- xfs_defer_init(tp, &dfops);
lock_flags |= XFS_ILOCK_EXCL;
xfs_ilock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 958e9b96dc6a..265e1f561157 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4857,15 +4857,7 @@ xlog_finish_defer_ops(
/* transfer all collected dfops to this transaction */
xfs_defer_move(tp->t_dfops, dfops);
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_cancel;
-
return xfs_trans_commit(tp);
-
-out_cancel:
- xfs_trans_cancel(tp);
- return error;
}
/*
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index df0783303887..c07c5a39d516 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -214,7 +214,6 @@ xfs_qm_scall_trunc_qfile(
{
struct xfs_inode *ip;
struct xfs_trans *tp;
- struct xfs_defer_ops dfops;
int error;
if (ino == NULLFSINO)
@@ -231,7 +230,6 @@ xfs_qm_scall_trunc_qfile(
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
goto out_put;
}
- xfs_defer_init(tp, &dfops);
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, 0);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index c3f2a0d3e6f5..865ba615dd76 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -364,7 +364,6 @@ xfs_reflink_allocate_cow(
xfs_fileoff_t offset_fsb = imap->br_startoff;
xfs_filblks_t count_fsb = imap->br_blockcount;
struct xfs_bmbt_irec got;
- struct xfs_defer_ops dfops;
struct xfs_trans *tp = NULL;
int nimaps, error = 0;
bool trimmed;
@@ -424,7 +423,6 @@ xfs_reflink_allocate_cow(
xfs_trans_ijoin(tp, ip, 0);
- xfs_defer_init(tp, &dfops);
nimaps = 1;
/* Allocate the entire reservation as unwritten blocks. */
@@ -432,15 +430,11 @@ xfs_reflink_allocate_cow(
XFS_BMAPI_COWFORK | XFS_BMAPI_PREALLOC,
resblks, imap, &nimaps);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
xfs_inode_set_cowblocks_tag(ip);
/* Finish up. */
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
-
error = xfs_trans_commit(tp);
if (error)
return error;
@@ -453,8 +447,7 @@ xfs_reflink_allocate_cow(
return -ENOSPC;
convert:
return xfs_reflink_convert_cow_extent(ip, imap, offset_fsb, count_fsb);
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
+out_trans_cancel:
xfs_trans_unreserve_quota_nblks(tp, ip, (long)resblks, 0,
XFS_QMOPT_RES_REGBLKS);
out:
@@ -624,7 +617,6 @@ xfs_reflink_end_cow(
struct xfs_trans *tp;
xfs_fileoff_t offset_fsb;
xfs_fileoff_t end_fsb;
- struct xfs_defer_ops dfops;
int error;
unsigned int resblks;
xfs_filblks_t rlen;
@@ -691,11 +683,11 @@ xfs_reflink_end_cow(
goto prev_extent;
/* Unmap the old blocks in the data fork. */
- xfs_defer_init(tp, &dfops);
+ ASSERT(tp->t_dfops && tp->t_firstblock == NULLFSBLOCK);
rlen = del.br_blockcount;
error = __xfs_bunmapi(tp, ip, del.br_startoff, &rlen, 0, 1);
if (error)
- goto out_defer;
+ goto out_cancel;
/* Trim the extent to whatever got unmapped. */
if (rlen) {
@@ -708,13 +700,13 @@ xfs_reflink_end_cow(
error = xfs_refcount_free_cow_extent(tp->t_mountp, tp->t_dfops,
del.br_startblock, del.br_blockcount);
if (error)
- goto out_defer;
+ goto out_cancel;
/* Map the new blocks into the data fork. */
error = xfs_bmap_map_extent(tp->t_mountp, tp->t_dfops, ip,
&del);
if (error)
- goto out_defer;
+ goto out_cancel;
/* Charge this new data fork mapping to the on-disk quota. */
xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_DELBCOUNT,
@@ -726,7 +718,7 @@ xfs_reflink_end_cow(
xfs_defer_ijoin(tp->t_dfops, ip);
error = xfs_defer_finish(&tp, tp->t_dfops);
if (error)
- goto out_defer;
+ goto out_cancel;
if (!xfs_iext_get_extent(ifp, &icur, &got))
break;
continue;
@@ -741,8 +733,6 @@ xfs_reflink_end_cow(
goto out;
return 0;
-out_defer:
- xfs_defer_cancel(tp->t_dfops);
out_cancel:
xfs_trans_cancel(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
@@ -998,7 +988,6 @@ xfs_reflink_remap_extent(
bool real_extent = xfs_bmap_is_real_extent(irec);
struct xfs_trans *tp;
unsigned int resblks;
- struct xfs_defer_ops dfops;
struct xfs_bmbt_irec uirec;
xfs_filblks_t rlen;
xfs_filblks_t unmap_len;
@@ -1039,10 +1028,10 @@ xfs_reflink_remap_extent(
/* Unmap the old blocks in the data fork. */
rlen = unmap_len;
while (rlen) {
- xfs_defer_init(tp, &dfops);
+ ASSERT(tp->t_dfops && tp->t_firstblock == NULLFSBLOCK);
error = __xfs_bunmapi(tp, ip, destoff, &rlen, 0, 1);
if (error)
- goto out_defer;
+ goto out_cancel;
/*
* Trim the extent to whatever got unmapped.
@@ -1063,12 +1052,12 @@ xfs_reflink_remap_extent(
/* Update the refcount tree */
error = xfs_refcount_increase_extent(mp, tp->t_dfops, &uirec);
if (error)
- goto out_defer;
+ goto out_cancel;
/* Map the new blocks into the data fork. */
error = xfs_bmap_map_extent(mp, tp->t_dfops, ip, &uirec);
if (error)
- goto out_defer;
+ goto out_cancel;
/* Update quota accounting. */
xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT,
@@ -1090,7 +1079,7 @@ xfs_reflink_remap_extent(
xfs_defer_ijoin(tp->t_dfops, ip);
error = xfs_defer_finish(&tp, tp->t_dfops);
if (error)
- goto out_defer;
+ goto out_cancel;
}
error = xfs_trans_commit(tp);
@@ -1099,8 +1088,6 @@ xfs_reflink_remap_extent(
goto out;
return 0;
-out_defer:
- xfs_defer_cancel(tp->t_dfops);
out_cancel:
xfs_trans_cancel(tp);
xfs_iunlock(ip, XFS_ILOCK_EXCL);
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index bc471d42a968..86d7d2f76226 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -761,7 +761,6 @@ xfs_growfs_rt_alloc(
struct xfs_buf *bp; /* temporary buffer for zeroing */
xfs_daddr_t d; /* disk block address */
int error; /* error return value */
- struct xfs_defer_ops dfops; /* list of freed blocks */
xfs_fsblock_t fsbno; /* filesystem block for bno */
struct xfs_bmbt_irec map; /* block map output */
int nmap; /* number of block maps */
@@ -786,7 +785,6 @@ xfs_growfs_rt_alloc(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
- xfs_defer_init(tp, &dfops);
/*
* Allocate blocks to the bitmap file.
*/
@@ -797,13 +795,10 @@ xfs_growfs_rt_alloc(
if (!error && nmap < 1)
error = -ENOSPC;
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
/*
* Free any blocks freed up in the transaction, then commit.
*/
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
error = xfs_trans_commit(tp);
if (error)
return error;
@@ -853,8 +848,6 @@ xfs_growfs_rt_alloc(
return 0;
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
return error;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index d1ab0afa2723..ce801aedbcdc 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -163,7 +163,6 @@ xfs_symlink(
struct xfs_inode *ip = NULL;
int error = 0;
int pathlen;
- struct xfs_defer_ops dfops;
bool unlock_dp_on_error = false;
xfs_fileoff_t first_fsb;
xfs_filblks_t fs_blocks;
@@ -241,12 +240,6 @@ xfs_symlink(
if (error)
goto out_trans_cancel;
- /*
- * Initialize the bmap freelist prior to calling either
- * bmapi or the directory create code.
- */
- xfs_defer_init(tp, &dfops);
-
/*
* Allocate an inode for the symlink.
*/
@@ -290,7 +283,7 @@ xfs_symlink(
error = xfs_bmapi_write(tp, ip, first_fsb, fs_blocks,
XFS_BMAPI_METADATA, resblks, mval, &nmaps);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
if (resblks)
resblks -= fs_blocks;
@@ -308,7 +301,7 @@ xfs_symlink(
BTOBB(byte_cnt), 0);
if (!bp) {
error = -ENOMEM;
- goto out_bmap_cancel;
+ goto out_trans_cancel;
}
bp->b_ops = &xfs_symlink_buf_ops;
@@ -337,7 +330,7 @@ xfs_symlink(
*/
error = xfs_dir_createname(tp, dp, link_name, ip->i_ino, resblks);
if (error)
- goto out_bmap_cancel;
+ goto out_trans_cancel;
xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
@@ -350,10 +343,6 @@ xfs_symlink(
xfs_trans_set_sync(tp);
}
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto out_bmap_cancel;
-
error = xfs_trans_commit(tp);
if (error)
goto out_release_inode;
@@ -365,8 +354,6 @@ xfs_symlink(
*ipp = ip;
return 0;
-out_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
out_trans_cancel:
xfs_trans_cancel(tp);
out_release_inode:
@@ -399,7 +386,6 @@ xfs_inactive_symlink_rmt(
xfs_buf_t *bp;
int done;
int error;
- struct xfs_defer_ops dfops;
int i;
xfs_mount_t *mp;
xfs_bmbt_irec_t mval[XFS_SYMLINK_MAPS];
@@ -438,7 +424,6 @@ xfs_inactive_symlink_rmt(
* Find the block(s) so we can inval and unmap them.
*/
done = 0;
- xfs_defer_init(tp, &dfops);
nmaps = ARRAY_SIZE(mval);
error = xfs_bmapi_read(ip, 0, xfs_symlink_blocks(mp, size),
mval, &nmaps, 0);
@@ -453,7 +438,7 @@ xfs_inactive_symlink_rmt(
XFS_FSB_TO_BB(mp, mval[i].br_blockcount), 0);
if (!bp) {
error = -ENOMEM;
- goto error_bmap_cancel;
+ goto error_trans_cancel;
}
xfs_trans_binval(tp, bp);
}
@@ -462,19 +447,14 @@ xfs_inactive_symlink_rmt(
*/
error = xfs_bunmapi(tp, ip, 0, size, 0, nmaps, &done);
if (error)
- goto error_bmap_cancel;
+ goto error_trans_cancel;
ASSERT(done);
- /*
- * Commit the first transaction. This logs the EFI and the inode.
- */
- xfs_defer_ijoin(tp->t_dfops, ip);
- error = xfs_defer_finish(&tp, tp->t_dfops);
- if (error)
- goto error_bmap_cancel;
/*
- * Commit the transaction containing extent freeing and EFDs.
+ * Commit the transaction. This first logs the EFI and the inode, then
+ * rolls and commits the transaction that frees the extents.
*/
+ xfs_defer_ijoin(tp->t_dfops, ip);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
error = xfs_trans_commit(tp);
if (error) {
@@ -492,8 +472,6 @@ xfs_inactive_symlink_rmt(
xfs_iunlock(ip, XFS_ILOCK_EXCL);
return 0;
-error_bmap_cancel:
- xfs_defer_cancel(tp->t_dfops);
error_trans_cancel:
xfs_trans_cancel(tp);
error_unlock:
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.