* Re: [PATCH 2/2] watchdog: Add new arm_smc_wdt watchdog driver
From: Guenter Roeck @ 2020-02-20 15:52 UTC (permalink / raw)
To: Evan Benn
Cc: Rob Herring, Wim Van Sebroeck, linux-watchdog, Anson Huang,
Dinh Nguyen, Catalin Marinas, LKML, Shawn Guo, Bjorn Andersson,
Marcin Juszkiewicz, Olof Johansson, Clément Péron,
Greg Kroah-Hartman, Jonathan Cameron, Mauro Carvalho Chehab,
Julius Werner, Leonard Crestez, Will Deacon, David S. Miller,
linux-arm-kernel
In-Reply-To: <CAKz_xw0fHgVBLdEoEoQ7OSAgBcvYBAowV0obWLsDUGNPotP55Q@mail.gmail.com>
On Thu, Feb 20, 2020 at 05:50:03PM +1100, Evan Benn wrote:
> > > + if ((int)res->a0 == PSCI_RET_NOT_SUPPORTED)
> > > + return -ENOTSUPP;
> >
> > -ENODEV would be better here.
> >
> > > + if ((int)res->a0 == PSCI_RET_INVALID_PARAMS)
> > > + return -EINVAL;
> > > + if ((int)res->a0 < 0)
> > > + return -EIO;
>
> In fixing this I found drivers/firmware/psci/psci.c:145
> Which also translates psci codes to errno codes, but uses EOPNOTSUPP:
>
> switch (errno) {
> case PSCI_RET_SUCCESS:
> return 0;
> case PSCI_RET_NOT_SUPPORTED:
> return -EOPNOTSUPP;
> case PSCI_RET_INVALID_PARAMS:
> case PSCI_RET_INVALID_ADDRESS:
> return -EINVAL;
> case PSCI_RET_DENIED:
> return -EPERM;
> };
>
> return -EINVAL;
>
> Are these more appropriate?
>
It is customary for driver probe functions to return -ENODEV if the
device is not supported. I don't see a reason why this driver should
behave any different. "but this other driver does it" is never a good
argument.
Having said that, yet, with the exception of -EOPNOTSUPP the other
return values would be more appropriate.
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/2] watchdog: Add new arm_smc_wdt watchdog driver
From: Guenter Roeck @ 2020-02-20 15:52 UTC (permalink / raw)
To: Evan Benn
Cc: LKML, Julius Werner, Bjorn Andersson, Mauro Carvalho Chehab,
Marcin Juszkiewicz, Catalin Marinas, Olof Johansson,
Leonard Crestez, Jonathan Cameron, Dinh Nguyen, linux-arm-kernel,
Greg Kroah-Hartman, Will Deacon, linux-watchdog, Rob Herring,
Wim Van Sebroeck, Clément Péron, Shawn Guo,
David S. Miller, Anson Huang
In-Reply-To: <CAKz_xw0fHgVBLdEoEoQ7OSAgBcvYBAowV0obWLsDUGNPotP55Q@mail.gmail.com>
On Thu, Feb 20, 2020 at 05:50:03PM +1100, Evan Benn wrote:
> > > + if ((int)res->a0 == PSCI_RET_NOT_SUPPORTED)
> > > + return -ENOTSUPP;
> >
> > -ENODEV would be better here.
> >
> > > + if ((int)res->a0 == PSCI_RET_INVALID_PARAMS)
> > > + return -EINVAL;
> > > + if ((int)res->a0 < 0)
> > > + return -EIO;
>
> In fixing this I found drivers/firmware/psci/psci.c:145
> Which also translates psci codes to errno codes, but uses EOPNOTSUPP:
>
> switch (errno) {
> case PSCI_RET_SUCCESS:
> return 0;
> case PSCI_RET_NOT_SUPPORTED:
> return -EOPNOTSUPP;
> case PSCI_RET_INVALID_PARAMS:
> case PSCI_RET_INVALID_ADDRESS:
> return -EINVAL;
> case PSCI_RET_DENIED:
> return -EPERM;
> };
>
> return -EINVAL;
>
> Are these more appropriate?
>
It is customary for driver probe functions to return -ENODEV if the
device is not supported. I don't see a reason why this driver should
behave any different. "but this other driver does it" is never a good
argument.
Having said that, yet, with the exception of -EOPNOTSUPP the other
return values would be more appropriate.
Guenter
^ permalink raw reply
* Re: [Xen-devel] [PATCH] rwlock: allow recursive read locking when already locked in write mode
From: Jan Beulich @ 2020-02-20 15:50 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Jürgen Groß, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
xen-devel
In-Reply-To: <20200220151734.GM4679@Air-de-Roger>
On 20.02.2020 16:17, Roger Pau Monné wrote:
> On Thu, Feb 20, 2020 at 04:02:55PM +0100, Jan Beulich wrote:
>> On 20.02.2020 15:11, Roger Pau Monné wrote:
>>> On Thu, Feb 20, 2020 at 01:48:54PM +0100, Jan Beulich wrote:
>>>> Another option is to use the recurse_cpu field of the
>>>> associated spin lock: The field is used for recursive locks
>>>> only, and hence the only conflict would be with
>>>> _spin_is_locked(), which we don't (and in the future then
>>>> also shouldn't) use on this lock.
>>>
>>> I looked into that also, but things get more complicated AFAICT, as it's
>>> not possible to atomically fetch the state of the lock and the owner
>>> CPU at the same time. Neither you could set the LOCKED bit and the CPU
>>> at the same time.
>>
>> There's no need to atomically fetch both afaics: The field is
>> valid only if the LOCKED bit is set. And when reading the
>> field you only care about the value being equal to
>> smp_processor_id(), i.e. it is fine to set LOCKED before
>> updating the CPU field on lock, and to reset the CPU field to
>> SPINLOCK_NO_CPU (or whatever it's called) before clearing
>> LOCKED.
>
> Yes that would require the usage of a sentinel value as 0 would be a
> valid processor ID.
>
> I would try to refrain from (abu)sing internal spinlock fields, as I
> think we can solve this just by using the cnts field on the current
> rwlock implementation.
>
> What issue do you have in mind that would prevent storing the CPU
> write locked in the cnts field?
The reduction of the number of bits used for other purposes.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: [PATCH 01/19] vfs: syscall: Add fsinfo() to query filesystem information [ver #16]
From: Darrick J. Wong @ 2020-02-20 15:48 UTC (permalink / raw)
To: David Howells
Cc: viro, raven, mszeredi, christian, linux-api, linux-fsdevel,
linux-kernel
In-Reply-To: <542411.1582194875@warthog.procyon.org.uk>
On Thu, Feb 20, 2020 at 10:34:35AM +0000, David Howells wrote:
> Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> > > + p->f_blocks.hi = 0;
> > > + p->f_blocks.lo = buf.f_blocks;
> >
> > Er... are there filesystems (besides that (xfs++)++ one) that require
> > u128 counters? I suspect that the Very Large Fields are for future
> > expandability, but I also wonder about the whether it's worth the
> > complexity of doing this, since the structures can always be
> > version-revved later.
>
> I'm making a relatively cheap allowance for future expansion. Dave Chinner
> has mentioned at one of the LSFs that 16EiB may be exceeded soon (though I
> hate to think of fscking such a beastie). I know that the YFS variant of AFS
> supports 96-bit vnode numbers (which I translate to inode numbers). What I'm
Aha, you /already/ have a use for larger-than-64bit numbers. Got it. :)
> trying to avoid is the problem we have with stat/statfs where under some
> circumstances we have to return an error (ERANGE?) because we can't represent
> the number if someone asks for an older version of the struct.
>
> Since the buffer is (meant to be) pre-cleared, the filesystem can just ignore
> the high word if it's never going to set it. In fact, fsinfo_generic_statfs
> doesn't need to set them either.
>
> > XFS inodes are u64 values...
> > ...and the max symlink target length is 1k, not PAGE_SIZE...
>
> Yeah, and AFS(YFS) has 96-bit inode numbers. The filesystem's fsinfo table is
> read first so that the filesystem can override this.
>
> > ...so is the usage model here that XFS should call fsinfo_generic_limits
> > to fill out the fsinfo_limits structure, modify the values in
> > ctx->buffer as appropriate for XFS, and then return the structure size?
>
> Actually, I should export some these so that you can do that. I'll export
> fsinfo_generic_{timestamp_info,supports,limits} for now.
Thank you.
> > > +#define FSINFO_ATTR_VOLUME_ID 0x05 /* Volume ID (string) */
> > > +#define FSINFO_ATTR_VOLUME_UUID 0x06 /* Volume UUID (LE uuid) */
> > > +#define FSINFO_ATTR_VOLUME_NAME 0x07 /* Volume name (string) */
> >
> > I think I've muttered about the distinction between volume id and
> > volume name before, but I'm still wondering how confusing that will be
> > for users? Let me check my assumptions, though:
> >
> > Volume ID is whatever's in super_block.s_id, which (at least for xfs and
> > ext4) is the device name (e.g. "sda1"). I guess that's useful for
> > correlating a thing you can call fsinfo() on against strings that were
> > logged in dmesg.
> >
> > Volume name I think is the fs label (e.g. "home"), which I think will
> > have to be implemented separately by each filesystem, and that's why
> > there's no generic vfs implementation.
>
> Yes. For AFS, for example, this would be the name of the volume (which may be
> changed), whereas the volume ID is the number in the protocol that actually
> refers to the volume (which cannot be changed).
>
> And, as you say, for blockdev mounts, the ID is the device name and the volume
> name is filesystem specific.
>
> > The 7 -> 0 -> 1 sequence here confused me until I figured out that
> > QUERY_TYPE is the mask for QUERY_{PATH,FD}.
>
> Changed to FSINFO_FLAGS_QUERY_MASK.
>
> > > +struct fsinfo_limits {
> > > +...
> > > + __u32 __reserved[1];
> >
> > I wonder if these structures ought to reserve more space than a single u32...
>
> No need. Part of the way the interface is designed is that the version number
> for a particular VSTRUCT-type attribute is also the length. So a newer
> version is also longer. All the old fields must be retained and filled in.
> New fields are tagged on the end.
>
> If userspace asks for an older version than is supported, it gets a truncated
> return. If it asks for a newer version, the extra fields it is expecting are
> all set to 0. Either way, the length (and thus the version) the kernel
> supports is returned - not the length copied.
>
> The __reserved fields are there because they represent padding (the struct is
> going to be aligned/padded according to __u64 in this case). Ideally, I'd
> mark the structs __packed, but this messes with the alignment and may make the
> compiler do funny tricks to get out any field larger than a byte.
>
> I've renamed them to __padding.
>
> > > +struct fsinfo_supports {
> > > + __u64 stx_attributes; /* What statx::stx_attributes are supported */
> > > + __u32 stx_mask; /* What statx::stx_mask bits are supported */
> > > + __u32 ioc_flags; /* What FS_IOC_* flags are supported */
> >
> > "IOC"? That just means 'ioctl'. Is this field supposed to return the
> > supported FS_IOC_GETFLAGS flags, or the supported FS_IOC_FSGETXATTR
> > flags?
>
> FS_IOC_[GS]ETFLAGS is what I meant.
>
> > I suspect it would also be a big help to be able to tell userspace which
> > of the flags can be set, and which can be cleared.
>
> How about:
>
> __u32 fs_ioc_getflags; /* What FS_IOC_GETFLAGS may return */
> __u32 fs_ioc_setflags_set; /* What FS_IOC_SETFLAGS may set */
> __u32 fs_ioc_setflags_clear; /* What FS_IOC_SETFLAGS may clear */
Looks good to me. At some point it'll be good to add another fsinfo
verb or something to query exactly what parts of struct fsxattr can be
set, cleared, or returned, but that can wait.
> > > +struct fsinfo_timestamp_one {
> > > + __s64 minimum; /* Minimum timestamp value in seconds */
> > > + __u64 maximum; /* Maximum timestamp value in seconds */
> >
> > Given that time64_t is s64, why is the maximum here u64?
>
> Well, I assume it extremely unlikely that the maximum can be before 1970, so
> there doesn't seem any need to allow the maximum to be negative. Furthermore,
> it would be feasible that you could encounter a filesystem with a u64
> filesystem that doesn't support dates before 1970.
>
> On the other hand, if Linux is still going when __s64 seconds from 1970 wraps,
> it will be impressive, but I'm not sure we'll be around to see it... Someone
> will have to cast a resurrection spell on Arnd to fix that one.
I fear we /all/ will be around, having forcibly been uploaded to The
Cloud because nobody else knows how to maintain the Linux kernel, and we
can't have the drinks dispenser on B deck going bonkers twice per stardate.
(j/k)
> However, since signed/unsigned comparisons may have issues, I can turn it into
> an __s64 also if that is preferred.
It /would/ be more consistent with the rest of the kernel, and since the
kernel & userspace don't support timestamps beyond 8Es, there's no point
in advertising a maximum beyond that.
--D
> David
>
^ permalink raw reply
* Re: Filesystem corruption after unreachable storage
From: Theodore Y. Ts'o @ 2020-02-20 15:50 UTC (permalink / raw)
To: Jean-Louis Dupond; +Cc: linux-ext4
In-Reply-To: <3a7bc899-31d9-51f2-1ea9-b3bef2a98913@dupond.be>
On Thu, Feb 20, 2020 at 10:08:44AM +0100, Jean-Louis Dupond wrote:
> dumpe2fs -> see attachment
Looking at the dumpe2fs output, it's interesting that it was "clean
with errors", without any error information being logged in the
superblock. What version of the kernel are you using? I'm guessing
it's a fairly old one?
> Fsck:
> # e2fsck -fy /dev/mapper/vg01-root
> e2fsck 1.44.5 (15-Dec-2018)
And that's a old version of e2fsck as well. Is this some kind of
stable/enterprise linux distro?
> Pass 1: Checking inodes, blocks, and sizes
> Inodes that were part of a corrupted orphan linked list found. Fix? yes
>
> Inode 165708 was part of the orphaned inode list. FIXED.
OK, this and the rest looks like it's relating to a file truncation or
deletion at the time of the disconnection.
> > > On KVM for example there is a unlimited timeout (afaik) until the
> > > storage is
> > > back, and the VM just continues running after storage recovery.
> > Well, you can adjust the SCSI timeout, if you want to give that a try....
> It has some other disadvantages? Or is it quite safe to increment the SCSI
> timeout?
It should be pretty safe.
Can you reliably reproduce the problem by disconnecting the machine
from the SAN?
- Ted
^ permalink raw reply
* Re: [cip-dev] [PATCH 4.19.y-cip 17/23] usb: typec: add dependency for TYPEC_HD3SS3220
From: Pavel Machek @ 2020-02-20 15:50 UTC (permalink / raw)
To: Marian-Cristian Rotariu; +Cc: cip-dev@lists.cip-project.org
In-Reply-To: <OSAPR01MB5028AA819CCF2EF35A68E18AAE130@OSAPR01MB5028.jpnprd01.prod.outlook.com>
[-- Attachment #1.1: Type: text/plain, Size: 1944 bytes --]
Hi!
> > On Tue 2020-02-18 14:05:14, Marian-Cristian Rotariu wrote:
> > > From: Mao Wenan <maowenan@huawei.com>
> > >
> > > commit da4b5d18dd949abdda7c8ea76c9483b5edd49616 upstream.
> > >
> > > If CONFIG_TYPEC_HD3SS3220=y, CONFIG_USB_ROLE_SWITCH=m, below
> > errors
> > > can be found:
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_remove':
> > > hd3ss3220.c:(.text+0x64): undefined reference to `usb_role_switch_put'
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_dr_set':
> > > hd3ss3220.c:(.text+0x154): undefined reference to
> > `usb_role_switch_set_role'
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_set_role':
> > > hd3ss3220.c:(.text+0x294): undefined reference to
> > `usb_role_switch_set_role'
> > > hd3ss3220.c:(.text+0x2f4): undefined reference to
> > `usb_role_switch_set_role'
> > > hd3ss3220.c:(.text+0x348): undefined reference to
> > `usb_role_switch_set_role'
> > > hd3ss3220.c:(.text+0x390): undefined reference to
> > `usb_role_switch_set_role'
> > > drivers/usb/typec/hd3ss3220.o: In function `hd3ss3220_probe':
> > > hd3ss3220.c:(.text+0x5e8): undefined reference to
> > `fwnode_usb_role_switch_get'
> > > hd3ss3220.c:(.text+0x8a4): undefined reference to `usb_role_switch_put'
> > > make: *** [vmlinux] Error 1
> >
> > I guess it is okay here, but for more serious bugs it would be nice to merge
> > the buggy patch with a fix -- not to break bisect.
>
> I kind of forgot about bisect. I was mostly focused on keeping the original patches
> from upstream (in this case, it is more about the commit message as the fix is trivial).
> I will send this patch in v2 also, since you mostly agreed with it. Please let me know
> if otherwise.
This patch is okay.
Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
cip-dev mailing list
cip-dev@lists.cip-project.org
https://lists.cip-project.org/mailman/listinfo/cip-dev
^ permalink raw reply
* Re: [igt-dev] [PATCH i-g-t] intel-ci: add a pre-merge blacklist to reduce the testing queue
From: Chris Wilson @ 2020-02-20 15:50 UTC (permalink / raw)
To: Martin Peres, igt-dev
In-Reply-To: <20200220153209.210767-1-martin.peres@linux.intel.com>
Quoting Martin Peres (2020-02-20 15:32:09)
> +###############################################################################
> +# This tests modesetting-vs-wedged which is a useful thing to check, but it
> +# seems like it mostly catches unrelated issues which are better caught by
> +# other tests.
> +#
> +# - shard-skl: 0.5% (~1 minute)
> +# - shard-kbl: 1% (~1 minute)
> +# - shard-apl: 0.6% (~1 minute)
> +# - shard-glk: 0.5% (~1 minute)
> +# - shard-icl: 0.6% (~1.5 minutes)
> +# - shard-tgl: 0.7% (~1.5 minutes)
> +#
> +# Data acquired on 2020-02-20 by Martin Peres
> +###############################################################################
> +igt@gem_eio@kms
Hah. It caught novel issues, which have not been split out into separate
regression tests.
Making sure kms is robust is not a bad thing; you can easily speed this
up by reducing the workload.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply
* Re: [PATCH 1/8] btrfs: make the extent buffer leak check per fs info
From: Josef Bacik @ 2020-02-20 15:49 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs, kernel-team
In-Reply-To: <78ac92dc-1e57-1d74-0b17-48832a36c649@suse.com>
On 2/19/20 8:59 AM, Nikolay Borisov wrote:
>
>
> On 14.02.20 г. 23:11 ч., Josef Bacik wrote:
>> I'm going to make the entire destruction of btrfs_root's controlled by
>> their refcount, so it will be helpful to notice if we're leaking their
>> eb's on umount.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>
> Overall looks good, one minor nit below though:
>
>> ---
>> fs/btrfs/ctree.h | 3 +++
>> fs/btrfs/disk-io.c | 3 +++
>> fs/btrfs/extent_io.c | 45 ++++++++++++++++++++++----------------------
>> fs/btrfs/extent_io.h | 7 +++++++
>> 4 files changed, 36 insertions(+), 22 deletions(-)
>>
>
> <snip>
>
>> @@ -35,42 +35,45 @@ static inline bool extent_state_in_tree(const struct extent_state *state)
>> }
>>
>> #ifdef CONFIG_BTRFS_DEBUG
>> -static LIST_HEAD(buffers);
>> static LIST_HEAD(states);
>> -
>> static DEFINE_SPINLOCK(leak_lock);
>
> This lock should be renamed to extent_state_leak_lock, otherwise it's
> not clear what it protects. I had to through its usage to figure it out.
> To take this further, why don't you make it a a per-fs_info lock as
> well? Extent states are per fs themselves.
>
Yeah I can follow up once this set is merged. Thanks,
Josef
^ permalink raw reply
* Re: [PATCH] selftest/lkdtm: Don't pollute 'git status'
From: Shuah Khan @ 2020-02-20 15:49 UTC (permalink / raw)
To: Christophe Leroy, Kees Cook
Cc: linux-mm, linuxppc-dev, linux-kernel, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK
In-Reply-To: <ca71e490-b4fa-bf53-7a60-b6954b9dd33e@c-s.fr>
On 2/20/20 7:58 AM, Christophe Leroy wrote:
> ping
>
> On 02/06/2020 08:11 AM, Christophe Leroy wrote:
>> Commit 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
>> added generation of lkdtm test scripts.
>>
>> Ignore those generated scripts when performing 'git status'
>>
>> Fixes: 46d1a0f03d66 ("selftests/lkdtm: Add tests for LKDTM targets")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> Without this, 'git status' now reports the following crap and real
> problems are drowned in the middle, that's annoying.
>
I will pull this in. Please cc linux-kselftest mailing list in the
future.
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH v7 22/24] iomap: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-xfs, linux-kernel, linux-f2fs-devel, cluster-devel,
linux-mm, ocfs2-devel, linux-fsdevel, linux-ext4, linux-erofs,
linux-btrfs
In-Reply-To: <20200219210103.32400-23-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:01PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Use the new readahead operation in iomap. Convert XFS and ZoneFS to
> use it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/iomap/buffered-io.c | 90 +++++++++++++++---------------------------
> fs/iomap/trace.h | 2 +-
> fs/xfs/xfs_aops.c | 13 +++---
> fs/zonefs/super.c | 7 ++--
> include/linux/iomap.h | 3 +-
> 5 files changed, 41 insertions(+), 74 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 31899e6cb0f8..66cf453f4bb7 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -214,9 +214,8 @@ iomap_read_end_io(struct bio *bio)
> struct iomap_readpage_ctx {
> struct page *cur_page;
> bool cur_page_in_bio;
> - bool is_readahead;
> struct bio *bio;
> - struct list_head *pages;
> + struct readahead_control *rac;
> };
>
> static void
> @@ -307,11 +306,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> if (ctx->bio)
> submit_bio(ctx->bio);
>
> - if (ctx->is_readahead) /* same as readahead_gfp_mask */
> + if (ctx->rac) /* same as readahead_gfp_mask */
> gfp |= __GFP_NORETRY | __GFP_NOWARN;
> ctx->bio = bio_alloc(gfp, min(BIO_MAX_PAGES, nr_vecs));
> ctx->bio->bi_opf = REQ_OP_READ;
> - if (ctx->is_readahead)
> + if (ctx->rac)
> ctx->bio->bi_opf |= REQ_RAHEAD;
> ctx->bio->bi_iter.bi_sector = sector;
> bio_set_dev(ctx->bio, iomap->bdev);
> @@ -367,36 +366,8 @@ iomap_readpage(struct page *page, const struct iomap_ops *ops)
> }
> EXPORT_SYMBOL_GPL(iomap_readpage);
>
> -static struct page *
> -iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos,
> - loff_t length, loff_t *done)
> -{
> - while (!list_empty(pages)) {
> - struct page *page = lru_to_page(pages);
> -
> - if (page_offset(page) >= (u64)pos + length)
> - break;
> -
> - list_del(&page->lru);
> - if (!add_to_page_cache_lru(page, inode->i_mapping, page->index,
> - GFP_NOFS))
> - return page;
> -
> - /*
> - * If we already have a page in the page cache at index we are
> - * done. Upper layers don't care if it is uptodate after the
> - * readpages call itself as every page gets checked again once
> - * actually needed.
> - */
> - *done += PAGE_SIZE;
> - put_page(page);
> - }
> -
> - return NULL;
> -}
> -
> static loff_t
> -iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> +iomap_readahead_actor(struct inode *inode, loff_t pos, loff_t length,
> void *data, struct iomap *iomap, struct iomap *srcmap)
> {
> struct iomap_readpage_ctx *ctx = data;
> @@ -404,10 +375,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
>
> while (done < length) {
> if (!ctx->cur_page) {
> - ctx->cur_page = iomap_next_page(inode, ctx->pages,
> - pos, length, &done);
> - if (!ctx->cur_page)
> - break;
> + ctx->cur_page = readahead_page(ctx->rac);
> ctx->cur_page_in_bio = false;
> }
> ret = iomap_readpage_actor(inode, pos + done, length - done,
> @@ -431,44 +399,48 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> return done;
> }
>
> -int
> -iomap_readpages(struct address_space *mapping, struct list_head *pages,
> - unsigned nr_pages, const struct iomap_ops *ops)
> +/**
> + * iomap_readahead - Attempt to read pages from a file.
> + * @rac: Describes the pages to be read.
> + * @ops: The operations vector for the filesystem.
> + *
> + * This function is for filesystems to call to implement their readahead
> + * address_space operation.
> + *
> + * Context: The file is pinned by the caller, and the pages to be read are
> + * all locked and have an elevated refcount. This function will unlock
> + * the pages (once I/O has completed on them, or I/O has been determined to
> + * not be necessary). It will also decrease the refcount once the pages
> + * have been submitted for I/O. After this point, the page may be removed
> + * from the page cache, and should not be referenced.
> + */
Isn't the context documentation something that belongs into the aop
documentation? I've never really seen the value of duplicating this
information in method instances, as it is just bound to be out of date
rather sooner than later.
^ permalink raw reply
* Re: [PATCH v7 22/24] iomap: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-fsdevel, linux-mm, linux-kernel, linux-btrfs, linux-erofs,
linux-ext4, linux-f2fs-devel, cluster-devel, ocfs2-devel,
linux-xfs
In-Reply-To: <20200219210103.32400-23-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:01PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Use the new readahead operation in iomap. Convert XFS and ZoneFS to
> use it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/iomap/buffered-io.c | 90 +++++++++++++++---------------------------
> fs/iomap/trace.h | 2 +-
> fs/xfs/xfs_aops.c | 13 +++---
> fs/zonefs/super.c | 7 ++--
> include/linux/iomap.h | 3 +-
> 5 files changed, 41 insertions(+), 74 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 31899e6cb0f8..66cf453f4bb7 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -214,9 +214,8 @@ iomap_read_end_io(struct bio *bio)
> struct iomap_readpage_ctx {
> struct page *cur_page;
> bool cur_page_in_bio;
> - bool is_readahead;
> struct bio *bio;
> - struct list_head *pages;
> + struct readahead_control *rac;
> };
>
> static void
> @@ -307,11 +306,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> if (ctx->bio)
> submit_bio(ctx->bio);
>
> - if (ctx->is_readahead) /* same as readahead_gfp_mask */
> + if (ctx->rac) /* same as readahead_gfp_mask */
> gfp |= __GFP_NORETRY | __GFP_NOWARN;
> ctx->bio = bio_alloc(gfp, min(BIO_MAX_PAGES, nr_vecs));
> ctx->bio->bi_opf = REQ_OP_READ;
> - if (ctx->is_readahead)
> + if (ctx->rac)
> ctx->bio->bi_opf |= REQ_RAHEAD;
> ctx->bio->bi_iter.bi_sector = sector;
> bio_set_dev(ctx->bio, iomap->bdev);
> @@ -367,36 +366,8 @@ iomap_readpage(struct page *page, const struct iomap_ops *ops)
> }
> EXPORT_SYMBOL_GPL(iomap_readpage);
>
> -static struct page *
> -iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos,
> - loff_t length, loff_t *done)
> -{
> - while (!list_empty(pages)) {
> - struct page *page = lru_to_page(pages);
> -
> - if (page_offset(page) >= (u64)pos + length)
> - break;
> -
> - list_del(&page->lru);
> - if (!add_to_page_cache_lru(page, inode->i_mapping, page->index,
> - GFP_NOFS))
> - return page;
> -
> - /*
> - * If we already have a page in the page cache at index we are
> - * done. Upper layers don't care if it is uptodate after the
> - * readpages call itself as every page gets checked again once
> - * actually needed.
> - */
> - *done += PAGE_SIZE;
> - put_page(page);
> - }
> -
> - return NULL;
> -}
> -
> static loff_t
> -iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> +iomap_readahead_actor(struct inode *inode, loff_t pos, loff_t length,
> void *data, struct iomap *iomap, struct iomap *srcmap)
> {
> struct iomap_readpage_ctx *ctx = data;
> @@ -404,10 +375,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
>
> while (done < length) {
> if (!ctx->cur_page) {
> - ctx->cur_page = iomap_next_page(inode, ctx->pages,
> - pos, length, &done);
> - if (!ctx->cur_page)
> - break;
> + ctx->cur_page = readahead_page(ctx->rac);
> ctx->cur_page_in_bio = false;
> }
> ret = iomap_readpage_actor(inode, pos + done, length - done,
> @@ -431,44 +399,48 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> return done;
> }
>
> -int
> -iomap_readpages(struct address_space *mapping, struct list_head *pages,
> - unsigned nr_pages, const struct iomap_ops *ops)
> +/**
> + * iomap_readahead - Attempt to read pages from a file.
> + * @rac: Describes the pages to be read.
> + * @ops: The operations vector for the filesystem.
> + *
> + * This function is for filesystems to call to implement their readahead
> + * address_space operation.
> + *
> + * Context: The file is pinned by the caller, and the pages to be read are
> + * all locked and have an elevated refcount. This function will unlock
> + * the pages (once I/O has completed on them, or I/O has been determined to
> + * not be necessary). It will also decrease the refcount once the pages
> + * have been submitted for I/O. After this point, the page may be removed
> + * from the page cache, and should not be referenced.
> + */
Isn't the context documentation something that belongs into the aop
documentation? I've never really seen the value of duplicating this
information in method instances, as it is just bound to be out of date
rather sooner than later.
^ permalink raw reply
* [Ocfs2-devel] [PATCH v7 22/24] iomap: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:49 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-fsdevel, linux-mm, linux-kernel, linux-btrfs, linux-erofs,
linux-ext4, linux-f2fs-devel, cluster-devel, ocfs2-devel,
linux-xfs
In-Reply-To: <20200219210103.32400-23-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:01PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Use the new readahead operation in iomap. Convert XFS and ZoneFS to
> use it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/iomap/buffered-io.c | 90 +++++++++++++++---------------------------
> fs/iomap/trace.h | 2 +-
> fs/xfs/xfs_aops.c | 13 +++---
> fs/zonefs/super.c | 7 ++--
> include/linux/iomap.h | 3 +-
> 5 files changed, 41 insertions(+), 74 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 31899e6cb0f8..66cf453f4bb7 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -214,9 +214,8 @@ iomap_read_end_io(struct bio *bio)
> struct iomap_readpage_ctx {
> struct page *cur_page;
> bool cur_page_in_bio;
> - bool is_readahead;
> struct bio *bio;
> - struct list_head *pages;
> + struct readahead_control *rac;
> };
>
> static void
> @@ -307,11 +306,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> if (ctx->bio)
> submit_bio(ctx->bio);
>
> - if (ctx->is_readahead) /* same as readahead_gfp_mask */
> + if (ctx->rac) /* same as readahead_gfp_mask */
> gfp |= __GFP_NORETRY | __GFP_NOWARN;
> ctx->bio = bio_alloc(gfp, min(BIO_MAX_PAGES, nr_vecs));
> ctx->bio->bi_opf = REQ_OP_READ;
> - if (ctx->is_readahead)
> + if (ctx->rac)
> ctx->bio->bi_opf |= REQ_RAHEAD;
> ctx->bio->bi_iter.bi_sector = sector;
> bio_set_dev(ctx->bio, iomap->bdev);
> @@ -367,36 +366,8 @@ iomap_readpage(struct page *page, const struct iomap_ops *ops)
> }
> EXPORT_SYMBOL_GPL(iomap_readpage);
>
> -static struct page *
> -iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos,
> - loff_t length, loff_t *done)
> -{
> - while (!list_empty(pages)) {
> - struct page *page = lru_to_page(pages);
> -
> - if (page_offset(page) >= (u64)pos + length)
> - break;
> -
> - list_del(&page->lru);
> - if (!add_to_page_cache_lru(page, inode->i_mapping, page->index,
> - GFP_NOFS))
> - return page;
> -
> - /*
> - * If we already have a page in the page cache at index we are
> - * done. Upper layers don't care if it is uptodate after the
> - * readpages call itself as every page gets checked again once
> - * actually needed.
> - */
> - *done += PAGE_SIZE;
> - put_page(page);
> - }
> -
> - return NULL;
> -}
> -
> static loff_t
> -iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> +iomap_readahead_actor(struct inode *inode, loff_t pos, loff_t length,
> void *data, struct iomap *iomap, struct iomap *srcmap)
> {
> struct iomap_readpage_ctx *ctx = data;
> @@ -404,10 +375,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
>
> while (done < length) {
> if (!ctx->cur_page) {
> - ctx->cur_page = iomap_next_page(inode, ctx->pages,
> - pos, length, &done);
> - if (!ctx->cur_page)
> - break;
> + ctx->cur_page = readahead_page(ctx->rac);
> ctx->cur_page_in_bio = false;
> }
> ret = iomap_readpage_actor(inode, pos + done, length - done,
> @@ -431,44 +399,48 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> return done;
> }
>
> -int
> -iomap_readpages(struct address_space *mapping, struct list_head *pages,
> - unsigned nr_pages, const struct iomap_ops *ops)
> +/**
> + * iomap_readahead - Attempt to read pages from a file.
> + * @rac: Describes the pages to be read.
> + * @ops: The operations vector for the filesystem.
> + *
> + * This function is for filesystems to call to implement their readahead
> + * address_space operation.
> + *
> + * Context: The file is pinned by the caller, and the pages to be read are
> + * all locked and have an elevated refcount. This function will unlock
> + * the pages (once I/O has completed on them, or I/O has been determined to
> + * not be necessary). It will also decrease the refcount once the pages
> + * have been submitted for I/O. After this point, the page may be removed
> + * from the page cache, and should not be referenced.
> + */
Isn't the context documentation something that belongs into the aop
documentation? I've never really seen the value of duplicating this
information in method instances, as it is just bound to be out of date
rather sooner than later.
^ permalink raw reply
* [Cluster-devel] [PATCH v7 22/24] iomap: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:49 UTC (permalink / raw)
To: cluster-devel.redhat.com
In-Reply-To: <20200219210103.32400-23-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:01PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> Use the new readahead operation in iomap. Convert XFS and ZoneFS to
> use it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
> fs/iomap/buffered-io.c | 90 +++++++++++++++---------------------------
> fs/iomap/trace.h | 2 +-
> fs/xfs/xfs_aops.c | 13 +++---
> fs/zonefs/super.c | 7 ++--
> include/linux/iomap.h | 3 +-
> 5 files changed, 41 insertions(+), 74 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 31899e6cb0f8..66cf453f4bb7 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -214,9 +214,8 @@ iomap_read_end_io(struct bio *bio)
> struct iomap_readpage_ctx {
> struct page *cur_page;
> bool cur_page_in_bio;
> - bool is_readahead;
> struct bio *bio;
> - struct list_head *pages;
> + struct readahead_control *rac;
> };
>
> static void
> @@ -307,11 +306,11 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> if (ctx->bio)
> submit_bio(ctx->bio);
>
> - if (ctx->is_readahead) /* same as readahead_gfp_mask */
> + if (ctx->rac) /* same as readahead_gfp_mask */
> gfp |= __GFP_NORETRY | __GFP_NOWARN;
> ctx->bio = bio_alloc(gfp, min(BIO_MAX_PAGES, nr_vecs));
> ctx->bio->bi_opf = REQ_OP_READ;
> - if (ctx->is_readahead)
> + if (ctx->rac)
> ctx->bio->bi_opf |= REQ_RAHEAD;
> ctx->bio->bi_iter.bi_sector = sector;
> bio_set_dev(ctx->bio, iomap->bdev);
> @@ -367,36 +366,8 @@ iomap_readpage(struct page *page, const struct iomap_ops *ops)
> }
> EXPORT_SYMBOL_GPL(iomap_readpage);
>
> -static struct page *
> -iomap_next_page(struct inode *inode, struct list_head *pages, loff_t pos,
> - loff_t length, loff_t *done)
> -{
> - while (!list_empty(pages)) {
> - struct page *page = lru_to_page(pages);
> -
> - if (page_offset(page) >= (u64)pos + length)
> - break;
> -
> - list_del(&page->lru);
> - if (!add_to_page_cache_lru(page, inode->i_mapping, page->index,
> - GFP_NOFS))
> - return page;
> -
> - /*
> - * If we already have a page in the page cache at index we are
> - * done. Upper layers don't care if it is uptodate after the
> - * readpages call itself as every page gets checked again once
> - * actually needed.
> - */
> - *done += PAGE_SIZE;
> - put_page(page);
> - }
> -
> - return NULL;
> -}
> -
> static loff_t
> -iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> +iomap_readahead_actor(struct inode *inode, loff_t pos, loff_t length,
> void *data, struct iomap *iomap, struct iomap *srcmap)
> {
> struct iomap_readpage_ctx *ctx = data;
> @@ -404,10 +375,7 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
>
> while (done < length) {
> if (!ctx->cur_page) {
> - ctx->cur_page = iomap_next_page(inode, ctx->pages,
> - pos, length, &done);
> - if (!ctx->cur_page)
> - break;
> + ctx->cur_page = readahead_page(ctx->rac);
> ctx->cur_page_in_bio = false;
> }
> ret = iomap_readpage_actor(inode, pos + done, length - done,
> @@ -431,44 +399,48 @@ iomap_readpages_actor(struct inode *inode, loff_t pos, loff_t length,
> return done;
> }
>
> -int
> -iomap_readpages(struct address_space *mapping, struct list_head *pages,
> - unsigned nr_pages, const struct iomap_ops *ops)
> +/**
> + * iomap_readahead - Attempt to read pages from a file.
> + * @rac: Describes the pages to be read.
> + * @ops: The operations vector for the filesystem.
> + *
> + * This function is for filesystems to call to implement their readahead
> + * address_space operation.
> + *
> + * Context: The file is pinned by the caller, and the pages to be read are
> + * all locked and have an elevated refcount. This function will unlock
> + * the pages (once I/O has completed on them, or I/O has been determined to
> + * not be necessary). It will also decrease the refcount once the pages
> + * have been submitted for I/O. After this point, the page may be removed
> + * from the page cache, and should not be referenced.
> + */
Isn't the context documentation something that belongs into the aop
documentation? I've never really seen the value of duplicating this
information in method instances, as it is just bound to be out of date
rather sooner than later.
^ permalink raw reply
* Re: A problem with live migration of UEFI virtual machines
From: Dr. David Alan Gilbert @ 2020-02-20 15:47 UTC (permalink / raw)
To: wuchenye1995; +Cc: devel@edk2.groups.io, edk2-devel, zhoujianjay, qemu-devel
In-Reply-To: <tencent_3CD8845EC159F0161725898B@qq.com>
* wuchenye1995 (wuchenye1995@gmail.com) wrote:
> We found a problem with live migration of UEFI virtual machines due to size of OVMF.fd changes.</div><div class=" selfdiv" style="height: 79.6875px; width: auto !important;"
> Specifically, the size of OVMF.fd in edk with low version such as edk-2.0-25 is <b>2MB</b> while the size of it in higher version such as edk-2.0-30 is <b>4MB</b>.
> When we migrate a UEFI virtual machine from the host with low version of edk2 to the host with higher one, qemu component will report an error in function
> qemu_ram_resize while
>checking size of ovmf_pcbios: Length mismatch: pc.bios: 0x200000 in != 0x400000: Invalid argument.
>We want to know how to solve this problem after updating the version of edk2.
When you migrate, you must migrate between identical configurations; so
you need ROM images (including edk2) that are the same size.
There's two answers;
a) Stick with the same version of the ROM between VMs you want to
migrate
b) Pad your ROM images to some larger size (e.g. 8MB) so that
even if they grow a little bigger then you don't hit the problem.
Dave
P.S. Please use plain text email
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* Re: [dpdk-dev] [PATCH v2] examples/fips_validation: fix incorrect string for CT length
From: David Marchand @ 2020-02-20 15:48 UTC (permalink / raw)
To: Anoob Joseph
Cc: Marko Kovacevic, Fan Zhang, Narayana Prasad, Akhil Goyal,
dev@dpdk.org
In-Reply-To: <VE1PR04MB6639BCF806B4FB4559B5C894E6130@VE1PR04MB6639.eurprd04.prod.outlook.com>
On Thu, Feb 20, 2020 at 11:15 AM Akhil Goyal <akhil.goyal@nxp.com> wrote:
> > The NIST test vectors use the string 'PTlen' to denote text lengths
> > in case of encrypt & decrypt operations. So the same string need to be
> > used while parsing PT and CT.
> >
> > Fixes: 2adb3b4e7e54 ("examples/fips_validation: fix AES-GCM cipher length
> > parsing")
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > ---
> >
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
Applied, thanks.
--
David Marchand
^ permalink raw reply
* Re: [dpdk-dev] [PATCH] net/mlx5: fix metadata split with encap action
From: Raslan Darawsheh @ 2020-02-20 15:48 UTC (permalink / raw)
To: Matan Azrad, dev@dpdk.org; +Cc: Slava Ovsiienko, stable@dpdk.org
In-Reply-To: <1582209810-20546-1-git-send-email-matan@mellanox.com>
Hi,
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Matan Azrad
> Sent: Thursday, February 20, 2020 4:44 PM
> To: dev@dpdk.org
> Cc: Slava Ovsiienko <viacheslavo@mellanox.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix metadata split with encap action
>
> In order to move the mbuf metadata from the WQE to the FDB steering
> domain, the PMD add for each NIC TX flow a new action to copy the
> metadata register REG_A to REG_C_0.
>
> This copy action is considered as modify header action from HW
> perspective.
>
> The HW doesn't support to do modify header action after ant
> encapsulation action.
>
> The split metadata function wrongly added the copy action in the end of
> the original actions list, hence, NIC egress flow with encapapsulation
> action failed when the PMD worked with dv_xmeta_en mode.
>
> Move the copy action to be before and back to back with the
> encapsulation action for the aforementioned case.
>
> Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")
> Cc: stable@dpdk.org
>
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_flow.c | 66
> ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 51 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index fa58546..60aab16 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2744,7 +2744,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> }
>
> /**
> - * Get QUEUE/RSS action from the action list.
> + * Get metadata split action information.
> *
> * @param[in] actions
> * Pointer to the list of actions.
> @@ -2753,18 +2753,38 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> * @param[out] qrss_type
> * Pointer to the action type to return. RTE_FLOW_ACTION_TYPE_END is
> returned
> * if no QUEUE/RSS is found.
> + * @param[out] encap_idx
> + * Pointer to the index of the encap action if exists, otherwise the last
> + * action index.
> *
> * @return
> * Total number of actions.
> */
> static int
> -flow_parse_qrss_action(const struct rte_flow_action actions[],
> - const struct rte_flow_action **qrss)
> +flow_parse_metadata_split_actions_info(const struct rte_flow_action
> actions[],
> + const struct rte_flow_action **qrss,
> + int *encap_idx)
> {
> + const struct rte_flow_action_raw_encap *raw_encap;
> int actions_n = 0;
> + int raw_decap_idx = -1;
>
> + *encap_idx = -1;
> for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> switch (actions->type) {
> + case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
> + case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
> + *encap_idx = actions_n;
> + break;
> + case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
> + raw_decap_idx = actions_n;
> + break;
> + case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> + raw_encap = actions->conf;
> + if (raw_encap->size >
> MLX5_ENCAPSULATION_DECISION_SIZE)
> + *encap_idx = raw_decap_idx != -1 ?
> + raw_decap_idx :
> actions_n;
> + break;
> case RTE_FLOW_ACTION_TYPE_QUEUE:
> case RTE_FLOW_ACTION_TYPE_RSS:
> *qrss = actions;
> @@ -2774,6 +2794,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> }
> actions_n++;
> }
> + if (*encap_idx == -1)
> + *encap_idx = actions_n;
> /* Count RTE_FLOW_ACTION_TYPE_END. */
> return actions_n + 1;
> }
> @@ -3739,6 +3761,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> * Number of actions in the list.
> * @param[out] error
> * Perform verbose error reporting if not NULL.
> + * @param[in] encap_idx
> + * The encap action inndex.
> *
> * @return
> * 0 on success, negative value otherwise
> @@ -3747,7 +3771,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> flow_mreg_tx_copy_prep(struct rte_eth_dev *dev,
> struct rte_flow_action *ext_actions,
> const struct rte_flow_action *actions,
> - int actions_n, struct rte_flow_error *error)
> + int actions_n, struct rte_flow_error *error,
> + int encap_idx)
> {
> struct mlx5_flow_action_copy_mreg *cp_mreg =
> (struct mlx5_flow_action_copy_mreg *)
> @@ -3762,15 +3787,24 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> if (ret < 0)
> return ret;
> cp_mreg->src = ret;
> - memcpy(ext_actions, actions,
> - sizeof(*ext_actions) * actions_n);
> - ext_actions[actions_n - 1] = (struct rte_flow_action){
> - .type = MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
> - .conf = cp_mreg,
> - };
> - ext_actions[actions_n] = (struct rte_flow_action){
> - .type = RTE_FLOW_ACTION_TYPE_END,
> - };
> + if (encap_idx != 0)
> + memcpy(ext_actions, actions, sizeof(*ext_actions) *
> encap_idx);
> + if (encap_idx == actions_n - 1) {
> + ext_actions[actions_n - 1] = (struct rte_flow_action){
> + .type =
> MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
> + .conf = cp_mreg,
> + };
> + ext_actions[actions_n] = (struct rte_flow_action){
> + .type = RTE_FLOW_ACTION_TYPE_END,
> + };
> + } else {
> + ext_actions[encap_idx] = (struct rte_flow_action){
> + .type =
> MLX5_RTE_FLOW_ACTION_TYPE_COPY_MREG,
> + .conf = cp_mreg,
> + };
> + memcpy(ext_actions + encap_idx + 1, actions + encap_idx,
> + sizeof(*ext_actions) * (actions_n -
> encap_idx));
> + }
> return 0;
> }
>
> @@ -3821,6 +3855,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> int mtr_sfx = 0;
> size_t act_size;
> int actions_n;
> + int encap_idx;
> int ret;
>
> /* Check whether extensive metadata feature is engaged. */
> @@ -3830,7 +3865,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> return flow_create_split_inner(dev, flow, NULL,
> prefix_layers,
> attr, items, actions, external,
> error);
> - actions_n = flow_parse_qrss_action(actions, &qrss);
> + actions_n = flow_parse_metadata_split_actions_info(actions, &qrss,
> + &encap_idx);
> if (qrss) {
> /* Exclude hairpin flows from splitting. */
> if (qrss->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
> @@ -3905,7 +3941,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
> "metadata flow");
> /* Create the action list appended with copy register. */
> ret = flow_mreg_tx_copy_prep(dev, ext_actions, actions,
> - actions_n, error);
> + actions_n, error, encap_idx);
> if (ret < 0)
> goto exit;
> }
> --
> 1.8.3.1
Fixed typo in commit log,
And patch applied to next-net-mlx,
Kindest regards
Raslan Darawsheh
^ permalink raw reply
* Re: [PATCH 3/8] btrfs: move the root freeing stuff into btrfs_put_root
From: Josef Bacik @ 2020-02-20 15:48 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs, kernel-team
In-Reply-To: <058f94f8-7fb6-9dfb-61e0-21dc989e22bc@suse.com>
On 2/19/20 10:10 AM, Nikolay Borisov wrote:
>
>
> On 14.02.20 г. 23:11 ч., Josef Bacik wrote:
>> There are a few different ways to free roots, either you allocated them
>> yourself and you just do
>>
>> free_extent_buffer(root->node);
>> free_extent_buffer(root->commit_node);
>> btrfs_put_root(root);
>>
>> Which is the pattern for log roots. Or for snapshots/subvolumes that
>> are being dropped you simply call btrfs_free_fs_root() which does all
>> the cleanup for you.
>>
>> Unify this all into btrfs_put_root(), so that we don't free up things
>> associated with the root until the last reference is dropped. This
>> makes the root freeing code much more significant.
>>
>> The only caveat is at close_ctree() time we have to free the extent
>> buffers for all of our main roots (extent_root, chunk_root, etc) because
>> we have to drop the btree_inode and we'll run into issues if we hold
>> onto those nodes until ->kill_sb() time. This will be addressed in the
>> future when we kill the btree_inode.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>
> Nit: This patch obsoleted the last comment in btrfs_init_fs_root, namely:
>
> /* The caller is responsible to call btrfs_free_fs_root */
>
>> ---
>> fs/btrfs/disk-io.c | 64 ++++++++++++++++++------------------
>> fs/btrfs/disk-io.h | 16 +--------
>> fs/btrfs/extent-tree.c | 7 ++--
>> fs/btrfs/extent_io.c | 16 +++++++--
>> fs/btrfs/free-space-tree.c | 2 --
>> fs/btrfs/qgroup.c | 7 +---
>> fs/btrfs/relocation.c | 4 ---
>> fs/btrfs/tests/btrfs-tests.c | 5 +--
>> fs/btrfs/tree-log.c | 6 ----
>> 9 files changed, 50 insertions(+), 77 deletions(-)
>>
>
> <snip>
>
>> @@ -4795,7 +4803,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>
>> static void __free_extent_buffer(struct extent_buffer *eb)
>> {
>> - btrfs_leak_debug_del(&eb->fs_info->eb_leak_lock, &eb->leak_list);
>> kmem_cache_free(extent_buffer_cache, eb);
>> }
>
> This function becomes a trivial wrapper so should be eliminated altogether.
>
> <snip>
>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 034f5f151a74..4fb7e3cc2aca 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -2549,10 +2549,6 @@ void free_reloc_roots(struct list_head *list)
>> reloc_root = list_entry(list->next, struct btrfs_root,
>> root_list);
>> __del_reloc_root(reloc_root);
>> - free_extent_buffer(reloc_root->node);
>> - free_extent_buffer(reloc_root->commit_root);
>> - reloc_root->node = NULL;
>> - reloc_root->commit_root = NULL;
>
> Shouldn't you do btrfs_put_root(reloc_root) here ?
No, but I can see how this is confusing. The reloc root is actually cleaned up
in clean_dirty_subvols(), so it's final put happens there. Thanks,
Josef
^ permalink raw reply
* Re: [PATCH v7 21/24] iomap: Restructure iomap_readpages_actor
From: Christoph Hellwig @ 2020-02-20 15:47 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-xfs, linux-kernel, linux-f2fs-devel, cluster-devel,
linux-mm, ocfs2-devel, linux-fsdevel, linux-ext4, linux-erofs,
linux-btrfs
In-Reply-To: <20200219210103.32400-22-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:00PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> By putting the 'have we reached the end of the page' condition at the end
> of the loop instead of the beginning, we can remove the 'submit the last
> page' code from iomap_readpages(). Also check that iomap_readpage_actor()
> didn't return 0, which would lead to an endless loop.
I'm obviously biassed a I wrote the original code, but I find the new
very much harder to understand (not that the previous one was easy, this
is tricky code..).
^ permalink raw reply
* Re: [PATCH v7 21/24] iomap: Restructure iomap_readpages_actor
From: Christoph Hellwig @ 2020-02-20 15:47 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-fsdevel, linux-mm, linux-kernel, linux-btrfs, linux-erofs,
linux-ext4, linux-f2fs-devel, cluster-devel, ocfs2-devel,
linux-xfs
In-Reply-To: <20200219210103.32400-22-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:00PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> By putting the 'have we reached the end of the page' condition at the end
> of the loop instead of the beginning, we can remove the 'submit the last
> page' code from iomap_readpages(). Also check that iomap_readpage_actor()
> didn't return 0, which would lead to an endless loop.
I'm obviously biassed a I wrote the original code, but I find the new
very much harder to understand (not that the previous one was easy, this
is tricky code..).
^ permalink raw reply
* [Ocfs2-devel] [PATCH v7 21/24] iomap: Restructure iomap_readpages_actor
From: Christoph Hellwig @ 2020-02-20 15:47 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-fsdevel, linux-mm, linux-kernel, linux-btrfs, linux-erofs,
linux-ext4, linux-f2fs-devel, cluster-devel, ocfs2-devel,
linux-xfs
In-Reply-To: <20200219210103.32400-22-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:00PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> By putting the 'have we reached the end of the page' condition at the end
> of the loop instead of the beginning, we can remove the 'submit the last
> page' code from iomap_readpages(). Also check that iomap_readpage_actor()
> didn't return 0, which would lead to an endless loop.
I'm obviously biassed a I wrote the original code, but I find the new
very much harder to understand (not that the previous one was easy, this
is tricky code..).
^ permalink raw reply
* [Cluster-devel] [PATCH v7 21/24] iomap: Restructure iomap_readpages_actor
From: Christoph Hellwig @ 2020-02-20 15:47 UTC (permalink / raw)
To: cluster-devel.redhat.com
In-Reply-To: <20200219210103.32400-22-willy@infradead.org>
On Wed, Feb 19, 2020 at 01:01:00PM -0800, Matthew Wilcox wrote:
> From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>
> By putting the 'have we reached the end of the page' condition at the end
> of the loop instead of the beginning, we can remove the 'submit the last
> page' code from iomap_readpages(). Also check that iomap_readpage_actor()
> didn't return 0, which would lead to an endless loop.
I'm obviously biassed a I wrote the original code, but I find the new
very much harder to understand (not that the previous one was easy, this
is tricky code..).
^ permalink raw reply
* Re: [PATCH v7 14/24] btrfs: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:46 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-xfs@vger.kernel.org, Johannes Thumshirn,
linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com,
linux-mm@kvack.org, ocfs2-devel@oss.oracle.com,
linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-erofs@lists.ozlabs.org, linux-btrfs@vger.kernel.org
In-Reply-To: <20200220134849.GV24185@bombadil.infradead.org>
On Thu, Feb 20, 2020 at 05:48:49AM -0800, Matthew Wilcox wrote:
> btrfs: Convert from readpages to readahead
>
> Implement the new readahead method in btrfs. Add a readahead_page_batch()
> to optimise fetching a batch of pages at once.
Shouldn't this readahead_page_batch heper go into a separate patch so
that it clearly stands out?
^ permalink raw reply
* Re: [Xen-devel] [PATCH] x86/vpt: update last_guest_time with cmpxchg and drop pl_time_lock
From: Jan Beulich @ 2020-02-20 15:47 UTC (permalink / raw)
To: Igor Druzhinin; +Cc: xen-devel, roger.pau, wl, andrew.cooper3
In-Reply-To: <eb6156eb-6a6d-28f5-c8ec-081f81444b99@citrix.com>
On 20.02.2020 16:37, Igor Druzhinin wrote:
> On 20/02/2020 08:27, Jan Beulich wrote:
>> On 19.02.2020 19:52, Igor Druzhinin wrote:
>>> On 19/02/2020 07:48, Jan Beulich wrote:
>>>> On 20.12.2019 22:39, Igor Druzhinin wrote:
>>>>> @@ -38,24 +37,22 @@ void hvm_init_guest_time(struct domain *d)
>>>>> uint64_t hvm_get_guest_time_fixed(const struct vcpu *v, uint64_t at_tsc)
>>>>> {
>>>>> struct pl_time *pl = v->domain->arch.hvm.pl_time;
>>>>> - u64 now;
>>>>> + s_time_t old, new, now = get_s_time_fixed(at_tsc) + pl->stime_offset;
>>>>>
>>>>> /* Called from device models shared with PV guests. Be careful. */
>>>>> ASSERT(is_hvm_vcpu(v));
>>>>>
>>>>> - spin_lock(&pl->pl_time_lock);
>>>>> - now = get_s_time_fixed(at_tsc) + pl->stime_offset;
>>>>> -
>>>>> if ( !at_tsc )
>>>>> {
>>>>> - if ( (int64_t)(now - pl->last_guest_time) > 0 )
>>>>> - pl->last_guest_time = now;
>>>>> - else
>>>>> - now = ++pl->last_guest_time;
>>>>> + do {
>>>>> + old = pl->last_guest_time;
>>>>> + new = now > pl->last_guest_time ? now : old + 1;
>>>>> + } while ( cmpxchg(&pl->last_guest_time, old, new) != old );
>>>>
>>>> I wonder whether you wouldn't better re-invoke get_s_time() in
>>>> case you need to retry here. See how the function previously
>>>> was called only after the lock was already acquired.
>>>
>>> If there is a concurrent writer, wouldn't it just update pl->last_guest_time
>>> with the new get_s_time() and then we subsequently would just use the new
>>> time on retry?
>>
>> Yes, it would, but the latency until the retry actually occurs
>> is unknown (in particular if Xen itself runs virtualized). I.e.
>> in the at_tsc == 0 case I think the value would better be
>> re-calculated on every iteration.
>
> Why does it need to be recalculated if a concurrent writer did this
> for us already anyway and (get_s_time_fixed(at_tsc) + pl->stime_offset)
> value is common for all of vCPUs? Yes, it might reduce jitter slightly
> but overall latency could come from any point (especially in case of
> rinning virtualized) and it's important just to preserve invariant that
> the value is monotonic across vCPUs.
I'm afraid I don't follow: If we rely on remote CPUs updating
pl->last_guest_time, then what we'd return is whatever was put
there plus one. Whereas the correct value might be dozens of
clocks further ahead.
>> Anther thing I notice only now are the multiple reads of
>> pl->last_guest_time. Wouldn't you better do
>>
>> do {
>> old = ACCESS_ONCE(pl->last_guest_time);
>> new = now > old ? now : old + 1;
>> } while ( cmpxchg(&pl->last_guest_time, old, new) != old );
>>
>> ?
>
> Fair enough, although even reading it multiple times wouldn't cause
> any harm as any inconsistency would be resolved by cmpxchg op.
Afaics "new", if calculated from a value latched _earlier_
than "old", could cause time to actually move backwards. Reads
can be re-ordered, after all.
> I'd
> prefer to make it in a separate commit to unify it with pv_soft_rdtsc().
I'd be fine if you changed pv_soft_rdtsc() first, and then
made the code here match. But I don't think the code should be
introduced in other than its (for the time being) final shape.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: [dpdk-dev] [PATCH v2] app/testpmd: fix return value in portlist parser
From: Govindharajan, Hariprasad @ 2020-02-20 15:46 UTC (permalink / raw)
To: Lu, Wenzhuo, Wu, Jingjing, Iremonger, Bernard
Cc: dev@dpdk.org, Yigit, Ferruh, stephen@networkplumber.org,
david.marchand@redhat.com
In-Reply-To: <1582213402-18941-1-git-send-email-hariprasad.govindharajan@intel.com>
> -----Original Message-----
> From: Govindharajan, Hariprasad <hariprasad.govindharajan@intel.com>
> Sent: Thursday, February 20, 2020 3:43 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>;
> stephen@networkplumber.org; david.marchand@redhat.com;
> Govindharajan, Hariprasad <hariprasad.govindharajan@intel.com>
> Subject: [PATCH v2] app/testpmd: fix return value in portlist parser
>
> The function parse_port_list() is designed to return unsigned int value. After
> sanitizing the inputs, it is returning -1. Changed it to return 0.
>
> Fixes: 2df00d562d20 ("app/testpmd: add --portlist option")
> Cc: hariprasad.govindharajan@intel.com
>
> Signed-off-by: Hariprasad Govindharajan
> <hariprasad.govindharajan@intel.com>
> ---
> app/test-pmd/config.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 9d95202..91db508 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2642,7 +2642,7 @@ parse_port_list(const char *list, unsigned int
> *values, unsigned int maxsize)
> unsigned int marked[maxsize];
>
> if (list == NULL || values == NULL)
> - return -1;
> + return 0;
>
> for (i = 0; i < (int)maxsize; i++)
> marked[i] = 0;
> --
> 2.7.4
This patch was acked previously by Bernard Iremonger.
Just changed the patch head line to correct format.
Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
^ permalink raw reply
* Re: [PATCH v7 14/24] btrfs: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:46 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Johannes Thumshirn, linux-fsdevel@vger.kernel.org,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-erofs@lists.ozlabs.org,
linux-ext4@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com,
ocfs2-devel@oss.oracle.com, linux-xfs@vger.kernel.org
In-Reply-To: <20200220134849.GV24185@bombadil.infradead.org>
On Thu, Feb 20, 2020 at 05:48:49AM -0800, Matthew Wilcox wrote:
> btrfs: Convert from readpages to readahead
>
> Implement the new readahead method in btrfs. Add a readahead_page_batch()
> to optimise fetching a batch of pages at once.
Shouldn't this readahead_page_batch heper go into a separate patch so
that it clearly stands out?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.