Re: [PATCH v2] cpuidle: Add 'above' and 'below' idle state metrics
On Mon, Dec 10, 2018 at 11:51 PM Peter Zijlstra wrote:
>
> On Mon, Dec 10, 2018 at 10:36:40PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Dec 10, 2018 at 1:21 PM Peter Zijlstra wrote:
>
> > > One question on this; why is this tracked unconditionally?
> >
> > Because I didn't quite see how to make that conditional in a sensible way.
>
> Something like:
>
> if (static_branch_unlikely(__tracepoint_idle_above) ||
> static_branch_unlikely(__tracepoint_idle_below)) {
>
> // do stuff that calls trace_idle_above() /
> // trace_idle_below().
>
> }
>
> > These things are counters and counting with the help of tracepoints
> > isn't particularly convenient (and one needs debugfs to be there to
> > use tracepoints and they require root access etc).
>
> Root only should not be a problem for a developer; and aren't these
> numbers only really interesting if you're prodding at the idle governor?
What about regression testing?
> > > Would not a tracepoint be better?; then there is no overhead in the
> > > normal case where nobody gives a crap about these here numbers.
> >
> > There is an existing tracepoint that in principle could be used to
> > produce this information, but it is such a major PITA in practice that
> > nobody does that. Guess why. :-)
>
> Sounds like you need to ship a convenient script or something :-)
That would be a very ugly script. Also I'd need to expose
drv->states[i].disabled somehow (that's not accessible from user space
now).
> > Also, the "usage" and "time" counters are there in sysfs, so why not these
> > two?
> >
> > And is the overhead really that horrible?
>
> Dunno; it could be cold cachelines, at which point it can be fairly
> expensive. Also, being stuck with API is fairly horrible if you want to
> 'fix' it.
All of the cache lines involved should've been touched earlier in this
code path by the governor. At least menu and the new one both touch
them.
The API part I'm not too worried about. I know it is useful and two
other people have told that to me already. :-)
Re: [PATCH 1/2] cpufreq: intel_pstate: Force HWP min perf before offline
On Friday, November 16, 2018 11:24:19 PM CET Srinivas Pandruvada wrote:
> Force HWP Request MAX = HWP Request MIN = HWP Capability MIN and EPP to
> 0xFF. In this way the performance limits on the offlined CPU will not
> influence performance limits on its sibling CPU, which is still online.
>
> If the sibling CPU is calling for higher performance, it will impact the
> max core performance. Here core performance will follow higher of the
> performance requests from each sibling.
>
> Reported-and-tested-by: Chen Yu
> Signed-off-by: Srinivas Pandruvada
> ---
> drivers/cpufreq/intel_pstate.c | 28 ++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 14f551a3d36e..501ab9f4489a 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -830,6 +830,28 @@ static void intel_pstate_hwp_set(unsigned int cpu)
> wrmsrl_on_cpu(cpu, MSR_HWP_REQUEST, value);
> }
>
> +static void intel_pstate_hwp_force_min_perf(int cpu)
> +{
> + u64 value;
> + int min_perf;
> +
> + value = all_cpu_data[cpu]->hwp_req_cached;
> + value &= ~GENMASK_ULL(31, 0);
> + min_perf = HWP_LOWEST_PERF(all_cpu_data[cpu]->hwp_cap_cached);
> +
> + /* Set hwp_max = hwp_min */
> + value |= HWP_MAX_PERF(min_perf);
> + value |= HWP_MIN_PERF(min_perf);
> +
> + /* Set EPP/EPB to min */
> + if (static_cpu_has(X86_FEATURE_HWP_EPP))
> + value |= HWP_ENERGY_PERF_PREFERENCE(HWP_EPP_POWERSAVE);
> + else
> + intel_pstate_set_epb(cpu, HWP_EPP_BALANCE_POWERSAVE);
> +
> + wrmsrl_on_cpu(cpu, MSR_HWP_REQUEST, value);
> +}
> +
> static int intel_pstate_hwp_save_state(struct cpufreq_policy *policy)
> {
> struct cpudata *cpu_data = all_cpu_data[policy->cpu];
> @@ -2093,10 +2115,12 @@ static void intel_pstate_stop_cpu(struct
> cpufreq_policy *policy)
> pr_debug("CPU %d exiting\n", policy->cpu);
>
> intel_pstate_clear_update_util_hook(policy->cpu);
> - if (hwp_active)
> + if (hwp_active) {
> intel_pstate_hwp_save_state(policy);
> - else
> + intel_pstate_hwp_force_min_perf(policy->cpu);
> + } else {
> intel_cpufreq_stop_cpu(policy);
> + }
> }
>
> static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
>
Both this one and the [2/2] applied, thanks!
Re: [PATCH 2/3] kbuild: generate asm-generic wrappers if mandatory headers are missing
Hi Christoph, On Fri, Dec 7, 2018 at 12:04 AM Christoph Hellwig wrote: > > On Wed, Dec 05, 2018 at 08:28:05PM +0900, Masahiro Yamada wrote: > > Some time ago, Sam pointed out a certain degree of overwrap between > > generic-y and mandatory-y. (https://lkml.org/lkml/2017/7/10/121) > > > > I a bit tweaked the meaning of mandatory-y; now it defines the minimum > > set of ASM headers that all architectures must have. > > > > If arch does not have specific implementation of a mandatory header, > > Kbuild will let it fallback to the asm-generic one by automatically > > generating a wrapper. This will allow to drop lots of redundant > > generic-y defines. > > > > Previously, "mandatory" was used in the context of UAPI, but I guess > > this can be extended to kernel space ASM headers. > > How useful is it to keep the generic-y behavior around at all vs making > everything useful mandatory? What I can tell is not all architectures support kvm_para.h, ucontext.h I guess they will stay as arch-specific generic-y, but I am not an expert in this area. kvm_para.h is missing csky, nds32, riscv. ucontext.h is missing in alpha, arm, m68k, parisc, sparc, xtensa bpf_perf_event.h could be promoted to mandatory-y ? All architectures have it. -- Best Regards Masahiro Yamada
Re: [PATCH 2/3] kbuild: generate asm-generic wrappers if mandatory headers are missing
Hi Sam, On Fri, Dec 7, 2018 at 3:06 AM Sam Ravnborg wrote: > > On Wed, Dec 05, 2018 at 08:28:05PM +0900, Masahiro Yamada wrote: > > Some time ago, Sam pointed out a certain degree of overwrap between > > generic-y and mandatory-y. (https://lkml.org/lkml/2017/7/10/121) > > > > I a bit tweaked the meaning of mandatory-y; now it defines the minimum > > set of ASM headers that all architectures must have. > > > > If arch does not have specific implementation of a mandatory header, > > Kbuild will let it fallback to the asm-generic one by automatically > > generating a wrapper. This will allow to drop lots of redundant > > generic-y defines. > > > > Previously, "mandatory" was used in the context of UAPI, but I guess > > this can be extended to kernel space ASM headers. > > > > Suggested-by: Sam Ravnborg > > Signed-off-by: Masahiro Yamada > > Nice work! > > For the full series: > Acked-by: Sam Ravnborg > > Have you considered to warn if generic-y contains a header listed > in mandatory-y - to prevent that they sneak back in. > And to catch when we lift a header from available to mandatory. Yes, I also thought of this, and probably we should do it. -- Best Regards Masahiro Yamada
Re: [PATCH v3] kbuild: Add support for DT binding schema checks
On Mon, Dec 10, 2018 at 10:39 PM Masahiro Yamada
wrote:
>
> Hi Rob,
>
> On Tue, Dec 11, 2018 at 5:50 AM Rob Herring wrote:
> >
> > This adds the build infrastructure for checking DT binding schema
> > documents and validating dts files using the binding schema.
> >
> > Check DT binding schema documents:
> > make dt_binding_check
> >
> > Build dts files and check using DT binding schema:
> > make dtbs_check
> >
> > Optionally, DT_SCHEMA_FILES can passed in with a schema file(s) to use
>
>
> Perhaps, "can be passed" ?
Yes.
[...]
> > +quiet_cmd_chk_binding = CHKDT $<
>
>
> In convention, the short log displays the target name
> instead of the prerequisite.
Yes, but here there is no target. We're just running a check on the source.
> If O=foo/bar is given, $< shows the full path,
> then log will look like follows:
>
>
> CHKDT
> /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/calxeda.yaml
> DTC Documentation/devicetree/bindings/arm/calxeda.example.dtb
> CHKDT
> /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/qcom.yaml
> DTC Documentation/devicetree/bindings/arm/qcom.example.dtb
> CHKDT
> /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/xilinx.yaml
> DTC Documentation/devicetree/bindings/arm/xilinx.example.dtb
> CHKDT
> /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/ti/nspire.yaml
> DTC Documentation/devicetree/bindings/arm/ti/nspire.example.dtb
> CHKDT
> /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/ti/ti,davinci.yaml
> DTC Documentation/devicetree/bindings/arm/ti/ti,davinci.example.dtb
> CHKDT
> /home/masahiro/ref/linux-devicetree/Documentation/devicetree/bindings/arm/altera.yaml
>
> You might think it is ugly.
I've changed it to this:
quiet_cmd_chk_binding = CHKDT $(patsubst $(srctree)/%,%,$<)
[...]
> > +$(obj)/%.example.dts: $(src)/%.yaml FORCE
> > + $(call if_changed,chk_binding)
> > +
> > +DT_TMP_SCHEMA := .schema.yaml.tmp
>
>
> BTW, why does this file start with a period?
> What is the meaning of '.tmp' extension?
Nothing really. Just named it something so it gets cleaned and ignored by git.
> > +extra-y += $(DT_TMP_SCHEMA)
> > +
> > +quiet_cmd_mk_schema = SCHEMA $@
> > + cmd_mk_schema = mkdir -p $(obj); \
> > + rm -f $@; \
> > + $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@
> > $(filter-out FORCE, $^)
>
>
> "mkdir -p $(obj)" is redundant.
>
>
> Why is 'rm -f $@' necessary ?
> Can't dt-mk-schema overwrite the output file?
It is for error case when the output file is not generated. I can
handle this within dt-mk-schema instead.
> > +DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml')
> > +DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS))
> > +
> > +extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES))
> > +extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES))
>
>
>
> I assume you intentionally did not do like this:
>
> extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS))
>
> From the commit description, DT_SCHEMA_FILES might be overridden by a user.
> So, I think this is OK.
>
>
>
>
> > +$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst
> > $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)))
>
> I do not understand this line.
> Why is it necessary?
>
> *.example.dtb files are generated anyway
> since they are listed in extra-y.
It is enforcing the ordering. Without it, the binding checks and
building .schema.yaml.tmp happen in parallel because both only have
the source files as dependencies. The '|' keeps the dependencies out
of the dependency list($^).
[...]
> > +cmd_dtc_yaml_chk = \
> > + if ! echo "/dts-v1/; /{};" | $(DTC) -I dts -O yaml > /dev/null ;
> > then \
> > + echo "*"; \
> > + echo "* dtc needs libyaml for DT schema validation
> > support."; \
> > + echo "* Install the necessary libyaml development package
> > from your distro."; \
> > + echo "*"; \
> > + false; \
> > + fi; \
> > + touch $@
> > +
> > +$(objtree)/scripts/dtc/.dtc-yaml-chk.tmp: $(DTC) FORCE
> > + $(call if_changed,dtc_yaml_chk)
>
>
> Hmm, this is kind of ugly.
>
> I'd rather check this in scripts/dtc/Makefile
>
>
>
> How about something like below?
Looks good.
> ifeq ($(wildcard /usr/include/yaml.h),)
> +ifeq ($(CHECK_DTBS),1)
I'm going to change this to "ifneq ($CHECK_DTBS),)" in case we start
supporting more than 1 value such as warning levels.
> +$(error dtc needs libyaml for DT schema validation support. \
> +Install the necessary libyaml development package.)
> +endif
> HOST_EXTRACFLAGS += -DNO_YAML
> else
> dtc-objs += yamltree.o
>
>
>
>
>
> One drawback of this approach is,
> this is not checked if you are using out-of-tree DTC.
> Probably, Rob is the only person that overrides DTC.
Sa
Re: [PATCH v13 00/25] kasan: add software tag-based mode for arm64
Hi Andrey, On Thu, Dec 06, 2018 at 01:24:18PM +0100, Andrey Konovalov wrote: > This patchset adds a new software tag-based mode to KASAN [1]. > (Initially this mode was called KHWASAN, but it got renamed, > see the naming rationale at the end of this section). > > The plan is to implement HWASan [2] for the kernel with the incentive, > that it's going to have comparable to KASAN performance, but in the same > time consume much less memory, trading that off for somewhat imprecise > bug detection and being supported only for arm64. For the arm64 parts: Acked-by: Will Deacon I assume that you plan to replace the current patches in -mm with this series? Cheers, Will
Re: [PATCH v13 05/25] kasan: add CONFIG_KASAN_GENERIC and CONFIG_KASAN_SW_TAGS
On Thu, Dec 06, 2018 at 01:24:23PM +0100, Andrey Konovalov wrote:
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 3e7dafb3ea80..39f668d5066b 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -16,9 +16,13 @@
> /* all clang versions usable with the kernel support KASAN ABI version 5 */
> #define KASAN_ABI_VERSION 5
>
> +#if __has_feature(address_sanitizer) || __has_feature(hwaddress_sanitizer)
> /* emulate gcc's __SANITIZE_ADDRESS__ flag */
> -#if __has_feature(address_sanitizer)
> #define __SANITIZE_ADDRESS__
> +#define __no_sanitize_address \
> + __attribute__((no_sanitize("address", "hwaddress")))
> +#else
> +#define __no_sanitize_address
> #endif
>
> /*
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 2010493e1040..5776da43da97 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -143,6 +143,12 @@
> #define KASAN_ABI_VERSION 3
> #endif
>
> +#if __has_attribute(__no_sanitize_address__)
> +#define __no_sanitize_address __attribute__((no_sanitize_address))
> +#else
> +#define __no_sanitize_address
> +#endif
Not really important but it's the name with leading and trailing
underscores that is tested with __has_attribute() but then it's
the naked 'no_sanitize_address' that is used in the attribute.
-- Luc
Re: [PATCH v13 00/25] kasan: add software tag-based mode for arm64
[moving akpm to To:] On Tue, Dec 11, 2018 at 04:57:27PM +0100, Andrey Konovalov wrote: > On Tue, Dec 11, 2018 at 4:18 PM Will Deacon wrote: > > On Thu, Dec 06, 2018 at 01:24:18PM +0100, Andrey Konovalov wrote: > > > This patchset adds a new software tag-based mode to KASAN [1]. > > > (Initially this mode was called KHWASAN, but it got renamed, > > > see the naming rationale at the end of this section). > > > > > > The plan is to implement HWASan [2] for the kernel with the incentive, > > > that it's going to have comparable to KASAN performance, but in the same > > > time consume much less memory, trading that off for somewhat imprecise > > > bug detection and being supported only for arm64. > > > > For the arm64 parts: > > > > Acked-by: Will Deacon > > > > I assume that you plan to replace the current patches in -mm with this > > series? > > > > Cheers, > > > > Will > > Hi Will, > > Yes, that was the intention of sending v13. Should have I sent a > separate patch with v12->v13 fixes instead? I don't know what's the > usual way to make changes to the patchset once it's in the mm tree. No, I was just checking that the intention was for akpm to pick this up in preference to the old stuff! That works for me, and the minor conflict with arm64 in next was already resolved by Stephen. Will
Re: [PATCH v13 00/25] kasan: add software tag-based mode for arm64
On Tue, Dec 11, 2018 at 4:18 PM Will Deacon wrote: > > Hi Andrey, > > On Thu, Dec 06, 2018 at 01:24:18PM +0100, Andrey Konovalov wrote: > > This patchset adds a new software tag-based mode to KASAN [1]. > > (Initially this mode was called KHWASAN, but it got renamed, > > see the naming rationale at the end of this section). > > > > The plan is to implement HWASan [2] for the kernel with the incentive, > > that it's going to have comparable to KASAN performance, but in the same > > time consume much less memory, trading that off for somewhat imprecise > > bug detection and being supported only for arm64. > > For the arm64 parts: > > Acked-by: Will Deacon > > I assume that you plan to replace the current patches in -mm with this > series? > > Cheers, > > Will Hi Will, Yes, that was the intention of sending v13. Should have I sent a separate patch with v12->v13 fixes instead? I don't know what's the usual way to make changes to the patchset once it's in the mm tree. Thanks!
Re: [PATCH v13 05/25] kasan: add CONFIG_KASAN_GENERIC and CONFIG_KASAN_SW_TAGS
On Tue, Dec 11, 2018 at 4:28 PM Luc Van Oostenryck
wrote:
>
> On Thu, Dec 06, 2018 at 01:24:23PM +0100, Andrey Konovalov wrote:
> > diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> > index 3e7dafb3ea80..39f668d5066b 100644
> > --- a/include/linux/compiler-clang.h
> > +++ b/include/linux/compiler-clang.h
> > @@ -16,9 +16,13 @@
> > /* all clang versions usable with the kernel support KASAN ABI version 5 */
> > #define KASAN_ABI_VERSION 5
> >
> > +#if __has_feature(address_sanitizer) || __has_feature(hwaddress_sanitizer)
> > /* emulate gcc's __SANITIZE_ADDRESS__ flag */
> > -#if __has_feature(address_sanitizer)
> > #define __SANITIZE_ADDRESS__
> > +#define __no_sanitize_address \
> > + __attribute__((no_sanitize("address", "hwaddress")))
> > +#else
> > +#define __no_sanitize_address
> > #endif
> >
> > /*
> > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> > index 2010493e1040..5776da43da97 100644
> > --- a/include/linux/compiler-gcc.h
> > +++ b/include/linux/compiler-gcc.h
> > @@ -143,6 +143,12 @@
> > #define KASAN_ABI_VERSION 3
> > #endif
> >
> > +#if __has_attribute(__no_sanitize_address__)
> > +#define __no_sanitize_address __attribute__((no_sanitize_address))
> > +#else
> > +#define __no_sanitize_address
> > +#endif
>
> Not really important but it's the name with leading and trailing
> underscores that is tested with __has_attribute() but then it's
> the naked 'no_sanitize_address' that is used in the attribute.
Hi Luc,
You're right. This shouldn't be important though, since "__" in
attribute names are optional AFAIK. It might make sense to fix it as a
separate patch when this series is merged.
Thanks!
Re: [PATCH v3] kbuild: Add support for DT binding schema checks
On Wed, Dec 12, 2018 at 12:13 AM Rob Herring wrote: > > > > +$(obj)/%.example.dts: $(src)/%.yaml FORCE > > > + $(call if_changed,chk_binding) > > > + > > > +DT_TMP_SCHEMA := .schema.yaml.tmp > > > > > > BTW, why does this file start with a period? > > What is the meaning of '.tmp' extension? > > Nothing really. Just named it something so it gets cleaned and ignored by git. It is cleaned whatever file name you use. See scripts/Makefile.clean __clean-files := $(extra-y) $(extra-m) $(extra-) \ $(always) $(targets) $(clean-files) \ $(hostprogs-y) $(hostprogs-m) $(hostprogs-) \ $(hostlibs-y) $(hostlibs-m) $(hostlibs-) \ $(hostcxxlibs-y) $(hostcxxlibs-m) $(extra-y) is cleaned. You are adding *.example.dts to .gitignore Why not "schema.yaml" ? > > > +extra-y += $(DT_TMP_SCHEMA) > > > + > > > +quiet_cmd_mk_schema = SCHEMA $@ > > > + cmd_mk_schema = mkdir -p $(obj); \ > > > + rm -f $@; \ > > > + $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ > > > $(filter-out FORCE, $^) > > > > > > "mkdir -p $(obj)" is redundant. > > > > > > Why is 'rm -f $@' necessary ? > > Can't dt-mk-schema overwrite the output file? > > It is for error case when the output file is not generated. I can > handle this within dt-mk-schema instead. > > > +DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml') > > > +DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) > > > + > > > +extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) > > > +extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)) > > > > > > > > I assume you intentionally did not do like this: > > > > extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS)) > > > > From the commit description, DT_SCHEMA_FILES might be overridden by a user. > > So, I think this is OK. > > > > > > > > > > > +$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst > > > $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES))) > > > > I do not understand this line. > > Why is it necessary? > > > > *.example.dtb files are generated anyway > > since they are listed in extra-y. > > It is enforcing the ordering. Without it, the binding checks and > building .schema.yaml.tmp happen in parallel because both only have > the source files as dependencies. The '|' keeps the dependencies out > of the dependency list($^). What kind problem would you see if the binding checks and building .schema.yaml.tmp happen in parallel? -- Best Regards Masahiro Yamada
Re: [PATCH v13 19/25] kasan: add hooks implementation for tag-based mode
Hi Andrey, On 06/12/2018 12:24, Andrey Konovalov wrote: > This commit adds tag-based KASAN specific hooks implementation and > adjusts common generic and tag-based KASAN ones. > > 1. When a new slab cache is created, tag-based KASAN rounds up the size of >the objects in this cache to KASAN_SHADOW_SCALE_SIZE (== 16). > > 2. On each kmalloc tag-based KASAN generates a random tag, sets the shadow >memory, that corresponds to this object to this tag, and embeds this >tag value into the top byte of the returned pointer. > > 3. On each kfree tag-based KASAN poisons the shadow memory with a random >tag to allow detection of use-after-free bugs. > > The rest of the logic of the hook implementation is very much similar to > the one provided by generic KASAN. Tag-based KASAN saves allocation and > free stack metadata to the slab object the same way generic KASAN does. > > Reviewed-by: Andrey Ryabinin > Reviewed-by: Dmitry Vyukov > Signed-off-by: Andrey Konovalov > --- > mm/kasan/common.c | 116 ++ > mm/kasan/kasan.h | 8 > mm/kasan/tags.c | 48 +++ > 3 files changed, 153 insertions(+), 19 deletions(-) > [...] > @@ -265,6 +290,8 @@ void kasan_cache_create(struct kmem_cache *cache, > unsigned int *size, > return; > } > > + cache->align = round_up(cache->align, KASAN_SHADOW_SCALE_SIZE); > + Did you consider to set ARCH_SLAB_MINALIGN instead of this round up? -- Regards, Vincenzo
Re: [PATCH v5 4/7] cgroup: cgroup v2 freezer
On 12/07, Roman Gushchin wrote:
>
> Cgroup v2 freezer tries to put tasks into a state similar to jobctl
> stop. This means that tasks can be killed, ptraced (using
> PTRACE_SEIZE*), and interrupted. It is possible to attach to
> a frozen task, get some information (e.g. read registers) and detach.
I fail to understand how this all supposed to work.
> @@ -368,6 +369,8 @@ static inline int signal_pending_state(long state, struct
> task_struct *p)
> return 0;
> if (!signal_pending(p))
> return 0;
> + if (unlikely(cgroup_task_frozen(p) && p->jobctl == JOBCTL_TRAP_FREEZE))
> + return __fatal_signal_pending(p);
I think I will never agree with this change ;) and I don't think it actually
helps.
> +void cgroup_enter_frozen(void)
> +{
> + if (!current->frozen) {
> + spin_lock_irq(&css_set_lock);
> + current->frozen = true;
> + cgroup_inc_frozen_cnt(task_dfl_cgroup(current), false, true);
> + spin_unlock_irq(&css_set_lock);
> + }
> +
> + __set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
So once again, suppose it races with PTRACE_INTERRUPT, or SIGSTOP, or something
else which should be handled by get_signal() before do_freezer_trap().
If (say) PTRACE_INTERRUPT comes before schedule it will be lost. Otherwise
the frozen task will react. This can't be right. Or I am totally confused.
Perhaps you can split this patch? start with cgroup_enter_frozen() using
TASK_KILLABLE, then teach it to handle ptrace/stop/etc? I think this way it
would be simpler to discuss the necessary changes and document what exactly
are you trying to do.
and btw what about suspend? try_to_freeze_tasks() will obviously fail
if there is a ->frozen thread?
Oleg.
Re: [PATCH 2/3] kbuild: generate asm-generic wrappers if mandatory headers are missing
On Tue, 11 Dec 2018 04:59:26 PST (-0800), yamada.masah...@socionext.com wrote: > Hi Christoph, > > > On Fri, Dec 7, 2018 at 12:04 AM Christoph Hellwig wrote: >> >> On Wed, Dec 05, 2018 at 08:28:05PM +0900, Masahiro Yamada wrote: >> > Some time ago, Sam pointed out a certain degree of overwrap between >> > generic-y and mandatory-y. (https://lkml.org/lkml/2017/7/10/121) >> > >> > I a bit tweaked the meaning of mandatory-y; now it defines the minimum >> > set of ASM headers that all architectures must have. >> > >> > If arch does not have specific implementation of a mandatory header, >> > Kbuild will let it fallback to the asm-generic one by automatically >> > generating a wrapper. This will allow to drop lots of redundant >> > generic-y defines. >> > >> > Previously, "mandatory" was used in the context of UAPI, but I guess >> > this can be extended to kernel space ASM headers. >> >> How useful is it to keep the generic-y behavior around at all vs making >> everything useful mandatory? > > > What I can tell is not all architectures > support kvm_para.h, ucontext.h > > I guess they will stay as arch-specific generic-y, > but I am not an expert in this area. > > > kvm_para.h is missing csky, nds32, riscv. It looks like RISC-V missed it and everyone else copied us. I don't see any reason why the generic version wouldn't work on RISC-V, as it just has failures for all the calls. > ucontext.h is missing in alpha, arm, m68k, parisc, sparc, xtensa > > > > bpf_perf_event.h could be promoted to mandatory-y ? > All architectures have it.
Re: [PATCH v11 00/13] Intel SGX1 support
On 12/10/18 3:12 PM, Josh Triplett wrote: >> Or maybe even python/shell scripts? It looked to me like virtual >> memory will be "interesting" for enclaves. > Memory management doesn't seem that hard to deal with. The problems are: 1. SGX enclave memory (EPC) is statically allocated at boot and can't grow or shrink 2. EPC is much smaller than regular RAM 3. The core VM has no comprehension of EPC use, thus can not help with its algorithms, like the LRU 4. The SGX driver implements its own VM which is substantially simpler than the core VM, but less feature-rich, fast, or scalable
Re: [PATCH v11 00/13] Intel SGX1 support
On Tue, Dec 11, 2018 at 10:10:38AM -0800, Dave Hansen wrote: > On 12/10/18 3:12 PM, Josh Triplett wrote: > >> Or maybe even python/shell scripts? It looked to me like virtual > >> memory will be "interesting" for enclaves. > > Memory management doesn't seem that hard to deal with. > > The problems are: > > 1. SGX enclave memory (EPC) is statically allocated at boot and can't >grow or shrink > 2. EPC is much smaller than regular RAM > 3. The core VM has no comprehension of EPC use, thus can not help >with its algorithms, like the LRU > 4. The SGX driver implements its own VM which is substantially simpler >than the core VM, but less feature-rich, fast, or scalable I'd also add: 5. Swapping EPC pages can only be done through SGX specific ISA that has strict concurrency requirements and enforces TLB flushing. 6. There are specialized types of EPC pages that have different swapping requirements than regular EPC pages. 7. EPC pages that are exposed to a KVM guest have yet another set of swapping requirements. In other words, extending the core VM to SGX EPC is painfully difficult.
Re: [PATCH v3] kbuild: Add support for DT binding schema checks
On Tue, Dec 11, 2018 at 10:03 AM Masahiro Yamada wrote: > > On Wed, Dec 12, 2018 at 12:13 AM Rob Herring wrote: > > > > > > > +$(obj)/%.example.dts: $(src)/%.yaml FORCE > > > > + $(call if_changed,chk_binding) > > > > + > > > > +DT_TMP_SCHEMA := .schema.yaml.tmp > > > > > > > > > BTW, why does this file start with a period? > > > What is the meaning of '.tmp' extension? > > > > Nothing really. Just named it something so it gets cleaned and ignored by > > git. > > > It is cleaned whatever file name you use. > > > See scripts/Makefile.clean > > __clean-files := $(extra-y) $(extra-m) $(extra-) \ >$(always) $(targets) $(clean-files) \ >$(hostprogs-y) $(hostprogs-m) $(hostprogs-) \ >$(hostlibs-y) $(hostlibs-m) $(hostlibs-) \ >$(hostcxxlibs-y) $(hostcxxlibs-m) > > > $(extra-y) is cleaned. True. > > > You are adding *.example.dts to .gitignore > > Why not "schema.yaml" ? Okay. I'll do "processed-schema.yaml" to give a bit better name of what it contains. > > > > > +extra-y += $(DT_TMP_SCHEMA) > > > > + > > > > +quiet_cmd_mk_schema = SCHEMA $@ > > > > + cmd_mk_schema = mkdir -p $(obj); \ > > > > + rm -f $@; \ > > > > + $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ > > > > $(filter-out FORCE, $^) > > > > > > > > > "mkdir -p $(obj)" is redundant. > > > > > > > > > Why is 'rm -f $@' necessary ? > > > Can't dt-mk-schema overwrite the output file? > > > > It is for error case when the output file is not generated. I can > > handle this within dt-mk-schema instead. > > > > +DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml') > > > > +DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) > > > > + > > > > +extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) > > > > +extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)) > > > > > > > > > > > > I assume you intentionally did not do like this: > > > > > > extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS)) > > > > > > From the commit description, DT_SCHEMA_FILES might be overridden by a > > > user. > > > So, I think this is OK. > > > > > > > > > > > > > > > > +$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst > > > > $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES))) > > > > > > I do not understand this line. > > > Why is it necessary? > > > > > > *.example.dtb files are generated anyway > > > since they are listed in extra-y. > > > > It is enforcing the ordering. Without it, the binding checks and > > building .schema.yaml.tmp happen in parallel because both only have > > the source files as dependencies. The '|' keeps the dependencies out > > of the dependency list($^). > > > What kind problem would you see if > the binding checks and building .schema.yaml.tmp > happen in parallel? In case of no errors in the binding docs, it doesn't matter. If there are errors, I don't want the dtbs validation to run if any schema doesn't validate. However, I played around with this a bit more and it seems like having the examples' dts/dtb in extra-y prevents that from happening. Does that match your expections? If so, then I think we can remove the dependency. Here's an example. SCHEMA Documentation/devicetree/bindings/.schema.yaml.tmp CHKDT Documentation/devicetree/bindings/arm/vt8500.yaml CHKDT Documentation/devicetree/bindings/arm/zte.yaml CHKDT Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.yaml CHKDT Documentation/devicetree/bindings/arm/ti/ti,davinci.yaml CHKDT Documentation/devicetree/bindings/arm/ti/nspire.yaml CHKDT Documentation/devicetree/bindings/arm/spear.yaml CHKDT Documentation/devicetree/bindings/arm/altera.yaml warning: no schema found in file: ../Documentation/devicetree/bindings/arm/xilinx.yaml /home/rob/proj/git/linux-2.6/Documentation/devicetree/bindings/arm/xilinx.yaml: ignoring, error in schema 'onOf' CHKDT Documentation/devicetree/bindings/arm/sti.yaml CHKDT Documentation/devicetree/bindings/arm/qcom.yaml CHKDT Documentation/devicetree/bindings/arm/primecell.yaml CHKDT Documentation/devicetree/bindings/arm/cpus.yaml CHKDT Documentation/devicetree/bindings/arm/sirf.yaml CHKDT Documentation/devicetree/bindings/arm/calxeda.yaml CHKDT Documentation/devicetree/bindings/arm/xilinx.yaml CHKDT Documentation/devicetree/bindings/example-schema.yaml /home/rob/proj/git/linux-2.6/Documentation/devicetree/bindings/arm/xilinx.yaml: properties:compatible:onOf: 'onOf' is not one of ['$ref', 'additionalItems', 'allOf', 'const', 'contains', 'default', 'dependencies', 'description', 'enum', 'items', 'minItems', 'minimum', 'maxItems', 'maximum', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'type', 'typeSize'] make[2]: *** [../Documentation/devicetree/bindings/Makefile:12: Documentation/devicetree/bindings/arm/xilinx.example.dts] Error 1 Rob
Re: [PATCH v5 4/7] cgroup: cgroup v2 freezer
On Tue, Dec 11, 2018 at 05:26:32PM +0100, Oleg Nesterov wrote:
> On 12/07, Roman Gushchin wrote:
> >
> > Cgroup v2 freezer tries to put tasks into a state similar to jobctl
> > stop. This means that tasks can be killed, ptraced (using
> > PTRACE_SEIZE*), and interrupted. It is possible to attach to
> > a frozen task, get some information (e.g. read registers) and detach.
>
> I fail to understand how this all supposed to work.
>
> > @@ -368,6 +369,8 @@ static inline int signal_pending_state(long state,
> > struct task_struct *p)
> > return 0;
> > if (!signal_pending(p))
> > return 0;
> > + if (unlikely(cgroup_task_frozen(p) && p->jobctl == JOBCTL_TRAP_FREEZE))
> > + return __fatal_signal_pending(p);
>
> I think I will never agree with this change ;) and I don't think it actually
> helps.
See below.
>
> > +void cgroup_enter_frozen(void)
> > +{
> > + if (!current->frozen) {
> > + spin_lock_irq(&css_set_lock);
> > + current->frozen = true;
> > + cgroup_inc_frozen_cnt(task_dfl_cgroup(current), false, true);
> > + spin_unlock_irq(&css_set_lock);
> > + }
> > +
> > + __set_current_state(TASK_INTERRUPTIBLE);
> > + schedule();
>
> So once again, suppose it races with PTRACE_INTERRUPT, or SIGSTOP, or
> something
> else which should be handled by get_signal() before do_freezer_trap().
>
> If (say) PTRACE_INTERRUPT comes before schedule it will be lost. Otherwise
> the frozen task will react. This can't be right. Or I am totally confused.
Why?
PTRACE_INTERRUPT will set JOBCTL_TRAP_STOP, so signal_pending_state()
will return true, schedule() will return immediately, and we'll handle the trap.
>
> Perhaps you can split this patch? start with cgroup_enter_frozen() using
> TASK_KILLABLE, then teach it to handle ptrace/stop/etc? I think this way it
> would be simpler to discuss the necessary changes and document what exactly
> are you trying to do.
>
> and btw what about suspend? try_to_freeze_tasks() will obviously fail
> if there is a ->frozen thread?
I have to think a bit more here, but something like this will probably work:
diff --git a/kernel/freezer.c b/kernel/freezer.c
index b162b74611e4..590ac4d10b02 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -134,7 +134,7 @@ bool freeze_task(struct task_struct *p)
return false;
spin_lock_irqsave(&freezer_lock, flags);
- if (!freezing(p) || frozen(p)) {
+ if (!freezing(p) || frozen(p) || cgroup_task_frozen()) {
spin_unlock_irqrestore(&freezer_lock, flags);
return false;
}
--
If the task is already frozen by the cgroup freezer, we don't have to do
anything additionally.
Thanks!
[PATCH v4] kbuild: Add support for DT binding schema checks
This adds the build infrastructure for checking DT binding schema documents and validating dts files using the binding schema. Check DT binding schema documents: make dt_binding_check Build dts files and check using DT binding schema: make dtbs_check Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to use for validation. This makes it easier to find and fix errors generated by a specific schema. Currently, the validation targets are separate from a normal build to avoid a hard dependency on the external DT schema project and because there are lots of warnings generated. Cc: Jonathan Corbet Cc: Mark Rutland Cc: Masahiro Yamada Cc: Michal Marek Cc: linux-doc@vger.kernel.org Cc: devicet...@vger.kernel.org Cc: linux-kbu...@vger.kernel.org Signed-off-by: Rob Herring --- v4: - Rework libyaml check and error message with Masahiro's version - Simplify build rules and dependencies .gitignore | 1 + Documentation/Makefile | 2 +- Documentation/devicetree/bindings/.gitignore | 2 ++ Documentation/devicetree/bindings/Makefile | 27 Makefile | 13 +++--- scripts/Makefile.lib | 24 +++-- scripts/dtc/Makefile | 4 +++ 7 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/.gitignore create mode 100644 Documentation/devicetree/bindings/Makefile diff --git a/.gitignore b/.gitignore index 97ba6b79834c..a20ac26aa2f5 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ *.bin *.bz2 *.c.[012]*.* +*.dt.yaml *.dtb *.dtb.S *.dwo diff --git a/Documentation/Makefile b/Documentation/Makefile index 2ca77ad0f238..9786957c6a35 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -2,7 +2,7 @@ # Makefile for Sphinx documentation # -subdir-y := +subdir-y := devicetree/bindings/ # You can set these variables from the command line. SPHINXBUILD = sphinx-build diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore new file mode 100644 index ..ef82fcfcccab --- /dev/null +++ b/Documentation/devicetree/bindings/.gitignore @@ -0,0 +1,2 @@ +*.example.dts +processed-schema.yaml diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile new file mode 100644 index ..6e5cef0ed6fb --- /dev/null +++ b/Documentation/devicetree/bindings/Makefile @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: GPL-2.0 +DT_DOC_CHECKER ?= dt-doc-validate +DT_EXTRACT_EX ?= dt-extract-example +DT_MK_SCHEMA ?= dt-mk-schema +DT_MK_SCHEMA_FLAGS := $(if $(DT_SCHEMA_FILES), -u) + +quiet_cmd_chk_binding = CHKDT $(patsubst $(srctree)/%,%,$<) + cmd_chk_binding = $(DT_DOC_CHECKER) $< ; \ +$(DT_EXTRACT_EX) $< > $@ + +$(obj)/%.example.dts: $(src)/%.yaml FORCE + $(call if_changed,chk_binding) + +DT_TMP_SCHEMA := processed-schema.yaml +extra-y += $(DT_TMP_SCHEMA) + +quiet_cmd_mk_schema = SCHEMA $@ + cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^) + +DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml') +DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) + +extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) +extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES)) + +$(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE + $(call if_changed,mk_schema) diff --git a/Makefile b/Makefile index 2f36db897895..a3e2db2a3119 100644 --- a/Makefile +++ b/Makefile @@ -1232,10 +1232,13 @@ ifneq ($(dtstree),) %.dtb: prepare3 scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ -PHONY += dtbs dtbs_install -dtbs: prepare3 scripts_dtc +PHONY += dtbs dtbs_install dt_binding_check +dtbs dtbs_check: prepare3 scripts_dtc $(Q)$(MAKE) $(build)=$(dtstree) +dtbs_check: export CHECK_DTBS=1 +dtbs_check: dt_binding_check + dtbs_install: $(Q)$(MAKE) $(dtbinst)=$(dtstree) @@ -1249,6 +1252,9 @@ PHONY += scripts_dtc scripts_dtc: scripts_basic $(Q)$(MAKE) $(build)=scripts/dtc +dt_binding_check: scripts_dtc + $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings + # --- # Modules @@ -1611,7 +1617,8 @@ clean: $(clean-dirs) $(call cmd,rmfiles) @find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \ \( -name '*.[aios]' -o -name '*.ko' -o -name '.*.cmd' \ - -o -name '*.ko.*' -o -name '*.dtb' -o -name '*.dtb.S' \ + -o -name '*.ko.*' \ + -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ -o -name '*.dwo' -o -name '*.lst' \ -o -name '*.su' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ diff --git a/scripts/Makefile.li
Re: [PATCH v13 00/25] kasan: add software tag-based mode for arm64
On Tue, 11 Dec 2018 16:00:19 + Will Deacon wrote: > > Yes, that was the intention of sending v13. Should have I sent a > > separate patch with v12->v13 fixes instead? I don't know what's the > > usual way to make changes to the patchset once it's in the mm tree. I usually convert replacement patches into deltas so people can see what changed. In this case it got messy so I dropped v12 and remerged v13 wholesale.
Re: [PATCH V2 3/7] fscrypt: remove filesystem specific build config option
Hi, On Tue, Dec 04, 2018 at 03:26:46PM +0530, Chandan Rajendra wrote: > In order to have a common code base for fscrypt "post read" processing > for all filesystems which support encryption, this commit removes > filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION) > and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose > value affects all the filesystems making use of fscrypt. > > Signed-off-by: Chandan Rajendra this patch causes a recursive dependency when trying to build ia64 images. make ARCH=ia64 allnoconfig: scripts/kconfig/conf --allnoconfig Kconfig arch/ia64/Kconfig:126:error: recursive dependency detected! arch/ia64/Kconfig:126: choice contains symbol IA64_HP_SIM arch/ia64/Kconfig:200: symbol IA64_HP_SIM is part of choice PM kernel/power/Kconfig:144: symbol PM is selected by PM_SLEEP kernel/power/Kconfig:104: symbol PM_SLEEP depends on HIBERNATE_CALLBACKS kernel/power/Kconfig:31:symbol HIBERNATE_CALLBACKS is selected by HIBERNATION kernel/power/Kconfig:34:symbol HIBERNATION depends on SWAP init/Kconfig:250: symbol SWAP depends on BLOCK block/Kconfig:5:symbol BLOCK is selected by UBIFS_FS fs/ubifs/Kconfig:1: symbol UBIFS_FS depends on MISC_FILESYSTEMS fs/Kconfig:220: symbol MISC_FILESYSTEMS is selected by ACPI_APEI drivers/acpi/apei/Kconfig:8:symbol ACPI_APEI depends on ACPI drivers/acpi/Kconfig:9: symbol ACPI depends on ARCH_SUPPORTS_ACPI drivers/acpi/Kconfig:6: symbol ARCH_SUPPORTS_ACPI is selected by IA64_HP_SIM arch/ia64/Kconfig:200: symbol IA64_HP_SIM is part of choice For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" scripts/kconfig/Makefile:69: recipe for target 'allnoconfig' failed Reverting the patch fixes the problem. Guenter --- bisect log: # bad: [60bec71b5acb0b469d07e73a348f6610236ae7fa] Add linux-next specific files for 20181211 # good: [40e020c129cfc991e8ab4736d2665351ffd1468d] Linux 4.20-rc6 git bisect start 'HEAD' 'v4.20-rc6' # bad: [9d9400c203d670e3b002336bf3a70a34c8023853] Merge remote-tracking branch 'spi-nor/spi-nor/next' git bisect bad 9d9400c203d670e3b002336bf3a70a34c8023853 # bad: [98d439a801d02e313d762fa08368b680e4c0d961] Merge remote-tracking branch 'vfs/for-next' git bisect bad 98d439a801d02e313d762fa08368b680e4c0d961 # good: [79271eb86e6aca36f962657ea4efa248fc84e2fa] Merge remote-tracking branch 'samsung-krzk/for-next' git bisect good 79271eb86e6aca36f962657ea4efa248fc84e2fa # good: [b506abfb5f33d7358cfc9818054e3a834813f7af] Merge remote-tracking branch 'mips/mips-next' git bisect good b506abfb5f33d7358cfc9818054e3a834813f7af # bad: [d58e2b0647688a51a96e373589c0597f493ba066] Merge remote-tracking branch 'befs/for-next' git bisect bad d58e2b0647688a51a96e373589c0597f493ba066 # good: [39f8d343f04868e14abdf1c7ca31d53855bf6f81] Merge remote-tracking branch 'risc-v/for-next' git bisect good 39f8d343f04868e14abdf1c7ca31d53855bf6f81 # good: [e2f3efdaf1b66f9ac1a5ae16a99a1ff5b0956515] Merge branch 'xtensa-cleanups' into xtensa-for-next git bisect good e2f3efdaf1b66f9ac1a5ae16a99a1ff5b0956515 # bad: [25da54f1d777ac4a612d9b036f8ac41baf746674] fsverity: Move verity status check to fsverity_prepare_setattr git bisect bad 25da54f1d777ac4a612d9b036f8ac41baf746674 # good: [a391d6149e706be54aeed8757769910f1b2445d4] fs-verity: add CRC-32C support git bisect good a391d6149e706be54aeed8757769910f1b2445d4 # good: [848a010287e6a02f1e46c344bbffeb987ed2a0aa] f2fs: use IS_ENCRYPTED() to check encryption status git bisect good 848a010287e6a02f1e46c344bbffeb987ed2a0aa # bad: [9f55ada08b452bde55c02ae9bc97c19990ff5c36] ext4: use IS_VERITY() to check inode's fsverity status git bisect bad 9f55ada08b452bde55c02ae9bc97c19990ff5c36 # bad: [824834dc4a5e595ef24ba44086efa56b97ad4990] Add S_VERITY and IS_VERITY() git bisect bad 824834dc4a5e595ef24ba44086efa56b97ad4990 # bad: [6956097c429aae498d26b8603b2dec39250b8940] fscrypt: remove filesystem specific build config option git bisect bad 6956097c429aae498d26b8603b2dec39250b8940 # first bad commit: [6956097c429aae498d26b8603b2dec39250b8940] fscrypt: remove filesystem specific build config option
Re: [PATCH V2 3/7] fscrypt: remove filesystem specific build config option
On Tue, Dec 11, 2018 at 05:52:11PM -0800, Guenter Roeck wrote: > Hi, > > On Tue, Dec 04, 2018 at 03:26:46PM +0530, Chandan Rajendra wrote: > > In order to have a common code base for fscrypt "post read" processing > > for all filesystems which support encryption, this commit removes > > filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION) > > and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose > > value affects all the filesystems making use of fscrypt. > > > > Signed-off-by: Chandan Rajendra > > this patch causes a recursive dependency when trying to build ia64 images. > > make ARCH=ia64 allnoconfig: > > scripts/kconfig/conf --allnoconfig Kconfig > arch/ia64/Kconfig:126:error: recursive dependency detected! > arch/ia64/Kconfig:126:choice contains symbol IA64_HP_SIM > arch/ia64/Kconfig:200:symbol IA64_HP_SIM is part of choice PM > kernel/power/Kconfig:144: symbol PM is selected by PM_SLEEP > kernel/power/Kconfig:104: symbol PM_SLEEP depends on HIBERNATE_CALLBACKS > kernel/power/Kconfig:31: symbol HIBERNATE_CALLBACKS is selected by > HIBERNATION > kernel/power/Kconfig:34: symbol HIBERNATION depends on SWAP > init/Kconfig:250: symbol SWAP depends on BLOCK > block/Kconfig:5: symbol BLOCK is selected by UBIFS_FS > fs/ubifs/Kconfig:1: symbol UBIFS_FS depends on MISC_FILESYSTEMS > fs/Kconfig:220: symbol MISC_FILESYSTEMS is selected by ACPI_APEI > drivers/acpi/apei/Kconfig:8: symbol ACPI_APEI depends on ACPI > drivers/acpi/Kconfig:9: symbol ACPI depends on ARCH_SUPPORTS_ACPI > drivers/acpi/Kconfig:6: symbol ARCH_SUPPORTS_ACPI is selected by > IA64_HP_SIM > arch/ia64/Kconfig:200:symbol IA64_HP_SIM is part of choice > For a resolution refer to Documentation/kbuild/kconfig-language.txt > subsection "Kconfig recursive dependency limitations" > > scripts/kconfig/Makefile:69: recipe for target 'allnoconfig' failed > > Reverting the patch fixes the problem. > Thanks for the report. Chandan, it appears the problem is UBIFS_FS selecting BLOCK. It's actually not necessary because now the parts of fs/crypto/ that depend on BLOCK are separated out into a separate file fs/crypto/bio.c that is only compiled with CONFIG_BLOCK. So how about just removing the selection of BLOCK from fs/ubifs/Kconfig: select BLOCK if FS_ENCRYPTION - Eric
Re: [PATCH v3] kbuild: Add support for DT binding schema checks
On Wed, Dec 12, 2018 at 3:36 AM Rob Herring wrote: > > On Tue, Dec 11, 2018 at 10:03 AM Masahiro Yamada > wrote: > > > > On Wed, Dec 12, 2018 at 12:13 AM Rob Herring wrote: > > > > > > > > > > +$(obj)/%.example.dts: $(src)/%.yaml FORCE > > > > > + $(call if_changed,chk_binding) > > > > > + > > > > > +DT_TMP_SCHEMA := .schema.yaml.tmp > > > > > > > > > > > > BTW, why does this file start with a period? > > > > What is the meaning of '.tmp' extension? > > > > > > Nothing really. Just named it something so it gets cleaned and ignored by > > > git. > > > > > > It is cleaned whatever file name you use. > > > > > > See scripts/Makefile.clean > > > > __clean-files := $(extra-y) $(extra-m) $(extra-) \ > >$(always) $(targets) $(clean-files) \ > >$(hostprogs-y) $(hostprogs-m) $(hostprogs-) \ > >$(hostlibs-y) $(hostlibs-m) $(hostlibs-) \ > >$(hostcxxlibs-y) $(hostcxxlibs-m) > > > > > > $(extra-y) is cleaned. > > True. > > > > > > > You are adding *.example.dts to .gitignore > > > > Why not "schema.yaml" ? > > Okay. I'll do "processed-schema.yaml" to give a bit better name of > what it contains. > > > > > > > > +extra-y += $(DT_TMP_SCHEMA) > > > > > + > > > > > +quiet_cmd_mk_schema = SCHEMA $@ > > > > > + cmd_mk_schema = mkdir -p $(obj); \ > > > > > + rm -f $@; \ > > > > > + $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ > > > > > $(filter-out FORCE, $^) > > > > > > > > > > > > "mkdir -p $(obj)" is redundant. > > > > > > > > > > > > Why is 'rm -f $@' necessary ? > > > > Can't dt-mk-schema overwrite the output file? > > > > > > It is for error case when the output file is not generated. I can > > > handle this within dt-mk-schema instead. > > > > > +DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml') > > > > > +DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) > > > > > + > > > > > +extra-y += $(patsubst $(src)/%.yaml,%.example.dts, > > > > > $(DT_SCHEMA_FILES)) > > > > > +extra-y += $(patsubst $(src)/%.yaml,%.example.dtb, > > > > > $(DT_SCHEMA_FILES)) > > > > > > > > > > > > > > > > I assume you intentionally did not do like this: > > > > > > > > extra-y += $(patsubst %.yaml,%.example.dtb, $(DT_DOCS)) > > > > > > > > From the commit description, DT_SCHEMA_FILES might be overridden by a > > > > user. > > > > So, I think this is OK. > > > > > > > > > > > > > > > > > > > > > +$(obj)/$(DT_TMP_SCHEMA): | $(addprefix $(obj)/,$(patsubst > > > > > $(src)/%.yaml,%.example.dtb, $(DT_SCHEMA_FILES))) > > > > > > > > I do not understand this line. > > > > Why is it necessary? > > > > > > > > *.example.dtb files are generated anyway > > > > since they are listed in extra-y. > > > > > > It is enforcing the ordering. Without it, the binding checks and > > > building .schema.yaml.tmp happen in parallel because both only have > > > the source files as dependencies. The '|' keeps the dependencies out > > > of the dependency list($^). > > > > > > What kind problem would you see if > > the binding checks and building .schema.yaml.tmp > > happen in parallel? > > In case of no errors in the binding docs, it doesn't matter. If there > are errors, I don't want the dtbs validation to run if any schema > doesn't validate. However, I played around with this a bit more and it > seems like having the examples' dts/dtb in extra-y prevents that from > happening. Does that match your expections? Exactly. If any error occurs in Documentation/devicetree/bindings/Makefile, Make terminates before proceeding to the dtbs_check stage. -- Best Regards Masahiro Yamada
Re: [PATCH v4] kbuild: Add support for DT binding schema checks
On Wed, Dec 12, 2018 at 5:24 AM Rob Herring wrote: > > This adds the build infrastructure for checking DT binding schema > documents and validating dts files using the binding schema. > > Check DT binding schema documents: > make dt_binding_check > > Build dts files and check using DT binding schema: > make dtbs_check > > Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to > use for validation. This makes it easier to find and fix errors > generated by a specific schema. > > Currently, the validation targets are separate from a normal build to > avoid a hard dependency on the external DT schema project and because > there are lots of warnings generated. > > Cc: Jonathan Corbet > Cc: Mark Rutland > Cc: Masahiro Yamada > Cc: Michal Marek > Cc: linux-doc@vger.kernel.org > Cc: devicet...@vger.kernel.org > Cc: linux-kbu...@vger.kernel.org > Signed-off-by: Rob Herring > --- > v4: > - Rework libyaml check and error message with Masahiro's version > - Simplify build rules and dependencies > Acked-by: Masahiro Yamada Thanks. -- Best Regards Masahiro Yamada
Re: [PATCH V2 3/7] fscrypt: remove filesystem specific build config option
On 12/11/18 6:48 PM, Eric Biggers wrote: On Tue, Dec 11, 2018 at 05:52:11PM -0800, Guenter Roeck wrote: Hi, On Tue, Dec 04, 2018 at 03:26:46PM +0530, Chandan Rajendra wrote: In order to have a common code base for fscrypt "post read" processing for all filesystems which support encryption, this commit removes filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION) and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose value affects all the filesystems making use of fscrypt. Signed-off-by: Chandan Rajendra this patch causes a recursive dependency when trying to build ia64 images. make ARCH=ia64 allnoconfig: scripts/kconfig/conf --allnoconfig Kconfig arch/ia64/Kconfig:126:error: recursive dependency detected! arch/ia64/Kconfig:126: choice contains symbol IA64_HP_SIM arch/ia64/Kconfig:200: symbol IA64_HP_SIM is part of choice PM kernel/power/Kconfig:144: symbol PM is selected by PM_SLEEP kernel/power/Kconfig:104: symbol PM_SLEEP depends on HIBERNATE_CALLBACKS kernel/power/Kconfig:31:symbol HIBERNATE_CALLBACKS is selected by HIBERNATION kernel/power/Kconfig:34:symbol HIBERNATION depends on SWAP init/Kconfig:250: symbol SWAP depends on BLOCK block/Kconfig:5:symbol BLOCK is selected by UBIFS_FS fs/ubifs/Kconfig:1: symbol UBIFS_FS depends on MISC_FILESYSTEMS fs/Kconfig:220: symbol MISC_FILESYSTEMS is selected by ACPI_APEI drivers/acpi/apei/Kconfig:8:symbol ACPI_APEI depends on ACPI drivers/acpi/Kconfig:9: symbol ACPI depends on ARCH_SUPPORTS_ACPI drivers/acpi/Kconfig:6: symbol ARCH_SUPPORTS_ACPI is selected by IA64_HP_SIM arch/ia64/Kconfig:200: symbol IA64_HP_SIM is part of choice For a resolution refer to Documentation/kbuild/kconfig-language.txt subsection "Kconfig recursive dependency limitations" scripts/kconfig/Makefile:69: recipe for target 'allnoconfig' failed Reverting the patch fixes the problem. Thanks for the report. Chandan, it appears the problem is UBIFS_FS selecting BLOCK. It's actually not necessary because now the parts of fs/crypto/ that depend on BLOCK are separated out into a separate file fs/crypto/bio.c that is only compiled with CONFIG_BLOCK. So how about just removing the selection of BLOCK from fs/ubifs/Kconfig: select BLOCK if FS_ENCRYPTION The recursion is gone if I make this change. Guenter
[PATCH V4 2/9] f2fs: use IS_ENCRYPTED() to check encryption status
This commit removes the f2fs specific f2fs_encrypted_inode() and makes
use of the generic IS_ENCRYPTED() macro to check for the encryption
status of an inode.
Acked-by: Chao Yu
Reviewed-by: Eric Biggers
Signed-off-by: Chandan Rajendra
---
fs/f2fs/data.c | 4 ++--
fs/f2fs/dir.c | 10 +-
fs/f2fs/f2fs.h | 7 +--
fs/f2fs/file.c | 10 +-
fs/f2fs/inode.c | 4 ++--
fs/f2fs/namei.c | 6 +++---
6 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 09d9fc1676a7..844ec573263e 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1491,7 +1491,7 @@ int f2fs_fiemap(struct inode *inode, struct
fiemap_extent_info *fieinfo,
}
if (size) {
- if (f2fs_encrypted_inode(inode))
+ if (IS_ENCRYPTED(inode))
flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
ret = fiemap_fill_next_extent(fieinfo, logical,
@@ -1764,7 +1764,7 @@ static inline bool check_inplace_update_policy(struct
inode *inode,
if (policy & (0x1 << F2FS_IPU_ASYNC) &&
fio && fio->op == REQ_OP_WRITE &&
!(fio->op_flags & REQ_SYNC) &&
- !f2fs_encrypted_inode(inode))
+ !IS_ENCRYPTED(inode))
return true;
/* this is only set during fdatasync */
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index bacc667950b6..cf9e2564388d 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -385,7 +385,7 @@ struct page *f2fs_init_inode_metadata(struct inode *inode,
struct inode *dir,
if (err)
goto put_error;
- if ((f2fs_encrypted_inode(dir) || dummy_encrypt) &&
+ if ((IS_ENCRYPTED(dir) || dummy_encrypt) &&
f2fs_may_encrypt(inode)) {
err = fscrypt_inherit_context(dir, inode, page, false);
if (err)
@@ -399,7 +399,7 @@ struct page *f2fs_init_inode_metadata(struct inode *inode,
struct inode *dir,
if (new_name) {
init_dent_inode(new_name, page);
- if (f2fs_encrypted_inode(dir))
+ if (IS_ENCRYPTED(dir))
file_set_enc_name(inode);
}
@@ -808,7 +808,7 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct
f2fs_dentry_ptr *d,
de_name.name = d->filename[bit_pos];
de_name.len = le16_to_cpu(de->name_len);
- if (f2fs_encrypted_inode(d->inode)) {
+ if (IS_ENCRYPTED(d->inode)) {
int save_len = fstr->len;
err = fscrypt_fname_disk_to_usr(d->inode,
@@ -852,7 +852,7 @@ static int f2fs_readdir(struct file *file, struct
dir_context *ctx)
struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
int err = 0;
- if (f2fs_encrypted_inode(inode)) {
+ if (IS_ENCRYPTED(inode)) {
err = fscrypt_get_encryption_info(inode);
if (err && err != -ENOKEY)
goto out;
@@ -914,7 +914,7 @@ static int f2fs_readdir(struct file *file, struct
dir_context *ctx)
static int f2fs_dir_open(struct inode *inode, struct file *filp)
{
- if (f2fs_encrypted_inode(inode))
+ if (IS_ENCRYPTED(inode))
return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
return 0;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index dadb5f468f20..368c7e78fe6b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3436,14 +3436,9 @@ void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi);
/*
* crypto support
*/
-static inline bool f2fs_encrypted_inode(struct inode *inode)
-{
- return file_is_encrypt(inode);
-}
-
static inline bool f2fs_encrypted_file(struct inode *inode)
{
- return f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode);
+ return IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode);
}
static inline void f2fs_set_encrypted_inode(struct inode *inode)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 87794b2a45ff..6c7ad15000b9 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -585,7 +585,7 @@ static int truncate_partial_data_page(struct inode *inode,
u64 from,
zero_user(page, offset, PAGE_SIZE - offset);
/* An encrypted inode should have a key and truncate the last page. */
- f2fs_bug_on(F2FS_I_SB(inode), cache_only &&
f2fs_encrypted_inode(inode));
+ f2fs_bug_on(F2FS_I_SB(inode), cache_only && IS_ENCRYPTED(inode));
if (!cache_only)
set_page_dirty(page);
f2fs_put_page(page, 1);
@@ -730,7 +730,7 @@ int f2fs_getattr(const struct path *path, struct kstat
*stat,
stat->attributes |= STATX_ATTR_APPEND;
if (flags & F2FS_COMPR_FL)
stat->attributes |= STATX_ATTR_COMPRESSED;
- if (f2fs_encrypted_inode(inode))
+ if (IS_ENCRYPTED(inode))
stat->attributes |= STATX_ATTR_ENC
[PATCH V4 1/9] ext4: use IS_ENCRYPTED() to check encryption status
This commit removes the ext4 specific ext4_encrypted_inode() and makes
use of the generic IS_ENCRYPTED() macro to check for the encryption
status of an inode.
Reviewed-by: Eric Biggers
Signed-off-by: Chandan Rajendra
---
fs/ext4/dir.c | 8
fs/ext4/ext4.h| 5 -
fs/ext4/ext4_jbd2.h | 2 +-
fs/ext4/extents.c | 4 ++--
fs/ext4/ialloc.c | 2 +-
fs/ext4/inode.c | 16 +++-
fs/ext4/move_extent.c | 3 +--
fs/ext4/namei.c | 8
fs/ext4/page-io.c | 3 +--
fs/ext4/readpage.c| 2 +-
10 files changed, 22 insertions(+), 31 deletions(-)
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index f93f9881ec18..fb7a64ea5679 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -111,7 +111,7 @@ static int ext4_readdir(struct file *file, struct
dir_context *ctx)
int dir_has_error = 0;
struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
- if (ext4_encrypted_inode(inode)) {
+ if (IS_ENCRYPTED(inode)) {
err = fscrypt_get_encryption_info(inode);
if (err && err != -ENOKEY)
return err;
@@ -138,7 +138,7 @@ static int ext4_readdir(struct file *file, struct
dir_context *ctx)
return err;
}
- if (ext4_encrypted_inode(inode)) {
+ if (IS_ENCRYPTED(inode)) {
err = fscrypt_fname_alloc_buffer(inode, EXT4_NAME_LEN, &fstr);
if (err < 0)
return err;
@@ -245,7 +245,7 @@ static int ext4_readdir(struct file *file, struct
dir_context *ctx)
offset += ext4_rec_len_from_disk(de->rec_len,
sb->s_blocksize);
if (le32_to_cpu(de->inode)) {
- if (!ext4_encrypted_inode(inode)) {
+ if (!IS_ENCRYPTED(inode)) {
if (!dir_emit(ctx, de->name,
de->name_len,
le32_to_cpu(de->inode),
@@ -613,7 +613,7 @@ static int ext4_dx_readdir(struct file *file, struct
dir_context *ctx)
static int ext4_dir_open(struct inode * inode, struct file * filp)
{
- if (ext4_encrypted_inode(inode))
+ if (IS_ENCRYPTED(inode))
return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
return 0;
}
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 80957f9d3cbe..2ae6ab88f218 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2297,11 +2297,6 @@ extern unsigned ext4_free_clusters_after_init(struct
super_block *sb,
struct ext4_group_desc *gdp);
ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
-static inline bool ext4_encrypted_inode(struct inode *inode)
-{
- return ext4_test_inode_flag(inode, EXT4_INODE_ENCRYPT);
-}
-
static inline bool ext4_verity_inode(struct inode *inode)
{
#ifdef CONFIG_EXT4_FS_VERITY
diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index 15b6dd733780..a1ac7e9245ec 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -411,7 +411,7 @@ static inline int ext4_inode_journal_mode(struct inode
*inode)
(ext4_test_inode_flag(inode, EXT4_INODE_JOURNAL_DATA) &&
!test_opt(inode->i_sb, DELALLOC))) {
/* We do not support data journalling for encrypted data */
- if (S_ISREG(inode->i_mode) && ext4_encrypted_inode(inode))
+ if (S_ISREG(inode->i_mode) && IS_ENCRYPTED(inode))
return EXT4_INODE_ORDERED_DATA_MODE; /* ordered */
return EXT4_INODE_JOURNAL_DATA_MODE;/* journal data */
}
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 240b6dea5441..79d986dbf5af 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3631,7 +3631,7 @@ static int ext4_ext_convert_to_initialized(handle_t
*handle,
max_zeroout = sbi->s_extent_max_zeroout_kb >>
(inode->i_sb->s_blocksize_bits - 10);
- if (ext4_encrypted_inode(inode))
+ if (IS_ENCRYPTED(inode))
max_zeroout = 0;
/*
@@ -4818,7 +4818,7 @@ long ext4_fallocate(struct file *file, int mode, loff_t
offset, loff_t len)
* leave it disabled for encrypted inodes for now. This is a
* bug we should fix
*/
- if (ext4_encrypted_inode(inode) &&
+ if (IS_ENCRYPTED(inode) &&
(mode & (FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE |
FALLOC_FL_ZERO_RANGE)))
return -EOPNOTSUPP;
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 2addcb8730e1..3002f110eb4f 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -771,7 +771,7 @@ struct inode *__ext4_new_inode(handle_t *handle, struct
inode *dir,
if (unlikely(ext4_forced_shutdown(sbi)))
return ERR_PTR(-EIO);
- if ((ext4_encrypted_inode(dir) ||
[PATCH V4 4/9] Add S_VERITY and IS_VERITY()
Similar to S_ENCRYPTED/IS_ENCRYPTED(), this commit adds
S_VERITY/IS_VERITY() to be able to check if a VFS inode has verity
information associated with it.
Reviewed-by: Eric Biggers
Signed-off-by: Chandan Rajendra
---
include/linux/fs.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 389a35e028bf..de602d9f8d0e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1938,6 +1938,7 @@ struct super_operations {
#define S_DAX 0 /* Make all the DAX code disappear */
#endif
#define S_ENCRYPTED16384 /* Encrypted file (using fs/crypto/) */
+#define S_VERITY 32768 /* Verity file (using fs/verity/) */
/*
* Note that nosuid etc flags are inode-specific: setting some file-system
@@ -1978,6 +1979,7 @@ static inline bool sb_rdonly(const struct super_block
*sb) { return sb->s_flags
#define IS_NOSEC(inode)((inode)->i_flags & S_NOSEC)
#define IS_DAX(inode) ((inode)->i_flags & S_DAX)
#define IS_ENCRYPTED(inode)((inode)->i_flags & S_ENCRYPTED)
+#define IS_VERITY(inode) ((inode)->i_flags & S_VERITY)
#define IS_WHITEOUT(inode) (S_ISCHR(inode->i_mode) && \
(inode)->i_rdev == WHITEOUT_DEV)
--
2.19.1
[PATCH V4 3/9] fscrypt: remove filesystem specific build config option
In order to have a common code base for fscrypt "post read" processing for all filesystems which support encryption, this commit removes filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION) and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose value affects all the filesystems making use of fscrypt. Reviewed-by: Eric Biggers Signed-off-by: Chandan Rajendra --- Documentation/filesystems/fscrypt.rst | 4 +- arch/mips/configs/generic_defconfig | 2 +- arch/nds32/configs/defconfig| 2 +- arch/s390/configs/debug_defconfig | 2 +- arch/s390/configs/performance_defconfig | 2 +- fs/crypto/Kconfig | 5 +- fs/crypto/fscrypt_private.h | 1 - fs/ext4/Kconfig | 15 - fs/ext4/dir.c | 2 - fs/ext4/ext4.h | 7 +- fs/ext4/inode.c | 8 +- fs/ext4/ioctl.c | 4 +- fs/ext4/namei.c | 10 +- fs/ext4/page-io.c | 6 +- fs/ext4/readpage.c | 2 +- fs/ext4/super.c | 6 +- fs/ext4/sysfs.c | 4 +- fs/f2fs/Kconfig | 12 +- fs/f2fs/f2fs.h | 7 +- fs/f2fs/super.c | 8 +- fs/f2fs/sysfs.c | 4 +- fs/ubifs/Kconfig| 13 +- fs/ubifs/Makefile | 2 +- fs/ubifs/ioctl.c| 4 +- fs/ubifs/sb.c | 2 +- fs/ubifs/super.c| 2 +- fs/ubifs/ubifs.h| 5 +- include/linux/fs.h | 4 +- include/linux/fscrypt.h | 416 +++- include/linux/fscrypt_notsupp.h | 231 - include/linux/fscrypt_supp.h| 204 31 files changed, 461 insertions(+), 535 deletions(-) delete mode 100644 include/linux/fscrypt_notsupp.h delete mode 100644 include/linux/fscrypt_supp.h diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst index cfbc18f0d9c9..92084762ad20 100644 --- a/Documentation/filesystems/fscrypt.rst +++ b/Documentation/filesystems/fscrypt.rst @@ -318,9 +318,9 @@ FS_IOC_SET_ENCRYPTION_POLICY can fail with the following errors: - ``ENOTEMPTY``: the file is unencrypted and is a nonempty directory - ``ENOTTY``: this type of filesystem does not implement encryption - ``EOPNOTSUPP``: the kernel was not configured with encryption - support for this filesystem, or the filesystem superblock has not + support for filesystems, or the filesystem superblock has not had encryption enabled on it. (For example, to use encryption on an - ext4 filesystem, CONFIG_EXT4_ENCRYPTION must be enabled in the + ext4 filesystem, CONFIG_FS_ENCRYPTION must be enabled in the kernel config, and the superblock must have had the "encrypt" feature flag enabled using ``tune2fs -O encrypt`` or ``mkfs.ext4 -O encrypt``.) diff --git a/arch/mips/configs/generic_defconfig b/arch/mips/configs/generic_defconfig index 684c9dcba126..b8a21b9b934a 100644 --- a/arch/mips/configs/generic_defconfig +++ b/arch/mips/configs/generic_defconfig @@ -63,7 +63,7 @@ CONFIG_HID_MONTEREY=y CONFIG_EXT4_FS=y CONFIG_EXT4_FS_POSIX_ACL=y CONFIG_EXT4_FS_SECURITY=y -CONFIG_EXT4_ENCRYPTION=y +CONFIG_FS_ENCRYPTION=y CONFIG_FANOTIFY=y CONFIG_FUSE_FS=y CONFIG_CUSE=y diff --git a/arch/nds32/configs/defconfig b/arch/nds32/configs/defconfig index 2546d8770785..65ce9259081b 100644 --- a/arch/nds32/configs/defconfig +++ b/arch/nds32/configs/defconfig @@ -74,7 +74,7 @@ CONFIG_GENERIC_PHY=y CONFIG_EXT4_FS=y CONFIG_EXT4_FS_POSIX_ACL=y CONFIG_EXT4_FS_SECURITY=y -CONFIG_EXT4_ENCRYPTION=y +CONFIG_FS_ENCRYPTION=y CONFIG_FUSE_FS=y CONFIG_MSDOS_FS=y CONFIG_VFAT_FS=y diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig index 259d1698ac50..e8868e0fba76 100644 --- a/arch/s390/configs/debug_defconfig +++ b/arch/s390/configs/debug_defconfig @@ -492,7 +492,6 @@ CONFIG_VIRTIO_INPUT=y CONFIG_EXT4_FS=y CONFIG_EXT4_FS_POSIX_ACL=y CONFIG_EXT4_FS_SECURITY=y -CONFIG_EXT4_ENCRYPTION=y CONFIG_JBD2_DEBUG=y CONFIG_JFS_FS=m CONFIG_JFS_POSIX_ACL=y @@ -512,6 +511,7 @@ CONFIG_BTRFS_DEBUG=y CONFIG_NILFS2_FS=m CONFIG_FS_DAX=y CONFIG_EXPORTFS_BLOCK_OPS=y +CONFIG_FS_ENCRYPTION=y CONFIG_FANOTIFY=y CONFIG_FANOTIFY_ACCESS_PERMISSIONS=y CONFIG_QUOTA_NETLINK_INTERFACE=y diff --git a/arch/s390/configs/performance_defconfig b/arch/s390/configs/performance_defconfig index 37fd60c20e22..ef1dd74b29f5 100644 --- a/arch/s390/configs/performance_defconfig +++ b/arch/s390/configs/performance_defconfig @@ -489,7 +489,6 @@ CONFIG_VIRTIO_INPUT=y CONFIG_EXT4_FS=y CONFIG_EXT4_FS_POSIX_ACL=y CONFIG_EXT4_FS_SECURITY=y -CONFIG_EXT4_ENCRYPTION=y CONFIG_JBD2_DEBUG=y CONFIG
[PATCH V4 0/9] Remove fs specific fscrypt and fsverity build config options
In order to have a common code base for fscrypt & fsverity "post read" processing across filesystems which implement fscrypt/fsverity, this commit removes filesystem specific build config option (CONFIG_EXT4_FS_ENCRYPTION, CONFIG_EXT4_FS_VERITY, CONFIG_F2FS_FS_ENCRYPTION, CONFIG_F2FS_FS_VERITY and CONFIG_UBIFS_FS_ENCRYPTION) and replaces it with build options (CONFIG_FS_ENCRYPTION and CONFIG_FS_VERITY) whose values affect all the filesystems making use of fscrypt and fsverity. The Kconfig recursive dependency issue reported by kbuild test bot for IA64 architecture still needs to be fixed. I should be able to fix the problem and post the next verison of patches by today. PS: Since I have access to only to x86 and ppc64le machines, I haven't tested the defconfig files for other architectures. Changelog: V3 -> V4: 1. For non-fsverity supported kernels, return success when fsverity_file_open() is invoked for non-fsverity files. V2 -> V3: 1. Remove unnecessary line breaks. 2. Remove the definition of f2fs_encrypted_inode(). 3. Fix Kconfig dependencies for fscrypt w.r.t F2FS and UBIFS. If F2FS is enabled in the kernel build configuration, F2FS_FS_XATTR is selected if FS_ENCRYPTION is enabled. Similarly, if UBIFS is enabled in the kernel build configuration, UBIFS_FS_XATTR and BLOCK is selected if FS_ENCRYPTION is enabled. 4. Two new patches have been added to move verity status check to fsverity_file_open() and fsverity_prepare_setattr(). 5. For patch "f2fs: use IS_VERITY() to check inode's fsverity status", the acked-by tag given by Chao Yu has been removed since I added an invocation to f2fs_set_inode_flags() inside f2fs_set_verity(). This is needed to have S_VERITY flag set on the corresponding VFS inode. V1 -> V2: 1. Address the following review comments provided by Eric Biggers, - In ext4_should_use_dax(), Use ext4_test_inode_flag() to check for fscrypt/fsverity status of an inode. - Update documentation associated with fscrypt & fsverity to refer to CONFIG_FS_ENCRYPTION and CONFIG_FS_VERITY build flags. - Remove filesystem specific fscrypt build configuration from defconfig files. - Provide a list of supported filesystems for CONFIG_FS_ENCRYPTION and CONFIG_FS_VERITY build flags. - Update comment describing S_VERITY flag. 2. Remove UBIFS specific encryption build option and make use of the generic CONFIG_FS_ENCRYPTION flag. RFC -> V1: 1. Add a new patch to implement S_VERITY/IS_VERITY(). 2. Split code that replaces filesystem specific routines with generic IS_ENCRYPTED() and IS_VERITY() calls into separate patches. Chandan Rajendra (9): ext4: use IS_ENCRYPTED() to check encryption status f2fs: use IS_ENCRYPTED() to check encryption status fscrypt: remove filesystem specific build config option Add S_VERITY and IS_VERITY() ext4: use IS_VERITY() to check inode's fsverity status f2fs: use IS_VERITY() to check inode's fsverity status fsverity: Remove filesystem specific build config option fsverity: Move verity status check to fsverity_file_open fsverity: Move verity status check to fsverity_prepare_setattr Documentation/filesystems/fscrypt.rst | 4 +- Documentation/filesystems/fsverity.rst | 4 +- arch/mips/configs/generic_defconfig | 2 +- arch/nds32/configs/defconfig| 2 +- arch/s390/configs/debug_defconfig | 2 +- arch/s390/configs/performance_defconfig | 2 +- fs/crypto/Kconfig | 5 +- fs/crypto/fscrypt_private.h | 1 - fs/ext4/Kconfig | 35 -- fs/ext4/dir.c | 10 +- fs/ext4/ext4.h | 23 +- fs/ext4/ext4_jbd2.h | 2 +- fs/ext4/extents.c | 4 +- fs/ext4/file.c | 8 +- fs/ext4/ialloc.c| 2 +- fs/ext4/inode.c | 40 ++- fs/ext4/ioctl.c | 4 +- fs/ext4/move_extent.c | 3 +- fs/ext4/namei.c | 18 +- fs/ext4/page-io.c | 9 +- fs/ext4/readpage.c | 10 +- fs/ext4/super.c | 13 +- fs/ext4/sysfs.c | 8 +- fs/f2fs/Kconfig | 32 +- fs/f2fs/data.c | 6 +- fs/f2fs/dir.c | 10 +- fs/f2fs/f2fs.h | 23 +- fs/f2fs/file.c | 28 +- fs/f2fs/inode.c | 8 +- fs/f2fs/namei.c | 6 +- fs/f2fs/super.c | 15 +- fs/f2fs/sysfs.c | 8 +- fs/ubifs/Kconfig| 13 +- fs/ubifs/Makefile | 2 +- fs/ubifs/ioctl.c| 4 +- fs/ubifs/sb.c | 2 +- fs/ubifs/super.c| 2 +
[PATCH V4 8/9] fsverity: Move verity status check to fsverity_file_open
Instead of conditionally checking for verity status of an inode before
invoking fsverity_file_open(), this commit moves the check inside the
definition of fsverity_file_open().
Signed-off-by: Chandan Rajendra
---
fs/ext4/file.c | 8 +++-
fs/f2fs/file.c | 8 +++-
fs/verity/setup.c| 3 +++
include/linux/fsverity.h | 5 -
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 30fbd663354f..b404a857cd48 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -444,11 +444,9 @@ static int ext4_file_open(struct inode * inode, struct
file * filp)
if (ret)
return ret;
- if (IS_VERITY(inode)) {
- ret = fsverity_file_open(inode, filp);
- if (ret)
- return ret;
- }
+ ret = fsverity_file_open(inode, filp);
+ if (ret)
+ return ret;
/*
* Set up the jbd2_inode if we are opening the inode for
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 2eb4821d95d1..925c0d9608da 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -491,11 +491,9 @@ static int f2fs_file_open(struct inode *inode, struct file
*filp)
if (err)
return err;
- if (IS_VERITY(inode)) {
- err = fsverity_file_open(inode, filp);
- if (err)
- return err;
- }
+ err = fsverity_file_open(inode, filp);
+ if (err)
+ return err;
filp->f_mode |= FMODE_NOWAIT;
diff --git a/fs/verity/setup.c b/fs/verity/setup.c
index 08b609127531..bc463dc601b1 100644
--- a/fs/verity/setup.c
+++ b/fs/verity/setup.c
@@ -771,6 +771,9 @@ static int setup_fsverity_info(struct inode *inode)
*/
int fsverity_file_open(struct inode *inode, struct file *filp)
{
+ if (!IS_VERITY(inode))
+ return 0;
+
if (filp->f_mode & FMODE_WRITE) {
pr_debug("Denying opening verity file (ino %lu) for write\n",
inode->i_ino);
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index ea8c418bd7d5..c770025606e1 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -60,7 +60,10 @@ static inline int fsverity_ioctl_measure(struct file *filp,
void __user *arg)
static inline int fsverity_file_open(struct inode *inode, struct file *filp)
{
- return -EOPNOTSUPP;
+ if (IS_VERITY(inode))
+ return -EOPNOTSUPP;
+
+ return 0;
}
static inline int fsverity_prepare_setattr(struct dentry *dentry,
--
2.19.1
[PATCH V4 9/9] fsverity: Move verity status check to fsverity_prepare_setattr
Instead of conditionally checking for verity status of an inode before
invoking fsverity_prepare_setattr(), this commit moves the check inside the
definition of fsverity_prepare_setattr().
Signed-off-by: Chandan Rajendra
---
fs/ext4/inode.c | 8 +++-
fs/f2fs/file.c | 8 +++-
fs/verity/setup.c| 2 +-
include/linux/fsverity.h | 5 -
4 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 703f55635824..44561e68886b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5510,11 +5510,9 @@ int ext4_setattr(struct dentry *dentry, struct iattr
*attr)
if (error)
return error;
- if (IS_VERITY(inode)) {
- error = fsverity_prepare_setattr(dentry, attr);
- if (error)
- return error;
- }
+ error = fsverity_prepare_setattr(dentry, attr);
+ if (error)
+ return error;
if (is_quota_modification(inode, attr)) {
error = dquot_initialize(inode);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 925c0d9608da..422907ca37c6 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -798,11 +798,9 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
if (err)
return err;
- if (IS_VERITY(inode)) {
- err = fsverity_prepare_setattr(dentry, attr);
- if (err)
- return err;
- }
+ err = fsverity_prepare_setattr(dentry, attr);
+ if (err)
+ return err;
if (is_quota_modification(inode, attr)) {
err = dquot_initialize(inode);
diff --git a/fs/verity/setup.c b/fs/verity/setup.c
index bc463dc601b1..154556d72691 100644
--- a/fs/verity/setup.c
+++ b/fs/verity/setup.c
@@ -796,7 +796,7 @@ EXPORT_SYMBOL_GPL(fsverity_file_open);
*/
int fsverity_prepare_setattr(struct dentry *dentry, struct iattr *attr)
{
- if (attr->ia_valid & ATTR_SIZE) {
+ if (IS_VERITY(d_inode(dentry)) && (attr->ia_valid & ATTR_SIZE)) {
pr_debug("Denying truncate of verity file (ino %lu)\n",
d_inode(dentry)->i_ino);
return -EPERM;
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index c770025606e1..9ccc8ad9652a 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -69,7 +69,10 @@ static inline int fsverity_file_open(struct inode *inode,
struct file *filp)
static inline int fsverity_prepare_setattr(struct dentry *dentry,
struct iattr *attr)
{
- return -EOPNOTSUPP;
+ if (IS_VERITY(d_inode(dentry)))
+ return -EOPNOTSUPP;
+
+ return 0;
}
static inline int fsverity_prepare_getattr(struct inode *inode)
--
2.19.1
[PATCH V4 5/9] ext4: use IS_VERITY() to check inode's fsverity status
This commit removes the ext4 specific ext4_verity_inode() and makes
use of the generic IS_ENCRYPTED() macro or ext4_test_inode_flag() to
check for the encryption status of an inode.
Reviewed-by: Eric Biggers
Signed-off-by: Chandan Rajendra
---
fs/ext4/ext4.h | 9 -
fs/ext4/file.c | 2 +-
fs/ext4/inode.c| 10 ++
fs/ext4/readpage.c | 2 +-
fs/ext4/super.c| 1 +
5 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index db21df885186..64bf9fb7ef18 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2296,15 +2296,6 @@ extern unsigned ext4_free_clusters_after_init(struct
super_block *sb,
struct ext4_group_desc *gdp);
ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
-static inline bool ext4_verity_inode(struct inode *inode)
-{
-#ifdef CONFIG_EXT4_FS_VERITY
- return ext4_test_inode_flag(inode, EXT4_INODE_VERITY);
-#else
- return false;
-#endif
-}
-
#ifdef CONFIG_FS_ENCRYPTION
static inline int ext4_fname_setup_filename(struct inode *dir,
const struct qstr *iname,
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index cb4b69ef01a2..30fbd663354f 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -444,7 +444,7 @@ static int ext4_file_open(struct inode * inode, struct file
* filp)
if (ret)
return ret;
- if (ext4_verity_inode(inode)) {
+ if (IS_VERITY(inode)) {
ret = fsverity_file_open(inode, filp);
if (ret)
return ret;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8a55255a6c32..703f55635824 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3883,7 +3883,7 @@ static ssize_t ext4_direct_IO(struct kiocb *iocb, struct
iov_iter *iter)
return 0;
#endif
- if (ext4_verity_inode(inode))
+ if (IS_VERITY(inode))
return 0;
/*
@@ -4724,7 +4724,7 @@ static bool ext4_should_use_dax(struct inode *inode)
return false;
if (ext4_test_inode_flag(inode, EXT4_INODE_ENCRYPT))
return false;
- if (ext4_verity_inode(inode))
+ if (ext4_test_inode_flag(inode, EXT4_INODE_VERITY))
return false;
return true;
}
@@ -4748,9 +4748,11 @@ void ext4_set_inode_flags(struct inode *inode)
new_fl |= S_DAX;
if (flags & EXT4_ENCRYPT_FL)
new_fl |= S_ENCRYPTED;
+ if (flags & EXT4_VERITY_FL)
+ new_fl |= S_VERITY;
inode_set_flags(inode, new_fl,
S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_DAX|
- S_ENCRYPTED);
+ S_ENCRYPTED|S_VERITY);
}
static blkcnt_t ext4_inode_blocks(struct ext4_inode *raw_inode,
@@ -5508,7 +5510,7 @@ int ext4_setattr(struct dentry *dentry, struct iattr
*attr)
if (error)
return error;
- if (ext4_verity_inode(inode)) {
+ if (IS_VERITY(inode)) {
error = fsverity_prepare_setattr(dentry, attr);
if (error)
return error;
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 7252f0a60cdb..2c037df629dd 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -206,7 +206,7 @@ static void mpage_end_io(struct bio *bio)
static inline loff_t ext4_readpage_limit(struct inode *inode)
{
#ifdef CONFIG_EXT4_FS_VERITY
- if (ext4_verity_inode(inode)) {
+ if (IS_VERITY(inode)) {
if (inode->i_verity_info)
/* limit to end of metadata region */
return fsverity_full_i_size(inode);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 16fb483a6f4a..35ed3c48f8d2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1344,6 +1344,7 @@ static int ext4_set_verity(struct inode *inode, loff_t
data_i_size)
err = ext4_reserve_inode_write(handle, inode, &iloc);
if (err == 0) {
ext4_set_inode_flag(inode, EXT4_INODE_VERITY);
+ ext4_set_inode_flags(inode);
EXT4_I(inode)->i_disksize = data_i_size;
err = ext4_mark_iloc_dirty(handle, inode, &iloc);
}
--
2.19.1
[PATCH V4 6/9] f2fs: use IS_VERITY() to check inode's fsverity status
This commit removes the f2fs specific f2fs_verity_file() and makes use
of the generic IS_VERITY() macro or file_is_verity() to check for the
verity status of an inode.
Reviewed-by: Eric Biggers
Signed-off-by: Chandan Rajendra
---
fs/f2fs/f2fs.h | 7 +--
fs/f2fs/file.c | 6 +++---
fs/f2fs/inode.c | 4 +++-
fs/f2fs/super.c | 1 +
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index f9c8b0afc1de..54bd93c7b630 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3448,18 +3448,13 @@ static inline void f2fs_set_encrypted_inode(struct
inode *inode)
#endif
}
-static inline bool f2fs_verity_file(struct inode *inode)
-{
- return file_is_verity(inode);
-}
-
/*
* Returns true if the reads of the inode's data need to undergo some
* postprocessing step, like decryption or authenticity verification.
*/
static inline bool f2fs_post_read_required(struct inode *inode)
{
- return f2fs_encrypted_file(inode) || f2fs_verity_file(inode);
+ return f2fs_encrypted_file(inode) || IS_VERITY(inode);
}
#define F2FS_FEATURE_FUNCS(name, flagname) \
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 6c7ad15000b9..2eb4821d95d1 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -491,7 +491,7 @@ static int f2fs_file_open(struct inode *inode, struct file
*filp)
if (err)
return err;
- if (f2fs_verity_file(inode)) {
+ if (IS_VERITY(inode)) {
err = fsverity_file_open(inode, filp);
if (err)
return err;
@@ -701,7 +701,7 @@ int f2fs_getattr(const struct path *path, struct kstat
*stat,
struct f2fs_inode *ri;
unsigned int flags;
- if (f2fs_verity_file(inode)) {
+ if (IS_VERITY(inode)) {
/*
* For fs-verity we need to override i_size with the original
* data i_size. This requires I/O to the file which with
@@ -800,7 +800,7 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
if (err)
return err;
- if (f2fs_verity_file(inode)) {
+ if (IS_VERITY(inode)) {
err = fsverity_prepare_setattr(dentry, attr);
if (err)
return err;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index d731d23aedee..ff9126616d65 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -45,9 +45,11 @@ void f2fs_set_inode_flags(struct inode *inode)
new_fl |= S_DIRSYNC;
if (file_is_encrypt(inode))
new_fl |= S_ENCRYPTED;
+ if (file_is_verity(inode))
+ new_fl |= S_VERITY;
inode_set_flags(inode, new_fl,
S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|
- S_ENCRYPTED);
+ S_ENCRYPTED|S_VERITY);
}
static void __get_inode_rdev(struct inode *inode, struct f2fs_inode *ri)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4287cf348d3c..73320202bd01 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2207,6 +2207,7 @@ static int f2fs_set_verity(struct inode *inode, loff_t
data_i_size)
return err;
file_set_verity(inode);
+ f2fs_set_inode_flags(inode);
f2fs_mark_inode_dirty_sync(inode, true);
return 0;
}
--
2.19.1
[PATCH V4 7/9] fsverity: Remove filesystem specific build config option
In order to have a common code base for fsverity "post read" processing
for all filesystems which support fsverity, this commit removes
filesystem specific build config option (e.g. CONFIG_EXT4_FS_VERITY)
and replaces it with a build option (i.e. CONFIG_FS_VERITY) whose
value affects all the filesystems making use of fsverity.
Reviewed-by: Eric Biggers
Signed-off-by: Chandan Rajendra
---
Documentation/filesystems/fsverity.rst | 4 ++--
fs/ext4/Kconfig| 20
fs/ext4/ext4.h | 2 --
fs/ext4/readpage.c | 4 ++--
fs/ext4/super.c| 6 +++---
fs/ext4/sysfs.c| 4 ++--
fs/f2fs/Kconfig| 20
fs/f2fs/data.c | 2 +-
fs/f2fs/f2fs.h | 2 --
fs/f2fs/super.c| 6 +++---
fs/f2fs/sysfs.c| 4 ++--
fs/verity/Kconfig | 3 ++-
fs/verity/fsverity_private.h | 1 -
include/linux/fs.h | 4 ++--
include/linux/fsverity.h | 7 +++
15 files changed, 22 insertions(+), 67 deletions(-)
diff --git a/Documentation/filesystems/fsverity.rst
b/Documentation/filesystems/fsverity.rst
index d633fc0567bd..bb208dad10d9 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -279,7 +279,7 @@ ext4
ext4 supports fs-verity since kernel version TODO.
-CONFIG_EXT4_FS_VERITY must be enabled in the kernel config. Also, the
+CONFIG_FS_VERITY must be enabled in the kernel config. Also, the
filesystem must have been formatted with ``-O verity``, or had
``tune2fs -O verity`` run on it. These require e2fsprogs v1.44.4-2 or
later. This e2fsprogs version is also required for e2fsck to
@@ -306,7 +306,7 @@ f2fs
f2fs supports fs-verity since kernel version TODO.
-CONFIG_F2FS_FS_VERITY must be enabled in the kernel config. Also, the
+CONFIG_FS_VERITY must be enabled in the kernel config. Also, the
filesystem must have been formatted with ``-O verity``. This requires
f2fs-tools v1.11.0 or later.
diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
index e1002bbf35bf..031e5a82d556 100644
--- a/fs/ext4/Kconfig
+++ b/fs/ext4/Kconfig
@@ -96,26 +96,6 @@ config EXT4_FS_SECURITY
If you are not using a security module that requires using
extended attributes for file security labels, say N.
-config EXT4_FS_VERITY
- bool "Ext4 Verity"
- depends on EXT4_FS
- select FS_VERITY
- help
- This option enables fs-verity for ext4. fs-verity is the
- dm-verity mechanism implemented at the file level. Userspace
- can append a Merkle tree (hash tree) to a file, then enable
- fs-verity on the file. ext4 will then transparently verify
- any data read from the file against the Merkle tree. The file
- is also made read-only.
-
- This serves as an integrity check, but the availability of the
- Merkle tree root hash also allows efficiently supporting
- various use cases where normally the whole file would need to
- be hashed at once, such as auditing and authenticity
- verification (appraisal).
-
- If unsure, say N.
-
config EXT4_DEBUG
bool "EXT4 debugging support"
depends on EXT4_FS
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 64bf9fb7ef18..bff8d639dd0c 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -41,8 +41,6 @@
#endif
#include
-
-#define __FS_HAS_VERITY IS_ENABLED(CONFIG_EXT4_FS_VERITY)
#include
#include
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 2c037df629dd..8717ac0a5bb2 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -158,7 +158,7 @@ static struct bio_post_read_ctx
*get_bio_post_read_ctx(struct inode *inode,
if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
post_read_steps |= 1 << STEP_DECRYPT;
-#ifdef CONFIG_EXT4_FS_VERITY
+#ifdef CONFIG_FS_VERITY
if (inode->i_verity_info != NULL &&
(index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
post_read_steps |= 1 << STEP_VERITY;
@@ -205,7 +205,7 @@ static void mpage_end_io(struct bio *bio)
static inline loff_t ext4_readpage_limit(struct inode *inode)
{
-#ifdef CONFIG_EXT4_FS_VERITY
+#ifdef CONFIG_FS_VERITY
if (IS_VERITY(inode)) {
if (inode->i_verity_info)
/* limit to end of metadata region */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 35ed3c48f8d2..0d169de59f76 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1316,7 +1316,7 @@ static const struct fscrypt_operations ext4_cryptops = {
};
#endif
-#ifdef CONFIG_EXT4_FS_VERITY
+#ifdef CONFIG_FS_VERITY
static int ext4_set_verity(struct inode *inode, loff_t data_i_size)
{
int err;
@@ -1402,7 +1402,7 @@ static const struct f
Re: [PATCH V2 3/7] fscrypt: remove filesystem specific build config option
On Wednesday, December 12, 2018 8:18:12 AM IST Eric Biggers wrote: > On Tue, Dec 11, 2018 at 05:52:11PM -0800, Guenter Roeck wrote: > > Hi, > > > > On Tue, Dec 04, 2018 at 03:26:46PM +0530, Chandan Rajendra wrote: > > > In order to have a common code base for fscrypt "post read" processing > > > for all filesystems which support encryption, this commit removes > > > filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION) > > > and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose > > > value affects all the filesystems making use of fscrypt. > > > > > > Signed-off-by: Chandan Rajendra > > > > this patch causes a recursive dependency when trying to build ia64 images. > > > > make ARCH=ia64 allnoconfig: > > > > scripts/kconfig/conf --allnoconfig Kconfig > > arch/ia64/Kconfig:126:error: recursive dependency detected! > > arch/ia64/Kconfig:126: choice contains symbol IA64_HP_SIM > > arch/ia64/Kconfig:200: symbol IA64_HP_SIM is part of choice PM > > kernel/power/Kconfig:144: symbol PM is selected by PM_SLEEP > > kernel/power/Kconfig:104: symbol PM_SLEEP depends on HIBERNATE_CALLBACKS > > kernel/power/Kconfig:31:symbol HIBERNATE_CALLBACKS is selected by > > HIBERNATION > > kernel/power/Kconfig:34:symbol HIBERNATION depends on SWAP > > init/Kconfig:250: symbol SWAP depends on BLOCK > > block/Kconfig:5:symbol BLOCK is selected by UBIFS_FS > > fs/ubifs/Kconfig:1: symbol UBIFS_FS depends on MISC_FILESYSTEMS > > fs/Kconfig:220: symbol MISC_FILESYSTEMS is selected by ACPI_APEI > > drivers/acpi/apei/Kconfig:8:symbol ACPI_APEI depends on ACPI > > drivers/acpi/Kconfig:9: symbol ACPI depends on ARCH_SUPPORTS_ACPI > > drivers/acpi/Kconfig:6: symbol ARCH_SUPPORTS_ACPI is selected by > > IA64_HP_SIM > > arch/ia64/Kconfig:200: symbol IA64_HP_SIM is part of choice > > For a resolution refer to Documentation/kbuild/kconfig-language.txt > > subsection "Kconfig recursive dependency limitations" > > > > scripts/kconfig/Makefile:69: recipe for target 'allnoconfig' failed > > > > Reverting the patch fixes the problem. > > > > Thanks for the report. Chandan, it appears the problem is UBIFS_FS selecting > BLOCK. It's actually not necessary because now the parts of fs/crypto/ that > depend on BLOCK are separated out into a separate file fs/crypto/bio.c that is > only compiled with CONFIG_BLOCK. So how about just removing the selection of > BLOCK from fs/ubifs/Kconfig: > > select BLOCK if FS_ENCRYPTION > Yes, I will do that. Also, Apologies for sending V4 of the patchset without checking for newer responses from you. -- chandan
Re: [PATCH] docs/networking: fix formatting of Intel drivers documentation
From: Mike Rapoport Date: Sun, 9 Dec 2018 18:09:51 +0200 > The documentation of Intel drivers is missing the heading adornment for > document titles. > > This causes the generated html to have TOC entries from these documents to > appear as top level TOC entries: > > * Linux* Base Driver for Intel(R) Ethernet Network Connection > * Contents > * Identifying Your Adapter > * Command Line Parameters > * AutoNeg > * Duplex > ... > > Add overline heading adornment to document titles. > > Signed-off-by: Mike Rapoport Jeff, I am assuming you picked this up. Let me know if you haven't and you want me to integrate it. Thanks.
