All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [libnftnl PATCH 3/7] set: Introduce NFTNL_SET_DESC_CONCAT_DATA
From: Pablo Neira Ayuso @ 2022-01-05 16:17 UTC (permalink / raw)
  To: Phil Sutter, netfilter-devel
In-Reply-To: <20211130174558.GF29413@orbyte.nwl.cc>

Hi Phil,

Sorry for taking a while to catch up on this.

On Tue, Nov 30, 2021 at 06:45:58PM +0100, Phil Sutter wrote:
> Hi Pablo,
> 
> On Tue, Nov 30, 2021 at 02:46:58PM +0100, Pablo Neira Ayuso wrote:
> > On Wed, Nov 24, 2021 at 06:22:38PM +0100, Phil Sutter wrote:
> > > Analogous to NFTNL_SET_DESC_CONCAT, introduce a data structure
> > > describing individual data lengths of elements' concatenated data
> > > fields.
> > > 
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > >  include/libnftnl/set.h | 1 +
> > >  include/set.h          | 2 ++
> > >  src/set.c              | 8 ++++++++
> > >  3 files changed, 11 insertions(+)
> > > 
> > > diff --git a/include/libnftnl/set.h b/include/libnftnl/set.h
> > > index 1ffb6c415260d..958bbc9065f67 100644
> > > --- a/include/libnftnl/set.h
> > > +++ b/include/libnftnl/set.h
> > > @@ -33,6 +33,7 @@ enum nftnl_set_attr {
> > >  	NFTNL_SET_EXPR,
> > >  	NFTNL_SET_EXPRESSIONS,
> > >  	NFTNL_SET_DESC_BYTEORDER,
> > > +	NFTNL_SET_DESC_CONCAT_DATA,
> > 
> > This information is already encoded in NFTNL_SET_DATA_TYPE, the
> > datatypes that are defined in libnftables have an explicit byteorder
> > and length.
> 
> We don't define data types in libnftnl, merely expressions and (with
> your patch) those define what byteorder the source/destination registers
> are supposed to be.

OK.

> > For concatenation, this information is stored in 6 bits (see
> > TYPE_BITS). By parsing the NFTNL_SET_DATA_TYPE field you can extract
> > both types (and byteorders) of the set definition.
> 
> For this to work, I would have to duplicate nftables' enum datatypes and
> in addition to that add an array defining each type's byteorder. I had
> considered this once, but didn't like the amount of duplication.
> 
> > For the typeof case, where a generic datatype such as integer is used,
> > this information is stored in the SET_USERDATA area.
> 
> This does not work for concatenated elements, right? At least I see e.g.
> NFTNL_UDATA_SET_KEYBYTEORDER being set to set->key->byteorder, so that's
> just a single value, no?
> 
> > This update for libnftnl is adding a third way to describe the
> > datatypes in the set, right?
> 
> Well, it extends the logic around NFTNL_SET_DESC_CONCAT to non-interval
> sets and to maps (adding the same data for the target part).
> 
> Then there is the new NFTNL_SET_DESC_BYTEORDER which defines the
> byteorder of each part of the key (and value in maps).

I think it would be good to skip these new NFTNL_SET_DESC_* attributes
since they are not used to pass a description to the kernel to help it
select the best set type. So, instead of nftnl_set_elem_snprintf_desc(),
it should be possible to add:

int nftnl_set_elem_snprintf2(char *buf, size_t size,
                             const struct nftnl_set *s,
                             const struct nftnl_set_elem *e,
                             uint32_t type, uint32_t flags)

(or pick a better name if you come up with any other alternative).

So the nftnl_set object provides the byteorder notation to display the
set element accordingly.

This requires an extra conversion from struct set to struct nftnl_set
for the debug case, that should be fine (--debug is slow path anyway).
If this needs to be speed up later on, it should be possible to keep
the nftnl_set object around as context.

Then, there is already NFTNL_UDATA_SET_KEYBYTEORDER and
NFTNL_UDATA_SET_DATABYTEORDER which store the byteorder for key and
data. I'm going to have a look at the userdata API since to see if I
can propose an easy API to set and to get userdata information (this
is currently a blob using TLVs that are stored in the kernel, but they
are not interpreted by the kernel, it's only useful context
information for userspace which is included in netlink dumps). This
should fill missing gap in my proposal.

Looking at your series, I think it's better it's better to avoid the
struct nftnl_set_desc definition that is exposed in the libnftnl
header, this will not allow for future extensions without breaking
binary compatibility. I understand your motivation is to avoid a
duplicated definition in the libnftnl and nftables codebase.

^ permalink raw reply

* Re: Add timestamp to dpdk.log
From: Stephen Hemminger @ 2022-01-05 16:17 UTC (permalink / raw)
  To: Soumya Muralidhar; +Cc: dev@dpdk.org
In-Reply-To: <25E66ADE-793E-4C7E-A70F-29EFD2AE7CEA@gigamon.com>

Your message is garbaled with lots of bogus URL's. Please avoid HTML mail
or weird links.

I use this. There was a more general version submitted as a patch to
DPDK but no one seemed interested.  The reason for redirecting stdout
is that are libraries we use that print to stdout and want to get timestamps
from them if debugging startup issues.


/*
 * Sample of redirecting DPDK log stream.
 *
 * If program is run for debugging then stdout will be a console and
 * put messages onto console stream with timestamps.  Otherwise, just
 * put message onto stderr which systemd journal handles.  No need for
 * syslog because that causes duplicate messages.
 */

static struct timespec log_t0; /* Start program in monotonic time */

/*
 * Write message to standard output with timestamp.
 * Use writev() so that timestamp and message do not get interleaved.
 */
static ssize_t
dpdk_log_tty_write(__rte_unused void *ctx, const char *buf, size_t size)
{
	struct timespec ts;
	struct iovec iov[2];
	char tbuf[64];

	/* format up monotonic timestamp */
	clock_gettime(CLOCK_MONOTONIC, &ts);

	ts.tv_sec -= log_t0.tv_sec;
	ts.tv_nsec -= log_t0.tv_nsec;
	if (ts.tv_nsec < 0) {
		--ts.tv_sec;
		ts.tv_nsec += NS_PER_S;
	}

	iov[0].iov_base = tbuf;
	iov[0].iov_len  = snprintf(tbuf, sizeof(tbuf), "[%8lu.%06lu] ",
				   ts.tv_sec, ts.tv_nsec / 1000u);

	/* extra cast is workaround to remove const qualifier */
	iov[1].iov_base = (void *)(uintptr_t)buf;
	iov[1].iov_len = size;

	return writev(STDOUT_FILENO, iov, 2);
}

static cookie_io_functions_t dpdk_log_tty_func = {
	.write = dpdk_log_tty_write,
};

/*
 * Print message to stderr with priority format so that 
 * Systemd journal can handle it. Alternative would be
 * to use syslog or use sd_journal; but less is more.
 */
static ssize_t
dpdk_log_write(__rte_unused void *ctx, const char *buf, size_t size)
{
	/* Syslog error levels are from 0 to 7, DPDK uses 1 to 8 */
	int priority = rte_log_cur_msg_loglevel() - 1;

	fprintf(stderr, "<%i>%.*s\n", priority, (int)size, buf);
	return size;
}

static cookie_io_functions_t dpdk_log_func = {
	.write = dpdk_log_write,
};

/*
 * Override default DPDK logging which duplicates messages
 * to both stderr and syslog.
 */
static void log_init(void)
{
	backplane_logtype = rte_log_register("backplane");
	if (backplane_logtype >= 0)
		rte_log_set_level(backplane_logtype, RTE_LOG_INFO);

	if (isatty(STDOUT_FILENO)) {
		clock_gettime(CLOCK_MONOTONIC, &log_t0);

		stdout = fopencookie(NULL, "w+", dpdk_log_tty_func);
		setlinebuf(stdout);
		rte_openlog_stream(stdout);
	} else {
		FILE *jf;

		jf = fopencookie(NULL, "w+", dpdk_log_func);
		rte_openlog_stream(jf);
	}
}

/* Put DPDK log back onto stderr */
static void log_uninit(void)
{
	FILE *logf;

	logf = rte_log_get_stream();
	if (logf != stderr) {
		rte_openlog_stream(stderr);
		fclose(logf);
	}
}

^ permalink raw reply

* Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process
From: Felix Kuehling @ 2022-01-05 16:16 UTC (permalink / raw)
  To: Christian König, Christian König, Daniel Vetter,
	Bhardwaj, Rajneesh, Adrian Reber
  Cc: daniel.vetter, amd-gfx, David Yat Sin, dri-devel,
	alexander.deucher, airlied
In-Reply-To: <1ab2558b-1af0-3319-dce6-b805320a49d0@gmail.com>

Am 2022-01-05 um 3:08 a.m. schrieb Christian König:
> Am 04.01.22 um 19:08 schrieb Felix Kuehling:
>> [+Adrian]
>>
>> Am 2021-12-23 um 2:05 a.m. schrieb Christian König:
>>
>>> Am 22.12.21 um 21:53 schrieb Daniel Vetter:
>>>> On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote:
>>>>
>>>> [SNIP]
>>>> Still sounds funky. I think minimally we should have an ack from CRIU
>>>> developers that this is officially the right way to solve this
>>>> problem. I
>>>> really don't want to have random one-off hacks that don't work across
>>>> the
>>>> board, for a problem where we (drm subsystem) really shouldn't be the
>>>> only
>>>> one with this problem. Where "this problem" means that the mmap
>>>> space is
>>>> per file description, and not per underlying inode or real device or
>>>> whatever. That part sounds like a CRIU problem, and I expect CRIU
>>>> folks
>>>> want a consistent solution across the board for this. Hence please
>>>> grab an
>>>> ack from them.
>>> Unfortunately it's a KFD design problem. AMD used a single device
>>> node, then mmaped different objects from the same offset to different
>>> processes and expected it to work the rest of the fs subsystem without
>>> churn.
>> This may be true for mmaps in the KFD device, but not for mmaps in the
>> DRM render nodes.
>
> Correct, yes.
>
>>> So yes, this is indeed because the mmap space is per file descriptor
>>> for the use case here.
>> No. This is a different problem.
>
> I was already wondering which mmaps through the KFD node we have left
> which cause problems here.

We still use the KFD FD for mapping doorbells and HDP flushing. These
are both SG BOs, so they cannot be CPU-mapped through render nodes. The
KFD FD is also used for mapping signal pages and CWSR trap handlers on
old APUs.

Those VMAs aren't causing the problem. They still map successfully on
restore.


>
>> The problem has to do with the way that DRM manages mmap permissions. In
>> order to be able to mmap an offset in the render node, there needs to be
>> a BO that was created in the same render node. If you fork a process, it
>> inherits the VMA.
>
> Yeah, so far it works like designed.
>
>> But KFD doesn't know anything about the inherited BOs
>> from the parent process.
>
> Ok, why that? When the KFD is reinitializing it's context why
> shouldn't it cleanup those VMAs?

That cleanup has to be initiated by user mode. Basically closing the old
KFD and DRM file descriptors, cleaning up all the user mode VM state,
unmapping all the VMAs, etc. Then it reopens KFD and the render nodes
and starts from scratch.

User mode will do this automatically when it tries to reinitialize ROCm.
However, in this case the child process doesn't do that (e.g. a python
application using the multi-processing package). The child process does
not use ROCm. But you're left with all the dangling VMAs in the child
process indefinitely.

Regards,
  Felix


>
>> Therefore those BOs don't get checkpointed and
>> restored in the child process. When the CRIU checkpoint is restored, our
>> CRIU plugin never creates a BO corresponding to the VMA in the child
>> process' render node FD. We've also lost the relationship between the
>> parent and child-process' render node FDs. After "fork" the render node
>> FD points to the same struct file in parent and child. After restoring
>> the CRIU checkpoint, they are separate struct files, created by separate
>> "open" system calls. Therefore the mmap call that restores the VMA fails
>> in the child process.
>>
>> At least for KFD, there is no point inheriting BOs from a child process,
>> because the GPU has no way of accessing the BOs in the child process.
>> The child process has no GPU address space, no user mode queues, no way
>> to do anything with the GPU before it completely reinitializes its KFD
>> context.
>>
>> We can workaround this issue in user mode with madvise(...,
>> MADV_DONTFORK). In fact we've already done this for some BOs to avoid a
>> memory leak in the parent process while a child process exists. But it's
>> slightly racy because there is a short time window where VMA exists
>> without the VM_DONTCOPY flag. A fork during that time window could still
>> create a child process with an inherited VMA.
>>
>> Therefore a safer solution is to set the vm_flags in the VMA in the
>> driver when the VMA is first created.
>
> Thanks for the full explanation, it makes much more sense now.
>
> Regards,
> Christian.
>
>>
>> Regards,
>>    Felix
>>
>>
>>> And thanks for pointing this out, this indeed makes the whole change
>>> extremely questionable.
>>>
>>> Regards,
>>> Christian.
>>>
>>>> Cheers, Daniel
>>>>
>

^ permalink raw reply

* [PATCH 2/2] platform: sifive_fu740: fix reset when watchdog is running
From: David Abdurachmanov @ 2022-01-05 16:16 UTC (permalink / raw)
  To: opensbi
In-Reply-To: <20220105163959.000046ce@maquefel.me>

On Wed, Jan 5, 2022 at 3:40 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> 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
> ?

I would suggest doing this too. Disable watchdog on the start.

IIRC watchdog DA9063 can do 131 seconds (max). Alternative would be to
reconfigure the timer for 131 seconds, kick it in OpenSBI to give the
most possible time and reboot. Hopefully that would be enough time to
reboot and take control of the watchdog again. I don't know if U-Boot
has a DA9063 watchdog driver.

Disabling it is probably a better option :) System boot with watchdog
disabled and then it can be enabled again.

david

>
> 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: [linux-next:master 9576/10864] include/linux/rcupdate.h:412:36: error: dereferencing pointer to incomplete type 'struct bpf_prog'
From: Jakub Kicinski @ 2022-01-05 16:15 UTC (permalink / raw)
  To: kernel test robot, kbuild-all
  Cc: Linux Memory Management List, Alexei Starovoitov
In-Reply-To: <202201051741.TQVhjCyh-lkp@intel.com>

On Wed, 5 Jan 2022 17:21:31 +0800 kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   7a769a3922d81cfc74ab4d90a9cc69485f260976
> commit: b6459415b384cb829f0b2a4268f211c789f6cf0b [9576/10864] net: Don't include filter.h from net/sock.h

Thanks for the report! 

I'm slightly confused how to read it.

The issues is already fixed by commit 7d714ff14d64 ("net: fixup build
after bpf header changes"), which is present in linux-next. Should this
report be marked as "already fixed" or "transient" somehow? Why is head
reported to be at 7a769a39 when it must in fact be at b6459415 for the
build to fail.

HEAD has a special meaning in git, it's the currently checked out
commit. Perhaps we can avoid using it with a different meaning?
Maybe put the hash on the "tree:" line in brackets instead?

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master (7a769a3922d8)


> config: i386-randconfig-m021-20220105 (https://download.01.org/0day-ci/archive/20220105/202201051741.TQVhjCyh-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b6459415b384cb829f0b2a4268f211c789f6cf0b
>         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>         git fetch --no-tags linux-next master
>         git checkout b6459415b384cb829f0b2a4268f211c789f6cf0b
>         # save the config file to linux build tree
>         mkdir build_dir
>         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ethernet/mellanox/mlx5/core/
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
>                     from include/linux/compiler.h:266,
>                     from arch/x86/include/asm/atomic.h:5,
>                     from include/linux/atomic.h:7,
>                     from include/linux/refcount.h:95,
>                     from include/net/act_api.h:9,
>                     from include/net/tc_act/tc_gact.h:5,
>                     from drivers/net/ethernet/mellanox/mlx5/core/en_main.c:33:
>    drivers/net/ethernet/mellanox/mlx5/core/en_main.c: In function 'mlx5e_alloc_rq':
> >> include/linux/rcupdate.h:412:36: error: dereferencing pointer to incomplete type 'struct bpf_prog'  
>      412 | #define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
>          |                                    ^~~~
>    include/asm-generic/rwonce.h:55:33: note: in definition of macro '__WRITE_ONCE'
>       55 |  *(volatile typeof(x) *)&(x) = (val);    \
>          |                                 ^~~
>    include/linux/rcupdate.h:854:3: note: in expansion of macro 'WRITE_ONCE'
>      854 |   WRITE_ONCE(p, RCU_INITIALIZER(v)); \
>          |   ^~~~~~~~~~
>    include/linux/rcupdate.h:854:17: note: in expansion of macro 'RCU_INITIALIZER'
>      854 |   WRITE_ONCE(p, RCU_INITIALIZER(v)); \
>          |                 ^~~~~~~~~~~~~~~
>    drivers/net/ethernet/mellanox/mlx5/core/en_main.c:569:2: note: in expansion of macro 'RCU_INIT_POINTER'
>      569 |  RCU_INIT_POINTER(rq->xdp_prog, params->xdp_prog);
>          |  ^~~~~~~~~~~~~~~~
> 
> 
> vim +412 include/linux/rcupdate.h
> 
> ca5ecddfa8fcbd Paul E. McKenney 2010-04-28  407  
> 462225ae47d717 Paul E. McKenney 2013-11-11  408  /**
> 462225ae47d717 Paul E. McKenney 2013-11-11  409   * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
> 462225ae47d717 Paul E. McKenney 2013-11-11  410   * @v: The value to statically initialize with.
> 462225ae47d717 Paul E. McKenney 2013-11-11  411   */
> 462225ae47d717 Paul E. McKenney 2013-11-11 @412  #define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
> 462225ae47d717 Paul E. McKenney 2013-11-11  413  
> 
> :::::: The code at line 412 was first introduced by commit
> :::::: 462225ae47d7175f886281d8a91708550cd5178c rcu: Add an RCU_INITIALIZER for global RCU-protected pointers
> 
> :::::: TO: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> :::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org



^ permalink raw reply

* Re: [linux-next:master 9576/10864] include/linux/rcupdate.h:412:36: error: dereferencing pointer to incomplete type 'struct bpf_prog'
From: Jakub Kicinski @ 2022-01-05 16:15 UTC (permalink / raw)
  To: kbuild-all
In-Reply-To: <202201051741.TQVhjCyh-lkp@intel.com>

[-- Attachment #1: Type: text/plain, Size: 4503 bytes --]

On Wed, 5 Jan 2022 17:21:31 +0800 kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   7a769a3922d81cfc74ab4d90a9cc69485f260976
> commit: b6459415b384cb829f0b2a4268f211c789f6cf0b [9576/10864] net: Don't include filter.h from net/sock.h

Thanks for the report! 

I'm slightly confused how to read it.

The issues is already fixed by commit 7d714ff14d64 ("net: fixup build
after bpf header changes"), which is present in linux-next. Should this
report be marked as "already fixed" or "transient" somehow? Why is head
reported to be at 7a769a39 when it must in fact be at b6459415 for the
build to fail.

HEAD has a special meaning in git, it's the currently checked out
commit. Perhaps we can avoid using it with a different meaning?
Maybe put the hash on the "tree:" line in brackets instead?

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master (7a769a3922d8)


> config: i386-randconfig-m021-20220105 (https://download.01.org/0day-ci/archive/20220105/202201051741.TQVhjCyh-lkp(a)intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b6459415b384cb829f0b2a4268f211c789f6cf0b
>         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>         git fetch --no-tags linux-next master
>         git checkout b6459415b384cb829f0b2a4268f211c789f6cf0b
>         # save the config file to linux build tree
>         mkdir build_dir
>         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ethernet/mellanox/mlx5/core/
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from ./arch/x86/include/generated/asm/rwonce.h:1,
>                     from include/linux/compiler.h:266,
>                     from arch/x86/include/asm/atomic.h:5,
>                     from include/linux/atomic.h:7,
>                     from include/linux/refcount.h:95,
>                     from include/net/act_api.h:9,
>                     from include/net/tc_act/tc_gact.h:5,
>                     from drivers/net/ethernet/mellanox/mlx5/core/en_main.c:33:
>    drivers/net/ethernet/mellanox/mlx5/core/en_main.c: In function 'mlx5e_alloc_rq':
> >> include/linux/rcupdate.h:412:36: error: dereferencing pointer to incomplete type 'struct bpf_prog'  
>      412 | #define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
>          |                                    ^~~~
>    include/asm-generic/rwonce.h:55:33: note: in definition of macro '__WRITE_ONCE'
>       55 |  *(volatile typeof(x) *)&(x) = (val);    \
>          |                                 ^~~
>    include/linux/rcupdate.h:854:3: note: in expansion of macro 'WRITE_ONCE'
>      854 |   WRITE_ONCE(p, RCU_INITIALIZER(v)); \
>          |   ^~~~~~~~~~
>    include/linux/rcupdate.h:854:17: note: in expansion of macro 'RCU_INITIALIZER'
>      854 |   WRITE_ONCE(p, RCU_INITIALIZER(v)); \
>          |                 ^~~~~~~~~~~~~~~
>    drivers/net/ethernet/mellanox/mlx5/core/en_main.c:569:2: note: in expansion of macro 'RCU_INIT_POINTER'
>      569 |  RCU_INIT_POINTER(rq->xdp_prog, params->xdp_prog);
>          |  ^~~~~~~~~~~~~~~~
> 
> 
> vim +412 include/linux/rcupdate.h
> 
> ca5ecddfa8fcbd Paul E. McKenney 2010-04-28  407  
> 462225ae47d717 Paul E. McKenney 2013-11-11  408  /**
> 462225ae47d717 Paul E. McKenney 2013-11-11  409   * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
> 462225ae47d717 Paul E. McKenney 2013-11-11  410   * @v: The value to statically initialize with.
> 462225ae47d717 Paul E. McKenney 2013-11-11  411   */
> 462225ae47d717 Paul E. McKenney 2013-11-11 @412  #define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
> 462225ae47d717 Paul E. McKenney 2013-11-11  413  
> 
> :::::: The code at line 412 was first introduced by commit
> :::::: 462225ae47d7175f886281d8a91708550cd5178c rcu: Add an RCU_INITIALIZER for global RCU-protected pointers
> 
> :::::: TO: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> :::::: CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply

* [PATCH 3/3] git-p4: don't print shell commands as python lists
From: Joel Holdsworth @ 2022-01-05 16:14 UTC (permalink / raw)
  To: git, Luke Diamand, Junio C Hamano
  Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
	Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
In-Reply-To: <20220105161451.56378-1-jholdsworth@nvidia.com>

Previously the git-p4 script would log commands as stringified
representations of the command parameter, leading to output such as
this:

Reading pipe: ['git', 'config', '--bool', 'git-p4.useclientspec']

Now that all commands list objects, this patch instead joins the
elements of the list into a single string so the output now looks more
readable:

Reading pipe: git config --bool git-p4.useclientspec

Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
 git-p4.py | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 7c1f238a56..27481b91de 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -287,14 +287,14 @@ def run_hook_command(cmd, param):
 
 def write_pipe(c, stdin, *k, **kw):
     if verbose:
-        sys.stderr.write('Writing pipe: %s\n' % str(c))
+        sys.stderr.write('Writing pipe: {}\n'.format(' '.join(c)))
 
     p = subprocess.Popen(c, stdin=subprocess.PIPE, *k, **kw)
     pipe = p.stdin
     val = pipe.write(stdin)
     pipe.close()
     if p.wait():
-        die('Command failed: %s' % str(c))
+        die('Command failed: {}'.format(' '.join(c)))
 
     return val
 
@@ -310,7 +310,7 @@ def read_pipe_full(c, *k, **kw):
         text.
     """
     if verbose:
-        sys.stderr.write('Reading pipe: %s\n' % str(c))
+        sys.stderr.write('Reading pipe: {}\n'.format(' '.join(c)))
 
     p = subprocess.Popen(
         c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, *k, **kw)
@@ -329,7 +329,7 @@ def read_pipe(c, ignore_error=False, raw=False, *k, **kw):
         if ignore_error:
             out = ""
         else:
-            die('Command failed: %s\nError: %s' % (str(c), err))
+            die('Command failed: {}\nError: {}'.format(' '.join(c), err))
     if not raw:
         out = decode_text_stream(out)
     return out
@@ -350,7 +350,7 @@ def p4_read_pipe(c, ignore_error=False, raw=False, *k, **kw):
 
 def read_pipe_lines(c, raw=False, *k, **kw):
     if verbose:
-        sys.stderr.write('Reading pipe: %s\n' % str(c))
+        sys.stderr.write('Reading pipe: {}\n'.format(' '.join(c)))
 
     p = subprocess.Popen(c, stdout=subprocess.PIPE, *k, **kw)
     pipe = p.stdout
@@ -358,7 +358,7 @@ def read_pipe_lines(c, raw=False, *k, **kw):
     if not raw:
         lines = [decode_text_stream(line) for line in lines]
     if pipe.close() or p.wait():
-        die('Command failed: %s' % str(c))
+        die('Command failed: {}'.format(' '.join(c)))
     return lines
 
 def p4_read_pipe_lines(c, *k, **kw):
@@ -397,7 +397,8 @@ def p4_has_move_command():
 
 def system(cmd, ignore_error=False, *k, **kw):
     if verbose:
-        sys.stderr.write("executing %s\n" % str(cmd))
+        sys.stderr.write("executing {}\n".format(
+            ' '.join(cmd) if isinstance(cmd, list) else cmd))
     retcode = subprocess.call(cmd, *k, **kw)
     if retcode and not ignore_error:
         raise CalledProcessError(retcode, cmd)
@@ -732,7 +733,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
 
     cmd = p4_build_cmd(["-G"] + cmd)
     if verbose:
-        sys.stderr.write("Opening pipe: %s\n" % str(cmd))
+        sys.stderr.write("Opening pipe: {}\n".format(' '.join(cmd)))
 
     # Use a temporary file to avoid deadlocks without
     # subprocess.communicate(), which would put another copy
-- 
2.34.1


^ permalink raw reply related

* [PATCH 2/3] git-p4: pass command arguments as lists instead of using shell
From: Joel Holdsworth @ 2022-01-05 16:14 UTC (permalink / raw)
  To: git, Luke Diamand, Junio C Hamano
  Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
	Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
In-Reply-To: <20220105161451.56378-1-jholdsworth@nvidia.com>

In the majority of the subprocess calls where shell=True was used, it
was only needed to parse command arguments by spaces. In each of these
cases, the commands are now being passed in as lists instead of strings.

This change aids the comprehensibility of the code. Constucting commands
and arguments using strings risks bugs from unsanitized inputs, and the
attendant complexity of properly quoting and escaping command arguments.

Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
 git-p4.py | 105 ++++++++++++++++++++++--------------------------------
 1 file changed, 43 insertions(+), 62 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 6a6aa4e928..7c1f238a56 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -108,10 +108,7 @@ def p4_build_cmd(cmd):
         # Provide a way to not pass this option by setting git-p4.retries to 0
         real_cmd += ["-r", str(retries)]
 
-    if not isinstance(cmd, list):
-        real_cmd = ' '.join(real_cmd) + ' ' + cmd
-    else:
-        real_cmd += cmd
+    real_cmd += cmd
 
     # now check that we can actually talk to the server
     global p4_access_checked
@@ -733,12 +730,7 @@ def isModeExecChanged(src_mode, dst_mode):
 def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
         errors_as_exceptions=False, *k, **kw):
 
-    if not isinstance(cmd, list):
-        cmd = "-G " + cmd
-    else:
-        cmd = ["-G"] + cmd
-
-    cmd = p4_build_cmd(cmd)
+    cmd = p4_build_cmd(["-G"] + cmd)
     if verbose:
         sys.stderr.write("Opening pipe: %s\n" % str(cmd))
 
@@ -861,7 +853,7 @@ def isValidGitDir(path):
     return git_dir(path) != None
 
 def parseRevision(ref):
-    return read_pipe("git rev-parse %s" % ref, shell=True).strip()
+    return read_pipe(["git", "rev-parse", ref]).strip()
 
 def branchExists(ref):
     rev = read_pipe(["git", "rev-parse", "-q", "--verify", ref],
@@ -967,13 +959,13 @@ def p4BranchesInGit(branchesAreInRemotes=True):
 
     branches = {}
 
-    cmdline = "git rev-parse --symbolic "
+    cmdline = ["git", "rev-parse", "--symbolic"]
     if branchesAreInRemotes:
-        cmdline += "--remotes"
+        cmdline.append("--remotes")
     else:
-        cmdline += "--branches"
+        cmdline.append("--branches")
 
-    for line in read_pipe_lines(cmdline, shell=True):
+    for line in read_pipe_lines(cmdline):
         line = line.strip()
 
         # only import to p4/
@@ -1036,7 +1028,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
 
     originPrefix = "origin/p4/"
 
-    for line in read_pipe_lines("git rev-parse --symbolic --remotes", shell=True):
+    for line in read_pipe_lines(["git", "rev-parse", "--symbolic", "--remotes"]):
         line = line.strip()
         if (not line.startswith(originPrefix)) or line.endswith("HEAD"):
             continue
@@ -1074,8 +1066,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
                               remoteHead, ','.join(settings['depot-paths'])))
 
         if update:
-            system("git update-ref %s %s" % (remoteHead, originHead),
-                shell=True)
+            system(["git", "update-ref", remoteHead, originHead])
 
 def originP4BranchesExist():
         return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
@@ -1189,7 +1180,7 @@ def getClientSpec():
     """Look at the p4 client spec, create a View() object that contains
        all the mappings, and return it."""
 
-    specList = p4CmdList("client -o", shell=True)
+    specList = p4CmdList(["client", "-o"])
     if len(specList) != 1:
         die('Output from "client -o" is %d lines, expecting 1' %
             len(specList))
@@ -1218,7 +1209,7 @@ def getClientSpec():
 def getClientRoot():
     """Grab the client directory."""
 
-    output = p4CmdList("client -o", shell=True)
+    output = p4CmdList(["client", "-o"])
     if len(output) != 1:
         die('Output from "client -o" is %d lines, expecting 1' % len(output))
 
@@ -1473,7 +1464,7 @@ def p4UserId(self):
         if self.myP4UserId:
             return self.myP4UserId
 
-        results = p4CmdList("user -o", shell=True)
+        results = p4CmdList(["user", "-o"])
         for r in results:
             if 'User' in r:
                 self.myP4UserId = r['User']
@@ -1498,7 +1489,7 @@ def getUserMapFromPerforceServer(self):
         self.users = {}
         self.emails = {}
 
-        for output in p4CmdList("users", shell=True):
+        for output in p4CmdList(["users"]):
             if "User" not in output:
                 continue
             self.users[output["User"]] = output["FullName"] + " <" + output["Email"] + ">"
@@ -1622,7 +1613,7 @@ def __init__(self):
             die("Large file system not supported for git-p4 submit command. Please remove it from config.")
 
     def check(self):
-        if len(p4CmdList("opened ...", shell=True)) > 0:
+        if len(p4CmdList(["opened", "..."])) > 0:
             die("You have files opened with perforce! Close them before starting the sync.")
 
     def separate_jobs_from_description(self, message):
@@ -1726,7 +1717,7 @@ def lastP4Changelist(self):
         # then gets used to patch up the username in the change. If the same
         # client spec is being used by multiple processes then this might go
         # wrong.
-        results = p4CmdList("client -o", shell=True)    # find the current client
+        results = p4CmdList(["client", "-o"])        # find the current client
         client = None
         for r in results:
             if 'Client' in r:
@@ -1742,7 +1733,7 @@ def lastP4Changelist(self):
 
     def modifyChangelistUser(self, changelist, newUser):
         # fixup the user field of a changelist after it has been submitted.
-        changes = p4CmdList("change -o %s" % changelist, shell=True)
+        changes = p4CmdList(["change", "-o", changelist])
         if len(changes) != 1:
             die("Bad output from p4 change modifying %s to user %s" %
                 (changelist, newUser))
@@ -1753,7 +1744,7 @@ def modifyChangelistUser(self, changelist, newUser):
         # p4 does not understand format version 3 and above
         input = marshal.dumps(c, 2)
 
-        result = p4CmdList("change -f -i", stdin=input, shell=True)
+        result = p4CmdList(["change", "-f", "-i"], stdin=input)
         for r in result:
             if 'code' in r:
                 if r['code'] == 'error':
@@ -1859,7 +1850,7 @@ def edit_template(self, template_file):
         if "P4EDITOR" in os.environ and (os.environ.get("P4EDITOR") != ""):
             editor = os.environ.get("P4EDITOR")
         else:
-            editor = read_pipe("git var GIT_EDITOR", shell=True).strip()
+            editor = read_pipe(["git", "var", "GIT_EDITOR"]).strip()
         system(["sh", "-c", ('%s "$@"' % editor), editor, template_file])
 
         # If the file was not saved, prompt to see if this patch should
@@ -1918,8 +1909,7 @@ def applyCommit(self, id):
         (p4User, gitEmail) = self.p4UserForCommit(id)
 
         diff = read_pipe_lines(
-            "git diff-tree -r %s \"%s^\" \"%s\"" % (self.diffOpts, id, id),
-            shell=True)
+            ["git", "diff-tree", "-r"] + self.diffOpts + ["{}^".format(id), id])
         filesToAdd = set()
         filesToChangeType = set()
         filesToDelete = set()
@@ -2405,17 +2395,17 @@ def run(self, args):
         #
         if self.detectRenames:
             # command-line -M arg
-            self.diffOpts = "-M"
+            self.diffOpts = ["-M"]
         else:
             # If not explicitly set check the config variable
             detectRenames = gitConfig("git-p4.detectRenames")
 
             if detectRenames.lower() == "false" or detectRenames == "":
-                self.diffOpts = ""
+                self.diffOpts = []
             elif detectRenames.lower() == "true":
-                self.diffOpts = "-M"
+                self.diffOpts = ["-M"]
             else:
-                self.diffOpts = "-M%s" % detectRenames
+                self.diffOpts = ["-M{}".format(detectRenames)]
 
         # no command-line arg for -C or --find-copies-harder, just
         # config variables
@@ -2423,12 +2413,12 @@ def run(self, args):
         if detectCopies.lower() == "false" or detectCopies == "":
             pass
         elif detectCopies.lower() == "true":
-            self.diffOpts += " -C"
+            self.diffOpts.append("-C")
         else:
-            self.diffOpts += " -C%s" % detectCopies
+            self.diffOpts.append("-C{}".format(detectCopies))
 
         if gitConfigBool("git-p4.detectCopiesHarder"):
-            self.diffOpts += " --find-copies-harder"
+            self.diffOpts.append("--find-copies-harder")
 
         num_shelves = len(self.update_shelve)
         if num_shelves > 0 and num_shelves != len(commits):
@@ -3376,12 +3366,9 @@ def getBranchMapping(self):
         lostAndFoundBranches = set()
 
         user = gitConfig("git-p4.branchUser")
-        if len(user) > 0:
-            command = "branches -u %s" % user
-        else:
-            command = "branches"
 
-        for info in p4CmdList(command, shell=True):
+        for info in p4CmdList(
+            ["branches"] + (["-u", user] if len(user) > 0 else [])):
             details = p4Cmd(["branch", "-o", info["branch"]])
             viewIdx = 0
             while "View%s" % viewIdx in details:
@@ -3472,9 +3459,8 @@ def gitCommitByP4Change(self, ref, change):
         while True:
             if self.verbose:
                 print("trying: earliest %s latest %s" % (earliestCommit, latestCommit))
-            next = read_pipe(
-                "git rev-list --bisect %s %s" % (latestCommit, earliestCommit),
-                shell=True).strip()
+            next = read_pipe(["git", "rev-list", "--bisect",
+                latestCommit, earliestCommit]).strip()
             if len(next) == 0:
                 if self.verbose:
                     print("argh")
@@ -3630,7 +3616,7 @@ def sync_origin_only(self):
             if self.hasOrigin:
                 if not self.silent:
                     print('Syncing with origin first, using "git fetch origin"')
-                system("git fetch origin", shell=True)
+                system(["git", "fetch", "origin"])
 
     def importHeadRevision(self, revision):
         print("Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch))
@@ -3797,8 +3783,8 @@ def run(self, args):
         if len(self.branch) == 0:
             self.branch = self.refPrefix + "master"
             if gitBranchExists("refs/heads/p4") and self.importIntoRemotes:
-                system("git update-ref %s refs/heads/p4" % self.branch, shell=True)
-                system("git branch -D p4", shell=True)
+                system(["git", "update-ref", self.branch, "refs/heads/p4"])
+                system(["git", "branch", "-D", "p4"])
 
         # accept either the command-line option, or the configuration variable
         if self.useClientSpec:
@@ -4001,7 +3987,7 @@ def run(self, args):
         # Cleanup temporary branches created during import
         if self.tempBranches != []:
             for branch in self.tempBranches:
-                read_pipe("git update-ref -d %s" % branch, shell=True)
+                read_pipe(["git", "update-ref", "-d", branch])
             os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
 
         # Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
@@ -4033,7 +4019,7 @@ def run(self, args):
     def rebase(self):
         if os.system("git update-index --refresh") != 0:
             die("Some files in your working directory are modified and different than what is in your index. You can use git update-index <filename> to bring the index up to date or stash away all your changes with git stash.");
-        if len(read_pipe("git diff-index HEAD --", shell=True)) > 0:
+        if len(read_pipe(["git", "diff-index", "HEAD", "--"])) > 0:
             die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash.");
 
         [upstream, settings] = findUpstreamBranchPoint()
@@ -4044,10 +4030,10 @@ def rebase(self):
         upstream = re.sub("~[0-9]+$", "", upstream)
 
         print("Rebasing the current branch onto %s" % upstream)
-        oldHead = read_pipe("git rev-parse HEAD", shell=True).strip()
-        system("git rebase %s" % upstream, shell=True)
-        system("git diff-tree --stat --summary -M %s HEAD --" % oldHead,
-            shell=True)
+        oldHead = read_pipe(["git", "rev-parse", "HEAD"]).strip()
+        system(["git", "rebase", upstream])
+        system(["git", "diff-tree", "--stat", "--summary", "-M", oldHead,
+            "HEAD", "--"])
         return True
 
 class P4Clone(P4Sync):
@@ -4124,7 +4110,7 @@ def run(self, args):
 
         # auto-set this variable if invoked with --use-client-spec
         if self.useClientSpec_from_options:
-            system("git config --bool git-p4.useclientspec true", shell=True)
+            system(["git", "config", "--bool", "git-p4.useclientspec", "true"])
 
         return True
 
@@ -4255,10 +4241,7 @@ def run(self, args):
         if originP4BranchesExist():
             createOrUpdateBranchesFromOrigin()
 
-        cmdline = "git rev-parse --symbolic "
-        cmdline += " --remotes"
-
-        for line in read_pipe_lines(cmdline, shell=True):
+        for line in read_pipe_lines(["git", "rev-parse", "--symbolic", "--remotes"]):
             line = line.strip()
 
             if not line.startswith('p4/') or line == "p4/HEAD":
@@ -4341,11 +4324,9 @@ def main():
             cmd.gitdir = os.path.abspath(".git")
             if not isValidGitDir(cmd.gitdir):
                 # "rev-parse --git-dir" without arguments will try $PWD/.git
-                cmd.gitdir = read_pipe(
-                    "git rev-parse --git-dir", shell=True).strip()
+                cmd.gitdir = read_pipe(["git", "rev-parse", "--git-dir"]).strip()
                 if os.path.exists(cmd.gitdir):
-                    cdup = read_pipe(
-                        "git rev-parse --show-cdup", shell=True).strip()
+                    cdup = read_pipe(["git", "rev-parse", "--show-cdup"]).strip()
                     if len(cdup) > 0:
                         chdir(cdup);
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH 1/3] git-p4: don't select shell mode using the type of the command argument
From: Joel Holdsworth @ 2022-01-05 16:14 UTC (permalink / raw)
  To: git, Luke Diamand, Junio C Hamano
  Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
	Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
In-Reply-To: <20220105161451.56378-1-jholdsworth@nvidia.com>

Previously, the script would invoke subprocess functions setting the
shell argument True if the command argument was a string, setting it
False otherwise.

This patch replaces this implicit type-driven behaviour with explicit
shell arguments specified by the caller.

The apparent motive for the implict behaviour is that the subprocess
functions do not divide command strings into args. Invoking
subprocess.call("echo hello") will attempt to execute a program by the
name "echo hello". With subprocess.call("echo hello", shell=True), sh
-c "echo hello" will be executed instead, which will cause the command
and args to be divided by spaces.

Eventually, all usage of shell=True, that is not necessary for some
purpose beyond parsing command strings, should be removed. For now,
this patch makes the usage of shells explicit.
---
 git-p4.py | 126 +++++++++++++++++++++++++++---------------------------
 1 file changed, 63 insertions(+), 63 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index cb37545455..6a6aa4e928 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -288,12 +288,11 @@ def run_hook_command(cmd, param):
     return subprocess.call(cli, shell=use_shell)
 
 
-def write_pipe(c, stdin):
+def write_pipe(c, stdin, *k, **kw):
     if verbose:
         sys.stderr.write('Writing pipe: %s\n' % str(c))
 
-    expand = not isinstance(c, list)
-    p = subprocess.Popen(c, stdin=subprocess.PIPE, shell=expand)
+    p = subprocess.Popen(c, stdin=subprocess.PIPE, *k, **kw)
     pipe = p.stdin
     val = pipe.write(stdin)
     pipe.close()
@@ -302,13 +301,13 @@ def write_pipe(c, stdin):
 
     return val
 
-def p4_write_pipe(c, stdin):
+def p4_write_pipe(c, stdin, *k, **kw):
     real_cmd = p4_build_cmd(c)
     if bytes is not str and isinstance(stdin, str):
         stdin = encode_text_stream(stdin)
-    return write_pipe(real_cmd, stdin)
+    return write_pipe(real_cmd, stdin, *k, **kw)
 
-def read_pipe_full(c):
+def read_pipe_full(c, *k, **kw):
     """ Read output from  command. Returns a tuple
         of the return status, stdout text and stderr
         text.
@@ -316,19 +315,19 @@ def read_pipe_full(c):
     if verbose:
         sys.stderr.write('Reading pipe: %s\n' % str(c))
 
-    expand = not isinstance(c, list)
-    p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=expand)
+    p = subprocess.Popen(
+        c, stdout=subprocess.PIPE, stderr=subprocess.PIPE, *k, **kw)
     (out, err) = p.communicate()
     return (p.returncode, out, decode_text_stream(err))
 
-def read_pipe(c, ignore_error=False, raw=False):
+def read_pipe(c, ignore_error=False, raw=False, *k, **kw):
     """ Read output from  command. Returns the output text on
         success. On failure, terminates execution, unless
         ignore_error is True, when it returns an empty string.
 
         If raw is True, do not attempt to decode output text.
     """
-    (retcode, out, err) = read_pipe_full(c)
+    (retcode, out, err) = read_pipe_full(c, *k, **kw)
     if retcode != 0:
         if ignore_error:
             out = ""
@@ -338,26 +337,25 @@ def read_pipe(c, ignore_error=False, raw=False):
         out = decode_text_stream(out)
     return out
 
-def read_pipe_text(c):
+def read_pipe_text(c, *k, **kw):
     """ Read output from a command with trailing whitespace stripped.
         On error, returns None.
     """
-    (retcode, out, err) = read_pipe_full(c)
+    (retcode, out, err) = read_pipe_full(c, *k, **kw)
     if retcode != 0:
         return None
     else:
         return decode_text_stream(out).rstrip()
 
-def p4_read_pipe(c, ignore_error=False, raw=False):
+def p4_read_pipe(c, ignore_error=False, raw=False, *k, **kw):
     real_cmd = p4_build_cmd(c)
-    return read_pipe(real_cmd, ignore_error, raw=raw)
+    return read_pipe(real_cmd, ignore_error, raw=raw, *k, **kw)
 
-def read_pipe_lines(c, raw=False):
+def read_pipe_lines(c, raw=False, *k, **kw):
     if verbose:
         sys.stderr.write('Reading pipe: %s\n' % str(c))
 
-    expand = not isinstance(c, list)
-    p = subprocess.Popen(c, stdout=subprocess.PIPE, shell=expand)
+    p = subprocess.Popen(c, stdout=subprocess.PIPE, *k, **kw)
     pipe = p.stdout
     lines = pipe.readlines()
     if not raw:
@@ -366,10 +364,10 @@ def read_pipe_lines(c, raw=False):
         die('Command failed: %s' % str(c))
     return lines
 
-def p4_read_pipe_lines(c):
+def p4_read_pipe_lines(c, *k, **kw):
     """Specifically invoke p4 on the command supplied. """
     real_cmd = p4_build_cmd(c)
-    return read_pipe_lines(real_cmd)
+    return read_pipe_lines(real_cmd, *k, **kw)
 
 def p4_has_command(cmd):
     """Ask p4 for help on this command.  If it returns an error, the
@@ -400,21 +398,19 @@ def p4_has_move_command():
     # assume it failed because @... was invalid changelist
     return True
 
-def system(cmd, ignore_error=False):
-    expand = not isinstance(cmd, list)
+def system(cmd, ignore_error=False, *k, **kw):
     if verbose:
         sys.stderr.write("executing %s\n" % str(cmd))
-    retcode = subprocess.call(cmd, shell=expand)
+    retcode = subprocess.call(cmd, *k, **kw)
     if retcode and not ignore_error:
         raise CalledProcessError(retcode, cmd)
 
     return retcode
 
-def p4_system(cmd):
+def p4_system(cmd, *k, **kw):
     """Specifically invoke p4 as the system command. """
     real_cmd = p4_build_cmd(cmd)
-    expand = not isinstance(real_cmd, list)
-    retcode = subprocess.call(real_cmd, shell=expand)
+    retcode = subprocess.call(real_cmd, *k, **kw)
     if retcode:
         raise CalledProcessError(retcode, real_cmd)
 
@@ -735,14 +731,12 @@ def isModeExecChanged(src_mode, dst_mode):
     return isModeExec(src_mode) != isModeExec(dst_mode)
 
 def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
-        errors_as_exceptions=False):
+        errors_as_exceptions=False, *k, **kw):
 
     if not isinstance(cmd, list):
         cmd = "-G " + cmd
-        expand = True
     else:
         cmd = ["-G"] + cmd
-        expand = False
 
     cmd = p4_build_cmd(cmd)
     if verbose:
@@ -763,10 +757,8 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
         stdin_file.flush()
         stdin_file.seek(0)
 
-    p4 = subprocess.Popen(cmd,
-                          shell=expand,
-                          stdin=stdin_file,
-                          stdout=subprocess.PIPE)
+    p4 = subprocess.Popen(
+        cmd, stdin=stdin_file, stdout=subprocess.PIPE, *k, **kw)
 
     result = []
     try:
@@ -819,8 +811,8 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
 
     return result
 
-def p4Cmd(cmd):
-    list = p4CmdList(cmd)
+def p4Cmd(cmd, *k, **kw):
+    list = p4CmdList(cmd, *k, **kw)
     result = {}
     for entry in list:
         result.update(entry)
@@ -869,7 +861,7 @@ def isValidGitDir(path):
     return git_dir(path) != None
 
 def parseRevision(ref):
-    return read_pipe("git rev-parse %s" % ref).strip()
+    return read_pipe("git rev-parse %s" % ref, shell=True).strip()
 
 def branchExists(ref):
     rev = read_pipe(["git", "rev-parse", "-q", "--verify", ref],
@@ -981,7 +973,7 @@ def p4BranchesInGit(branchesAreInRemotes=True):
     else:
         cmdline += "--branches"
 
-    for line in read_pipe_lines(cmdline):
+    for line in read_pipe_lines(cmdline, shell=True):
         line = line.strip()
 
         # only import to p4/
@@ -1044,7 +1036,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
 
     originPrefix = "origin/p4/"
 
-    for line in read_pipe_lines("git rev-parse --symbolic --remotes"):
+    for line in read_pipe_lines("git rev-parse --symbolic --remotes", shell=True):
         line = line.strip()
         if (not line.startswith(originPrefix)) or line.endswith("HEAD"):
             continue
@@ -1082,7 +1074,8 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
                               remoteHead, ','.join(settings['depot-paths'])))
 
         if update:
-            system("git update-ref %s %s" % (remoteHead, originHead))
+            system("git update-ref %s %s" % (remoteHead, originHead),
+                shell=True)
 
 def originP4BranchesExist():
         return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
@@ -1196,7 +1189,7 @@ def getClientSpec():
     """Look at the p4 client spec, create a View() object that contains
        all the mappings, and return it."""
 
-    specList = p4CmdList("client -o")
+    specList = p4CmdList("client -o", shell=True)
     if len(specList) != 1:
         die('Output from "client -o" is %d lines, expecting 1' %
             len(specList))
@@ -1225,7 +1218,7 @@ def getClientSpec():
 def getClientRoot():
     """Grab the client directory."""
 
-    output = p4CmdList("client -o")
+    output = p4CmdList("client -o", shell=True)
     if len(output) != 1:
         die('Output from "client -o" is %d lines, expecting 1' % len(output))
 
@@ -1480,7 +1473,7 @@ def p4UserId(self):
         if self.myP4UserId:
             return self.myP4UserId
 
-        results = p4CmdList("user -o")
+        results = p4CmdList("user -o", shell=True)
         for r in results:
             if 'User' in r:
                 self.myP4UserId = r['User']
@@ -1505,7 +1498,7 @@ def getUserMapFromPerforceServer(self):
         self.users = {}
         self.emails = {}
 
-        for output in p4CmdList("users"):
+        for output in p4CmdList("users", shell=True):
             if "User" not in output:
                 continue
             self.users[output["User"]] = output["FullName"] + " <" + output["Email"] + ">"
@@ -1629,7 +1622,7 @@ def __init__(self):
             die("Large file system not supported for git-p4 submit command. Please remove it from config.")
 
     def check(self):
-        if len(p4CmdList("opened ...")) > 0:
+        if len(p4CmdList("opened ...", shell=True)) > 0:
             die("You have files opened with perforce! Close them before starting the sync.")
 
     def separate_jobs_from_description(self, message):
@@ -1733,7 +1726,7 @@ def lastP4Changelist(self):
         # then gets used to patch up the username in the change. If the same
         # client spec is being used by multiple processes then this might go
         # wrong.
-        results = p4CmdList("client -o")        # find the current client
+        results = p4CmdList("client -o", shell=True)    # find the current client
         client = None
         for r in results:
             if 'Client' in r:
@@ -1749,7 +1742,7 @@ def lastP4Changelist(self):
 
     def modifyChangelistUser(self, changelist, newUser):
         # fixup the user field of a changelist after it has been submitted.
-        changes = p4CmdList("change -o %s" % changelist)
+        changes = p4CmdList("change -o %s" % changelist, shell=True)
         if len(changes) != 1:
             die("Bad output from p4 change modifying %s to user %s" %
                 (changelist, newUser))
@@ -1760,7 +1753,7 @@ def modifyChangelistUser(self, changelist, newUser):
         # p4 does not understand format version 3 and above
         input = marshal.dumps(c, 2)
 
-        result = p4CmdList("change -f -i", stdin=input)
+        result = p4CmdList("change -f -i", stdin=input, shell=True)
         for r in result:
             if 'code' in r:
                 if r['code'] == 'error':
@@ -1866,7 +1859,7 @@ def edit_template(self, template_file):
         if "P4EDITOR" in os.environ and (os.environ.get("P4EDITOR") != ""):
             editor = os.environ.get("P4EDITOR")
         else:
-            editor = read_pipe("git var GIT_EDITOR").strip()
+            editor = read_pipe("git var GIT_EDITOR", shell=True).strip()
         system(["sh", "-c", ('%s "$@"' % editor), editor, template_file])
 
         # If the file was not saved, prompt to see if this patch should
@@ -1924,7 +1917,9 @@ def applyCommit(self, id):
 
         (p4User, gitEmail) = self.p4UserForCommit(id)
 
-        diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (self.diffOpts, id, id))
+        diff = read_pipe_lines(
+            "git diff-tree -r %s \"%s^\" \"%s\"" % (self.diffOpts, id, id),
+            shell=True)
         filesToAdd = set()
         filesToChangeType = set()
         filesToDelete = set()
@@ -2060,7 +2055,7 @@ def applyCommit(self, id):
         #
         # Apply the patch for real, and do add/delete/+x handling.
         #
-        system(applyPatchCmd)
+        system(applyPatchCmd, shell=True)
 
         for f in filesToChangeType:
             p4_edit(f, "-t", "auto")
@@ -3386,7 +3381,7 @@ def getBranchMapping(self):
         else:
             command = "branches"
 
-        for info in p4CmdList(command):
+        for info in p4CmdList(command, shell=True):
             details = p4Cmd(["branch", "-o", info["branch"]])
             viewIdx = 0
             while "View%s" % viewIdx in details:
@@ -3477,7 +3472,9 @@ def gitCommitByP4Change(self, ref, change):
         while True:
             if self.verbose:
                 print("trying: earliest %s latest %s" % (earliestCommit, latestCommit))
-            next = read_pipe("git rev-list --bisect %s %s" % (latestCommit, earliestCommit)).strip()
+            next = read_pipe(
+                "git rev-list --bisect %s %s" % (latestCommit, earliestCommit),
+                shell=True).strip()
             if len(next) == 0:
                 if self.verbose:
                     print("argh")
@@ -3633,7 +3630,7 @@ def sync_origin_only(self):
             if self.hasOrigin:
                 if not self.silent:
                     print('Syncing with origin first, using "git fetch origin"')
-                system("git fetch origin")
+                system("git fetch origin", shell=True)
 
     def importHeadRevision(self, revision):
         print("Doing initial import of %s from revision %s into %s" % (' '.join(self.depotPaths), revision, self.branch))
@@ -3800,8 +3797,8 @@ def run(self, args):
         if len(self.branch) == 0:
             self.branch = self.refPrefix + "master"
             if gitBranchExists("refs/heads/p4") and self.importIntoRemotes:
-                system("git update-ref %s refs/heads/p4" % self.branch)
-                system("git branch -D p4")
+                system("git update-ref %s refs/heads/p4" % self.branch, shell=True)
+                system("git branch -D p4", shell=True)
 
         # accept either the command-line option, or the configuration variable
         if self.useClientSpec:
@@ -4004,7 +4001,7 @@ def run(self, args):
         # Cleanup temporary branches created during import
         if self.tempBranches != []:
             for branch in self.tempBranches:
-                read_pipe("git update-ref -d %s" % branch)
+                read_pipe("git update-ref -d %s" % branch, shell=True)
             os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
 
         # Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
@@ -4036,7 +4033,7 @@ def run(self, args):
     def rebase(self):
         if os.system("git update-index --refresh") != 0:
             die("Some files in your working directory are modified and different than what is in your index. You can use git update-index <filename> to bring the index up to date or stash away all your changes with git stash.");
-        if len(read_pipe("git diff-index HEAD --")) > 0:
+        if len(read_pipe("git diff-index HEAD --", shell=True)) > 0:
             die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash.");
 
         [upstream, settings] = findUpstreamBranchPoint()
@@ -4047,9 +4044,10 @@ def rebase(self):
         upstream = re.sub("~[0-9]+$", "", upstream)
 
         print("Rebasing the current branch onto %s" % upstream)
-        oldHead = read_pipe("git rev-parse HEAD").strip()
-        system("git rebase %s" % upstream)
-        system("git diff-tree --stat --summary -M %s HEAD --" % oldHead)
+        oldHead = read_pipe("git rev-parse HEAD", shell=True).strip()
+        system("git rebase %s" % upstream, shell=True)
+        system("git diff-tree --stat --summary -M %s HEAD --" % oldHead,
+            shell=True)
         return True
 
 class P4Clone(P4Sync):
@@ -4126,7 +4124,7 @@ def run(self, args):
 
         # auto-set this variable if invoked with --use-client-spec
         if self.useClientSpec_from_options:
-            system("git config --bool git-p4.useclientspec true")
+            system("git config --bool git-p4.useclientspec true", shell=True)
 
         return True
 
@@ -4260,7 +4258,7 @@ def run(self, args):
         cmdline = "git rev-parse --symbolic "
         cmdline += " --remotes"
 
-        for line in read_pipe_lines(cmdline):
+        for line in read_pipe_lines(cmdline, shell=True):
             line = line.strip()
 
             if not line.startswith('p4/') or line == "p4/HEAD":
@@ -4343,9 +4341,11 @@ def main():
             cmd.gitdir = os.path.abspath(".git")
             if not isValidGitDir(cmd.gitdir):
                 # "rev-parse --git-dir" without arguments will try $PWD/.git
-                cmd.gitdir = read_pipe("git rev-parse --git-dir").strip()
+                cmd.gitdir = read_pipe(
+                    "git rev-parse --git-dir", shell=True).strip()
                 if os.path.exists(cmd.gitdir):
-                    cdup = read_pipe("git rev-parse --show-cdup").strip()
+                    cdup = read_pipe(
+                        "git rev-parse --show-cdup", shell=True).strip()
                     if len(cdup) > 0:
                         chdir(cdup);
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH 0/3] git-p4: Rationalise command construction
From: Joel Holdsworth @ 2022-01-05 16:14 UTC (permalink / raw)
  To: git, Luke Diamand, Junio C Hamano
  Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
	Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth

This patch-set removes the mixing of commands constructed using strings
with commands constructed using python lists. The goal being to simplify
the code by standardising on the usage of lists throughout the script.

It also attempts to make usage of shell execution clearer by changing
the code to require the caller to explicitly request execution-in-shell
if required.

With the script changed over to using lists every, there is also a patch
to improve the printing of log messages as command strings rather than
as stringified python lists

Joel Holdsworth (3):
  git-p4: don't select shell mode using the type of the command argument
  git-p4: pass command arguments as lists instead of using shell
  git-p4: don't print shell commands as python lists

 git-p4.py | 176 ++++++++++++++++++++++++------------------------------
 1 file changed, 79 insertions(+), 97 deletions(-)

-- 
2.34.1


^ permalink raw reply

* Re: [PATCH v4] powerpc/pseries: read the lpar name from the firmware
From: Laurent Dufour @ 2022-01-05 16:13 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Nathan Lynch, linuxppc-dev, linux-kernel
In-Reply-To: <20211207171109.22793-1-ldufour@linux.ibm.com>

Happy New Year, Michael!

Do you consider taking that patch soon?

Thanks,
Laurent.

On 07/12/2021, 18:11:09, Laurent Dufour wrote:
> The LPAR name may be changed after the LPAR has been started in the HMC.
> In that case lparstat command is not reporting the updated value because it
> reads it from the device tree which is read at boot time.
> 
> However this value could be read from RTAS.
> 
> Adding this value in the /proc/powerpc/lparcfg output allows to read the
> updated value.
> 
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
> v4:
>  address Nathan's new comments limiting size of the buffer.
> v3:
>  address Michael's comments.
> v2:
>  address Nathan's comments.
>  change title to partition_name aligning with existing partition_id
> ---
>  arch/powerpc/platforms/pseries/lparcfg.c | 54 ++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
> index f71eac74ea92..058d9a5fe545 100644
> --- a/arch/powerpc/platforms/pseries/lparcfg.c
> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
> @@ -311,6 +311,59 @@ static void parse_mpp_x_data(struct seq_file *m)
>  		seq_printf(m, "coalesce_pool_spurr=%ld\n", mpp_x_data.pool_spurr_cycles);
>  }
>  
> +/*
> + * PAPR defines, in section "7.3.16 System Parameters Option", the token 55 to
> + * read the LPAR name, and the largest output data to 4000 + 2 bytes length.
> + */
> +#define SPLPAR_LPAR_NAME_TOKEN	55
> +#define GET_SYS_PARM_BUF_SIZE	4002
> +#if GET_SYS_PARM_BUF_SIZE > RTAS_DATA_BUF_SIZE
> +#error "GET_SYS_PARM_BUF_SIZE is larger than RTAS_DATA_BUF_SIZE"
> +#endif
> +static void read_lpar_name(struct seq_file *m)
> +{
> +	int rc, len, token;
> +	union {
> +		char raw_buffer[GET_SYS_PARM_BUF_SIZE];
> +		struct {
> +			__be16 len;
> +			char name[GET_SYS_PARM_BUF_SIZE-2];
> +		};
> +	} *local_buffer;
> +
> +	token = rtas_token("ibm,get-system-parameter");
> +	if (token == RTAS_UNKNOWN_SERVICE)
> +		return;
> +
> +	local_buffer = kmalloc(sizeof(*local_buffer), GFP_KERNEL);
> +	if (!local_buffer)
> +		return;
> +
> +	do {
> +		spin_lock(&rtas_data_buf_lock);
> +		memset(rtas_data_buf, 0, sizeof(*local_buffer));
> +		rc = rtas_call(token, 3, 1, NULL, SPLPAR_LPAR_NAME_TOKEN,
> +			       __pa(rtas_data_buf), sizeof(*local_buffer));
> +		if (!rc)
> +			memcpy(local_buffer->raw_buffer, rtas_data_buf,
> +			       sizeof(local_buffer->raw_buffer));
> +		spin_unlock(&rtas_data_buf_lock);
> +	} while (rtas_busy_delay(rc));
> +
> +	if (!rc) {
> +		/* Force end of string */
> +		len = min((int) be16_to_cpu(local_buffer->len),
> +			  (int) sizeof(local_buffer->name)-1);
> +		local_buffer->name[len] = '\0';
> +
> +		seq_printf(m, "partition_name=%s\n", local_buffer->name);
> +	} else
> +		pr_err_once("Error calling get-system-parameter (0x%x)\n", rc);
> +
> +	kfree(local_buffer);
> +}
> +
> +
>  #define SPLPAR_CHARACTERISTICS_TOKEN 20
>  #define SPLPAR_MAXLENGTH 1026*(sizeof(char))
>  
> @@ -496,6 +549,7 @@ static int pseries_lparcfg_data(struct seq_file *m, void *v)
>  
>  	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
>  		/* this call handles the ibm,get-system-parameter contents */
> +		read_lpar_name(m);
>  		parse_system_parameter_string(m);
>  		parse_ppp_data(m);
>  		parse_mpp_data(m);


^ permalink raw reply

* Re: [OE-core] [PATCH 2/2] package.bbclass: don't skip kernel and kernel modules
From: Bruce Ashfield @ 2022-01-05 16:14 UTC (permalink / raw)
  To: Saul Wold
  Cc: Richard Purdie, Patches and discussions about the oe-core layer,
	Joshua Watt, Bruce Ashfield
In-Reply-To: <b90b0597-15e4-f19d-2431-9afe77d6c33c@windriver.com>

On Tue, Jan 4, 2022 at 5:08 PM Saul Wold <Saul.Wold@windriver.com> wrote:
>
>
>
> On 12/22/21 01:09, Richard Purdie wrote:
> > On Tue, 2021-12-21 at 11:08 -0800, Saul Wold wrote:
> >> Stop ignoring or skipping the kernel and kernel modules code in the
> >> split debug and striping functions, this will allow create_spdx to
> >> process the kernel and modules.
> >>
> >> Signed-off-by: Saul Wold <saul.wold@windriver.com>
> >> ---
> >>   meta/classes/package.bbclass | 8 ++------
> >>   1 file changed, 2 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> >> index 84eafbd529..4b7fe4f1e1 100644
> >> --- a/meta/classes/package.bbclass
> >> +++ b/meta/classes/package.bbclass
> >> @@ -390,10 +390,6 @@ def splitdebuginfo(file, dvar, debugdir, debuglibdir, debugappend, debugsrcdir,
> >>       dvar = d.getVar('PKGD')
> >>       objcopy = d.getVar("OBJCOPY")
> >>
> >> -    # We ignore kernel modules, we don't generate debug info files.
> >> -    if file.find("/lib/modules/") != -1 and file.endswith(".ko"):
> >> -        return (file, sources)
> >> -
> >>       newmode = None
> >>       if not os.access(file, os.W_OK) or os.access(file, os.R_OK):
> >>           origmode = os.stat(file)[stat.ST_MODE]
> >> @@ -1147,7 +1143,7 @@ python split_and_strip_files () {
> >>
> >>                   if file.endswith(".ko") and file.find("/lib/modules/") != -1:
> >>                       kernmods.append(file)
> >> -                    continue
> >> +
> >>                   if oe.package.is_static_lib(file):
> >>                       staticlibs.append(file)
> >>                       continue
> >> @@ -1165,7 +1161,7 @@ python split_and_strip_files () {
> >>                       continue
> >>                   # Check its an executable
> >>                   if (s[stat.ST_MODE] & stat.S_IXUSR) or (s[stat.ST_MODE] & stat.S_IXGRP) or (s[stat.ST_MODE] & stat.S_IXOTH) \
> >> -                        or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)):
> >> +                        or ((file.startswith(libdir) or file.startswith(baselibdir)) and (".so" in f or ".node" in f)) or (f.startswith('vmlinux') or ".ko" in f):
> >>
> >>                       if cpath.islink(file):
> >>                           checkelflinks[file] = ltarget
> >
> > edgerouter:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/62/builds/4513
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/111/builds/2507/steps/11/logs/stdio
> >
> So I have been digging into this and it seems that an option was added a
> decade ago or so to strip the kernel/vmlinux when it's too big, this was
> done for at least the routerstationpro according to bug #3515 [0], and
> persists with the edgerouter, although I am not sure if it would still
> actually be required as the edgerouter also uses the
> KERNEL_ALT_IMAGETYPE to create a smaller binary kernel image.

I recall when we added that! It was used for some other boards as
well, but most of them aren't around anymore.

>
> The change I proposed causes the all kernels to be stripped all the time
> as part of the split_and_strip_files(). As I see it there few different
> options:

Having some way to have a custom set of sections to strip (along with
skipping stripping (but that can be done via the standard inhibit)) is
something I'd suggest we preserve. But I suppose if you inhibit stripping,
you'll stop both the packaging one and the kernel custom one ?

>
> 1) Set KERNEL_IMAGE_EXTRA_STRIP_SECTIONS = "" in create_spdx.bbclass
>    - This solves the problem with create_spdx.bbclass is in use, but not
> the general case

What are you considering the general case in this instance ? Meaning a
non-spdx user of that same board, will run into issues with the already
stripped ? If they can inhibit the do_package stripping, there is a way around
it.

>
> 2) Remove the KERNEL_IMAGE_EXTRA_STRIP_SECTIONS from edgerouter.conf
>    - Will solve the edgerouter case but may not solve other usages
> unknown to me.
>    - Does anyone know of other machines/layers usage of this variable?
>

See above. There are some machines, and even if not common, it is
something I'd like to preserve.

> 3) deprecate the kernel.bbclass:do_strip function in favor of using the
> split_and_strip_files() of package.bbclass
>

I'd prefer to not do #3.

> 4) Change error to warning in packaging.bbclass for the kernel only
>    - This would explain that a kernel image (vmlinux) is already
> stripped and extended package data would not be available for for SPDX
> creation.

#4 is what came to mind for me. We already have special cases for the
kernel, so this isn't making things more complex .. or maybe there's a
more elegant "co-operative" section removal flag that the kernel bbclass
can set, and then the packaging not error or automatically inhibit the
QA check ?

But #1 is my second choice.

Bruce

>
> RP, Bruce, Joshua: Thoughts?
>
> Sau!
>
> [0] https://bugzilla.yoctoproject.org/show_bug.cgi?id=3515
>
>
> > qemux86 musl cryptodev:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/64/builds/4512/steps/11/logs/stdio
> >
> > qemux86-64 musl cryptodev:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/45/builds/4526
> >
> > qemux86 cryptodev:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/52/builds/4476/steps/11/logs/stdio
> >
> I tried these and have not been able to reproduce the failure.
>
>
>
> > selftest failure in linux-yocto:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/2981/steps/14/logs/stdio
> > (file truncated makes it sound like a race?)
> >
> > stap kernel module failure:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/110/builds/3395/steps/13/logs/stdio
> > (race/intermittent?)
> >
> > another kernel module race:
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/63/builds/4480/steps/16/logs/stdio
> >
> Might need to try this on the AB once I resolve the kernel stripping
> issue above.
>
> Sau!
>
> > Cheers,
> >
> > Richard
> >
> >
> >
>
> --
> Sau!
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#160172): https://lists.openembedded.org/g/openembedded-core/message/160172
> Mute This Topic: https://lists.openembedded.org/mt/87884056/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II


^ permalink raw reply

* Re: [PATCH v3] ASoC: cs4265: Add a remove() function
From: Mark Brown @ 2022-01-05 16:12 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: alsa-devel, Fabio Estevam, ckeepax
In-Reply-To: <20220104180613.639317-1-festevam@gmail.com>

On Tue, 4 Jan 2022 15:06:13 -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> When the reset_gpio GPIO is used, it is better to put the codec
> back into reset state when the driver unbinds.
> 
> Add a remove() function to accomplish that.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: cs4265: Add a remove() function
      commit: a319cb32e7cfd2703db3a883ce260a7b06729895

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* Re: [PATCH v4] powerpc/pseries: read the lpar name from the firmware
From: Laurent Dufour @ 2022-01-05 16:13 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Nathan Lynch, linux-kernel, linuxppc-dev
In-Reply-To: <20211207171109.22793-1-ldufour@linux.ibm.com>

Happy New Year, Michael!

Do you consider taking that patch soon?

Thanks,
Laurent.

On 07/12/2021, 18:11:09, Laurent Dufour wrote:
> The LPAR name may be changed after the LPAR has been started in the HMC.
> In that case lparstat command is not reporting the updated value because it
> reads it from the device tree which is read at boot time.
> 
> However this value could be read from RTAS.
> 
> Adding this value in the /proc/powerpc/lparcfg output allows to read the
> updated value.
> 
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
> v4:
>  address Nathan's new comments limiting size of the buffer.
> v3:
>  address Michael's comments.
> v2:
>  address Nathan's comments.
>  change title to partition_name aligning with existing partition_id
> ---
>  arch/powerpc/platforms/pseries/lparcfg.c | 54 ++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
> index f71eac74ea92..058d9a5fe545 100644
> --- a/arch/powerpc/platforms/pseries/lparcfg.c
> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
> @@ -311,6 +311,59 @@ static void parse_mpp_x_data(struct seq_file *m)
>  		seq_printf(m, "coalesce_pool_spurr=%ld\n", mpp_x_data.pool_spurr_cycles);
>  }
>  
> +/*
> + * PAPR defines, in section "7.3.16 System Parameters Option", the token 55 to
> + * read the LPAR name, and the largest output data to 4000 + 2 bytes length.
> + */
> +#define SPLPAR_LPAR_NAME_TOKEN	55
> +#define GET_SYS_PARM_BUF_SIZE	4002
> +#if GET_SYS_PARM_BUF_SIZE > RTAS_DATA_BUF_SIZE
> +#error "GET_SYS_PARM_BUF_SIZE is larger than RTAS_DATA_BUF_SIZE"
> +#endif
> +static void read_lpar_name(struct seq_file *m)
> +{
> +	int rc, len, token;
> +	union {
> +		char raw_buffer[GET_SYS_PARM_BUF_SIZE];
> +		struct {
> +			__be16 len;
> +			char name[GET_SYS_PARM_BUF_SIZE-2];
> +		};
> +	} *local_buffer;
> +
> +	token = rtas_token("ibm,get-system-parameter");
> +	if (token == RTAS_UNKNOWN_SERVICE)
> +		return;
> +
> +	local_buffer = kmalloc(sizeof(*local_buffer), GFP_KERNEL);
> +	if (!local_buffer)
> +		return;
> +
> +	do {
> +		spin_lock(&rtas_data_buf_lock);
> +		memset(rtas_data_buf, 0, sizeof(*local_buffer));
> +		rc = rtas_call(token, 3, 1, NULL, SPLPAR_LPAR_NAME_TOKEN,
> +			       __pa(rtas_data_buf), sizeof(*local_buffer));
> +		if (!rc)
> +			memcpy(local_buffer->raw_buffer, rtas_data_buf,
> +			       sizeof(local_buffer->raw_buffer));
> +		spin_unlock(&rtas_data_buf_lock);
> +	} while (rtas_busy_delay(rc));
> +
> +	if (!rc) {
> +		/* Force end of string */
> +		len = min((int) be16_to_cpu(local_buffer->len),
> +			  (int) sizeof(local_buffer->name)-1);
> +		local_buffer->name[len] = '\0';
> +
> +		seq_printf(m, "partition_name=%s\n", local_buffer->name);
> +	} else
> +		pr_err_once("Error calling get-system-parameter (0x%x)\n", rc);
> +
> +	kfree(local_buffer);
> +}
> +
> +
>  #define SPLPAR_CHARACTERISTICS_TOKEN 20
>  #define SPLPAR_MAXLENGTH 1026*(sizeof(char))
>  
> @@ -496,6 +549,7 @@ static int pseries_lparcfg_data(struct seq_file *m, void *v)
>  
>  	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
>  		/* this call handles the ibm,get-system-parameter contents */
> +		read_lpar_name(m);
>  		parse_system_parameter_string(m);
>  		parse_ppp_data(m);
>  		parse_mpp_data(m);


^ permalink raw reply

* Re: [PATCH 0/8] Add low power hibernation support to cs35l41
From: Mark Brown @ 2022-01-05 16:12 UTC (permalink / raw)
  To: Charles Keepax; +Cc: patches, alsa-devel, lgirdwood, tiwai, david.rhodes
In-Reply-To: <20220105113026.18955-1-ckeepax@opensource.cirrus.com>

On Wed, 5 Jan 2022 11:30:18 +0000, Charles Keepax wrote:
> This patch series adds support for the low power hibernation feature
> on cs35l41. This allows the DSP memory to be retained whilst the
> device enters a very low power state.
> 
> Patches 1-6 can happily be applied straight away and are mostly bug
> fixes to set things up for the series specifically around getting the
> cache handling corrected in the driver.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/8] ASoC: cs35l41: Add cs35l51/53 IDs
      commit: dcf821319474edde7e85b95608a4539703a2b67d
[2/8] ASoC: cs35l41: Remove incorrect comment
      commit: 4e7c3cd87db8d9350062a25a8476f90fd1cbc4c9
[3/8] ASoC: cs35l41: Correct DSP power down
      commit: 56852cf4b2179fb90068a49538501f31c2de18ea
[4/8] ASoC: cs35l41: Correct handling of some registers in the cache
      commit: 5f2f539901b0d9bda722637521a11b7f7cf753f1
[5/8] firmware: cs_dsp: Clear core reset for cache
      commit: 7aa1cc1091e0a424e9e7711ca381ebe98b6865bc
[6/8] ASoC: wm_adsp: Add support for "toggle" preloaders
      commit: ba235634b138cd9d012dbe983e7920481211e132
[7/8] ASoC: cs35l41: Update handling of test key registers
      (no commit info)
[8/8] ASoC: cs35l41: Add support for hibernate memory retention mode
      (no commit info)

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: don't call free_mmap_offset when purging
From: Patchwork @ 2022-01-05 16:13 UTC (permalink / raw)
  To: Matthew Auld; +Cc: intel-gfx
In-Reply-To: <20220105145835.142950-1-matthew.auld@intel.com>

== Series Details ==

Series: series starting with [1/4] drm/i915: don't call free_mmap_offset when purging
URL   : https://patchwork.freedesktop.org/series/98509/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5198a35603a2 drm/i915: don't call free_mmap_offset when purging
-:22: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#22: 
<4> [749.062928] CPU: 0 PID: 1643 Comm: gem_madvise Tainted: G     U  W         5.16.0-rc8-CI-CI_DRM_11046+ #1

total: 0 errors, 1 warnings, 0 checks, 7 lines checked
d478a7195b96 drm/i915/ttm: only fault WILLNEED objects
62769350a16a drm/i915/ttm: ensure we unmap when purging
c6d2e5c11332 drm/i915/ttm: ensure we unmap when shrinking



^ permalink raw reply

* Re: [PATCH 0/7] y2038: cond_wait_prologue64 and related fixes
From: Jan Kiszka @ 2022-01-05 16:13 UTC (permalink / raw)
  To: Bezdeka, Florian (T CED SES-DE), xenomai@xenomai.org
In-Reply-To: <034ac263dbb354ed8af7b575b0e884d97f8968c8.camel@siemens.com>

On 05.01.22 16:08, Bezdeka, Florian (T CED SES-DE) wrote:
> On Wed, 2022-01-05 at 15:58 +0100, Jan Kiszka wrote:
>> On 05.01.22 15:56, Bezdeka, Florian (T CED SES-DE) wrote:
>>> On Wed, 2022-01-05 at 15:43 +0100, Jan Kiszka wrote:
>>>> On 05.01.22 15:06, Florian Bezdeka wrote:
>>>>> Hi all,
>>>>>
>>>>> this is the last missing POSIX related y2038 affected syscall in
>>>>> Xenomai. With this applied we have two Xenomai specific syscalls
>>>>> missing:
>>>>>
>>>>>   - sc_cobalt_thread_setschedparam_ex
>>>>>   - sc_cobalt_thread_getschedparam_ex
>>>>>
>>>>> While adding tests for the introduced cond_wait_prologue64 I hit a
>>>>> kernel OOPS due to insuficient validation of user provided pointers.
>>>>> That has been addressed as well.
>>>>
>>>> Thanks for both! Is it possibly to move the fixes the front? That would
>>>> also ensure that I can easily pick them into stable.
>>>
>>> Yes. Patch 4 and 7 could be moved to the front easily. Do you want me
>>> to split patch 2 into the y2038 and non y2038 part, or does that not
>>> qualify for stable at all?
>>
>> Can I reorder things myself, or does patch 4 break (patch 7 does not,
>> already checked)? Then I just change the application order while doing
>> git am.
> 
> No breakage expected. The only "problematic" one would be patch 2 as it
> touches y2038 as well as non-y2038 syscall definitions. Let me know if
> I should split that into two parts (which would allow the non y2038
> related cleanup to be applied to stable separately)

The annotation patch is not needed for stable.

I'm applying now 4 7 1 2 3 5 6 and will kick off testing.

Thanks,
Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


^ permalink raw reply

* Re: [PATCH v2 1/2] EDAC/amd64: Check register values from all UMCs
From: Yazen Ghannam @ 2022-01-05 16:12 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-edac, linux-kernel, mchehab, tony.luck, james.morse, rric,
	Smita.KoralahalliChannabasappa, william.roche
In-Reply-To: <Yc2ZzMT+Mg5xCvjI@zn.tnic>

On Thu, Dec 30, 2021 at 12:36:44PM +0100, Borislav Petkov wrote:
> On Thu, Dec 16, 2021 at 04:08:18PM +0000, Yazen Ghannam wrote:
> > No, that's a good question. And actually the assumption is incorrect. It is
> > allowed to have different DIMM types in a system though all DIMMs on a single
> > UMC must match.
> 
> Oh fun, really?!
> 
> So a single system can have DDR4 *and* DDR5 on the same board?!
>

Well, I don't know about that specifically. There are some restrictions, but
you could have UDIMMs and RDIMMs of the same generation, at least.
 
> So then that
> 
> 	pvt->dram_type
> 
> is insufficient to store the DIMM type for a pvt. If you have multiple
> UMCs on a pvt and all have different type DIMMs, then you need the
> relevant DIMM type when you dump it in sysfs...
> 
> Which then means, you need ->dram_type to be per UMC...
> 
> And also, I'm assuming the hw already enforces that DIMMs on a single
> UMC must match - it simply won't boot if they don't so you don't have to
> enforce that, at least.
> 
> > Do you recommend a follow up patch or should this one be reworked?
> 
> This one is insufficient, I'm afraid.
> 
> One way to address this is, you could use pvt->umc at the places where
> dram_type is used and assign directly to the dimm->mtype thing. But then
> you'd need a way to map each struct dimm_info *dimm to the UMC so that
> you can determine the correct DIMM type.
> 

I did send a patch that did something like this.
https://lkml.kernel.org/r/20211228200615.412999-2-yazen.ghannam@amd.com

Though this got a build warning report, so I need to follow up on that.

> Which would make pvt->dram_type redundant and can be removed.
>

I kept this so as to not break legacy systems. But I'll look at it again. I
think you may be right.

Thanks,
Yazen 

^ permalink raw reply

* Re: [PATCH net-next v4 3/8] net/funeth: probing and netdev ops
From: Andrew Lunn @ 2022-01-05 16:12 UTC (permalink / raw)
  To: Dimitris Michailidis; +Cc: Jakub Kicinski, davem, netdev
In-Reply-To: <CAOkoqZmxHZ6KTZQPe+w23E_UPYWLNRiU8gVX32EFsNXgyzkucg@mail.gmail.com>

> > > +     if ((notif->link_state | notif->missed_events) & FUN_PORT_FLAG_MAC_DOWN)
> > > +             netif_carrier_off(netdev);
> > > +     if (notif->link_state & FUN_PORT_FLAG_NH_DOWN)
> > > +             netif_dormant_on(netdev);
> > > +     if (notif->link_state & FUN_PORT_FLAG_NH_UP)
> > > +             netif_dormant_off(netdev);
> >
> > What does this do?
> 
> FW may get exclusive access to the ports in some cases and during those times
> host traffic isn't serviced. Changing a port to dormant is its way of
> telling the host
> the port is unavailable though it has link up.

Quoting RFC2863

3.1.12.  New states for IfOperStatus

   Three new states have been added to ifOperStatus: 'dormant',
   'notPresent', and 'lowerLayerDown'.

   The dormant state indicates that the relevant interface is not
   actually in a condition to pass packets (i.e., it is not 'up') but is
   in a "pending" state, waiting for some external event.  For "on-
   demand" interfaces, this new state identifies the situation where the
   interface is waiting for events to place it in the up state.
   Examples of such events might be:

   (1)   having packets to transmit before establishing a connection to
         a remote system;

   (2)   having a remote system establish a connection to the interface
         (e.g. dialing up to a slip-server).

I can see this being valid if your FW is doing 802.1X. But i'm not
sure it is valid for other use cases. What exactly is your firmware
doing which stops it from handling frames?

	Andrew

^ permalink raw reply

* Re: [PATCH v2] btrfs: Remove redundant assignment of slot and leaf
From: Filipe Manana @ 2022-01-05 16:11 UTC (permalink / raw)
  To: Jiapeng Chong; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, Abaci Robot
In-Reply-To: <20220105150758.29670-1-jiapeng.chong@linux.alibaba.com>

On Wed, Jan 05, 2022 at 11:07:58PM +0800, Jiapeng Chong wrote:
> From: chongjiapeng <jiapeng.chong@linux.alibaba.com>
> 
> slot and leaf are being initialized to path->slots[0] and
> path->nodes[0], but this is never read as slot and leaf
> is overwritten later on. Remove the redundant assignment.
> 
> Cleans up the following clang-analyzer warning:
> 
> fs/btrfs/tree-log.c:6125:7: warning: Value stored to 'slot' during its
> initialization is never read [clang-analyzer-deadcode.DeadStores].
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: chongjiapeng <jiapeng.chong@linux.alibaba.com>
> ---
> Changes in v2:
>   -Remove redundant assignment of leaf.
> 
>  fs/btrfs/tree-log.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 4b89ac769347..d99cda0acd95 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -6188,8 +6188,6 @@ static int log_new_ancestors(struct btrfs_trans_handle *trans,
>  		if (ret < 0)
>  			return ret;
>  
> -		leaf = path->nodes[0];
> -		slot = path->slots[0];

No, this is not correct.

Right before those assignments we called btrfs_search_slot(), which updates
path->nodes and path->slots, and we need those updated values below.

The redundant assignments are not these two, but instead the ones when the
variables are declared at the top of the loop:

   struct extent_buffer *leaf = path->nodes[0];
   int slot = path->slots[0];

Thanks.

>  		if (slot >= btrfs_header_nritems(leaf)) {
>  			ret = btrfs_next_leaf(root, path);
>  			if (ret < 0)
> -- 
> 2.19.1.6.gb485710b
> 

^ permalink raw reply

* Re: [PATCH v3 0/5] aio: fix use-after-free and missing wakeups
From: Jens Axboe @ 2022-01-05 16:11 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Linus Torvalds, Alexander Viro, Benjamin LaHaise, linux-aio,
	linux-fsdevel, Linux Kernel Mailing List, Ramji Jiyani,
	Christoph Hellwig, Oleg Nesterov, Martijn Coenen, stable
In-Reply-To: <YdW4sApUUBi/5UHh@sol.localdomain>

On 1/5/22 7:26 AM, Eric Biggers wrote:
> On Thu, Dec 09, 2021 at 02:46:45PM -0700, Jens Axboe wrote:
>> On 12/9/21 11:00 AM, Linus Torvalds wrote:
>>> On Wed, Dec 8, 2021 at 5:06 PM Eric Biggers <ebiggers@kernel.org> wrote:
>>>>
>>>> Careful review is appreciated; the aio poll code is very hard to work
>>>> with, and it doesn't appear to have many tests.  I've verified that it
>>>> passes the libaio test suite, which provides some coverage of poll.
>>>>
>>>> Note, it looks like io_uring has the same bugs as aio poll.  I haven't
>>>> tried to fix io_uring.
>>>
>>> I'm hoping Jens is looking at the io_ring case, but I'm also assuming
>>> that I'll just get a pull request for this at some point.
>>
>> Yes, when I saw this original posting I did discuss it with Pavel as
>> well, and we agree that the same issue exists there. Which isn't too
>> surprising, as that's where the io_uring poll code from originally.
>>
> 
> Jens, any update on fixing the io_uring version of the bug?  Note,
> syzbot has managed to use io_uring poll to hit the WARN_ON_ONCE() that
> I added in __wake_up_pollfree(), which proves that it is broken.

There are two parts to this, first part is queued up for 5.17 for a few
weeks. Work in progress...

-- 
Jens Axboe


^ permalink raw reply

* Re: [PATCH v2 05/10] nfs: Add export support for weak cache consistency attributes
From: Daire Byrne @ 2022-01-05 16:10 UTC (permalink / raw)
  To: trondmy; +Cc: Chuck Lever, J. Bruce Fields, linux-nfs
In-Reply-To: <20211219013803.324724-6-trondmy@kernel.org>

Hi,

I was interested in testing these patches with our re-export
workloads. However, it looks like this specific patch and the previous
one (nfsd: Distinguish between required and optional NFSv3 post-op
attributes) are breaking re-exports in such a way as to cause
applications to randomly crash out with sigbus errors.

At a guess, loading applications and memory mapping data files are
particularly good at triggering this. I can see no errors logged on
either the re-export server or the eventual client (e.g. stale
filehandles). We mostly re-export NFSv4.2 servers to NFSv3 clients but
we do also have a few NFSv3 servers re-exported as NFSv3  too
(Netapps).

This only happens with patches 4 + 5 and vanilla 5.16-rc6 does not
have any issues. I haven't had a chance to dig much deeper yet, but
thought I'd flag it anyway.

Daire

On Sun, 19 Dec 2021 at 01:44, <trondmy@kernel.org> wrote:
>
> From: Trond Myklebust <trond.myklebust@primarydata.com>
>
> Allow knfsd to request weak cache consistency attributes on files that
> have delegations and/or have up to date attribute caches by propagating
> the information to NFS that the attributes being requested are optional.

^ permalink raw reply

* MES:
From: Mustafa Ayvaz @ 2022-01-05 15:50 UTC (permalink / raw)
  To: linux-omap

Hello linux-omap,

I was only wondering if you got my previous email? I have been 
trying to reach you on your email linux-omap@vger.kernel.org , 
kindly get back to me swiftly, it is very important.

Thanks
Mustafa Ayvaz
mustafa@ayvazburosu.com

^ permalink raw reply

* MES:
From: Mustafa Ayvaz @ 2022-01-05 15:50 UTC (permalink / raw)
  To: netdev

Hello netdev,

I was only wondering if you got my previous email? I have been 
trying to reach you on your email netdev@vger.kernel.org , kindly 
get back to me swiftly, it is very important.

Thanks
Mustafa Ayvaz
mustafa@ayvazburosu.com

^ permalink raw reply

* Re: Aw: Re: barebox - rk3568
From: Ahmad Fatoum @ 2022-01-05 16:08 UTC (permalink / raw)
  To: Frank Wunderlich; +Cc: barebox
In-Reply-To: <trinity-cf421ba4-410b-4e0e-bb2f-60fef962bc97-1641397322119@3c-app-gmx-bs58>

On 05.01.22 16:42, Frank Wunderlich wrote:
>> Gesendet: Mittwoch, 05. Januar 2022 um 13:08 Uhr
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> An: "Frank Wunderlich" <frank-w@public-files.de>, barebox@lists.infradead.org
>> Betreff: Re: barebox - rk3568
> 
>>> https://www.barebox.org/doc/latest/boards/rockchip.html#rockchip-rk3568
>>>
>>> says it starts from sector 32, my first block for uboot (idblock.bin=spl+raminit) starts at 64, full uboot in partition at 8M.
>>
>> You probably figured it out by now, but the discrepancy is likely due to differing
>> block sizes. barebox documentation has `dd bs=1024`, while the default is 512.
>> boot firmware is a single chunk with barebox, so no separate second stage bootloader
>> partition.
> 
> thanks, this indeed explain the different offset, which is same after resolving the double blocksize
> 
>>> As it differs a bit from evb can i add new dts like in uboot?
>>
>> You can check out the commit adding the EVB or the Pine64 Quartz64:
>> 8d14f8e898b4 ("arm: rockchip: add support for the quartz64 board")
>>
>> Feel free to send patches for your board along the same lines. :)
> 
> currently evb is working well, so i try to add bootscripts and maybe an menu (if i found out how) :)

It's not EVB anymore though, if you replace the device tree and skip the board code. ;)

> 
> regards Frank
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply


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.