All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: slow snd_rawmidi_drain() for VirMidi devcies
From: Stefan Sauer @ 2022-01-05 13:39 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel
In-Reply-To: <s5hiluzjy3d.wl-tiwai@suse.de>

-   28,78%     0,00%  rawmidi_alsa  [snd_rawmidi]         [k]
snd_rawmidi_drain_output
   - snd_rawmidi_drain_output
      - 26,59% msleep
           schedule_timeout
           schedule
         + __schedule
      + 2,13% schedule_timeout

Seems to be because of:
https://github.com/torvalds/linux/blob/master/sound/core/rawmidi.c#L244
if (substream->ops->drain)
  substream->ops->drain(substream);
else
  msleep(50);

It see what fileops have no drain impl:
https://github.com/torvalds/linux/blob/master/sound/core/rawmidi.c#L1708
and I see
https://github.com/torvalds/linux/blob/master/sound/core/seq/seq_virmidi.c#L329
that define no drain op. Not which ones are actually used here :/

The docs confirm teh 50 ms wait though:
https://github.com/torvalds/linux/blob/master/Documentation/sound/kernel-api/writing-an-alsa-driver.rst#drain-callback

would it make sense to have a dummy drain function for the seq_virmidi
output ops?

Stefan


Am Di., 4. Jan. 2022 um 16:54 Uhr schrieb Takashi Iwai <tiwai@suse.de>:

> On Sat, 01 Jan 2022 12:49:13 +0100,
> Stefan Sauer wrote:
> >
> > hi,
> >
> > I've tried to link BitwigStudio to the webapp cables.gl over virmidi.
> > Unfortunately Bitwig Studio only supports rawmidi. What I discovered is
> > that there is a strange slowness when sending data to virmidi caused
> > by snd_rawmidi_drain().
> >
> > I've posted two tiny, self-contained c apps to:
> > https://gist.github.com/ensonic/c7588b87fa6c1fa94a8f753b1e0aa394
> > See some examples below. 2 observations:
> > * snd_rawmidi_type() is *not* reporting virmidi as VIRTUAL
> > * snd_rawmidi_drain() takes about 60ms! on virtual vs. less that 0.1 ms
> on
> > usb midi (I checked all my hw midi and the worst was avg=1ms on physical
> > midi image unitor8)
> >
> > When comparing the implementations:
> >
> https://github.com/alsa-project/alsa-lib/blob/master/src/rawmidi/rawmidi_virt.c#L173
> >
> https://github.com/alsa-project/alsa-lib/blob/master/src/rawmidi/rawmidi_hw.c#L164
> > I see that the hw one results in an IOCTL which I can see when striking
> the
> > code and I wonder if this is the root cause? Why is rawmidi_virt.c not
> used
> > for virmidi?
> > >From poking at snd_rawmidi_open_conf() I have not yet figured where
> this is
> > decided ....
> >
> > Stefan
> >
> > > amidi -l
> > Dir Device    Name
> > IO  hw:0,0,0  Scarlett 18i20 USB MIDI 1
> > IO  hw:3,0,0  nanoKEY2 nanoKEY2 _ KEYBOARD
> > IO  hw:5,0,0  nanoKONTROL nanoKONTROL _ SLIDE
> > IO  hw:10,0    Virtual Raw MIDI (16 subdevices)
> > IO  hw:11,0    Virtual Raw MIDI (16 subdevices)
> >
> > # using direct i/o to virmidi - all good
> > > ./rawmidi_oss /dev/midi11 0
> > Using device '/dev/midi11' without draining
> > write took min=  0.0015 ms, avg=  0.0016 ms, max=  0.0110 ms
> > > ./rawmidi_oss /dev/midi11 1
> > Using device '/dev/midi11' with draining
> > write took min=  0.0015 ms, avg=  0.0017 ms, max=  0.0101 ms
> > drain took min=  0.0001 ms, avg=  0.0001 ms, max=  0.0008 ms
> >
> > # using snd_rawmidi to virmidi - slow drain operations
> > > ./rawmidi_alsa hw:11,0 0
> > Using device 'hw:11,0' without draining
> > SND_RAWMIDI_TYPE_HW
> > write took min=  0.0010 ms, avg=  0.0011 ms, max=  0.0056 ms
> > > ./rawmidi_alsa hw:11,0 1
> > Using device 'hw:11,0' with draining
> > SND_RAWMIDI_TYPE_HW
> > write took min=  0.0016 ms, avg=  0.0040 ms, max=  0.0077 ms
> > drain took min= 55.9951 ms, avg= 60.4330 ms, max= 64.0653 ms
> >
> > # using snd_rawmidi to usb hw - all good
> > > ./rawmidi_alsa hw:3,0 0
> > Using device 'hw:3,0' without draining
> > SND_RAWMIDI_TYPE_HW
> > write took min=  0.0012 ms, avg=  0.0015 ms, max=  0.0121 ms
> > > ./rawmidi_alsa hw:3,0 1
> > Using device 'hw:3,0' with draining
> > SND_RAWMIDI_TYPE_HW
> > write took min=  0.0024 ms, avg=  0.0032 ms, max=  0.0110 ms
> > drain took min=  0.0293 ms, avg=  0.0636 ms, max=  0.2277 ms
>
> This kind of thing needs profiling.  You can try perf or whatever
> available, and identify which call takes long.  My wild guess is
> something about snd_seq_sync_output_queue(), maybe poll syscall takes
> unexpected long.
>
>
> thanks,
>
> Takashi
>

^ permalink raw reply

* [PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running
From: Nikita Shubin @ 2022-01-05 13:39 UTC (permalink / raw)
  To: opensbi
In-Reply-To: <20220105072039.2609845-2-aurelien@aurel32.net>

Hello Aurelien!

Adding David from SiFive...

On Wed,  5 Jan 2022 08:20:39 +0100
Aurelien Jarno <aurelien@aurel32.net> wrote:

> When the watchdog is running the HiFive Unmatched board does not
> reboot properly and shuts down itself a few seconds after reboot, in
> the early stages of the u-boot loading. On a Linux kernel this
> happens when the da9063_wdt module is loaded. This does not happen if
> the module is unloaded before reboot or if the watchdog module is
> loaded with "stop_on_reboot=1".

| A running application is typically in ACTIVE mode. The DA9063
| transitions to ACTIVE mode after the host processor performs at least
| one initial ?alive? watchdog write (or alternatively an initial
| assertion of the KEEP_ACT port) inside the target time window. If the
| WATCHDOG function is disabled by setting TWDSCALE to zero, the DA9063
| transitions to ACTIVE mode when all of the sequencer IDs in the POWER
| domain are complete.

Is this that's case mentioned ? What if we press a reset key when
watchdog is enabled ? Or if it was reseted by thermal sensor ?

Can we disable watchdog on start instead of disabling it before a reset
?

Could you please give us a link to actual report ?

> 
> Fix that by stopping the watchdog before attempting to reset the
> board. This is done by zeroing the TWDSCALE field of CONTROL_D
> register, unless it was already set to 0.
> 
> Reported-by: Tianon Gravi <tianon@debian.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  platform/generic/sifive_fu740.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/platform/generic/sifive_fu740.c
> b/platform/generic/sifive_fu740.c index 866e924..f595c04 100644
> --- a/platform/generic/sifive_fu740.c
> +++ b/platform/generic/sifive_fu740.c
> @@ -22,6 +22,7 @@
>  
>  #define DA9063_REG_PAGE_CON		0x00
>  #define DA9063_REG_CONTROL_A		0x0e
> +#define DA9063_REG_CONTROL_D		0x11
>  #define DA9063_REG_CONTROL_F		0x13
>  #define DA9063_REG_DEVICE_ID		0x81
>  
> @@ -29,6 +30,8 @@
>  #define DA9063_CONTROL_A_M_POWER_EN	(1 << 5)
>  #define DA9063_CONTROL_A_STANDBY	(1 << 3)
>  
> +#define DA9063_CONTROL_D_TWDSCALE_MASK	0x07
> +
>  #define DA9063_CONTROL_F_WAKEUP	(1 << 2)
>  #define DA9063_CONTROL_F_SHUTDOWN	(1 << 1)
>  
> @@ -79,6 +82,27 @@ static inline int da9063_sanity_check(struct
> i2c_adapter *adap, uint32_t reg) return 0;
>  }
>  
> +static inline int da9063_stop_watchdog(struct i2c_adapter *adap,
> uint32_t reg) +{
> +	uint8_t val;
> +	int rc = i2c_adapter_reg_write(adap, reg,
> +					DA9063_REG_PAGE_CON, 0x00);
> +
> +	if (rc)
> +		return rc;
> +
> +	rc = i2c_adapter_reg_read(adap, reg, DA9063_REG_CONTROL_D,
> &val);
> +	if (rc)
> +		return rc;
> +
> +	if ((val & DA9063_CONTROL_D_TWDSCALE_MASK) == 0)
> +		return 0;
> +
> +	val &= ~DA9063_CONTROL_D_TWDSCALE_MASK;
> +
> +	return i2c_adapter_reg_write(adap, reg,
> DA9063_REG_CONTROL_D, val); +}
> +
>  static inline int da9063_shutdown(struct i2c_adapter *adap, uint32_t
> reg) {
>  	int rc = i2c_adapter_reg_write(adap, reg,
> @@ -133,6 +157,7 @@ static void da9063_system_reset(u32 type, u32
> reason) break;
>  		case SBI_SRST_RESET_TYPE_COLD_REBOOT:
>  		case SBI_SRST_RESET_TYPE_WARM_REBOOT:
> +			da9063_stop_watchdog(adap, reg);
>  			da9063_reset(adap, reg);
>  			break;
>  		}



^ permalink raw reply

* Re: [PATCH] drivers:iio:dac make expression evaluation 64-bit
From: Dan Carpenter @ 2022-01-05 13:39 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Mihail Chindris, open list:IIO SUBSYSTEM AND DRIVERS, open list,
	kernel, kernel-janitors
In-Reply-To: <YcIpAKV7Cmi0o7PU@debian-BULLSEYE-live-builder-AMD64>

On Wed, Dec 22, 2021 at 12:20:32AM +0500, Muhammad Usama Anjum wrote:
> Two 32-bit values are being evaluated using 32-bit arithmetic and then
> passed to s64 type. It is wrong. Expression should be evaluated using
> 64-bit arithmetic and then passed.
> 
> Fixes: 8f2b54824b ("drivers:iio:dac: Add AD3552R driver support")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  drivers/iio/dac/ad3552r.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> index 97f13c0b9631..b03d3c7cd4c4 100644
> --- a/drivers/iio/dac/ad3552r.c
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -770,7 +770,7 @@ static void ad3552r_calc_gain_and_offset(struct ad3552r_desc *dac, s32 ch)
>  	dac->ch_data[ch].scale_dec = DIV_ROUND_CLOSEST((s64)rem * 1000000,
>  							65536);
>  
> -	dac->ch_data[ch].offset_int = div_s64_rem(v_min * 65536, span, &rem);
> +	dac->ch_data[ch].offset_int = div_s64_rem(v_min * 65536L, span, &rem);

"v_min" is relatively close to zero on a number line so this can't
overflow.  There is no way that this change affects anything at runtime
(except making the code a tiny tiny bit slower).

And it should be 65536LL for 32 bit systems?

But I just don't see the point of this change.  Presumably it is to make
a static analyzer happy?

regards,
dan carpenter


^ permalink raw reply

* RE: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
From: Schwarz, Konrad @ 2022-01-05 13:25 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers, Ralf Ramsauer
In-Reply-To: <CAKmqyKNjPRt2kgyELPQF_c0eGwXajapYgcMYsLja7H3EuvVq+A@mail.gmail.com>

Hi,

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Tuesday, January 4, 2022 23:12
> To: Schwarz, Konrad (T CED SES-DE) <konrad.schwarz@siemens.com>
> Subject: Re: [PATCH v2 4/5] RISC-V: Typed CSRs in gdbserver
> 
> On Wed, Jan 5, 2022 at 1:56 AM Konrad Schwarz
> <konrad.schwarz@siemens.com> wrote:
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 9f41954894..557b4afe0e 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -3,6 +3,7 @@
> >   *
> >   * Copyright (c) 2016-2017 Sagar Karandikar, sagark@eecs.berkeley.edu
> >   * Copyright (c) 2017-2018 SiFive, Inc.
> > + * Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com
> 
> Please don't add these to existing files. In this case you have just
> added a newline to this file

Sorry, I don't know how that slipped through.
I originally didn't have any copyright messages, to which my company objected,
and I guess I overcompensated.
 
> diff --git a/target/riscv/csr32-op-gdbserver.h b/target/riscv/csr32-op-gdbserver.h
> > new file mode 100644
> > index 0000000000..e8ec527f23
> > --- /dev/null
> > +++ b/target/riscv/csr32-op-gdbserver.h
> > @@ -0,0 +1,109 @@
> > +/* Copyright (c) 2021 Siemens AG, konrad.schwarz@siemens.com */
> 
> All of these files should have the usual file boiler plate

OK, I'll try to figure out something.

> > +#if !defined(CONFIG_USER_ONLY)
> > +#  ifdef TARGET_RISCV64
> > +#    include "csr64-op-gdbserver.h"
> > +#  elif defined TARGET_RISCV64
> > +#    include "csr32-op-gdbserver.h"
> 
> This doesn't look right. `if defined TARGET_RISCV64` -> `include
> "csr32-op-gdbserver.h"`?

You are quite right. 

> Also this should be dynamic instead of based on the build time CPU, as
> the user could use a 32-bit CPU on a 64-bit target build.

I'm not sure.  The machine itself is still a 64-bit machine; its CSRs are 64 bits long.
`csr.c', which implements the CSRs contains an instance of # ifdef TARGET_RISCV64,
so that part of the implementation is not dynamic either.

Also, someone who is developing 32-bit system software can easily
sidestep the issue by using the 32-bit target.

Regards
Konrad

^ permalink raw reply

* Re: Expensive tcp_collapse with high tcp_rmem limit
From: Eric Dumazet @ 2022-01-05 13:38 UTC (permalink / raw)
  To: Daniel Dao
  Cc: netdev, kernel-team, linux-kernel, David Miller, Jakub Kicinski,
	Hideaki YOSHIFUJI, Marek Majkowski
In-Reply-To: <CA+wXwBRbLq6SW39qCD8GNG98YD5BJR2MFXmJV2zU1xwFjC-V0A@mail.gmail.com>

On Wed, Jan 5, 2022 at 4:15 AM Daniel Dao <dqminh@cloudflare.com> wrote:
>
> Hello,
>
> We are looking at increasing the maximum value of TCP receive buffer in order
> to take better advantage of high BDP links. For historical reasons (
> https://blog.cloudflare.com/the-story-of-one-latency-spike/), this was set to
> a lower than default value.
>
> We are still occasionally seeing long time spent in tcp_collapse, and the time
> seems to be proportional with max rmem. For example, with net.ipv4.tcp_rmem = 8192 2097152 16777216,
> we observe tcp_collapse latency with the following bpftrace command:
>

I suggest you add more traces, like the payload/truesize ratio when
these events happen.
and tp->rcv_ssthresh, sk->sk_rcvbuf

TCP stack by default assumes a conservative [1] payload/truesize ratio of 50%

Meaning that a 16MB sk->rcvbuf would translate to a TCP RWIN of 8MB.

I suspect that you use XDP, and standard MTU=1500.
Drivers in XDP mode use one page (4096 bytes on x86) per incoming frame.
In this case, the ratio is ~1428/4096 = 35%

This is one of the reason we switched to a 4K MTU at Google, because we
have an effective ratio close to 100% (even if XDP was used)

[1] The 50% ratio of TCP is defeated with small MSS, and malicious traffic.


>   bpftrace -e 'kprobe:tcp_collapse { @start[tid] = nsecs; } kretprobe:tcp_collapse /@start[tid] != 0/ { $us = (nsecs - @start[tid])/1000; @us = hist($us); delete(@start[tid]); printf("%ld us\n", $us);} interval:s:6000 { exit(); }'
>   Attaching 3 probes...
>   15496 us
>   14301 us
>   12248 us
>   @us:
>   [8K, 16K)              3 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@|
>
> Spending up to 16ms with 16MiB maximum receive buffer seems high.  Are there any
> recommendations on possible approaches to reduce the tcp_collapse latency ?
> Would clamping the duration of a tcp_collapse call be reasonable, since we only
> need to spend enough time to free space to queue the required skb ?

It depends if the incoming skb is queued in in-order queue or
out-of-order queue.
For out-of-orders, we have a strategy in tcp_prune_ofo_queue() which
should work reasonably well after commit
72cd43ba64fc17 tcp: free batches of packets in tcp_prune_ofo_queue()

Given the nature of tcp_collapse(), limiting it to even 1ms of processing time
would still allow for malicious traffic to hurt you quite a lot.

^ permalink raw reply

* Re: [PATCH] serial: imx: reduce RX interrupt frequency
From: Sergey Organov @ 2022-01-05 13:37 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Greg Kroah-Hartman, Fabio Estevam, linux-serial, Jiri Slaby,
	Tomasz Moń, k.drobinski, NXP Linux Team,
	Pengutronix Kernel Team, Shawn Guo, Sascha Hauer
In-Reply-To: <20220105130431.b3vb2icesuedaavk@pengutronix.de>

Marc Kleine-Budde <mkl@pengutronix.de> writes:

> On 05.01.2022 16:00:34, Sergey Organov wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> 
>> > On Tue, Jan 04, 2022 at 11:32:03AM +0100, Tomasz Moń wrote:
>> >> Triggering RX interrupt for every byte defeats the purpose of aging
>> >> timer and leads to interrupt starvation at high baud rates.
>> >> 
>> >> Increase receiver trigger level to 8 to increase the minimum period
>> >> between RX interrupts to 8 characters time. The tradeoff is increased
>> >> latency. In the worst case scenario, where RX data has intercharacter
>> >> delay slightly less than aging timer (8 characters time), it can take
>> >> up to 63 characters time for the interrupt to be raised since the
>> >> reception of the first character.
>> >
>> > Why can't you do this dynamically based on the baud rate so as to always
>> > work properly for all speeds without increased delays for slower ones?
>> 
>> I don't like the idea of dynamic threshold as I don't think increased
>> complexity is worth it.
>> 
>> In fact the threshold works "properly" on any baud rate, as maximum
>> latency is proportional to the current baud rate, and if somebody does
>> care about *absolute* latency, increasing baud rate is the primary
>> solution.
>
> Nope - this only works if you have both sides under control.... Which is
> not the case in our $CUSTROMER's use case.

Yep, if one can't use primary solution, they need to come up with
something else. Where is that historical "low-latency" bit, by the way?

Thanks,
-- Sergey Organov

^ permalink raw reply

* [Buildroot] [Bug 14501] lttng-modules v2.11 package fails to build
From: bugzilla @ 2022-01-05 13:36 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <bug-14501-163@https.bugs.busybox.net/>

https://bugs.busybox.net/show_bug.cgi?id=14501

--- Comment #1 from Thomas Petazzoni <thomas.petazzoni@bootlin.com> ---
Thanks for the bug report. Could you provide the Buildroot .config that allows
to reproduce this issue ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply

* Re: [PATCH v4] ima: silence measurement list hexdump during kexec
From: Mimi Zohar @ 2022-01-05 13:35 UTC (permalink / raw)
  To: Bruno Meneguele; +Cc: linux-integrity, linux-kernel
In-Reply-To: <20211229020303.357610-1-bmeneg@redhat.com>

On Tue, 2021-12-28 at 23:03 -0300, Bruno Meneguele wrote:
> Direclty calling print_hex_dump() dumps the IMA measurement list on soft

^Directly

> resets (kexec) straight to the syslog (kmsg/dmesg) without considering the
> DEBUG flag or the dynamic debug state, causing the output to be always
> printed, including during boot time.
> 
> Since this output is only valid for IMA debugging, but not necessary on
> normal kexec operation, print_hex_dump_debug() adheres to the pr_debug()
> behavior: the dump is only printed to syslog when DEBUG is defined or when
> explicitly requested by the user through dynamic debugging.
> 
> Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>

Thanks, Bruno.  This patch is now queued in #next-integrity-testing.

Mimi


^ permalink raw reply

* Re: [PATCH] serial: imx: reduce RX interrupt frequency
From: Sergey Organov @ 2022-01-05 13:34 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Tomasz Moń, linux-serial, Jiri Slaby,
	Fabio Estevam, Sascha Hauer, NXP Linux Team,
	Pengutronix Kernel Team, Shawn Guo, k.drobinski
In-Reply-To: <20220104224900.u3omfbilejx2jawr@pengutronix.de>

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> On Tue, Jan 04, 2022 at 12:38:01PM +0100, Greg Kroah-Hartman wrote:
>> On Tue, Jan 04, 2022 at 12:13:06PM +0100, Tomasz Moń wrote:
>> > On 04.01.2022 11:54, Greg Kroah-Hartman wrote:
>> > > Why can't you do this dynamically based on the baud rate so as to always
>> > > work properly for all speeds without increased delays for slower ones?
>> > 
>> > Could you please advise on which baud rates to consider as slow? Does it
>> > sound good to have the old trigger level for rates up to and including
>> > 115200 and the new one for faster ones?
>> 
>> You tell me, you are the one seeing this issue and are seeing delays on
>> slower values with your change.  Do some testing to see where the curve
>> is.
>
> Maybe it's more sensible to make this even more dynamic: e.g. keep it at
> 1 as default and increase the water level when a certain irq frequency
> is reached?

Too complex, and too many questions, I'm afraid. What is "irq
frequency", exactly? For this particular driver, or overall system?
Measured on what time interval? What is the threshold? Do we drop the
water level back to 1 when "irq frequency" is down again? Will we just
create re-configure storm at some conditions? Etc.....

Personally, I don't think this AI is worth the trouble. If at all, this
will need to be generic implementation on upper level of TTY subsystem.
I suspect a lot of UARTs have the feature nowadays, and it'd be a bad
idea to implement the same complex policy in every separate driver.

I'm not in favor of dependency on baud rate as well, but if any,
dependency on baud rate looks better for me, being more straightforward.

For what it's worth, I'm +1 for the original version of the patch. I
work with RS232 ports a lot and always set the threshold high in my
drivers. Where latency matters, there are usually methods to get proper
upper-level protocols to reduce it, though ioctl() to set the level
manually might be useful at some extreme conditions.

Thanks,
-- Sergey Organov

^ permalink raw reply

* [PATCH][TRIVIAL] btrfs-progs: Allow autodetect_object_types() to handle the link.
From: Goffredo Baroncelli @ 2022-01-05 13:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Boris Burkov, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>


Hi All,

Boris Burkov reported a problem when "btrfs prop get" is invoked on a link of a block
device. This happens when btrfs is invoked on a LVM device (which typically is
a link with an user friendly name to the real block device). In this case btrfs
reports 'ERROR: object is not a btrfs object'.

------------------
Steps to reproduce:

$ sudo losetup -f disk-1.img 
$ sudo losetup -f disk-2.img 
$ sudo losetup -O NAME,BACK-FILE
NAME       BACK-FILE
/dev/loop1 /home/ghigo/test-allocation-hint/disk-2.img
/dev/loop0 /home/ghigo/test-allocation-hint/disk-1.img
                                  
$ cd /dev/
$ mv loop1 loop1-tmp
$ ln -sf loop1-tmp loop1
$ ls -l /dev/loop[01]*
brw-rw---- 1 root disk 7, 0 Jan  5 13:42 /dev/loop0
lrwxrwxrwx 1 root root    9 Jan  5 13:41 /dev/loop1 -> loop1-tmp
brw-rw---- 1 root disk 7, 1 Jan  5 13:42 /dev/loop1-tmp

$ sudo mkfs.btrfs /dev/loop[0-1]
[....]
$ sudo mount /dev/loop1 mnt/

$ # this is the error
$ sudo btrfs prop get /dev/loop1
ERROR: object is not a btrfs object: /dev/loop1

$ # this is what should happen
$ sudo btrfs prop get /dev/loop0
label=

------------------

The cause is in the function autodetect_object_types() that detects the type of
btrfs object passed. If the object is an "inode" type (e.g. file, link...) it
returns the type of object. If the object is a block device (it doesn't matter
if it is in a btrfs filesystem), it returns it as block device.
However it doesn't handle well the case where the object passed is a link
to a block device (which could be a valid btrfs object). For example
LVM/DM creates link to block devices.

This patch adds a further call to stat() (instead of reusing the previous lstat()
returned value) when btrfs-progs checks if the object is a block device.

Reported-by: Boris Burkov <boris@bur.io>
Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 cmds/property.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cmds/property.c b/cmds/property.c
index 59ef997c..97dc5ae1 100644
--- a/cmds/property.c
+++ b/cmds/property.c
@@ -391,6 +391,14 @@ static int autodetect_object_types(const char *object, int *types_out)
 			types |= prop_object_root;
 	}
 
+	/*
+	 * Do a new stat to follow a possible link
+	 */
+	ret = stat(object, &st);
+	if (ret < 0) {
+		ret = -errno;
+		goto out;
+	}
 	if (S_ISBLK(st.st_mode))
 		types |= prop_object_dev;
 
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH 1/8] dt-bindings: rng: apm, x-gene-rng: convert APM RNG to dtschema
From: Rob Herring @ 2022-01-05 13:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Nishanth Menon, Alexandre Belloni, Tomer Maimon, Tony Lindgren,
	Matt Mackall, Tali Perry, Khuong Dinh, Herbert Xu, openbmc,
	Nancy Yuen, Ludovic Desroches, Patrice Chotard, devicetree,
	Santosh Shilimkar, linux-arm-kernel, Benjamin Fair, Avi Fishman,
	Patrick Venture, linux-kernel, linux-crypto, Pali Rohár
In-Reply-To: <20211227183251.132525-1-krzysztof.kozlowski@canonical.com>

On Mon, Dec 27, 2021 at 07:32:44PM +0100, Krzysztof Kozlowski wrote:
> Convert the APM X-Gene RNG bindings to DT schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  .../devicetree/bindings/rng/apm,rng.txt       | 17 -------
>  .../bindings/rng/apm,x-gene-rng.yaml          | 47 +++++++++++++++++++
>  2 files changed, 47 insertions(+), 17 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rng/apm,rng.txt
>  create mode 100644 Documentation/devicetree/bindings/rng/apm,x-gene-rng.yaml

Series applied, thanks.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/8] dt-bindings: rng: apm, x-gene-rng: convert APM RNG to dtschema
From: Rob Herring @ 2022-01-05 13:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Nishanth Menon, Alexandre Belloni, Tomer Maimon, Tony Lindgren,
	Matt Mackall, Tali Perry, Khuong Dinh, Herbert Xu, openbmc,
	Ludovic Desroches, Patrice Chotard, devicetree, Santosh Shilimkar,
	linux-arm-kernel, Benjamin Fair, Avi Fishman, Patrick Venture,
	Nicolas Ferre, linux-kernel, linux-crypto, Pali Rohár
In-Reply-To: <20211227183251.132525-1-krzysztof.kozlowski@canonical.com>

On Mon, Dec 27, 2021 at 07:32:44PM +0100, Krzysztof Kozlowski wrote:
> Convert the APM X-Gene RNG bindings to DT schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  .../devicetree/bindings/rng/apm,rng.txt       | 17 -------
>  .../bindings/rng/apm,x-gene-rng.yaml          | 47 +++++++++++++++++++
>  2 files changed, 47 insertions(+), 17 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rng/apm,rng.txt
>  create mode 100644 Documentation/devicetree/bindings/rng/apm,x-gene-rng.yaml

Series applied, thanks.

Rob

^ permalink raw reply

* Re: [PATCH 1/8] dt-bindings: rng: apm,x-gene-rng: convert APM RNG to dtschema
From: Rob Herring @ 2022-01-05 13:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Matt Mackall, Herbert Xu, Nicolas Ferre, Alexandre Belloni,
	Ludovic Desroches, Avi Fishman, Tomer Maimon, Tali Perry,
	Patrick Venture, Nancy Yuen, Benjamin Fair, Khuong Dinh,
	Patrice Chotard, Nishanth Menon, Santosh Shilimkar,
	Pali Rohár, Tony Lindgren, linux-crypto, devicetree,
	linux-kernel, linux-arm-kernel, openbmc
In-Reply-To: <20211227183251.132525-1-krzysztof.kozlowski@canonical.com>

On Mon, Dec 27, 2021 at 07:32:44PM +0100, Krzysztof Kozlowski wrote:
> Convert the APM X-Gene RNG bindings to DT schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>  .../devicetree/bindings/rng/apm,rng.txt       | 17 -------
>  .../bindings/rng/apm,x-gene-rng.yaml          | 47 +++++++++++++++++++
>  2 files changed, 47 insertions(+), 17 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rng/apm,rng.txt
>  create mode 100644 Documentation/devicetree/bindings/rng/apm,x-gene-rng.yaml

Series applied, thanks.

Rob

^ permalink raw reply

* Re: [Buildroot] candidate packages : ros2 and wxwidgets
From: Thomas Petazzoni @ 2022-01-05 13:29 UTC (permalink / raw)
  To: Thierry Bultel; +Cc: buildroot
In-Reply-To: <2209e11a-bda9-5193-9b4d-20a5f0fa34e3@linatsea.fr>

Hello Thierry,

On Wed, 5 Jan 2022 10:46:46 +0100
Thierry Bultel <thierry.bultel@linatsea.fr> wrote:

> For a number of companies, I can see a growing interest in having ros2 
> support in buildroot.
> Currently, people wanting to prototype (or make a product) a robotics 
> applications, are stuck
> to either yocto, or, worse, to a desktop-like distribution.
> 
> Another topic is charts displaying for sailing. The most famous 
> application (mostly used
> on raspberry by sailors) is OpenCpn, which is great but relies on wxWidgets.
> 
> Whereas I am looking for a sponsor for adding ros2 to buildroot, 
> (because it is a quite significant work,
> that implies to add a new build framework), I am thinking about using my 
> (little) free time for wxWidgets (and OpenCpn later).
> There has been an attempt is the past 
> (https://bugs.busybox.net/show_bug.cgi?id=261) but it does not
> seem to have lead to anything else since that time.
> 
> What are your thought about these both packages ? Any ideas, or links to 
> some WIP stuff ?

These packages would certainly be very welcome. ROS2 especially makes a
lot of sense.

Some prior work:

  https://github.com/mchalain/br_ros
  http://lists.busybox.net/pipermail/buildroot/2019-March/246497.html

I'll be happy to help in the review of such packages.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply

* RE: [PATCH v2 3/5] RISC-V: 'info gmem' to show hypervisor guest -> physical address translations
From: Schwarz, Konrad @ 2022-01-05 13:09 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers, Ralf Ramsauer
In-Reply-To: <CAKmqyKNq6ewguvDAH_v=4N5qPQAMgZSJNU8pJRC91X-Sh_WvXA@mail.gmail.com>

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Tuesday, January 4, 2022 23:03
> On Wed, Jan 5, 2022 at 1:55 AM Konrad Schwarz
> <konrad.schwarz@siemens.com> wrote:
> >
> > This is analog to the existing 'info mem' command and is implemented
> > using the same machinery.

> >  hmp-commands-info.hx         |  16 +++++
> >  include/monitor/hmp-target.h |   2 +
> >  target/riscv/monitor.c       | 135 +++++++++++++++++++++++++----------
> >  3 files changed, 117 insertions(+), 36 deletions(-)
> >
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index 407a1da800..fa519f0129 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -237,6 +237,22 @@ SRST
> >      Show the active virtual memory mappings.
> >  ERST
> >
> > +#if defined TARGET_RISCV
> > +    {
> > +        .name       = "gmem",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "show the hypervisor guest's physical address"
> > +                   " translation",
> > +        .cmd        = hmp_info_gmem,
> > +    },
> > +#endif
> 
> I don't think we want RISC-V specific commands. Could we not just
> extend `info mem` instead?

Considering that the similar commands `info tlb' and `info mem'
are target dependent (i.e. specific to I386, SH4, SPARC,
PCC, XTENSA and M68K, respectively to I386 and RISC-V),
I honestly do not see a problem here.

Obviously, other architectures are free to add their own implementation
of the `info gmem' functionality, so the list of architectures
supporting this command might grow in future.  The command itself
is not specific to RISC-V.

PS: I will be taking your other points to heart on the re-roll,
I just wanted to get this out of the way before attempting it.

Regards,
Konrad

^ permalink raw reply

* RE: [PATCH v4 0/4] kvm/arm: New VMID allocator based on asid
From: Shameerali Kolothum Thodi @ 2022-01-05 13:25 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org
  Cc: maz@kernel.org, will@kernel.org, catalin.marinas@arm.com,
	james.morse@arm.com, julien.thierry.kdev@gmail.com,
	suzuki.poulose@arm.com, jean-philippe@linaro.org,
	Alexandru.Elisei@arm.com, qperret@google.com, Jonathan Cameron,
	Linuxarm
In-Reply-To: <20211122121844.867-1-shameerali.kolothum.thodi@huawei.com>

Hi,

A gentle ping on this series. Please take a look and let me know the new approach
taken in this revision is good enough or not. 

Appreciate your feedback.

Thanks,
Shameer

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Shameer Kolothum
> Sent: 22 November 2021 12:19
> To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> linux-kernel@vger.kernel.org
> Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> james.morse@arm.com; julien.thierry.kdev@gmail.com;
> suzuki.poulose@arm.com; jean-philippe@linaro.org;
> Alexandru.Elisei@arm.com; qperret@google.com; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>
> Subject: [PATCH v4 0/4] kvm/arm: New VMID allocator based on asid
> 
> Changes from v3:
> - Main change is in patch #4, where the VMID is now set to an
>   invalid one on vCPU schedule out. Introduced an
>   INVALID_ACTIVE_VMID which is basically a VMID 0 with generation 1.
>   Since the basic allocator algorithm reserves vmid #0, it is never
>   used as an active VMID. This (hopefully) will fix the issue of
>   unnecessarily reserving VMID space with active_vmids when those
>   VMs are no longer active[0] and at the same time address the
>   problem noted in v3 wherein everything ends up in slow-path[1].
> 
> Testing:
>  -Run with VMID bit set to 4 and maxcpus to 8 on D06. The test
>   involves running concurrently 50 guests with 4 vCPUs. Each
>   guest will then execute hackbench 5 times before exiting.
>   No crash was observed for a 4-day continuous run.
>   The latest branch is here,
>    https://github.com/hisilicon/kernel-dev/tree/private-v5.16-rc1-vmid-v4
> 
>  -TLA+ model. Modified the asidalloc model to incorporate the new
>   VMID algo. The main differences are,
>   -flush_tlb_all() instead of local_tlb_flush_all() on rollover.
>   -Introduced INVALID_VMID and vCPU Sched Out logic.
>   -No CnP (Removed UniqueASIDAllCPUs & UniqueASIDActiveTask invariants).
>   -Removed  UniqueVMIDPerCPU invariant for now as it looks like
>    because of the speculative fetching with flush_tlb_all() there
>    is a small window where this gets triggered. If I change the
>    logic back to local_flush_tlb_all(), UniqueVMIDPerCPU seems to
>    be fine. With my limited knowledge on TLA+ model, it is not
>    clear to me whether this is a problem with the above logic
>    or the VMID model implementation. Really appreciate any help
>    with the model.
>    The initial VMID TLA+ model is here,
>    https://github.com/shamiali2008/kernel-tla/tree/private-vmidalloc-v1
> 
> Please take a look and let me know.
> 
> Thanks,
> Shameer
> 
> [0]
> https://lore.kernel.org/kvmarm/20210721160614.GC11003@willie-the-truck/
> [1]
> https://lore.kernel.org/kvmarm/20210803114034.GB30853@willie-the-truck/
> 
> History:
> --------
> v2 --> v3
>   -Dropped adding a new static key and cpufeature for retrieving
>    supported VMID bits. Instead, we now make use of the
>    kvm_arm_vmid_bits variable (patch #2).
> 
>   -Since we expect less frequent rollover in the case of VMIDs,
>    the TLB invalidation is now broadcasted on rollover instead
>    of keeping per CPU flush_pending info and issuing a local
>    context flush.
> 
>   -Clear active_vmids on vCPU schedule out to avoid unnecessarily
>    reserving the VMID space(patch #3).
> 
>   -I have kept the struct kvm_vmid as it is for now(instead of a
>    typedef as suggested), as we may soon add another variable to
>    it when we introduce Pinned KVM VMID support.
> 
> RFCv1 --> v2
>    -Dropped "pinned VMID" support for now.
>    -Dropped RFC tag.
> RFCv1
> 
> https://lore.kernel.org/kvmarm/20210506165232.1969-1-shameerali.kolothu
> m.thodi@huawei.com/
> 
> Julien Grall (1):
>   KVM: arm64: Align the VMID allocation with the arm64 ASID
> 
> Shameer Kolothum (3):
>   KVM: arm64: Introduce a new VMID allocator for KVM
>   KVM: arm64: Make VMID bits accessible outside of allocator
>   KVM: arm64: Make active_vmids invalid on vCPU schedule out
> 
>  arch/arm64/include/asm/kvm_host.h     |  10 +-
>  arch/arm64/include/asm/kvm_mmu.h      |   4 +-
>  arch/arm64/kernel/image-vars.h        |   3 +
>  arch/arm64/kvm/Makefile               |   2 +-
>  arch/arm64/kvm/arm.c                  | 106 +++-----------
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c |   3 +-
>  arch/arm64/kvm/mmu.c                  |   1 -
>  arch/arm64/kvm/vmid.c                 | 196
> ++++++++++++++++++++++++++
>  8 files changed, 228 insertions(+), 97 deletions(-)  create mode 100644
> arch/arm64/kvm/vmid.c
> 
> --
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs
From: Lazar, Lijo @ 2022-01-05 13:26 UTC (permalink / raw)
  To: Christian König, Andrey Grodzovsky, dri-devel, amd-gfx
  Cc: horace.chen, daniel, Monk.Liu
In-Reply-To: <6be71531-40f1-fcad-f54c-03f6e14064f9@amd.com>



On 1/5/2022 6:45 PM, Christian König wrote:
> Am 05.01.22 um 14:11 schrieb Lazar, Lijo:
>> On 1/5/2022 6:01 PM, Christian König wrote:
>>> Am 05.01.22 um 10:54 schrieb Lazar, Lijo:
>>>> On 12/23/2021 3:35 AM, Andrey Grodzovsky wrote:
>>>>> Use reset domain wq also for non TDR gpu recovery trigers
>>>>> such as sysfs and RAS. We must serialize all possible
>>>>> GPU recoveries to gurantee no concurrency there.
>>>>> For TDR call the original recovery function directly since
>>>>> it's already executed from within the wq. For others just
>>>>> use a wrapper to qeueue work and wait on it to finish.
>>>>>
>>>>> v2: Rename to amdgpu_recover_work_struct
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 
>>>>> +++++++++++++++++++++-
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c    |  2 +-
>>>>>   3 files changed, 35 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index b5ff76aae7e0..8e96b9a14452 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -1296,6 +1296,8 @@ bool amdgpu_device_has_job_running(struct 
>>>>> amdgpu_device *adev);
>>>>>   bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
>>>>>   int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>>>>                     struct amdgpu_job* job);
>>>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>>>> +                  struct amdgpu_job *job);
>>>>>   void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
>>>>>   int amdgpu_device_pci_reset(struct amdgpu_device *adev);
>>>>>   bool amdgpu_device_need_post(struct amdgpu_device *adev);
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> index 7c063fd37389..258ec3c0b2af 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> @@ -4979,7 +4979,7 @@ static void amdgpu_device_recheck_guilty_jobs(
>>>>>    * Returns 0 for success or an error on failure.
>>>>>    */
>>>>>   -int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>>>>                     struct amdgpu_job *job)
>>>>>   {
>>>>>       struct list_head device_list, *device_list_handle = NULL;
>>>>> @@ -5237,6 +5237,37 @@ int amdgpu_device_gpu_recover(struct 
>>>>> amdgpu_device *adev,
>>>>>       return r;
>>>>>   }
>>>>>   +struct amdgpu_recover_work_struct {
>>>>> +    struct work_struct base;
>>>>> +    struct amdgpu_device *adev;
>>>>> +    struct amdgpu_job *job;
>>>>> +    int ret;
>>>>> +};
>>>>> +
>>>>> +static void amdgpu_device_queue_gpu_recover_work(struct 
>>>>> work_struct *work)
>>>>> +{
>>>>> +    struct amdgpu_recover_work_struct *recover_work = 
>>>>> container_of(work, struct amdgpu_recover_work_struct, base);
>>>>> +
>>>>> +    recover_work->ret = 
>>>>> amdgpu_device_gpu_recover_imp(recover_work->adev, recover_work->job);
>>>>> +}
>>>>> +/*
>>>>> + * Serialize gpu recover into reset domain single threaded wq
>>>>> + */
>>>>> +int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>>>> +                    struct amdgpu_job *job)
>>>>> +{
>>>>> +    struct amdgpu_recover_work_struct work = {.adev = adev, .job = 
>>>>> job};
>>>>> +
>>>>> +    INIT_WORK(&work.base, amdgpu_device_queue_gpu_recover_work);
>>>>> +
>>>>> +    if (!queue_work(adev->reset_domain.wq, &work.base))
>>>>> +        return -EAGAIN;
>>>>> +
>>>>
>>>> The decision to schedule a reset is made at this point. Subsequent 
>>>> accesses to hardware may not be reliable. So should the flag 
>>>> in_reset be set here itself rather than waiting for the work to 
>>>> start execution?
>>>
>>> No, when we race and lose the VM is completely lost and probably 
>>> restarted by the hypervisor.
>>>
>>> And when we race and win we properly set the flag before signaling 
>>> the hypervisor that it can continue with the reset.
>>>
>>
>> I was talking about baremetal case. When this was synchronous, 
>> in_reset flag is set as one of the first things and amdgpu_in_reset is 
>> checked to prevent further hardware accesses. This design only changes 
>> the recover part and doesn't change the hardware perspective. 
> 
>> Potential accesses from other processes need to be blocked as soon as 
>> we determine a reset is required.
> 
> That's an incorrect assumption.
> 
> Accessing the hardware is perfectly ok as long as the reset hasn't 
> started yet. In other words even when the hardware is locked up you can 
> still happily read/write registers or access the VRAM BAR.
> 

Not sure if that is 100% correct like a recovery triggered by RAS error 
(depends on the access done).

Thanks,
Lijo

> Only when the hardware is currently performing a reset, then we can't 
> touch it or there might be unfortunate consequences (usually complete 
> system lockup).
> 
> Regards,
> Christian.
> 
>> Are we expecting the work to be immediately executed and set the flags?
>>
>> Thanks,
>> Lijo
>>
>>>> Also, what about having the reset_active or in_reset flag in the 
>>>> reset_domain itself?
>>>
>>> Of hand that sounds like a good idea.
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>> +    flush_work(&work.base);
>>>>> +
>>>>> +    return work.ret;
>>>>> +}
>>>>> +
>>>>>   /**
>>>>>    * amdgpu_device_get_pcie_info - fence pcie info about the PCIE slot
>>>>>    *
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>>> index bfc47bea23db..38c9fd7b7ad4 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>>> @@ -63,7 +63,7 @@ static enum drm_gpu_sched_stat 
>>>>> amdgpu_job_timedout(struct drm_sched_job *s_job)
>>>>>             ti.process_name, ti.tgid, ti.task_name, ti.pid);
>>>>>         if (amdgpu_device_should_recover_gpu(ring->adev)) {
>>>>> -        amdgpu_device_gpu_recover(ring->adev, job);
>>>>> +        amdgpu_device_gpu_recover_imp(ring->adev, job);
>>>>>       } else {
>>>>>           drm_sched_suspend_timeout(&ring->sched);
>>>>>           if (amdgpu_sriov_vf(adev))
>>>>>
>>>
> 

^ permalink raw reply

* Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs
From: Lazar, Lijo @ 2022-01-05 13:26 UTC (permalink / raw)
  To: Christian König, Andrey Grodzovsky, dri-devel, amd-gfx
  Cc: horace.chen, Monk.Liu
In-Reply-To: <6be71531-40f1-fcad-f54c-03f6e14064f9@amd.com>



On 1/5/2022 6:45 PM, Christian König wrote:
> Am 05.01.22 um 14:11 schrieb Lazar, Lijo:
>> On 1/5/2022 6:01 PM, Christian König wrote:
>>> Am 05.01.22 um 10:54 schrieb Lazar, Lijo:
>>>> On 12/23/2021 3:35 AM, Andrey Grodzovsky wrote:
>>>>> Use reset domain wq also for non TDR gpu recovery trigers
>>>>> such as sysfs and RAS. We must serialize all possible
>>>>> GPU recoveries to gurantee no concurrency there.
>>>>> For TDR call the original recovery function directly since
>>>>> it's already executed from within the wq. For others just
>>>>> use a wrapper to qeueue work and wait on it to finish.
>>>>>
>>>>> v2: Rename to amdgpu_recover_work_struct
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  2 ++
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 33 
>>>>> +++++++++++++++++++++-
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c    |  2 +-
>>>>>   3 files changed, 35 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index b5ff76aae7e0..8e96b9a14452 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -1296,6 +1296,8 @@ bool amdgpu_device_has_job_running(struct 
>>>>> amdgpu_device *adev);
>>>>>   bool amdgpu_device_should_recover_gpu(struct amdgpu_device *adev);
>>>>>   int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>>>>                     struct amdgpu_job* job);
>>>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>>>> +                  struct amdgpu_job *job);
>>>>>   void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
>>>>>   int amdgpu_device_pci_reset(struct amdgpu_device *adev);
>>>>>   bool amdgpu_device_need_post(struct amdgpu_device *adev);
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> index 7c063fd37389..258ec3c0b2af 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>>> @@ -4979,7 +4979,7 @@ static void amdgpu_device_recheck_guilty_jobs(
>>>>>    * Returns 0 for success or an error on failure.
>>>>>    */
>>>>>   -int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>>>> +int amdgpu_device_gpu_recover_imp(struct amdgpu_device *adev,
>>>>>                     struct amdgpu_job *job)
>>>>>   {
>>>>>       struct list_head device_list, *device_list_handle = NULL;
>>>>> @@ -5237,6 +5237,37 @@ int amdgpu_device_gpu_recover(struct 
>>>>> amdgpu_device *adev,
>>>>>       return r;
>>>>>   }
>>>>>   +struct amdgpu_recover_work_struct {
>>>>> +    struct work_struct base;
>>>>> +    struct amdgpu_device *adev;
>>>>> +    struct amdgpu_job *job;
>>>>> +    int ret;
>>>>> +};
>>>>> +
>>>>> +static void amdgpu_device_queue_gpu_recover_work(struct 
>>>>> work_struct *work)
>>>>> +{
>>>>> +    struct amdgpu_recover_work_struct *recover_work = 
>>>>> container_of(work, struct amdgpu_recover_work_struct, base);
>>>>> +
>>>>> +    recover_work->ret = 
>>>>> amdgpu_device_gpu_recover_imp(recover_work->adev, recover_work->job);
>>>>> +}
>>>>> +/*
>>>>> + * Serialize gpu recover into reset domain single threaded wq
>>>>> + */
>>>>> +int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>>>> +                    struct amdgpu_job *job)
>>>>> +{
>>>>> +    struct amdgpu_recover_work_struct work = {.adev = adev, .job = 
>>>>> job};
>>>>> +
>>>>> +    INIT_WORK(&work.base, amdgpu_device_queue_gpu_recover_work);
>>>>> +
>>>>> +    if (!queue_work(adev->reset_domain.wq, &work.base))
>>>>> +        return -EAGAIN;
>>>>> +
>>>>
>>>> The decision to schedule a reset is made at this point. Subsequent 
>>>> accesses to hardware may not be reliable. So should the flag 
>>>> in_reset be set here itself rather than waiting for the work to 
>>>> start execution?
>>>
>>> No, when we race and lose the VM is completely lost and probably 
>>> restarted by the hypervisor.
>>>
>>> And when we race and win we properly set the flag before signaling 
>>> the hypervisor that it can continue with the reset.
>>>
>>
>> I was talking about baremetal case. When this was synchronous, 
>> in_reset flag is set as one of the first things and amdgpu_in_reset is 
>> checked to prevent further hardware accesses. This design only changes 
>> the recover part and doesn't change the hardware perspective. 
> 
>> Potential accesses from other processes need to be blocked as soon as 
>> we determine a reset is required.
> 
> That's an incorrect assumption.
> 
> Accessing the hardware is perfectly ok as long as the reset hasn't 
> started yet. In other words even when the hardware is locked up you can 
> still happily read/write registers or access the VRAM BAR.
> 

Not sure if that is 100% correct like a recovery triggered by RAS error 
(depends on the access done).

Thanks,
Lijo

> Only when the hardware is currently performing a reset, then we can't 
> touch it or there might be unfortunate consequences (usually complete 
> system lockup).
> 
> Regards,
> Christian.
> 
>> Are we expecting the work to be immediately executed and set the flags?
>>
>> Thanks,
>> Lijo
>>
>>>> Also, what about having the reset_active or in_reset flag in the 
>>>> reset_domain itself?
>>>
>>> Of hand that sounds like a good idea.
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>> +    flush_work(&work.base);
>>>>> +
>>>>> +    return work.ret;
>>>>> +}
>>>>> +
>>>>>   /**
>>>>>    * amdgpu_device_get_pcie_info - fence pcie info about the PCIE slot
>>>>>    *
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>>> index bfc47bea23db..38c9fd7b7ad4 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
>>>>> @@ -63,7 +63,7 @@ static enum drm_gpu_sched_stat 
>>>>> amdgpu_job_timedout(struct drm_sched_job *s_job)
>>>>>             ti.process_name, ti.tgid, ti.task_name, ti.pid);
>>>>>         if (amdgpu_device_should_recover_gpu(ring->adev)) {
>>>>> -        amdgpu_device_gpu_recover(ring->adev, job);
>>>>> +        amdgpu_device_gpu_recover_imp(ring->adev, job);
>>>>>       } else {
>>>>>           drm_sched_suspend_timeout(&ring->sched);
>>>>>           if (amdgpu_sriov_vf(adev))
>>>>>
>>>
> 

^ permalink raw reply

* Re: [PATCH v2 10/35] brcmfmac: firmware: Allow platform to override macaddr
From: Hector Martin @ 2022-01-05 13:26 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Kalle Valo, David S. Miller, Jakub Kicinski, Rob Herring,
	Rafael J. Wysocki, Len Brown, Arend van Spriel, Franky Lin,
	Hante Meuleman, Chi-hsien Lin, Wright Feng, Dmitry Osipenko,
	Sven Peter, Alyssa Rosenzweig, Mark Kettenis,
	Rafał Miłecki, Pieter-Paul Giesberts, Linus Walleij,
	Hans de Goede, John W. Linville, brian m. carlson,
	open list:TI WILINK WIRELES..., netdev, devicetree,
	Linux Kernel Mailing List, ACPI Devel Maling List,
	open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
	SHA-cyfmac-dev-list
In-Reply-To: <CAHp75VcU1vVSucvegmSiMLoKBoPoGW5XLmqVUG0vXGdeafm2Jw@mail.gmail.com>

On 04/01/2022 23.23, Andy Shevchenko wrote:
> On Tue, Jan 4, 2022 at 9:29 AM Hector Martin <marcan@marcan.st> wrote:
>>
>> On Device Tree platforms, it is customary to be able to set the MAC
>> address via the Device Tree, as it is often stored in system firmware.
>> This is particularly relevant for Apple ARM64 platforms, where this
>> information comes from system configuration and passed through by the
>> bootloader into the DT.
>>
>> Implement support for this by fetching the platform MAC address and
>> adding or replacing the macaddr= property in nvram. This becomes the
>> dongle's default MAC address.
>>
>> On platforms with an SROM MAC address, this overrides it. On platforms
>> without one, such as Apple ARM64 devices, this is required for the
>> firmware to boot (it will fail if it does not have a valid MAC at all).
> 
> ...
> 
>> +#define BRCMF_FW_MACADDR_FMT                   "macaddr=%pM"
>> +#define BRCMF_FW_MACADDR_LEN                   (7 + ETH_ALEN * 3)
> 
> ...
> 
>>                 if (strncmp(&nvp->data[nvp->entry], "boardrev", 8) == 0)
>>                         nvp->boardrev_found = true;
>> +               /* strip macaddr if platform MAC overrides */
>> +               if (nvp->strip_mac &&
>> +                   strncmp(&nvp->data[nvp->entry], "macaddr", 7) == 0)
> 
> If it has no side effects, I would rather swap the operands of && so
> you match string first (it will be in align with above code at least,
> although I haven't checked bigger context).

I usually check for trivial flags before calling more expensive
functions because it's more efficient in the common case. Obviously here
performance doesn't matter though.

> 
> ....
> 
>> +static void brcmf_fw_add_macaddr(struct nvram_parser *nvp, u8 *mac)
>> +{
>> +       snprintf(&nvp->nvram[nvp->nvram_len], BRCMF_FW_MACADDR_LEN + 1,
>> +                BRCMF_FW_MACADDR_FMT, mac);
> 
> Please, avoid using implict format string, it's dangerous from security p.o.v.

What do you mean by implicit format string? The format string is at the
top of the file and its length is right next to it, which makes it
harder for them to accidentally fall out of sync.

+#define BRCMF_FW_MACADDR_FMT			"macaddr=%pM"
+#define BRCMF_FW_MACADDR_LEN			(7 + ETH_ALEN * 3)

> 
>> +       nvp->nvram_len += BRCMF_FW_MACADDR_LEN + 1;
> 
> Also, with temporary variable the code can be better to read
> 
> size_t mac_len = ...;
> 

Sure.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

^ permalink raw reply

* [RFC PATCH] ceph: add new "nopagecache" option
From: Jeff Layton @ 2022-01-05 13:26 UTC (permalink / raw)
  To: ceph-devel; +Cc: idryomov, majianpeng

CephFS is a bit unlike most other filesystems in that it only
conditionally does buffered I/O based on the caps that it gets from the
MDS. In most cases, unless there is contended access for an inode the
MDS does give Fbc caps to the client, so the unbuffered codepaths are
only infrequently traveled and are difficult to test.

At one time, the "-o sync" mount option would give you this behavior,
but that was removed in commit 7ab9b3807097 ("ceph: Don't use
ceph-sync-mode for synchronous-fs.").

Add a new mount option to tell the client to ignore Fbc caps when doing
I/O, and to use the synchronous codepaths exclusively, even on
non-O_DIRECT file descriptors. We already have an ioctl that forces this
behavior on a per-inode basis, so we can just always set the CEPH_F_SYNC
flag on such mounts.

Additionally, this patch also changes the client to not request Fbc when
doing direct I/O. We aren't using the cache with O_DIRECT so we don't
have any need for those caps.

Cc: majianpeng <majianpeng@gmail.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/file.c  | 24 +++++++++++++++---------
 fs/ceph/super.c | 11 +++++++++++
 fs/ceph/super.h |  1 +
 3 files changed, 27 insertions(+), 9 deletions(-)

I've been working with this patch in order to test the synchronous
codepaths for content encryption. I think though that this might make
sense to take into mainline, as it could be helpful for troubleshooting,
and ensuring that these codepaths are regularly tested.

Either way, I think the cap handling changes probably make sense on
their own. I can split that out if we don't want to take the mount
option in as well.

Thoughts?

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index c138e8126286..7de5db51c3d0 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -204,6 +204,8 @@ static int ceph_init_file_info(struct inode *inode, struct file *file,
 					int fmode, bool isdir)
 {
 	struct ceph_inode_info *ci = ceph_inode(inode);
+	struct ceph_mount_options *opt =
+		ceph_inode_to_client(&ci->vfs_inode)->mount_options;
 	struct ceph_file_info *fi;
 
 	dout("%s %p %p 0%o (%s)\n", __func__, inode, file,
@@ -225,6 +227,9 @@ static int ceph_init_file_info(struct inode *inode, struct file *file,
 		if (!fi)
 			return -ENOMEM;
 
+		if (opt->flags & CEPH_MOUNT_OPT_NOPAGECACHE)
+			fi->flags |= CEPH_F_SYNC;
+
 		file->private_data = fi;
 	}
 
@@ -1536,7 +1541,7 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	struct ceph_inode_info *ci = ceph_inode(inode);
 	bool direct_lock = iocb->ki_flags & IOCB_DIRECT;
 	ssize_t ret;
-	int want, got = 0;
+	int want = 0, got = 0;
 	int retry_op = 0, read = 0;
 
 again:
@@ -1551,13 +1556,14 @@ static ssize_t ceph_read_iter(struct kiocb *iocb, struct iov_iter *to)
 	else
 		ceph_start_io_read(inode);
 
+	if (!(fi->flags & CEPH_F_SYNC) && !direct_lock)
+		want |= CEPH_CAP_FILE_CACHE;
 	if (fi->fmode & CEPH_FILE_MODE_LAZY)
-		want = CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO;
-	else
-		want = CEPH_CAP_FILE_CACHE;
+		want |= CEPH_CAP_FILE_LAZYIO;
+
 	ret = ceph_get_caps(filp, CEPH_CAP_FILE_RD, want, -1, &got);
 	if (ret < 0) {
-		if (iocb->ki_flags & IOCB_DIRECT)
+		if (direct_lock)
 			ceph_end_io_direct(inode);
 		else
 			ceph_end_io_read(inode);
@@ -1691,7 +1697,7 @@ static ssize_t ceph_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	struct ceph_osd_client *osdc = &fsc->client->osdc;
 	struct ceph_cap_flush *prealloc_cf;
 	ssize_t count, written = 0;
-	int err, want, got;
+	int err, want = 0, got;
 	bool direct_lock = false;
 	u32 map_flags;
 	u64 pool_flags;
@@ -1766,10 +1772,10 @@ static ssize_t ceph_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	dout("aio_write %p %llx.%llx %llu~%zd getting caps. i_size %llu\n",
 	     inode, ceph_vinop(inode), pos, count, i_size_read(inode));
+	if (!(fi->flags & CEPH_F_SYNC) && !direct_lock)
+		want |= CEPH_CAP_FILE_BUFFER;
 	if (fi->fmode & CEPH_FILE_MODE_LAZY)
-		want = CEPH_CAP_FILE_BUFFER | CEPH_CAP_FILE_LAZYIO;
-	else
-		want = CEPH_CAP_FILE_BUFFER;
+		want |= CEPH_CAP_FILE_LAZYIO;
 	got = 0;
 	err = ceph_get_caps(file, CEPH_CAP_FILE_WR, want, pos + count, &got);
 	if (err < 0)
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 8d6daea351f6..d7e604a56fd9 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -160,6 +160,7 @@ enum {
 	Opt_quotadf,
 	Opt_copyfrom,
 	Opt_wsync,
+	Opt_pagecache,
 };
 
 enum ceph_recover_session_mode {
@@ -201,6 +202,7 @@ static const struct fs_parameter_spec ceph_mount_parameters[] = {
 	fsparam_string	("mon_addr",			Opt_mon_addr),
 	fsparam_u32	("wsize",			Opt_wsize),
 	fsparam_flag_no	("wsync",			Opt_wsync),
+	fsparam_flag_no	("pagecache",			Opt_pagecache),
 	{}
 };
 
@@ -567,6 +569,12 @@ static int ceph_parse_mount_param(struct fs_context *fc,
 		else
 			fsopt->flags |= CEPH_MOUNT_OPT_ASYNC_DIROPS;
 		break;
+	case Opt_pagecache:
+		if (result.negated)
+			fsopt->flags |= CEPH_MOUNT_OPT_NOPAGECACHE;
+		else
+			fsopt->flags &= ~CEPH_MOUNT_OPT_NOPAGECACHE;
+		break;
 	default:
 		BUG();
 	}
@@ -702,6 +710,9 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
 	if (!(fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS))
 		seq_puts(m, ",wsync");
 
+	if (fsopt->flags & CEPH_MOUNT_OPT_NOPAGECACHE)
+		seq_puts(m, ",nopagecache");
+
 	if (fsopt->wsize != CEPH_MAX_WRITE_SIZE)
 		seq_printf(m, ",wsize=%u", fsopt->wsize);
 	if (fsopt->rsize != CEPH_MAX_READ_SIZE)
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index f9b1bbf26c1b..5c6d9384b7be 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -46,6 +46,7 @@
 #define CEPH_MOUNT_OPT_NOQUOTADF       (1<<13) /* no root dir quota in statfs */
 #define CEPH_MOUNT_OPT_NOCOPYFROM      (1<<14) /* don't use RADOS 'copy-from' op */
 #define CEPH_MOUNT_OPT_ASYNC_DIROPS    (1<<15) /* allow async directory ops */
+#define CEPH_MOUNT_OPT_NOPAGECACHE     (1<<16) /* bypass pagecache altogether */
 
 #define CEPH_MOUNT_OPT_DEFAULT			\
 	(CEPH_MOUNT_OPT_DCACHE |		\
-- 
2.33.1


^ permalink raw reply related

* Re: [PATCH net v3] net/smc: Reset conn->lgr when link group registration fails
From: Karsten Graul @ 2022-01-05 13:25 UTC (permalink / raw)
  To: Wen Gu, dust.li, davem, kuba; +Cc: linux-s390, netdev, linux-kernel
In-Reply-To: <23b607fe-95da-ea8a-8dda-900a51572b90@linux.alibaba.com>

On 05/01/2022 09:55, Wen Gu wrote:
> On 2022/1/5 3:54 pm, dust.li wrote:
> 
>>> -        if (rc)
>>> +        if (rc) {
>>> +            spin_lock_bh(lgr_lock);
>>> +            if (!list_empty(&lgr->list))
>>> +                list_del_init(&lgr->list);
>>> +            spin_unlock_bh(lgr_lock);
>>> +            __smc_lgr_terminate(lgr, true);
>>
>> What about adding a smc_lgr_terminate() wrapper and put list_del_init()
>> and __smc_lgr_terminate() into it ?
> 
> Adding a new wrapper is a good idea. But I think the logic here is relatively simple.
> So instead of wrapping them, I coded them like what smc_lgr_cleanup_early() does.

It might look cleaner with the following changes:
- adopt smc_lgr_cleanup_early() to take only an lgr as parameter and remove the call to smc_conn_free()
- change smc_conn_abort() (which is the only caller of smc_lgr_cleanup_early() right now), always
  call smc_conn_free() and if (local_first) additionally call smc_lgr_cleanup_early() 
  (hold a local copy of the lgr for this call)
- finally call smc_lgr_cleanup_early(lgr) from smc_conn_create()

This should be the same processing, but the smc_conn_free() is moved to smc_conn_abort() where
it looks to be a better place for this call. And smc_lgr_cleanup_early() takes only care of an lgr.

What do you think? Did I miss something?

^ permalink raw reply

* Re: [PATCH] nitrogen6x: add missing pinctrl to fix mmc
From: Fabio Estevam @ 2022-01-05 13:25 UTC (permalink / raw)
  To: Gary Bisson, Stefano Babic, Tom Rini; +Cc: U-Boot-Denx, Troy Kisky
In-Reply-To: <20220105131753.1137235-1-gary.bisson@boundarydevices.com>

Hi Gary,

On Wed, Jan 5, 2022 at 10:18 AM Gary Bisson
<gary.bisson@boundarydevices.com> wrote:
>
> Since commit f7ac30b042d, the pin muxing for mmc was removed from the
> board file to be managed by DM_MMC which requires PINCTRL to work. It
> made the change for sabrelite but nitrogen configs were forgotten.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>

Since it is a bug fix, it would be nice to get included in the 2022.01 release.

^ permalink raw reply

* RE: [PATCH v4 0/4] kvm/arm: New VMID allocator based on asid
From: Shameerali Kolothum Thodi @ 2022-01-05 13:25 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org
  Cc: jean-philippe@linaro.org, maz@kernel.org, Linuxarm,
	Jonathan Cameron, catalin.marinas@arm.com, will@kernel.org
In-Reply-To: <20211122121844.867-1-shameerali.kolothum.thodi@huawei.com>

Hi,

A gentle ping on this series. Please take a look and let me know the new approach
taken in this revision is good enough or not. 

Appreciate your feedback.

Thanks,
Shameer

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Shameer Kolothum
> Sent: 22 November 2021 12:19
> To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> linux-kernel@vger.kernel.org
> Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> james.morse@arm.com; julien.thierry.kdev@gmail.com;
> suzuki.poulose@arm.com; jean-philippe@linaro.org;
> Alexandru.Elisei@arm.com; qperret@google.com; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>
> Subject: [PATCH v4 0/4] kvm/arm: New VMID allocator based on asid
> 
> Changes from v3:
> - Main change is in patch #4, where the VMID is now set to an
>   invalid one on vCPU schedule out. Introduced an
>   INVALID_ACTIVE_VMID which is basically a VMID 0 with generation 1.
>   Since the basic allocator algorithm reserves vmid #0, it is never
>   used as an active VMID. This (hopefully) will fix the issue of
>   unnecessarily reserving VMID space with active_vmids when those
>   VMs are no longer active[0] and at the same time address the
>   problem noted in v3 wherein everything ends up in slow-path[1].
> 
> Testing:
>  -Run with VMID bit set to 4 and maxcpus to 8 on D06. The test
>   involves running concurrently 50 guests with 4 vCPUs. Each
>   guest will then execute hackbench 5 times before exiting.
>   No crash was observed for a 4-day continuous run.
>   The latest branch is here,
>    https://github.com/hisilicon/kernel-dev/tree/private-v5.16-rc1-vmid-v4
> 
>  -TLA+ model. Modified the asidalloc model to incorporate the new
>   VMID algo. The main differences are,
>   -flush_tlb_all() instead of local_tlb_flush_all() on rollover.
>   -Introduced INVALID_VMID and vCPU Sched Out logic.
>   -No CnP (Removed UniqueASIDAllCPUs & UniqueASIDActiveTask invariants).
>   -Removed  UniqueVMIDPerCPU invariant for now as it looks like
>    because of the speculative fetching with flush_tlb_all() there
>    is a small window where this gets triggered. If I change the
>    logic back to local_flush_tlb_all(), UniqueVMIDPerCPU seems to
>    be fine. With my limited knowledge on TLA+ model, it is not
>    clear to me whether this is a problem with the above logic
>    or the VMID model implementation. Really appreciate any help
>    with the model.
>    The initial VMID TLA+ model is here,
>    https://github.com/shamiali2008/kernel-tla/tree/private-vmidalloc-v1
> 
> Please take a look and let me know.
> 
> Thanks,
> Shameer
> 
> [0]
> https://lore.kernel.org/kvmarm/20210721160614.GC11003@willie-the-truck/
> [1]
> https://lore.kernel.org/kvmarm/20210803114034.GB30853@willie-the-truck/
> 
> History:
> --------
> v2 --> v3
>   -Dropped adding a new static key and cpufeature for retrieving
>    supported VMID bits. Instead, we now make use of the
>    kvm_arm_vmid_bits variable (patch #2).
> 
>   -Since we expect less frequent rollover in the case of VMIDs,
>    the TLB invalidation is now broadcasted on rollover instead
>    of keeping per CPU flush_pending info and issuing a local
>    context flush.
> 
>   -Clear active_vmids on vCPU schedule out to avoid unnecessarily
>    reserving the VMID space(patch #3).
> 
>   -I have kept the struct kvm_vmid as it is for now(instead of a
>    typedef as suggested), as we may soon add another variable to
>    it when we introduce Pinned KVM VMID support.
> 
> RFCv1 --> v2
>    -Dropped "pinned VMID" support for now.
>    -Dropped RFC tag.
> RFCv1
> 
> https://lore.kernel.org/kvmarm/20210506165232.1969-1-shameerali.kolothu
> m.thodi@huawei.com/
> 
> Julien Grall (1):
>   KVM: arm64: Align the VMID allocation with the arm64 ASID
> 
> Shameer Kolothum (3):
>   KVM: arm64: Introduce a new VMID allocator for KVM
>   KVM: arm64: Make VMID bits accessible outside of allocator
>   KVM: arm64: Make active_vmids invalid on vCPU schedule out
> 
>  arch/arm64/include/asm/kvm_host.h     |  10 +-
>  arch/arm64/include/asm/kvm_mmu.h      |   4 +-
>  arch/arm64/kernel/image-vars.h        |   3 +
>  arch/arm64/kvm/Makefile               |   2 +-
>  arch/arm64/kvm/arm.c                  | 106 +++-----------
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c |   3 +-
>  arch/arm64/kvm/mmu.c                  |   1 -
>  arch/arm64/kvm/vmid.c                 | 196
> ++++++++++++++++++++++++++
>  8 files changed, 228 insertions(+), 97 deletions(-)  create mode 100644
> arch/arm64/kvm/vmid.c
> 
> --
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply

* RE: [PATCH v4 0/4] kvm/arm: New VMID allocator based on asid
From: Shameerali Kolothum Thodi @ 2022-01-05 13:25 UTC (permalink / raw)
  To: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org
  Cc: maz@kernel.org, will@kernel.org, catalin.marinas@arm.com,
	james.morse@arm.com, julien.thierry.kdev@gmail.com,
	suzuki.poulose@arm.com, jean-philippe@linaro.org,
	Alexandru.Elisei@arm.com, qperret@google.com, Jonathan Cameron,
	Linuxarm
In-Reply-To: <20211122121844.867-1-shameerali.kolothum.thodi@huawei.com>

Hi,

A gentle ping on this series. Please take a look and let me know the new approach
taken in this revision is good enough or not. 

Appreciate your feedback.

Thanks,
Shameer

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org]
> On Behalf Of Shameer Kolothum
> Sent: 22 November 2021 12:19
> To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> linux-kernel@vger.kernel.org
> Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> james.morse@arm.com; julien.thierry.kdev@gmail.com;
> suzuki.poulose@arm.com; jean-philippe@linaro.org;
> Alexandru.Elisei@arm.com; qperret@google.com; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>
> Subject: [PATCH v4 0/4] kvm/arm: New VMID allocator based on asid
> 
> Changes from v3:
> - Main change is in patch #4, where the VMID is now set to an
>   invalid one on vCPU schedule out. Introduced an
>   INVALID_ACTIVE_VMID which is basically a VMID 0 with generation 1.
>   Since the basic allocator algorithm reserves vmid #0, it is never
>   used as an active VMID. This (hopefully) will fix the issue of
>   unnecessarily reserving VMID space with active_vmids when those
>   VMs are no longer active[0] and at the same time address the
>   problem noted in v3 wherein everything ends up in slow-path[1].
> 
> Testing:
>  -Run with VMID bit set to 4 and maxcpus to 8 on D06. The test
>   involves running concurrently 50 guests with 4 vCPUs. Each
>   guest will then execute hackbench 5 times before exiting.
>   No crash was observed for a 4-day continuous run.
>   The latest branch is here,
>    https://github.com/hisilicon/kernel-dev/tree/private-v5.16-rc1-vmid-v4
> 
>  -TLA+ model. Modified the asidalloc model to incorporate the new
>   VMID algo. The main differences are,
>   -flush_tlb_all() instead of local_tlb_flush_all() on rollover.
>   -Introduced INVALID_VMID and vCPU Sched Out logic.
>   -No CnP (Removed UniqueASIDAllCPUs & UniqueASIDActiveTask invariants).
>   -Removed  UniqueVMIDPerCPU invariant for now as it looks like
>    because of the speculative fetching with flush_tlb_all() there
>    is a small window where this gets triggered. If I change the
>    logic back to local_flush_tlb_all(), UniqueVMIDPerCPU seems to
>    be fine. With my limited knowledge on TLA+ model, it is not
>    clear to me whether this is a problem with the above logic
>    or the VMID model implementation. Really appreciate any help
>    with the model.
>    The initial VMID TLA+ model is here,
>    https://github.com/shamiali2008/kernel-tla/tree/private-vmidalloc-v1
> 
> Please take a look and let me know.
> 
> Thanks,
> Shameer
> 
> [0]
> https://lore.kernel.org/kvmarm/20210721160614.GC11003@willie-the-truck/
> [1]
> https://lore.kernel.org/kvmarm/20210803114034.GB30853@willie-the-truck/
> 
> History:
> --------
> v2 --> v3
>   -Dropped adding a new static key and cpufeature for retrieving
>    supported VMID bits. Instead, we now make use of the
>    kvm_arm_vmid_bits variable (patch #2).
> 
>   -Since we expect less frequent rollover in the case of VMIDs,
>    the TLB invalidation is now broadcasted on rollover instead
>    of keeping per CPU flush_pending info and issuing a local
>    context flush.
> 
>   -Clear active_vmids on vCPU schedule out to avoid unnecessarily
>    reserving the VMID space(patch #3).
> 
>   -I have kept the struct kvm_vmid as it is for now(instead of a
>    typedef as suggested), as we may soon add another variable to
>    it when we introduce Pinned KVM VMID support.
> 
> RFCv1 --> v2
>    -Dropped "pinned VMID" support for now.
>    -Dropped RFC tag.
> RFCv1
> 
> https://lore.kernel.org/kvmarm/20210506165232.1969-1-shameerali.kolothu
> m.thodi@huawei.com/
> 
> Julien Grall (1):
>   KVM: arm64: Align the VMID allocation with the arm64 ASID
> 
> Shameer Kolothum (3):
>   KVM: arm64: Introduce a new VMID allocator for KVM
>   KVM: arm64: Make VMID bits accessible outside of allocator
>   KVM: arm64: Make active_vmids invalid on vCPU schedule out
> 
>  arch/arm64/include/asm/kvm_host.h     |  10 +-
>  arch/arm64/include/asm/kvm_mmu.h      |   4 +-
>  arch/arm64/kernel/image-vars.h        |   3 +
>  arch/arm64/kvm/Makefile               |   2 +-
>  arch/arm64/kvm/arm.c                  | 106 +++-----------
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c |   3 +-
>  arch/arm64/kvm/mmu.c                  |   1 -
>  arch/arm64/kvm/vmid.c                 | 196
> ++++++++++++++++++++++++++
>  8 files changed, 228 insertions(+), 97 deletions(-)  create mode 100644
> arch/arm64/kvm/vmid.c
> 
> --
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH RFC] can: isotp: convert struct tpcon::{idx,len} to unsigned int
From: Marc Kleine-Budde @ 2022-01-05 13:24 UTC (permalink / raw)
  To: linux-can; +Cc: Marc Kleine-Budde, Oliver Hartkopp, syzbot+4c63f36709a642f801c5

In isotp_rcv_ff() 32 bit of data received over the network is assigned
to struct tpcon::len. Later in that function the length is checked for
the maximal supported length against MAX_MSG_LENGTH.

As struct tpcon::len is an "int" this check does not work, if the
provided length overflows the "int".

Later on struct tpcon::idx is compared against struct tpcon::len.

To fix this problem this patch converts both struct tpcon::{idx,len}
to unsigned int.

Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
\# Cc: stable@vger.kernel.org
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Reported-by: syzbot+4c63f36709a642f801c5@syzkaller.appspotmail.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index df6968b28bf4..02cbcb2ecf0d 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -119,8 +119,8 @@ enum {
 };
 
 struct tpcon {
-	int idx;
-	int len;
+	unsigned int idx;
+	unsigned int len;
 	u32 state;
 	u8 bs;
 	u8 sn;
-- 
2.34.1



^ permalink raw reply related


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.