Re: [PATCH v9 12/32] virtio_ring: packed: extract the logic of alloc queue
On Tue, 12 Apr 2022 14:28:24 +0800, Jason Wang wrote:
>
> 在 2022/4/6 上午11:43, Xuan Zhuo 写道:
> > Separate the logic of packed to create vring queue.
> >
> > For the convenience of passing parameters, add a structure
> > vring_packed.
> >
> > This feature is required for subsequent virtuqueue reset vring.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > drivers/virtio/virtio_ring.c | 70
> > 1 file changed, 56 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 33864134a744..ea451ae2aaef 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1817,19 +1817,17 @@ static struct vring_desc_extra
> > *vring_alloc_desc_extra(unsigned int num)
> > return desc_extra;
> > }
> >
> > -static struct virtqueue *vring_create_virtqueue_packed(
> > - unsigned int index,
> > - unsigned int num,
> > - unsigned int vring_align,
> > - struct virtio_device *vdev,
> > - bool weak_barriers,
> > - bool may_reduce_num,
> > - bool context,
> > - bool (*notify)(struct virtqueue *),
> > - void (*callback)(struct virtqueue *),
> > - const char *name)
> > +static int vring_alloc_queue_packed(struct virtio_device *vdev,
> > + u32 num,
> > + struct vring_packed_desc **_ring,
> > + struct vring_packed_desc_event **_driver,
> > + struct vring_packed_desc_event **_device,
> > + dma_addr_t *_ring_dma_addr,
> > + dma_addr_t *_driver_event_dma_addr,
> > + dma_addr_t *_device_event_dma_addr,
> > + size_t *_ring_size_in_bytes,
> > + size_t *_event_size_in_bytes)
> > {
> > - struct vring_virtqueue *vq;
> > struct vring_packed_desc *ring;
> > struct vring_packed_desc_event *driver, *device;
> > dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr;
> > @@ -1857,6 +1855,52 @@ static struct virtqueue
> > *vring_create_virtqueue_packed(
> > if (!device)
> > goto err_device;
> >
> > + *_ring = ring;
> > + *_driver = driver;
> > + *_device = device;
> > + *_ring_dma_addr = ring_dma_addr;
> > + *_driver_event_dma_addr = driver_event_dma_addr;
> > + *_device_event_dma_addr = device_event_dma_addr;
> > + *_ring_size_in_bytes = ring_size_in_bytes;
> > + *_event_size_in_bytes= event_size_in_bytes;
>
>
> I wonder if we can simply factor out split and packed from struct
> vring_virtqueue:
>
> struct vring_virtqueue {
> union {
> struct {} split;
> struct {} packed;
> };
> };
>
> to
>
> struct vring_virtqueue_split {};
> struct vring_virtqueue_packed {};
>
> Then we can do things like:
>
> vring_create_virtqueue_packed(struct virtio_device *vdev, u32 num,
> struct vring_virtqueue_packed *packed);
>
> and
>
> vring_vritqueue_attach_packed(struct vring_virtqueue *vq, struct
> vring_virtqueue_packed packed);
This idea is very similar to my previous idea, just without introducing a new
structure.
I'd be more than happy to revise this.
Thanks.
>
> Thanks
>
>
> > +
> > + return 0;
> > +
> > +err_device:
> > + vring_free_queue(vdev, event_size_in_bytes, driver,
> > driver_event_dma_addr);
> > +
> > +err_driver:
> > + vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr);
> > +
> > +err_ring:
> > + return -ENOMEM;
> > +}
> > +
> > +static struct virtqueue *vring_create_virtqueue_packed(
> > + unsigned int index,
> > + unsigned int num,
> > + unsigned int vring_align,
> > + struct virtio_device *vdev,
> > + bool weak_barriers,
> > + bool may_reduce_num,
> > + bool context,
> > + bool (*notify)(struct virtqueue *),
> > + void (*callback)(struct virtqueue *),
> > + const char *name)
> > +{
> > + dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr;
> > + struct vring_packed_desc_event *driver, *device;
> > + size_t ring_size_in_bytes, event_size_in_bytes;
> > + struct vring_packed_desc *ring;
> > + struct vring_virtqueue *vq;
> > +
> > + if (vring_alloc_queue_packed(vdev, num, , , ,
> > +_dma_addr, _event_dma_addr,
> > +_event_dma_addr,
> > +_size_in_bytes,
> > +_size_in_bytes))
> > + goto err_ring;
> > +
> > vq = kmalloc(sizeof(*vq), GFP_KERNEL);
> > if (!vq)
> > goto err_vq;
> > @@ -1939,9 +1983,7 @@ static struct virtqueue
> > *vring_create_virtqueue_packed(
> > kfree(vq);
> > err_vq:
> > vring_free_queue(vdev, event_size_in_bytes, device,
> > device_event_dma_addr);
> > -err_device:
> > vring_free_queue(vdev, event_size_in_bytes,
Re: [PATCH v9 23/32] virtio_pci: queue_reset: support VIRTIO_F_RING_RESET
On Tue, 12 Apr 2022 15:07:58 +0800, Jason Wang wrote:
>
> 在 2022/4/6 上午11:43, Xuan Zhuo 写道:
> > This patch implements virtio pci support for QUEUE RESET.
> >
> > Performing reset on a queue is divided into these steps:
> >
> > 1. notify the device to reset the queue
> > 2. recycle the buffer submitted
> > 3. reset the vring (may re-alloc)
> > 4. mmap vring to device, and enable the queue
> >
> > This patch implements virtio_reset_vq(), virtio_enable_resetq() in the
> > pci scenario.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > drivers/virtio/virtio_pci_common.c | 8 +--
> > drivers/virtio/virtio_pci_modern.c | 84 ++
> > drivers/virtio/virtio_ring.c | 2 +
> > include/linux/virtio.h | 1 +
> > 4 files changed, 92 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c
> > b/drivers/virtio/virtio_pci_common.c
> > index fdbde1db5ec5..863d3a8a0956 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -248,9 +248,11 @@ static void vp_del_vq(struct virtqueue *vq)
> > struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
> > unsigned long flags;
> >
> > - spin_lock_irqsave(_dev->lock, flags);
> > - list_del(>node);
> > - spin_unlock_irqrestore(_dev->lock, flags);
> > + if (!vq->reset) {
>
>
> On which condition that we may hit this path?
>
>
> > + spin_lock_irqsave(_dev->lock, flags);
> > + list_del(>node);
> > + spin_unlock_irqrestore(_dev->lock, flags);
> > + }
> >
> > vp_dev->del_vq(info);
> > kfree(info);
> > diff --git a/drivers/virtio/virtio_pci_modern.c
> > b/drivers/virtio/virtio_pci_modern.c
> > index 49a4493732cf..cb5d38f1c9c8 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device
> > *vdev, u64 features)
> > if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > +
> > + if (features & BIT_ULL(VIRTIO_F_RING_RESET))
> > + __virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
> > }
> >
> > /* virtio config->finalize_features() implementation */
> > @@ -199,6 +202,83 @@ static int vp_active_vq(struct virtqueue *vq, u16
> > msix_vec)
> > return 0;
> > }
> >
> > +static int vp_modern_reset_vq(struct virtqueue *vq)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
> > + struct virtio_pci_modern_device *mdev = _dev->mdev;
> > + struct virtio_pci_vq_info *info;
> > + unsigned long flags;
> > +
> > + if (!virtio_has_feature(vq->vdev, VIRTIO_F_RING_RESET))
> > + return -ENOENT;
> > +
> > + vp_modern_set_queue_reset(mdev, vq->index);
> > +
> > + info = vp_dev->vqs[vq->index];
> > +
> > + /* delete vq from irq handler */
> > + spin_lock_irqsave(_dev->lock, flags);
> > + list_del(>node);
> > + spin_unlock_irqrestore(_dev->lock, flags);
> > +
> > + INIT_LIST_HEAD(>node);
> > +
> > + /* For the case where vq has an exclusive irq, to prevent the irq from
> > +* being received again and the pending irq, call disable_irq().
> > +*
> > +* In the scenario based on shared interrupts, vq will be searched from
> > +* the queue virtqueues. Since the previous list_del() has been deleted
> > +* from the queue, it is impossible for vq to be called in this case.
> > +* There is no need to close the corresponding interrupt.
> > +*/
> > + if (vp_dev->per_vq_vectors && info->msix_vector != VIRTIO_MSI_NO_VECTOR)
> > + disable_irq(pci_irq_vector(vp_dev->pci_dev, info->msix_vector));
>
>
> See the previous discussion and the revert of the first try to harden
> the interrupt. We probably can't use disable_irq() since it conflicts
> with the affinity managed IRQ that is used by some drivers.
>
> We need to use synchonize_irq() and per virtqueue flag instead. As
> mentioned in previous patches, this could be done on top of my rework on
> the IRQ hardening .
OK, the next version will contain hardened features by per virtqueue flag.
Thanks.
>
>
> > +
> > + vq->reset = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int vp_modern_enable_reset_vq(struct virtqueue *vq)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
> > + struct virtio_pci_modern_device *mdev = _dev->mdev;
> > + struct virtio_pci_vq_info *info;
> > + unsigned long flags, index;
> > + int err;
> > +
> > + if (!vq->reset)
> > + return -EBUSY;
> > +
> > + index = vq->index;
> > + info = vp_dev->vqs[index];
> > +
> > + /* check queue reset status */
> > + if (vp_modern_get_queue_reset(mdev, index) != 1)
> > + return -EBUSY;
> > +
> > + err = vp_active_vq(vq, info->msix_vector);
> > + if (err)
> > + return err;
> > +
> > + if
Re: [PATCH] fuse: avoid unnecessary spinlock bump
On 4/11/22 10:00 PM, Vivek Goyal wrote: > On Mon, Apr 11, 2022 at 03:20:05PM +0200, Bernd Schubert wrote: > > So for testing DAX, I have to rely on out of tree patches from qemu > here if any changes in virtiofs client happen. > > https://gitlab.com/virtio-fs/qemu/-/tree/virtio-fs-dev > > Jeffle is probably relying on their own virtiofsd implementation for DAX > testing. > Actually I also use the C version virtiofsd in the above described repository for testing :) -- Thanks, Jeffle ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V2 4/5] virtio-pci: implement synchronize_vqs()
On Wed, Apr 13, 2022 at 12:49 AM Cornelia Huck wrote:
>
> On Tue, Apr 12 2022, Halil Pasic wrote:
>
> > On Mon, 11 Apr 2022 16:27:41 +0200
> > Cornelia Huck wrote:
>
> >> My main concern is that we would need to synchronize against a single
> >> interrupt that covers all kinds of I/O interrupts, not just a single
> >> device...
> >>
> >
> > Could we synchronize on struct airq_info's lock member? If we were
> > to grab all of these that might be involved...
>
> Hm, that could possibly narrow the sync down to a subset, which seems
> better. For devices still using classic interrupts, per-device sync
> would be easy.
>
> >
> > AFAIU for the synchronize implementation we need a lock or a set of locks
> > that contain all the possible vring_interrupt() calls with the queuues
> > that belong to the given device as a critical section. That way, one
> > has the acquire's and release's in place so that the vrign_interrupt()
> > either guaranteed to finish before the change of driver_ready is
> > guaranteed to be complete, or it is guaranteed to see the change.
> >
> > In any case, I guess we should first get clear on the first part. I.e.
> > when do we want to allow host->guest notifications.
>
> Also, whether we just care about vring interrupts, or general device
> interrupts (not sure if a config change interrupt may also trigger
> things we do not want to trigger?)
I think only vring interrupts, since the config interrupt hardening is
done via 22b7050a024d7 ("virtio: defer config changed notifications")
Thanks
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 01/32] virtio: add helper virtqueue_get_vring_max_size()
On Tue, 12 Apr 2022 10:41:03 +0800, Jason Wang wrote:
>
> 在 2022/4/6 上午11:43, Xuan Zhuo 写道:
> > Record the maximum queue num supported by the device.
> >
> > virtio-net can display the maximum (supported by hardware) ring size in
> > ethtool -g eth0.
> >
> > When the subsequent patch implements vring reset, it can judge whether
> > the ring size passed by the driver is legal based on this.
> >
> > Signed-off-by: Xuan Zhuo
> > ---
> > arch/um/drivers/virtio_uml.c | 1 +
> > drivers/platform/mellanox/mlxbf-tmfifo.c | 2 ++
> > drivers/remoteproc/remoteproc_virtio.c | 2 ++
> > drivers/s390/virtio/virtio_ccw.c | 3 +++
> > drivers/virtio/virtio_mmio.c | 2 ++
> > drivers/virtio/virtio_pci_legacy.c | 2 ++
> > drivers/virtio/virtio_pci_modern.c | 2 ++
> > drivers/virtio/virtio_ring.c | 14 ++
> > drivers/virtio/virtio_vdpa.c | 2 ++
> > include/linux/virtio.h | 2 ++
> > 10 files changed, 32 insertions(+)
> >
> > diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
> > index ba562d68dc04..904993d15a85 100644
> > --- a/arch/um/drivers/virtio_uml.c
> > +++ b/arch/um/drivers/virtio_uml.c
> > @@ -945,6 +945,7 @@ static struct virtqueue *vu_setup_vq(struct
> > virtio_device *vdev,
> > goto error_create;
> > }
> > vq->priv = info;
> > + vq->num_max = num;
> > num = virtqueue_get_vring_size(vq);
> >
> > if (vu_dev->protocol_features &
> > diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c
> > b/drivers/platform/mellanox/mlxbf-tmfifo.c
> > index 38800e86ed8a..1ae3c56b66b0 100644
> > --- a/drivers/platform/mellanox/mlxbf-tmfifo.c
> > +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
> > @@ -959,6 +959,8 @@ static int mlxbf_tmfifo_virtio_find_vqs(struct
> > virtio_device *vdev,
> > goto error;
> > }
> >
> > + vq->num_max = vring->num;
> > +
> > vqs[i] = vq;
> > vring->vq = vq;
> > vq->priv = vring;
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > b/drivers/remoteproc/remoteproc_virtio.c
> > index 70ab496d0431..7611755d0ae2 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -125,6 +125,8 @@ static struct virtqueue *rp_find_vq(struct
> > virtio_device *vdev,
> > return ERR_PTR(-ENOMEM);
> > }
> >
> > + vq->num_max = len;
>
>
> I wonder if this is correct.
>
> It looks to me len is counted in bytes:
>
> /**
> * struct rproc_vring - remoteproc vring state
> * @va: virtual address
> * @len: length, in bytes
> * @da: device address
> * @align: vring alignment
> * @notifyid: rproc-specific unique vring index
> * @rvdev: remote vdev
> * @vq: the virtqueue of this vring
> */
> struct rproc_vring {
> void *va;
> int len;
> u32 da;
> u32 align;
> int notifyid;
> struct rproc_vdev *rvdev;
> struct virtqueue *vq;
> };
>
I think this comment is incorrect because here len is passed as num to
vring_new_virtqueue().
There is also this usage:
/* actual size of vring (in bytes) */
size = PAGE_ALIGN(vring_size(rvring->len, rvring->align));
And this value comes from here:
static int
rproc_parse_vring(struct rproc_vdev *rvdev, struct fw_rsc_vdev *rsc,
int i)
{
struct rproc *rproc = rvdev->rproc;
struct device *dev = >dev;
struct fw_rsc_vdev_vring *vring = >vring[i];
struct rproc_vring *rvring = >vring[i];
dev_dbg(dev, "vdev rsc: vring%d: da 0x%x, qsz %d, align %d\n",
i, vring->da, vring->num, vring->align);
/* verify queue size and vring alignment are sane */
if (!vring->num || !vring->align) {
dev_err(dev, "invalid qsz (%d) or alignment (%d)\n",
vring->num, vring->align);
return -EINVAL;
}
>rvring->len = vring->num;
rvring->align = vring->align;
rvring->rvdev = rvdev;
return 0;
}
/**
* struct fw_rsc_vdev_vring - vring descriptor entry
* @da: device address
* @align: the alignment between the consumer and producer parts of the vring
* @num: num of buffers supported by this vring (must be power of two)
* @notifyid: a unique rproc-wide notify index for this vring. This notify
* index is used when kicking a remote processor, to let it know that this
* vring is triggered.
* @pa: physical address
*
* This descriptor is not a resource entry by itself; it is part of the
* vdev resource type (see below).
*
* Note that @da should either contain the device address where
* the remote processor is expecting the vring, or indicate that
* dynamically allocation of the
Re: [PATCH V2 4/5] virtio-pci: implement synchronize_vqs()
On Tue, Apr 12 2022, Halil Pasic wrote: > On Mon, 11 Apr 2022 16:27:41 +0200 > Cornelia Huck wrote: >> My main concern is that we would need to synchronize against a single >> interrupt that covers all kinds of I/O interrupts, not just a single >> device... >> > > Could we synchronize on struct airq_info's lock member? If we were > to grab all of these that might be involved... Hm, that could possibly narrow the sync down to a subset, which seems better. For devices still using classic interrupts, per-device sync would be easy. > > AFAIU for the synchronize implementation we need a lock or a set of locks > that contain all the possible vring_interrupt() calls with the queuues > that belong to the given device as a critical section. That way, one > has the acquire's and release's in place so that the vrign_interrupt() > either guaranteed to finish before the change of driver_ready is > guaranteed to be complete, or it is guaranteed to see the change. > > In any case, I guess we should first get clear on the first part. I.e. > when do we want to allow host->guest notifications. Also, whether we just care about vring interrupts, or general device interrupts (not sure if a config change interrupt may also trigger things we do not want to trigger?) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/2] vdpa: Add support for querying vendor statistics
Hi Eli,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.18-rc2 next-20220412]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/Eli-Cohen/Show-statistics-for-a-vdpa-device/20220412-212129
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
ce522ba9ef7e2d9fb22a39eb3371c0c64e2a433e
config: i386-randconfig-a004-20220411
(https://download.01.org/0day-ci/archive/20220413/202204130003.froyiuxa-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/813a1bf827634a8d7e148b133e2849fac37819cd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Eli-Cohen/Show-statistics-for-a-vdpa-device/20220412-212129
git checkout 813a1bf827634a8d7e148b133e2849fac37819cd
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=i386 SHELL=/bin/bash drivers/usb/typec/ drivers/vdpa/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/vdpa/vdpa.c:1122:61: warning: implicit conversion from 'int' to
>> 's16' (aka 'short') changes value from 65535 to -1 [-Wconstant-conversion]
[VDPA_ATTR_DEV_QUEUE_INDEX] = NLA_POLICY_RANGE(NLA_U32, 0, 65535),
~^~
include/net/netlink.h:396:9: note: expanded from macro 'NLA_POLICY_RANGE'
.max = _max \
^~~~
1 warning generated.
vim +1122 drivers/vdpa/vdpa.c
1114
1115 static const struct nla_policy vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
1116 [VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
1117 [VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
1118 [VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
1119 [VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
1120 /* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
1121 [VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
> 1122 [VDPA_ATTR_DEV_QUEUE_INDEX] = NLA_POLICY_RANGE(NLA_U32, 0,
> 65535),
1123 };
1124
--
0-DAY CI Kernel Test Service
https://01.org/lkp
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 2/5] mm: page_isolation: check specified range for unmovable pages
On 12.04.22 17:01, Zi Yan wrote:
> On 12 Apr 2022, at 10:49, David Hildenbrand wrote:
>
>> On 12.04.22 16:07, Zi Yan wrote:
>>> On 12 Apr 2022, at 9:10, David Hildenbrand wrote:
>>>
On 06.04.22 17:18, Zi Yan wrote:
> From: Zi Yan
>
> Enable set_migratetype_isolate() to check specified sub-range for
> unmovable pages during isolation. Page isolation is done
> at MAX_ORDER_NR_PAEGS granularity, but not all pages within that
> granularity are intended to be isolated. For example,
> alloc_contig_range(), which uses page isolation, allows ranges without
> alignment. This commit makes unmovable page check only look for
> interesting pages, so that page isolation can succeed for any
> non-overlapping ranges.
>
> Signed-off-by: Zi Yan
> ---
[...]
> /*
> - * This function checks whether pageblock includes unmovable pages or
> not.
> + * This function checks whether the range [start_pfn, end_pfn) includes
> + * unmovable pages or not. The range must fall into a single pageblock
> and
> + * consequently belong to a single zone.
> *
> * PageLRU check without isolation or lru_lock could race so that
> * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
> @@ -28,12 +30,14 @@
> * cannot get removed (e.g., via memory unplug) concurrently.
> *
> */
> -static struct page *has_unmovable_pages(struct zone *zone, struct page
> *page,
> - int migratetype, int flags)
> +static struct page *has_unmovable_pages(unsigned long start_pfn,
> unsigned long end_pfn,
> + int migratetype, int flags)
> {
> - unsigned long iter = 0;
> - unsigned long pfn = page_to_pfn(page);
> - unsigned long offset = pfn % pageblock_nr_pages;
> + unsigned long pfn = start_pfn;
> + struct page *page = pfn_to_page(pfn);
Just do
struct page *page = pfn_to_page(start_pfn);
struct zone *zone = page_zone(page);
here. No need to lookup the zone again in the loop because, as you
document "must ... belong to a single zone.".
Then, there is also no need to initialize "pfn" here. In the loop header
is sufficient.
>>>
>>> Sure.
>>>
> +
> + VM_BUG_ON(ALIGN_DOWN(start_pfn, pageblock_nr_pages) !=
> + ALIGN_DOWN(end_pfn - 1, pageblock_nr_pages));
>
> if (is_migrate_cma_page(page)) {
> /*
> @@ -47,8 +51,11 @@ static struct page *has_unmovable_pages(struct zone
> *zone, struct page *page,
> return page;
> }
>
> - for (; iter < pageblock_nr_pages - offset; iter++) {
> - page = pfn_to_page(pfn + iter);
> + for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> + struct zone *zone;
> +
> + page = pfn_to_page(pfn);
> + zone = page_zone(page);
>
> /*
>* Both, bootmem allocations and memory holes are marked
> @@ -85,7 +92,7 @@ static struct page *has_unmovable_pages(struct zone
> *zone, struct page *page,
> }
>
> skip_pages = compound_nr(head) - (page - head);
> - iter += skip_pages - 1;
> + pfn += skip_pages - 1;
> continue;
> }
>
> @@ -97,7 +104,7 @@ static struct page *has_unmovable_pages(struct zone
> *zone, struct page *page,
>*/
> if (!page_ref_count(page)) {
> if (PageBuddy(page))
> - iter += (1 << buddy_order(page)) - 1;
> + pfn += (1 << buddy_order(page)) - 1;
> continue;
> }
>
> @@ -134,11 +141,18 @@ static struct page *has_unmovable_pages(struct zone
> *zone, struct page *page,
> return NULL;
> }
>
> -static int set_migratetype_isolate(struct page *page, int migratetype,
> int isol_flags)
> +/*
> + * This function set pageblock migratetype to isolate if no unmovable
> page is
> + * present in [start_pfn, end_pfn). The pageblock must intersect with
> + * [start_pfn, end_pfn).
> + */
> +static int set_migratetype_isolate(struct page *page, int migratetype,
> int isol_flags,
> + unsigned long start_pfn, unsigned long end_pfn)
I think we might be able do better, eventually not passing start_pfn at
all. Hmm.
>>>
>>> IMHO, having start_pfn and end_pfn in the parameter list would make the
>>> interface easier to understand. Otherwise if we remove start_pfn,
>>> the caller needs to adjust @page to be within the range of [start_pfn,
>>> end_pfn)
>>>
I think we want to pull out the
start_isolate_page_range()/undo_isolate_page_range() interface change
into a separate patch.
>>>
>>>
Re: [PATCH v10 2/5] mm: page_isolation: check specified range for unmovable pages
On 12.04.22 16:07, Zi Yan wrote:
> On 12 Apr 2022, at 9:10, David Hildenbrand wrote:
>
>> On 06.04.22 17:18, Zi Yan wrote:
>>> From: Zi Yan
>>>
>>> Enable set_migratetype_isolate() to check specified sub-range for
>>> unmovable pages during isolation. Page isolation is done
>>> at MAX_ORDER_NR_PAEGS granularity, but not all pages within that
>>> granularity are intended to be isolated. For example,
>>> alloc_contig_range(), which uses page isolation, allows ranges without
>>> alignment. This commit makes unmovable page check only look for
>>> interesting pages, so that page isolation can succeed for any
>>> non-overlapping ranges.
>>>
>>> Signed-off-by: Zi Yan
>>> ---
>>
>> [...]
>>
>>> /*
>>> - * This function checks whether pageblock includes unmovable pages or not.
>>> + * This function checks whether the range [start_pfn, end_pfn) includes
>>> + * unmovable pages or not. The range must fall into a single pageblock and
>>> + * consequently belong to a single zone.
>>> *
>>> * PageLRU check without isolation or lru_lock could race so that
>>> * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
>>> @@ -28,12 +30,14 @@
>>> * cannot get removed (e.g., via memory unplug) concurrently.
>>> *
>>> */
>>> -static struct page *has_unmovable_pages(struct zone *zone, struct page
>>> *page,
>>> -int migratetype, int flags)
>>> +static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned
>>> long end_pfn,
>>> + int migratetype, int flags)
>>> {
>>> - unsigned long iter = 0;
>>> - unsigned long pfn = page_to_pfn(page);
>>> - unsigned long offset = pfn % pageblock_nr_pages;
>>> + unsigned long pfn = start_pfn;
>>> + struct page *page = pfn_to_page(pfn);
>>
>>
>> Just do
>>
>> struct page *page = pfn_to_page(start_pfn);
>> struct zone *zone = page_zone(page);
>>
>> here. No need to lookup the zone again in the loop because, as you
>> document "must ... belong to a single zone.".
>>
>> Then, there is also no need to initialize "pfn" here. In the loop header
>> is sufficient.
>>
>
> Sure.
>
>>> +
>>> + VM_BUG_ON(ALIGN_DOWN(start_pfn, pageblock_nr_pages) !=
>>> + ALIGN_DOWN(end_pfn - 1, pageblock_nr_pages));
>>>
>>> if (is_migrate_cma_page(page)) {
>>> /*
>>> @@ -47,8 +51,11 @@ static struct page *has_unmovable_pages(struct zone
>>> *zone, struct page *page,
>>> return page;
>>> }
>>>
>>> - for (; iter < pageblock_nr_pages - offset; iter++) {
>>> - page = pfn_to_page(pfn + iter);
>>> + for (pfn = start_pfn; pfn < end_pfn; pfn++) {
>>> + struct zone *zone;
>>> +
>>> + page = pfn_to_page(pfn);
>>> + zone = page_zone(page);
>>>
>>> /*
>>> * Both, bootmem allocations and memory holes are marked
>>> @@ -85,7 +92,7 @@ static struct page *has_unmovable_pages(struct zone
>>> *zone, struct page *page,
>>> }
>>>
>>> skip_pages = compound_nr(head) - (page - head);
>>> - iter += skip_pages - 1;
>>> + pfn += skip_pages - 1;
>>> continue;
>>> }
>>>
>>> @@ -97,7 +104,7 @@ static struct page *has_unmovable_pages(struct zone
>>> *zone, struct page *page,
>>> */
>>> if (!page_ref_count(page)) {
>>> if (PageBuddy(page))
>>> - iter += (1 << buddy_order(page)) - 1;
>>> + pfn += (1 << buddy_order(page)) - 1;
>>> continue;
>>> }
>>>
>>> @@ -134,11 +141,18 @@ static struct page *has_unmovable_pages(struct zone
>>> *zone, struct page *page,
>>> return NULL;
>>> }
>>>
>>> -static int set_migratetype_isolate(struct page *page, int migratetype, int
>>> isol_flags)
>>> +/*
>>> + * This function set pageblock migratetype to isolate if no unmovable page
>>> is
>>> + * present in [start_pfn, end_pfn). The pageblock must intersect with
>>> + * [start_pfn, end_pfn).
>>> + */
>>> +static int set_migratetype_isolate(struct page *page, int migratetype, int
>>> isol_flags,
>>> + unsigned long start_pfn, unsigned long end_pfn)
>>
>> I think we might be able do better, eventually not passing start_pfn at
>> all. Hmm.
>
> IMHO, having start_pfn and end_pfn in the parameter list would make the
> interface easier to understand. Otherwise if we remove start_pfn,
> the caller needs to adjust @page to be within the range of [start_pfn,
> end_pfn)
>
>>
>> I think we want to pull out the
>> start_isolate_page_range()/undo_isolate_page_range() interface change
>> into a separate patch.
>
> You mean a patch just adding
>
> unsigned long isolate_start = pfn_max_align_down(start_pfn);
> unsigned long isolate_end = pfn_max_align_up(end_pfn);
>
> in start_isolate_page_range()/undo_isolate_page_range()?
>
> Yes I can do that.
I think we have to be careful with memory
Re: [PATCH v10 2/5] mm: page_isolation: check specified range for unmovable pages
On 06.04.22 17:18, Zi Yan wrote:
> From: Zi Yan
>
> Enable set_migratetype_isolate() to check specified sub-range for
> unmovable pages during isolation. Page isolation is done
> at MAX_ORDER_NR_PAEGS granularity, but not all pages within that
> granularity are intended to be isolated. For example,
> alloc_contig_range(), which uses page isolation, allows ranges without
> alignment. This commit makes unmovable page check only look for
> interesting pages, so that page isolation can succeed for any
> non-overlapping ranges.
>
> Signed-off-by: Zi Yan
> ---
[...]
> /*
> - * This function checks whether pageblock includes unmovable pages or not.
> + * This function checks whether the range [start_pfn, end_pfn) includes
> + * unmovable pages or not. The range must fall into a single pageblock and
> + * consequently belong to a single zone.
> *
> * PageLRU check without isolation or lru_lock could race so that
> * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
> @@ -28,12 +30,14 @@
> * cannot get removed (e.g., via memory unplug) concurrently.
> *
> */
> -static struct page *has_unmovable_pages(struct zone *zone, struct page *page,
> - int migratetype, int flags)
> +static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned
> long end_pfn,
> + int migratetype, int flags)
> {
> - unsigned long iter = 0;
> - unsigned long pfn = page_to_pfn(page);
> - unsigned long offset = pfn % pageblock_nr_pages;
> + unsigned long pfn = start_pfn;
> + struct page *page = pfn_to_page(pfn);
Just do
struct page *page = pfn_to_page(start_pfn);
struct zone *zone = page_zone(page);
here. No need to lookup the zone again in the loop because, as you
document "must ... belong to a single zone.".
Then, there is also no need to initialize "pfn" here. In the loop header
is sufficient.
> +
> + VM_BUG_ON(ALIGN_DOWN(start_pfn, pageblock_nr_pages) !=
> + ALIGN_DOWN(end_pfn - 1, pageblock_nr_pages));
>
> if (is_migrate_cma_page(page)) {
> /*
> @@ -47,8 +51,11 @@ static struct page *has_unmovable_pages(struct zone *zone,
> struct page *page,
> return page;
> }
>
> - for (; iter < pageblock_nr_pages - offset; iter++) {
> - page = pfn_to_page(pfn + iter);
> + for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> + struct zone *zone;
> +
> + page = pfn_to_page(pfn);
> + zone = page_zone(page);
>
> /*
>* Both, bootmem allocations and memory holes are marked
> @@ -85,7 +92,7 @@ static struct page *has_unmovable_pages(struct zone *zone,
> struct page *page,
> }
>
> skip_pages = compound_nr(head) - (page - head);
> - iter += skip_pages - 1;
> + pfn += skip_pages - 1;
> continue;
> }
>
> @@ -97,7 +104,7 @@ static struct page *has_unmovable_pages(struct zone *zone,
> struct page *page,
>*/
> if (!page_ref_count(page)) {
> if (PageBuddy(page))
> - iter += (1 << buddy_order(page)) - 1;
> + pfn += (1 << buddy_order(page)) - 1;
> continue;
> }
>
> @@ -134,11 +141,18 @@ static struct page *has_unmovable_pages(struct zone
> *zone, struct page *page,
> return NULL;
> }
>
> -static int set_migratetype_isolate(struct page *page, int migratetype, int
> isol_flags)
> +/*
> + * This function set pageblock migratetype to isolate if no unmovable page is
> + * present in [start_pfn, end_pfn). The pageblock must intersect with
> + * [start_pfn, end_pfn).
> + */
> +static int set_migratetype_isolate(struct page *page, int migratetype, int
> isol_flags,
> + unsigned long start_pfn, unsigned long end_pfn)
I think we might be able do better, eventually not passing start_pfn at
all. Hmm.
I think we want to pull out the
start_isolate_page_range()/undo_isolate_page_range() interface change
into a separate patch.
Let me try to give it a shot, I'll try hacking something up real quick
to see if we can do better.
--
Thanks,
David / dhildenb
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 0/5] Use pageblock_order for cma and alloc_contig_range alignment.
On 06.04.22 17:18, Zi Yan wrote: > From: Zi Yan > > Hi David, Hi! > > This patchset tries to remove the MAX_ORDER-1 alignment requirement for CMA > and alloc_contig_range(). It prepares for my upcoming changes to make > MAX_ORDER adjustable at boot time[1]. It is on top of mmotm-2022-04-05-15-54. Sorry for the late reply, I've got way too many irons in the fire right now. > > I also added "Content-Type: text/plain; charset=UTF-8" to all email bodies > explicitly, please let me know if you still cannot see the emails in a > proper format. Oh, thanks! But no need to work around Mimecast mailing issues on your side. This just has to be fixed for good on the RH side ... I yet heave to give #3 a thorough review, sorry for not commenting on that earlier. It's a bit more involved, especially, with all the possible corner cases :) -- Thanks, David / dhildenb ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Re: [PATCH v4 0/8] Introduce akcipher service for virtio-crypto
On 4/12/22 17:47, Paolo Bonzini wrote: In our plan, the feature is designed for HTTPS offloading case and other applications which use kernel RSA/ecdsa by keyctl syscall. Hi Zhenwei, what is the % of time spent doing asymmetric key operations in your benchmark? I am not very familiar with crypto acceleration but my understanding has always been that most time is spent doing either hashing (for signing) or symmetric key operations (for encryption). If I understand correctly, without support for acceleration these patches are more of a demonstration of virtio-crypto, or usable for testing purposes. Hi, Paolo This is the perf result of nginx+openssl CPU calculation, the heavy load from openssl uses the most time(as same as you mentioned). 27.37%26.00% nginxlibcrypto.so.1.1 [.] __bn_sqrx8x_reduction 20.58%19.52% nginxlibcrypto.so.1.1 [.] mulx4x_internal 16.73%15.89% nginxlibcrypto.so.1.1 [.] bn_sqrx8x_internal 8.79% 0.00% nginx[unknown] [k] 7.26% 0.00% nginx[unknown] [.] 0x89388669992a0cbc 7.00% 0.00% nginx[unknown] [k] 0x45f0e480d5f2a58e 6.76% 0.02% nginx[kernel.kallsyms] [k] entry_SYSCALL_64_after_hwframe 6.74% 0.02% nginx[kernel.kallsyms] [k] do_syscall_64 6.61% 0.00% nginx[unknown] [.] 0xa75a60d7820f9ffb 6.47% 0.00% nginx[unknown] [k] 0xe91223f6da36254c 5.51% 0.01% nginx[kernel.kallsyms] [k] asm_common_interrupt 5.46% 0.01% nginx[kernel.kallsyms] [k] common_interrupt 5.16% 0.04% nginx[kernel.kallsyms] [k] __softirqentry_text_start 4.92% 0.01% nginx[kernel.kallsyms] [k] irq_exit_rcu 4.91% 0.04% nginx[kernel.kallsyms] [k] net_rx_action This is the result of nginx+openssl keyctl offload(virtio crypto + host keyctl + Intel QAT): 30.38% 0.08% nginx[kernel.kallsyms] [k] entry_SYSCALL_64_after_hwframe 30.29% 0.07% nginx[kernel.kallsyms] [k] do_syscall_64 23.84% 0.00% nginx[unknown] [k] 14.24% 0.03% nginx[kernel.kallsyms] [k] asm_common_interrupt 14.06% 0.05% nginx[kernel.kallsyms] [k] common_interrupt 12.99% 0.11% nginx[kernel.kallsyms] [k] __softirqentry_text_start 12.27% 0.12% nginx[kernel.kallsyms] [k] net_rx_action 12.13% 0.03% nginx[kernel.kallsyms] [k] __napi_poll 12.06% 0.06% nginx[kernel.kallsyms] [k] irq_exit_rcu 10.49% 0.14% nginxlibssl.so.1.1 [.] tls_process_client_key_exchange 10.21% 0.12% nginx[virtio_net] [k] virtnet_poll 10.13% 0.04% nginxlibc-2.28.so [.] syscall 10.12% 0.03% nginxkctl-engine.so[.] kctl_rsa_priv_dec 10.02% 0.02% nginxkctl-engine.so[.] kctl_hw_rsa_priv_func 9.98% 0.01% nginxlibkeyutils.so.1.10 [.] keyctl_pkey_decrypt 9.95% 0.02% nginxlibkeyutils.so.1.10 [.] keyctl 9.77% 0.03% nginx[kernel.kallsyms] [k] keyctl_pkey_e_d_s 8.97% 0.00% nginx[unknown] [k] 0x7f4adbb81f0b 8.78% 0.08% nginxlibpthread-2.28.so[.] __libc_write 8.49% 0.05% nginx[kernel.kallsyms] [k] netif_receive_skb_list_internal The RSA part gets reduced, and the QPS of https improves to ~200%. Something may be ignored in this cover letter: [4] Currently RSA is supported only in builtin driver. This driver is supposed to test the full feature without other software(Ex vhost process) and hardware dependence. -> Yes, this patch is a demonstration of virtio-crypto. [5] keyctl backend is in development, we will post this feature in Q2-2022. keyctl backend can use hardware acceleration(Ex, Intel QAT). -> This is our plan. Currently it's still in developing. Would it be possible to extend virtio-crypto to use keys already in the host keyctl, or in a PKCS#11 smartcard, so that virtio-crypto could also provide the functionality of an HSM? Or does the standard require that the keys are provided by the guest? Paolo I'm very interested in this, I'll try in Q3-2022 or later. -- zhenwei pi ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 0/8] Introduce akcipher service for virtio-crypto
In our plan, the feature is designed for HTTPS offloading case and other applications which use kernel RSA/ecdsa by keyctl syscall. Hi Zhenwei, what is the % of time spent doing asymmetric key operations in your benchmark? I am not very familiar with crypto acceleration but my understanding has always been that most time is spent doing either hashing (for signing) or symmetric key operations (for encryption). If I understand correctly, without support for acceleration these patches are more of a demonstration of virtio-crypto, or usable for testing purposes. Would it be possible to extend virtio-crypto to use keys already in the host keyctl, or in a PKCS#11 smartcard, so that virtio-crypto could also provide the functionality of an HSM? Or does the standard require that the keys are provided by the guest? Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V2 4/5] virtio-pci: implement synchronize_vqs()
On Tue, 12 Apr 2022 10:24:35 +0800 Jason Wang wrote: > > Regarding the question "are we safe against notifications before > > indicators have been registered" I think we really need to think about > > something like Secure Execution. We don't have, and we are unlikely > > to have in hardware virtio-ccw implementations, and for a malicious > > hypervisor > > that has full access to the guest memory hardening makes no sense. > > Does s390 have something like memory encryption? (I guess yes). In the > case of x86 VM encryption, the I/O buffers were now done via software > IOTLB, that's why hardening of the virtio driver is needed to prevent > the hypervisor to poke the swiotlb etc. Yep! Secure Execution is a confidential computing solution which is much like encrypted guest memory, except for one gets exceptions when trying to access private memory instead of ending up with garbage because of the encryption. These improvements are IMHO relevant to us! Regards, Halil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 11/15] drm/shmem-helper: Add generic memory shrinker
Hi Dmitry,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20220411]
[cannot apply to drm/drm-next v5.18-rc2 v5.18-rc1 v5.17 v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/Dmitry-Osipenko/Add-generic-memory-shrinker-to-VirtIO-GPU-and-Panfrost-DRM-drivers/20220412-060325
base:d12d7e1cfe38e0c36d28c7a9fbbc436ad0d17c14
config: hexagon-randconfig-r045-20220411
(https://download.01.org/0day-ci/archive/20220412/202204121504.glr3fhqe-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/683ba8a9d72ba7770a61a9266a2b33949f3874f2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Dmitry-Osipenko/Add-generic-memory-shrinker-to-VirtIO-GPU-and-Panfrost-DRM-drivers/20220412-060325
git checkout 683ba8a9d72ba7770a61a9266a2b33949f3874f2
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_gem_shmem_helper.c:289:11: warning: variable 'new_state'
>> is used uninitialized whenever 'if' condition is false
>> [-Wsometimes-uninitialized]
else if (shmem->madv < 0)
^~~
drivers/gpu/drm/drm_gem_shmem_helper.c:292:46: note: uninitialized use
occurs here
drm_gem_shmem_set_pages_state_locked(shmem, new_state);
^
drivers/gpu/drm/drm_gem_shmem_helper.c:289:7: note: remove the 'if' if its
condition is always true
else if (shmem->madv < 0)
^~~~
drivers/gpu/drm/drm_gem_shmem_helper.c:278:2: note: variable 'new_state' is
declared here
enum drm_gem_shmem_pages_state new_state;
^
1 warning generated.
vim +289 drivers/gpu/drm/drm_gem_shmem_helper.c
273
274 static void drm_gem_shmem_update_pages_state_locked(struct
drm_gem_shmem_object *shmem)
275 {
276 struct drm_gem_object *obj = >base;
277 struct drm_gem_shmem_shrinker *gem_shrinker =
obj->dev->shmem_shrinker;
278 enum drm_gem_shmem_pages_state new_state;
279
280 if (!gem_shrinker || obj->import_attach)
281 return;
282
283 mutex_lock(_shrinker->lock);
284
285 if (!shmem->madv)
286 new_state = DRM_GEM_SHMEM_PAGES_STATE_ACTIVE;
287 else if (shmem->madv > 0)
288 new_state = DRM_GEM_SHMEM_PAGES_STATE_PURGEABLE;
> 289 else if (shmem->madv < 0)
290 new_state = DRM_GEM_SHMEM_PAGES_STATE_PURGED;
291
292 drm_gem_shmem_set_pages_state_locked(shmem, new_state);
293
294 mutex_unlock(_shrinker->lock);
295 }
296
--
0-DAY CI Kernel Test Service
https://01.org/lkp
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 11/15] drm/shmem-helper: Add generic memory shrinker
Hi Dmitry,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20220411]
[cannot apply to drm/drm-next v5.18-rc2 v5.18-rc1 v5.17 v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/Dmitry-Osipenko/Add-generic-memory-shrinker-to-VirtIO-GPU-and-Panfrost-DRM-drivers/20220412-060325
base:d12d7e1cfe38e0c36d28c7a9fbbc436ad0d17c14
config: i386-randconfig-a005-20220411
(https://download.01.org/0day-ci/archive/20220412/202204121523.qvmxovzg-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/683ba8a9d72ba7770a61a9266a2b33949f3874f2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Dmitry-Osipenko/Add-generic-memory-shrinker-to-VirtIO-GPU-and-Panfrost-DRM-drivers/20220412-060325
git checkout 683ba8a9d72ba7770a61a9266a2b33949f3874f2
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/drm_gem_shmem_helper.c:289:11: warning: variable 'new_state'
>> is used uninitialized whenever 'if' condition is false
>> [-Wsometimes-uninitialized]
else if (shmem->madv < 0)
^~~
drivers/gpu/drm/drm_gem_shmem_helper.c:292:46: note: uninitialized use
occurs here
drm_gem_shmem_set_pages_state_locked(shmem, new_state);
^
drivers/gpu/drm/drm_gem_shmem_helper.c:289:7: note: remove the 'if' if its
condition is always true
else if (shmem->madv < 0)
^~~~
drivers/gpu/drm/drm_gem_shmem_helper.c:278:2: note: variable 'new_state' is
declared here
enum drm_gem_shmem_pages_state new_state;
^
1 warning generated.
vim +289 drivers/gpu/drm/drm_gem_shmem_helper.c
273
274 static void drm_gem_shmem_update_pages_state_locked(struct
drm_gem_shmem_object *shmem)
275 {
276 struct drm_gem_object *obj = >base;
277 struct drm_gem_shmem_shrinker *gem_shrinker =
obj->dev->shmem_shrinker;
278 enum drm_gem_shmem_pages_state new_state;
279
280 if (!gem_shrinker || obj->import_attach)
281 return;
282
283 mutex_lock(_shrinker->lock);
284
285 if (!shmem->madv)
286 new_state = DRM_GEM_SHMEM_PAGES_STATE_ACTIVE;
287 else if (shmem->madv > 0)
288 new_state = DRM_GEM_SHMEM_PAGES_STATE_PURGEABLE;
> 289 else if (shmem->madv < 0)
290 new_state = DRM_GEM_SHMEM_PAGES_STATE_PURGED;
291
292 drm_gem_shmem_set_pages_state_locked(shmem, new_state);
293
294 mutex_unlock(_shrinker->lock);
295 }
296
--
0-DAY CI Kernel Test Service
https://01.org/lkp
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 29/32] virtio_net: get ringparam by virtqueue_get_vring_max_size()
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Use virtqueue_get_vring_max_size() in virtnet_get_ringparam() to set
tx,rx_max_pending.
Signed-off-by: Xuan Zhuo
---
Acked-by: Jason Wang
drivers/net/virtio_net.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index dad497a47b3a..96d96c666c8c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2177,10 +2177,10 @@ static void virtnet_get_ringparam(struct net_device
*dev,
{
struct virtnet_info *vi = netdev_priv(dev);
- ring->rx_max_pending = virtqueue_get_vring_size(vi->rq[0].vq);
- ring->tx_max_pending = virtqueue_get_vring_size(vi->sq[0].vq);
- ring->rx_pending = ring->rx_max_pending;
- ring->tx_pending = ring->tx_max_pending;
+ ring->rx_max_pending = virtqueue_get_vring_max_size(vi->rq[0].vq);
+ ring->tx_max_pending = virtqueue_get_vring_max_size(vi->sq[0].vq);
+ ring->rx_pending = virtqueue_get_vring_size(vi->rq[0].vq);
+ ring->tx_pending = virtqueue_get_vring_size(vi->sq[0].vq);
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 28/32] virtio_net: set the default max ring size by find_vqs()
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Use virtio_find_vqs_ctx_size() to specify the maximum ring size of tx,
rx at the same time.
| rx/tx ring size
---
speed == UNKNOWN or < 10G| 1024
speed < 40G | 4096
speed >= 40G | 8192
Call virtnet_update_settings() once before calling init_vqs() to update
speed.
Signed-off-by: Xuan Zhuo
---
Acked-by: Jason Wang
drivers/net/virtio_net.c | 42
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a801ea40908f..dad497a47b3a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2861,6 +2861,29 @@ static unsigned int mergeable_min_buf_len(struct
virtnet_info *vi, struct virtqu
(unsigned int)GOOD_PACKET_LEN);
}
+static void virtnet_config_sizes(struct virtnet_info *vi, u32 *sizes)
+{
+ u32 i, rx_size, tx_size;
+
+ if (vi->speed == SPEED_UNKNOWN || vi->speed < SPEED_1) {
+ rx_size = 1024;
+ tx_size = 1024;
+
+ } else if (vi->speed < SPEED_4) {
+ rx_size = 1024 * 4;
+ tx_size = 1024 * 4;
+
+ } else {
+ rx_size = 1024 * 8;
+ tx_size = 1024 * 8;
+ }
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ sizes[rxq2vq(i)] = rx_size;
+ sizes[txq2vq(i)] = tx_size;
+ }
+}
+
static int virtnet_find_vqs(struct virtnet_info *vi)
{
vq_callback_t **callbacks;
@@ -2868,6 +2891,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
int ret = -ENOMEM;
int i, total_vqs;
const char **names;
+ u32 *sizes;
bool *ctx;
/* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by
@@ -2895,10 +2919,15 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
ctx = NULL;
}
+ sizes = kmalloc_array(total_vqs, sizeof(*sizes), GFP_KERNEL);
+ if (!sizes)
+ goto err_sizes;
+
/* Parameters for control virtqueue, if any */
if (vi->has_cvq) {
callbacks[total_vqs - 1] = NULL;
names[total_vqs - 1] = "control";
+ sizes[total_vqs - 1] = 64;
}
/* Allocate/initialize parameters for send/receive virtqueues */
@@ -2913,8 +2942,10 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
ctx[rxq2vq(i)] = true;
}
- ret = virtio_find_vqs_ctx(vi->vdev, total_vqs, vqs, callbacks,
- names, ctx, NULL);
+ virtnet_config_sizes(vi, sizes);
+
+ ret = virtio_find_vqs_ctx_size(vi->vdev, total_vqs, vqs, callbacks,
+ names, sizes, ctx, NULL);
if (ret)
goto err_find;
@@ -2934,6 +2965,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
err_find:
+ kfree(sizes);
+err_sizes:
kfree(ctx);
err_ctx:
kfree(names);
@@ -3252,6 +3285,9 @@ static int virtnet_probe(struct virtio_device *vdev)
vi->curr_queue_pairs = num_online_cpus();
vi->max_queue_pairs = max_queue_pairs;
+ virtnet_init_settings(dev);
+ virtnet_update_settings(vi);
+
/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
err = init_vqs(vi);
if (err)
@@ -3264,8 +3300,6 @@ static int virtnet_probe(struct virtio_device *vdev)
netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
- virtnet_init_settings(dev);
-
if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
vi->failover = net_failover_create(vi->dev);
if (IS_ERR(vi->failover)) {
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 27/32] virtio: add helper virtio_find_vqs_ctx_size()
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Introduce helper virtio_find_vqs_ctx_size() to call find_vqs and specify
the maximum size of each vq ring.
Signed-off-by: Xuan Zhuo
Acked-by: Jason Wang
---
include/linux/virtio_config.h | 12
1 file changed, 12 insertions(+)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 0f7def7ddfd2..22e29c926946 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -235,6 +235,18 @@ int virtio_find_vqs_ctx(struct virtio_device *vdev,
unsigned nvqs,
ctx, desc);
}
+static inline
+int virtio_find_vqs_ctx_size(struct virtio_device *vdev, u32 nvqs,
+struct virtqueue *vqs[],
+vq_callback_t *callbacks[],
+const char * const names[],
+u32 sizes[],
+const bool *ctx, struct irq_affinity *desc)
+{
+ return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, sizes,
+ ctx, desc);
+}
+
/**
* virtio_device_ready - enable vq use in probe function
* @vdev: the device
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 5.17 184/343] virtio_console: eliminate anonymous module_init & module_exit
From: Randy Dunlap
[ Upstream commit fefb8a2a941338d871e2d83fbd65fbfa068857bd ]
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Signed-off-by: Randy Dunlap
Reviewed-by: Amit Shah
Cc: virtualization@lists.linux-foundation.org
Cc: Arnd Bergmann
Link: https://lore.kernel.org/r/20220316192010.19001-3-rdun...@infradead.org
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/char/virtio_console.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e3c430539a17..9fa3c76a267f 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2245,7 +2245,7 @@ static struct virtio_driver virtio_rproc_serial = {
.remove = virtcons_remove,
};
-static int __init init(void)
+static int __init virtio_console_init(void)
{
int err;
@@ -2280,7 +2280,7 @@ static int __init init(void)
return err;
}
-static void __exit fini(void)
+static void __exit virtio_console_fini(void)
{
reclaim_dma_bufs();
@@ -2290,8 +2290,8 @@ static void __exit fini(void)
class_destroy(pdrvdata.class);
debugfs_remove_recursive(pdrvdata.debugfs_dir);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_console_init);
+module_exit(virtio_console_fini);
MODULE_DESCRIPTION("Virtio console driver");
MODULE_LICENSE("GPL");
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 26/32] virtio_mmio: support the arg sizes of find_vqs()
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Virtio MMIO support the new parameter sizes of find_vqs().
Signed-off-by: Xuan Zhuo
---
Acked-by: Jason Wang
drivers/virtio/virtio_mmio.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 9d5a674bdeec..51cf51764a92 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -347,7 +347,7 @@ static void vm_del_vqs(struct virtio_device *vdev)
static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index,
void (*callback)(struct virtqueue *vq),
- const char *name, bool ctx)
+ const char *name, u32 size, bool ctx)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
struct virtio_mmio_vq_info *info;
@@ -382,8 +382,11 @@ static struct virtqueue *vm_setup_vq(struct virtio_device
*vdev, unsigned index,
goto error_new_virtqueue;
}
+ if (!size || size > num)
+ size = num;
+
/* Create the vring */
- vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev,
+ vq = vring_create_virtqueue(index, size, VIRTIO_MMIO_VRING_ALIGN, vdev,
true, true, ctx, vm_notify, callback, name);
if (!vq) {
err = -ENOMEM;
@@ -484,6 +487,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned
nvqs,
}
vqs[i] = vm_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
+sizes ? sizes[i] : 0,
ctx ? ctx[i] : false);
if (IS_ERR(vqs[i])) {
vm_del_vqs(vdev);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 25/32] virtio_pci: support the arg sizes of find_vqs()
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Virtio PCI supports new parameter sizes of find_vqs().
Signed-off-by: Xuan Zhuo
Acked-by: Jason Wang
---
drivers/virtio/virtio_pci_common.c | 18 ++
drivers/virtio/virtio_pci_common.h | 1 +
drivers/virtio/virtio_pci_legacy.c | 6 +-
drivers/virtio/virtio_pci_modern.c | 10 +++---
4 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c
b/drivers/virtio/virtio_pci_common.c
index 826ea2e35d54..23976c61583f 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -208,6 +208,7 @@ static int vp_request_msix_vectors(struct virtio_device
*vdev, int nvectors,
static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned
index,
void (*callback)(struct virtqueue *vq),
const char *name,
+u32 size,
bool ctx,
u16 msix_vec)
{
@@ -220,7 +221,7 @@ static struct virtqueue *vp_setup_vq(struct virtio_device
*vdev, unsigned index,
if (!info)
return ERR_PTR(-ENOMEM);
- vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, ctx,
+ vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, size, ctx,
msix_vec);
if (IS_ERR(vq))
goto out_info;
@@ -314,7 +315,7 @@ void vp_del_vqs(struct virtio_device *vdev)
static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
- const char * const names[], bool per_vq_vectors,
+ const char * const names[], u32 sizes[], bool per_vq_vectors,
const bool *ctx,
struct irq_affinity *desc)
{
@@ -357,8 +358,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
unsigned nvqs,
else
msix_vec = VP_MSIX_VQ_VECTOR;
vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
-ctx ? ctx[i] : false,
-msix_vec);
+sizes ? sizes[i] : 0,
+ctx ? ctx[i] : false, msix_vec);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
goto error_find;
@@ -388,7 +389,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev,
unsigned nvqs,
static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
- const char * const names[], const bool *ctx)
+ const char * const names[], u32 sizes[], const bool *ctx)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
int i, err, queue_idx = 0;
@@ -410,6 +411,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev,
unsigned nvqs,
continue;
}
vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
+sizes ? sizes[i] : 0,
ctx ? ctx[i] : false,
VIRTIO_MSI_NO_VECTOR);
if (IS_ERR(vqs[i])) {
@@ -433,15 +435,15 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
int err;
/* Try MSI-X with one vector per queue. */
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
desc);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, sizes, true,
ctx, desc);
if (!err)
return 0;
/* Fallback: MSI-X with one vector for config, one shared for queues. */
- err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
desc);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, sizes, false,
ctx, desc);
if (!err)
return 0;
/* Finally fall back to regular interrupts. */
- return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx);
+ return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, sizes, ctx);
}
const char *vp_bus_name(struct virtio_device *vdev)
diff --git a/drivers/virtio/virtio_pci_common.h
b/drivers/virtio/virtio_pci_common.h
index 859eed559e10..fbf5a6d4b164 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -81,6 +81,7 @@ struct virtio_pci_device {
unsigned idx,
void (*callback)(struct virtqueue *vq),
const char *name,
+ u32 size,
bool ctx,
u16 msix_vec);
void
Re: [PATCH v9 24/32] virtio: find_vqs() add arg sizes
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
find_vqs() adds a new parameter sizes to specify the size of each vq
vring.
0 means use the maximum size supported by the backend.
Does this mean driver still need to prepare a array of 0 or it can
simply pass NULL to find_vqs()?
Thanks
In the split scenario, the meaning of size is the largest size, because
it may be limited by memory, the virtio core will try a smaller size.
And the size is power of 2.
Signed-off-by: Xuan Zhuo
Acked-by: Hans de Goede
Reviewed-by: Mathieu Poirier
---
arch/um/drivers/virtio_uml.c | 2 +-
drivers/platform/mellanox/mlxbf-tmfifo.c | 1 +
drivers/remoteproc/remoteproc_virtio.c | 1 +
drivers/s390/virtio/virtio_ccw.c | 1 +
drivers/virtio/virtio_mmio.c | 1 +
drivers/virtio/virtio_pci_common.c | 2 +-
drivers/virtio/virtio_pci_common.h | 2 +-
drivers/virtio/virtio_pci_modern.c | 7 +--
drivers/virtio/virtio_vdpa.c | 1 +
include/linux/virtio_config.h| 14 +-
10 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
index 904993d15a85..6af98d130840 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -998,7 +998,7 @@ static struct virtqueue *vu_setup_vq(struct virtio_device
*vdev,
static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
- const char * const names[], const bool *ctx,
+ const char * const names[], u32 sizes[], const bool *ctx,
struct irq_affinity *desc)
{
struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c
b/drivers/platform/mellanox/mlxbf-tmfifo.c
index 1ae3c56b66b0..8be13d416f48 100644
--- a/drivers/platform/mellanox/mlxbf-tmfifo.c
+++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
@@ -928,6 +928,7 @@ static int mlxbf_tmfifo_virtio_find_vqs(struct
virtio_device *vdev,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
const char * const names[],
+ u32 sizes[],
const bool *ctx,
struct irq_affinity *desc)
{
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index 7611755d0ae2..baad31c9da45 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -158,6 +158,7 @@ static int rproc_virtio_find_vqs(struct virtio_device
*vdev, unsigned int nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
const char * const names[],
+u32 sizes[],
const bool * ctx,
struct irq_affinity *desc)
{
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 468da60b56c5..f0c814a54e78 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -634,6 +634,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev,
unsigned nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
const char * const names[],
+ u32 sizes[],
const bool *ctx,
struct irq_affinity *desc)
{
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index a41abc8051b9..9d5a674bdeec 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -461,6 +461,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned
nvqs,
struct virtqueue *vqs[],
vq_callback_t *callbacks[],
const char * const names[],
+ u32 sizes[],
const bool *ctx,
struct irq_affinity *desc)
{
diff --git a/drivers/virtio/virtio_pci_common.c
b/drivers/virtio/virtio_pci_common.c
index 863d3a8a0956..826ea2e35d54 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -427,7 +427,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev,
unsigned nvqs,
/* the config->find_vqs() implementation */
int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
struct virtqueue *vqs[], vq_callback_t *callbacks[],
- const char * const names[], const bool *ctx,
+ const char * const names[], u32 sizes[], const bool *ctx,
struct irq_affinity
Re: [PATCH v9 23/32] virtio_pci: queue_reset: support VIRTIO_F_RING_RESET
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
This patch implements virtio pci support for QUEUE RESET.
Performing reset on a queue is divided into these steps:
1. notify the device to reset the queue
2. recycle the buffer submitted
3. reset the vring (may re-alloc)
4. mmap vring to device, and enable the queue
This patch implements virtio_reset_vq(), virtio_enable_resetq() in the
pci scenario.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_pci_common.c | 8 +--
drivers/virtio/virtio_pci_modern.c | 84 ++
drivers/virtio/virtio_ring.c | 2 +
include/linux/virtio.h | 1 +
4 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c
b/drivers/virtio/virtio_pci_common.c
index fdbde1db5ec5..863d3a8a0956 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -248,9 +248,11 @@ static void vp_del_vq(struct virtqueue *vq)
struct virtio_pci_vq_info *info = vp_dev->vqs[vq->index];
unsigned long flags;
- spin_lock_irqsave(_dev->lock, flags);
- list_del(>node);
- spin_unlock_irqrestore(_dev->lock, flags);
+ if (!vq->reset) {
On which condition that we may hit this path?
+ spin_lock_irqsave(_dev->lock, flags);
+ list_del(>node);
+ spin_unlock_irqrestore(_dev->lock, flags);
+ }
vp_dev->del_vq(info);
kfree(info);
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 49a4493732cf..cb5d38f1c9c8 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -34,6 +34,9 @@ static void vp_transport_features(struct virtio_device *vdev,
u64 features)
if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
__virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+
+ if (features & BIT_ULL(VIRTIO_F_RING_RESET))
+ __virtio_set_bit(vdev, VIRTIO_F_RING_RESET);
}
/* virtio config->finalize_features() implementation */
@@ -199,6 +202,83 @@ static int vp_active_vq(struct virtqueue *vq, u16 msix_vec)
return 0;
}
+static int vp_modern_reset_vq(struct virtqueue *vq)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
+ struct virtio_pci_modern_device *mdev = _dev->mdev;
+ struct virtio_pci_vq_info *info;
+ unsigned long flags;
+
+ if (!virtio_has_feature(vq->vdev, VIRTIO_F_RING_RESET))
+ return -ENOENT;
+
+ vp_modern_set_queue_reset(mdev, vq->index);
+
+ info = vp_dev->vqs[vq->index];
+
+ /* delete vq from irq handler */
+ spin_lock_irqsave(_dev->lock, flags);
+ list_del(>node);
+ spin_unlock_irqrestore(_dev->lock, flags);
+
+ INIT_LIST_HEAD(>node);
+
+ /* For the case where vq has an exclusive irq, to prevent the irq from
+* being received again and the pending irq, call disable_irq().
+*
+* In the scenario based on shared interrupts, vq will be searched from
+* the queue virtqueues. Since the previous list_del() has been deleted
+* from the queue, it is impossible for vq to be called in this case.
+* There is no need to close the corresponding interrupt.
+*/
+ if (vp_dev->per_vq_vectors && info->msix_vector != VIRTIO_MSI_NO_VECTOR)
+ disable_irq(pci_irq_vector(vp_dev->pci_dev, info->msix_vector));
See the previous discussion and the revert of the first try to harden
the interrupt. We probably can't use disable_irq() since it conflicts
with the affinity managed IRQ that is used by some drivers.
We need to use synchonize_irq() and per virtqueue flag instead. As
mentioned in previous patches, this could be done on top of my rework on
the IRQ hardening .
+
+ vq->reset = true;
+
+ return 0;
+}
+
+static int vp_modern_enable_reset_vq(struct virtqueue *vq)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
+ struct virtio_pci_modern_device *mdev = _dev->mdev;
+ struct virtio_pci_vq_info *info;
+ unsigned long flags, index;
+ int err;
+
+ if (!vq->reset)
+ return -EBUSY;
+
+ index = vq->index;
+ info = vp_dev->vqs[index];
+
+ /* check queue reset status */
+ if (vp_modern_get_queue_reset(mdev, index) != 1)
+ return -EBUSY;
+
+ err = vp_active_vq(vq, info->msix_vector);
+ if (err)
+ return err;
+
+ if (vq->callback) {
+ spin_lock_irqsave(_dev->lock, flags);
+ list_add(>node, _dev->virtqueues);
+ spin_unlock_irqrestore(_dev->lock, flags);
+ } else {
+ INIT_LIST_HEAD(>node);
+ }
+
+ vp_modern_set_queue_enable(_dev->mdev, index, true);
+
+ if (vp_dev->per_vq_vectors && info->msix_vector != VIRTIO_MSI_NO_VECTOR)
+
[PATCH 5.16 140/285] virtio_console: eliminate anonymous module_init & module_exit
From: Randy Dunlap
[ Upstream commit fefb8a2a941338d871e2d83fbd65fbfa068857bd ]
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Signed-off-by: Randy Dunlap
Reviewed-by: Amit Shah
Cc: virtualization@lists.linux-foundation.org
Cc: Arnd Bergmann
Link: https://lore.kernel.org/r/20220316192010.19001-3-rdun...@infradead.org
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/char/virtio_console.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index f864b17be7e3..35025f283bf6 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2245,7 +2245,7 @@ static struct virtio_driver virtio_rproc_serial = {
.remove = virtcons_remove,
};
-static int __init init(void)
+static int __init virtio_console_init(void)
{
int err;
@@ -2280,7 +2280,7 @@ static int __init init(void)
return err;
}
-static void __exit fini(void)
+static void __exit virtio_console_fini(void)
{
reclaim_dma_bufs();
@@ -2290,8 +2290,8 @@ static void __exit fini(void)
class_destroy(pdrvdata.class);
debugfs_remove_recursive(pdrvdata.debugfs_dir);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_console_init);
+module_exit(virtio_console_fini);
MODULE_DESCRIPTION("Virtio console driver");
MODULE_LICENSE("GPL");
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 22/32] virtio_pci: queue_reset: extract the logic of active vq for modern pci
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Introduce vp_active_vq() to configure vring to backend after vq attach
vring. And configure vq vector if necessary.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_pci_modern.c | 46 ++
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 86d301f272b8..49a4493732cf 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -176,6 +176,29 @@ static void vp_reset(struct virtio_device *vdev)
vp_disable_cbs(vdev);
}
+static int vp_active_vq(struct virtqueue *vq, u16 msix_vec)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
+ struct virtio_pci_modern_device *mdev = _dev->mdev;
+ unsigned long index;
+
+ index = vq->index;
+
+ /* activate the queue */
+ vp_modern_set_queue_size(mdev, index, virtqueue_get_vring_size(vq));
+ vp_modern_queue_address(mdev, index, virtqueue_get_desc_addr(vq),
+ virtqueue_get_avail_addr(vq),
+ virtqueue_get_used_addr(vq));
+
+ if (msix_vec != VIRTIO_MSI_NO_VECTOR) {
+ msix_vec = vp_modern_queue_vector(mdev, index, msix_vec);
+ if (msix_vec == VIRTIO_MSI_NO_VECTOR)
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
{
return vp_modern_config_vector(_dev->mdev, vector);
@@ -220,32 +243,19 @@ static struct virtqueue *setup_vq(struct
virtio_pci_device *vp_dev,
vq->num_max = num;
- /* activate the queue */
- vp_modern_set_queue_size(mdev, index, virtqueue_get_vring_size(vq));
- vp_modern_queue_address(mdev, index, virtqueue_get_desc_addr(vq),
- virtqueue_get_avail_addr(vq),
- virtqueue_get_used_addr(vq));
+ err = vp_active_vq(vq, msix_vec);
+ if (err)
+ goto err;
vq->priv = (void __force *)vp_modern_map_vq_notify(mdev, index, NULL);
if (!vq->priv) {
err = -ENOMEM;
- goto err_map_notify;
- }
-
- if (msix_vec != VIRTIO_MSI_NO_VECTOR) {
- msix_vec = vp_modern_queue_vector(mdev, index, msix_vec);
- if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
- err = -EBUSY;
- goto err_assign_vector;
- }
+ goto err;
}
return vq;
-err_assign_vector:
- if (!mdev->notify_base)
- pci_iounmap(mdev->pci_dev, (void __iomem __force *)vq->priv);
We need keep this or anything I missed?
Thanks
-err_map_notify:
+err:
vring_del_virtqueue(vq);
return ERR_PTR(err);
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re:
On Tue, Mar 29, 2022 at 10:35:21AM +0200, Thomas Gleixner wrote: > On Mon, Mar 28 2022 at 06:40, Michael S. Tsirkin wrote: > > On Mon, Mar 28, 2022 at 02:18:22PM +0800, Jason Wang wrote: > >> > > So I think we might talk different issues: > >> > > > >> > > 1) Whether request_irq() commits the previous setups, I think the > >> > > answer is yes, since the spin_unlock of desc->lock (release) can > >> > > guarantee this though there seems no documentation around > >> > > request_irq() to say this. > >> > > > >> > > And I can see at least drivers/video/fbdev/omap2/omapfb/dss/dispc.c is > >> > > using smp_wmb() before the request_irq(). > > That's a complete bogus example especially as there is not a single > smp_rmb() which pairs with the smp_wmb(). > > >> > > And even if write is ordered we still need read to be ordered to be > >> > > paired with that. > > > > IMO it synchronizes with the CPU to which irq is > > delivered. Otherwise basically all drivers would be broken, > > wouldn't they be? > > I don't know whether it's correct on all platforms, but if not > > we need to fix request_irq. > > There is nothing to fix: > > request_irq() >raw_spin_lock_irq(desc->lock); // ACQUIRE > >raw_spin_unlock_irq(desc->lock); // RELEASE > > interrupt() >raw_spin_lock(desc->lock); // ACQUIRE >set status to IN_PROGRESS >raw_spin_unlock(desc->lock); // RELEASE >invoke handler() > > So anything which the driver set up _before_ request_irq() is visible to > the interrupt handler. No? > > >> What happens if an interrupt is raised in the middle like: > >> > >> smp_store_release(dev->irq_soft_enabled, true) > >> IRQ handler > >> synchornize_irq() > > This is bogus. The obvious order of things is: > > dev->ok = false; > request_irq(); > > moar_setup(); > synchronize_irq(); // ACQUIRE + RELEASE > dev->ok = true; > > The reverse operation on teardown: > > dev->ok = false; > synchronize_irq(); // ACQUIRE + RELEASE > > teardown(); > > So in both cases a simple check in the handler is sufficient: > > handler() > if (!dev->ok) > return; Does this need to be if (!READ_ONCE(dev->ok)) ? > I'm not understanding what you folks are trying to "fix" here. If any > driver does this in the wrong order, then the driver is broken. > > Sure, you can do the same with: > > dev->ok = false; > request_irq(); > moar_setup(); > smp_wmb(); > dev->ok = true; > > for the price of a smp_rmb() in the interrupt handler: > > handler() > if (!dev->ok) > return; > smp_rmb(); > > but that's only working for the setup case correctly and not for > teardown. > > Thanks, > > tglx ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 5.15 135/277] virtio_console: eliminate anonymous module_init & module_exit
From: Randy Dunlap
[ Upstream commit fefb8a2a941338d871e2d83fbd65fbfa068857bd ]
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Signed-off-by: Randy Dunlap
Reviewed-by: Amit Shah
Cc: virtualization@lists.linux-foundation.org
Cc: Arnd Bergmann
Link: https://lore.kernel.org/r/20220316192010.19001-3-rdun...@infradead.org
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/char/virtio_console.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 3adf04766e98..77bc993d7513 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2236,7 +2236,7 @@ static struct virtio_driver virtio_rproc_serial = {
.remove = virtcons_remove,
};
-static int __init init(void)
+static int __init virtio_console_init(void)
{
int err;
@@ -2271,7 +2271,7 @@ static int __init init(void)
return err;
}
-static void __exit fini(void)
+static void __exit virtio_console_fini(void)
{
reclaim_dma_bufs();
@@ -2281,8 +2281,8 @@ static void __exit fini(void)
class_destroy(pdrvdata.class);
debugfs_remove_recursive(pdrvdata.debugfs_dir);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_console_init);
+module_exit(virtio_console_fini);
MODULE_DESCRIPTION("Virtio console driver");
MODULE_LICENSE("GPL");
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 21/32] virtio_pci: queue_reset: update struct virtio_pci_common_cfg and option functions
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Add queue_reset in virtio_pci_common_cfg, and add related operation
functions.
For not breaks uABI, add a new struct virtio_pci_common_cfg_reset.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_pci_modern_dev.c | 36 ++
include/linux/virtio_pci_modern.h | 2 ++
include/uapi/linux/virtio_pci.h| 7 +
3 files changed, 45 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern_dev.c
b/drivers/virtio/virtio_pci_modern_dev.c
index e8b3ff2b9fbc..8c74b00bc511 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -3,6 +3,7 @@
#include
#include
#include
+#include
/*
* vp_modern_map_capability - map a part of virtio pci capability
@@ -463,6 +464,41 @@ void vp_modern_set_status(struct virtio_pci_modern_device
*mdev,
}
EXPORT_SYMBOL_GPL(vp_modern_set_status);
+/*
+ * vp_modern_get_queue_reset - get the queue reset status
+ * @mdev: the modern virtio-pci device
+ * @index: queue index
+ */
+int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16 index)
+{
+ struct virtio_pci_common_cfg_reset __iomem *cfg;
+
+ cfg = (struct virtio_pci_common_cfg_reset __iomem *)mdev->common;
+
+ vp_iowrite16(index, >cfg.queue_select);
+ return vp_ioread16(>queue_reset);
+}
+EXPORT_SYMBOL_GPL(vp_modern_get_queue_reset);
+
+/*
+ * vp_modern_set_queue_reset - reset the queue
+ * @mdev: the modern virtio-pci device
+ * @index: queue index
+ */
+void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16
index)
+{
+ struct virtio_pci_common_cfg_reset __iomem *cfg;
+
+ cfg = (struct virtio_pci_common_cfg_reset __iomem *)mdev->common;
+
+ vp_iowrite16(index, >cfg.queue_select);
+ vp_iowrite16(1, >queue_reset);
+
+ while (vp_ioread16(>queue_reset) != 1)
+ msleep(1);
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_queue_reset);
+
/*
* vp_modern_queue_vector - set the MSIX vector for a specific virtqueue
* @mdev: the modern virtio-pci device
diff --git a/include/linux/virtio_pci_modern.h
b/include/linux/virtio_pci_modern.h
index eb2bd9b4077d..cc4154dd7b28 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -106,4 +106,6 @@ void __iomem * vp_modern_map_vq_notify(struct
virtio_pci_modern_device *mdev,
u16 index, resource_size_t *pa);
int vp_modern_probe(struct virtio_pci_modern_device *mdev);
void vp_modern_remove(struct virtio_pci_modern_device *mdev);
+int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16
index);
+void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16
index);
#endif
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 22bec9bd0dfc..d9462efd6ce8 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -173,6 +173,13 @@ struct virtio_pci_common_cfg_notify {
__le16 padding;
};
+struct virtio_pci_common_cfg_reset {
+ struct virtio_pci_common_cfg cfg;
+
+ __le16 queue_notify_data; /* read-write */
+ __le16 queue_reset; /* read-write */
+};
I prefer to use a separate patch for the uAPI change.
Other looks good.
Thanks
+
/* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
struct virtio_pci_cfg_cap {
struct virtio_pci_cap cap;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 18/32] virtio_ring: introduce virtqueue_resize()
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Introduce virtqueue_resize() to implement the resize of vring.
Based on these, the driver can dynamically adjust the size of the vring.
For example: ethtool -G.
virtqueue_resize() implements resize based on the vq reset function. In
case of failure to allocate a new vring, it will give up resize and use
the original vring.
During this process, if the re-enable reset vq fails, the vq can no
longer be used. Although the probability of this situation is not high.
The parameter recycle is used to recycle the buffer that is no longer
used.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 69
include/linux/virtio.h | 3 ++
2 files changed, 72 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 06f66b15c86c..6250e19fc5bf 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2554,6 +2554,75 @@ struct virtqueue *vring_create_virtqueue(
}
EXPORT_SYMBOL_GPL(vring_create_virtqueue);
+/**
+ * virtqueue_resize - resize the vring of vq
+ * @_vq: the struct virtqueue we're talking about.
+ * @num: new ring num
+ * @recycle: callback for recycle the useless buffer
+ *
+ * When it is really necessary to create a new vring, it will set the current
vq
+ * into the reset state. Then call the passed callback to recycle the buffer
+ * that is no longer used. Only after the new vring is successfully created,
the
+ * old vring will be released.
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error.
Should we document that the virtqueue is kept unchanged (still
available) on (specific) failure?
+ */
+int virtqueue_resize(struct virtqueue *_vq, u32 num,
+void (*recycle)(struct virtqueue *vq, void *buf))
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ struct virtio_device *vdev = vq->vq.vdev;
+ bool packed;
+ void *buf;
+ int err;
+
+ if (!vq->we_own_ring)
+ return -EINVAL;
+
+ if (num > vq->vq.num_max)
+ return -E2BIG;
+
+ if (!num)
+ return -EINVAL;
+
+ packed = virtio_has_feature(vdev, VIRTIO_F_RING_PACKED) ? true : false;
+
+ if ((packed ? vq->packed.vring.num : vq->split.vring.num) == num)
+ return 0;
+
+ if (!vdev->config->reset_vq)
+ return -ENOENT;
+
+ if (!vdev->config->enable_reset_vq)
+ return -ENOENT;
+
+ err = vdev->config->reset_vq(_vq);
+ if (err)
+ return err;
+
+ while ((buf = virtqueue_detach_unused_buf(_vq)) != NULL)
+ recycle(_vq, buf);
+
+ if (packed) {
+ err = virtqueue_resize_packed(_vq, num);
+ if (err)
+ virtqueue_reinit_packed(vq);
Calling reinit here seems a little bit odd, it looks more like a reset
of the virtqueue. Consider we may re-use virtqueue reset for more
purpose, I wonder if we need a helper like:
virtqueue_resize() {
vdev->config->reset_vq(_vq);
if (packed)
virtqueue_reinit_packed(_vq)
else
virtqueue_reinit_split(_vq)
}
Thanks
+ } else {
+ err = virtqueue_resize_split(_vq, num);
+ if (err)
+ virtqueue_reinit_split(vq);
+ }
+
+ if (vdev->config->enable_reset_vq(_vq))
+ return -EBUSY;
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(virtqueue_resize);
+
/* Only available for split ring */
struct virtqueue *vring_new_virtqueue(unsigned int index,
unsigned int num,
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d59adc4be068..c86ff02e0ca0 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -91,6 +91,9 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq);
dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
+int virtqueue_resize(struct virtqueue *vq, u32 num,
+void (*recycle)(struct virtqueue *vq, void *buf));
+
/**
* virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 5.10 089/171] virtio_console: eliminate anonymous module_init & module_exit
From: Randy Dunlap
[ Upstream commit fefb8a2a941338d871e2d83fbd65fbfa068857bd ]
Eliminate anonymous module_init() and module_exit(), which can lead to
confusion or ambiguity when reading System.map, crashes/oops/bugs,
or an initcall_debug log.
Give each of these init and exit functions unique driver-specific
names to eliminate the anonymous names.
Example 1: (System.map)
832fc78c t init
832fc79e t init
832fc8f8 t init
Example 2: (initcall_debug log)
calling init+0x0/0x12 @ 1
initcall init+0x0/0x12 returned 0 after 15 usecs
calling init+0x0/0x60 @ 1
initcall init+0x0/0x60 returned 0 after 2 usecs
calling init+0x0/0x9a @ 1
initcall init+0x0/0x9a returned 0 after 74 usecs
Signed-off-by: Randy Dunlap
Reviewed-by: Amit Shah
Cc: virtualization@lists.linux-foundation.org
Cc: Arnd Bergmann
Link: https://lore.kernel.org/r/20220316192010.19001-3-rdun...@infradead.org
Signed-off-by: Greg Kroah-Hartman
Signed-off-by: Sasha Levin
---
drivers/char/virtio_console.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 3dd4deb60adb..6d361420ffe8 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2239,7 +2239,7 @@ static struct virtio_driver virtio_rproc_serial = {
.remove = virtcons_remove,
};
-static int __init init(void)
+static int __init virtio_console_init(void)
{
int err;
@@ -2276,7 +2276,7 @@ static int __init init(void)
return err;
}
-static void __exit fini(void)
+static void __exit virtio_console_fini(void)
{
reclaim_dma_bufs();
@@ -2286,8 +2286,8 @@ static void __exit fini(void)
class_destroy(pdrvdata.class);
debugfs_remove_recursive(pdrvdata.debugfs_dir);
}
-module_init(init);
-module_exit(fini);
+module_init(virtio_console_init);
+module_exit(virtio_console_fini);
MODULE_DESCRIPTION("Virtio console driver");
MODULE_LICENSE("GPL");
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 12/32] virtio_ring: packed: extract the logic of alloc queue
在 2022/4/6 上午11:43, Xuan Zhuo 写道:
Separate the logic of packed to create vring queue.
For the convenience of passing parameters, add a structure
vring_packed.
This feature is required for subsequent virtuqueue reset vring.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 70
1 file changed, 56 insertions(+), 14 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 33864134a744..ea451ae2aaef 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1817,19 +1817,17 @@ static struct vring_desc_extra
*vring_alloc_desc_extra(unsigned int num)
return desc_extra;
}
-static struct virtqueue *vring_create_virtqueue_packed(
- unsigned int index,
- unsigned int num,
- unsigned int vring_align,
- struct virtio_device *vdev,
- bool weak_barriers,
- bool may_reduce_num,
- bool context,
- bool (*notify)(struct virtqueue *),
- void (*callback)(struct virtqueue *),
- const char *name)
+static int vring_alloc_queue_packed(struct virtio_device *vdev,
+ u32 num,
+ struct vring_packed_desc **_ring,
+ struct vring_packed_desc_event **_driver,
+ struct vring_packed_desc_event **_device,
+ dma_addr_t *_ring_dma_addr,
+ dma_addr_t *_driver_event_dma_addr,
+ dma_addr_t *_device_event_dma_addr,
+ size_t *_ring_size_in_bytes,
+ size_t *_event_size_in_bytes)
{
- struct vring_virtqueue *vq;
struct vring_packed_desc *ring;
struct vring_packed_desc_event *driver, *device;
dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr;
@@ -1857,6 +1855,52 @@ static struct virtqueue *vring_create_virtqueue_packed(
if (!device)
goto err_device;
+ *_ring = ring;
+ *_driver = driver;
+ *_device = device;
+ *_ring_dma_addr = ring_dma_addr;
+ *_driver_event_dma_addr = driver_event_dma_addr;
+ *_device_event_dma_addr = device_event_dma_addr;
+ *_ring_size_in_bytes = ring_size_in_bytes;
+ *_event_size_in_bytes= event_size_in_bytes;
I wonder if we can simply factor out split and packed from struct
vring_virtqueue:
struct vring_virtqueue {
union {
struct {} split;
struct {} packed;
};
};
to
struct vring_virtqueue_split {};
struct vring_virtqueue_packed {};
Then we can do things like:
vring_create_virtqueue_packed(struct virtio_device *vdev, u32 num,
struct vring_virtqueue_packed *packed);
and
vring_vritqueue_attach_packed(struct vring_virtqueue *vq, struct
vring_virtqueue_packed packed);
Thanks
+
+ return 0;
+
+err_device:
+ vring_free_queue(vdev, event_size_in_bytes, driver,
driver_event_dma_addr);
+
+err_driver:
+ vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr);
+
+err_ring:
+ return -ENOMEM;
+}
+
+static struct virtqueue *vring_create_virtqueue_packed(
+ unsigned int index,
+ unsigned int num,
+ unsigned int vring_align,
+ struct virtio_device *vdev,
+ bool weak_barriers,
+ bool may_reduce_num,
+ bool context,
+ bool (*notify)(struct virtqueue *),
+ void (*callback)(struct virtqueue *),
+ const char *name)
+{
+ dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr;
+ struct vring_packed_desc_event *driver, *device;
+ size_t ring_size_in_bytes, event_size_in_bytes;
+ struct vring_packed_desc *ring;
+ struct vring_virtqueue *vq;
+
+ if (vring_alloc_queue_packed(vdev, num, , , ,
+_dma_addr, _event_dma_addr,
+_event_dma_addr,
+_size_in_bytes,
+_size_in_bytes))
+ goto err_ring;
+
vq = kmalloc(sizeof(*vq), GFP_KERNEL);
if (!vq)
goto err_vq;
@@ -1939,9 +1983,7 @@ static struct virtqueue *vring_create_virtqueue_packed(
kfree(vq);
err_vq:
vring_free_queue(vdev, event_size_in_bytes, device,
device_event_dma_addr);
-err_device:
vring_free_queue(vdev, event_size_in_bytes, driver,
driver_event_dma_addr);
-err_driver:
vring_free_queue(vdev, ring_size_in_bytes, ring, ring_dma_addr);
err_ring:
return NULL;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
