All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH V4 4/5] virtio: introduce a vDPA based transport
From: Jason Gunthorpe @ 2020-02-20 15:19 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
	tiwei.bie@intel.com, maxime.coquelin@redhat.com,
	cunming.liang@intel.com, zhihong.wang@intel.com,
	rob.miller@broadcom.com, xiao.w.wang@intel.com,
	haotian.wang@sifive.com, lingshan.zhu@intel.com,
	eperezma@redhat.com, lulu@redhat.com, Parav Pandit,
	kevin.tian@intel.com, stefanha@redhat.com, rdunlap@infradead.org,
	hch@infradead.org, aadam@redhat.com, Jiri Pirko, Shahaf Shuler,
	hanand@xilinx.com, mhabets@solarflare.com
In-Reply-To: <20200220061141.29390-5-jasowang@redhat.com>

On Thu, Feb 20, 2020 at 02:11:40PM +0800, Jason Wang wrote:
> +static int virtio_vdpa_probe(struct vdpa_device *vdpa)
> +{
> +	const struct vdpa_config_ops *ops = vdpa->config;
> +	struct virtio_vdpa_device *vd_dev;
> +	int ret = -EINVAL;
> +
> +	vd_dev = kzalloc(sizeof(*vd_dev), GFP_KERNEL);
> +	if (!vd_dev)
> +		return -ENOMEM;
> +
> +	vd_dev->vdev.dev.parent = vdpa_get_dma_dev(vdpa);
> +	vd_dev->vdev.dev.release = virtio_vdpa_release_dev;
> +	vd_dev->vdev.config = &virtio_vdpa_config_ops;
> +	vd_dev->vdpa = vdpa;
> +	INIT_LIST_HEAD(&vd_dev->virtqueues);
> +	spin_lock_init(&vd_dev->lock);
> +
> +	vd_dev->vdev.id.device = ops->get_device_id(vdpa);
> +	if (vd_dev->vdev.id.device == 0)
> +		goto err;
> +
> +	vd_dev->vdev.id.vendor = ops->get_vendor_id(vdpa);
> +	ret = register_virtio_device(&vd_dev->vdev);
> +	if (ret)
> +		goto err;

This error unwind is wrong. register_virtio_device() does
device_initialize() as it's first action. After that point error
unwind must be done with put_device() - particularly calling
kfree(vd_dev) after doing dev_set_name() leaks memory.

Looks like about half of the register_virtio_device() users did this
right, the others not. Perhaps you should fix them too...

Jason

^ permalink raw reply

* Re: [PATCH V4 4/5] virtio: introduce a vDPA based transport
From: Jason Gunthorpe @ 2020-02-20 15:19 UTC (permalink / raw)
  To: Jason Wang
  Cc: kvm@vger.kernel.org, mst@redhat.com, mhabets@solarflare.com,
	virtualization@lists.linux-foundation.org,
	rob.miller@broadcom.com, lulu@redhat.com, hanand@xilinx.com,
	hch@infradead.org, eperezma@redhat.com, haotian.wang@sifive.com,
	Shahaf Shuler, Parav Pandit, Jiri Pirko, xiao.w.wang@intel.com,
	stefanha@redhat.com, zhihong.wang@intel.com,
	rdunlap@infradead.org, linux-kernel@vger.kernel.org,
	maxime.coquelin
In-Reply-To: <20200220061141.29390-5-jasowang@redhat.com>

On Thu, Feb 20, 2020 at 02:11:40PM +0800, Jason Wang wrote:
> +static int virtio_vdpa_probe(struct vdpa_device *vdpa)
> +{
> +	const struct vdpa_config_ops *ops = vdpa->config;
> +	struct virtio_vdpa_device *vd_dev;
> +	int ret = -EINVAL;
> +
> +	vd_dev = kzalloc(sizeof(*vd_dev), GFP_KERNEL);
> +	if (!vd_dev)
> +		return -ENOMEM;
> +
> +	vd_dev->vdev.dev.parent = vdpa_get_dma_dev(vdpa);
> +	vd_dev->vdev.dev.release = virtio_vdpa_release_dev;
> +	vd_dev->vdev.config = &virtio_vdpa_config_ops;
> +	vd_dev->vdpa = vdpa;
> +	INIT_LIST_HEAD(&vd_dev->virtqueues);
> +	spin_lock_init(&vd_dev->lock);
> +
> +	vd_dev->vdev.id.device = ops->get_device_id(vdpa);
> +	if (vd_dev->vdev.id.device == 0)
> +		goto err;
> +
> +	vd_dev->vdev.id.vendor = ops->get_vendor_id(vdpa);
> +	ret = register_virtio_device(&vd_dev->vdev);
> +	if (ret)
> +		goto err;

This error unwind is wrong. register_virtio_device() does
device_initialize() as it's first action. After that point error
unwind must be done with put_device() - particularly calling
kfree(vd_dev) after doing dev_set_name() leaks memory.

Looks like about half of the register_virtio_device() users did this
right, the others not. Perhaps you should fix them too...

Jason

^ permalink raw reply

* Re: [PATCH v1 05/13] migrate/ram: Handle RAM block resizes during precopy
From: David Hildenbrand @ 2020-02-20 15:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Juan Quintela, Michael S. Tsirkin,
	Richard Henderson, Dr . David Alan Gilbert, Peter Xu,
	Shannon Zhao, Igor Mammedov, Paolo Bonzini, Alex Bennée,
	Richard Henderson
In-Reply-To: <20200219161725.115218-6-david@redhat.com>

On 19.02.20 17:17, David Hildenbrand wrote:
> Resizing while migrating is dangerous and does not work as expected.
> The whole migration code works on the usable_length of ram blocks and does
> not expect this to change at random points in time.
> 
> In the case of precopy, the ram block size must not change on the source,
> after syncing the RAM block list in ram_save_setup(), so as long as the
> guest is still running on the source.
> 
> Resizing can be trigger *after* (but not during) a reset in
> ACPI code by the guest
> - hw/arm/virt-acpi-build.c:acpi_ram_update()
> - hw/i386/acpi-build.c:acpi_ram_update()
> 
> Use the ram block notifier to get notified about resizes. Let's simply
> cancel migration and indicate the reason. We'll continue running on the
> source. No harm done.
> 
> Update the documentation. Postcopy will be handled separately.
> 
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Shannon Zhao <shannon.zhao@linaro.org>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Peter Xu <peterx@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  exec.c                |  5 +++--
>  include/exec/memory.h | 10 ++++++----
>  migration/migration.c |  9 +++++++--
>  migration/migration.h |  1 +
>  migration/ram.c       | 41 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 58 insertions(+), 8 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index b75250e773..8b015821d6 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2120,8 +2120,9 @@ static int memory_try_enable_merging(void *addr, size_t len)
>      return qemu_madvise(addr, len, QEMU_MADV_MERGEABLE);
>  }
>  
> -/* Only legal before guest might have detected the memory size: e.g. on
> - * incoming migration, or right after reset.
> +/*
> + * Resizing RAM while migrating can result in the migration being canceled.
> + * Care has to be taken if the guest might have already detected the memory.
>   *
>   * As memory core doesn't know how is memory accessed, it is up to
>   * resize callback to update device state and/or add assertions to detect
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e85b7de99a..de111347e8 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -113,7 +113,7 @@ typedef struct IOMMUNotifier IOMMUNotifier;
>  #define RAM_SHARED     (1 << 1)
>  
>  /* Only a portion of RAM (used_length) is actually used, and migrated.
> - * This used_length size can change across reboots.
> + * Resizing RAM while migrating can result in the migration being canceled.
>   */
>  #define RAM_RESIZEABLE (1 << 2)
>  
> @@ -843,7 +843,9 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
>   *                                     RAM.  Accesses into the region will
>   *                                     modify memory directly.  Only an initial
>   *                                     portion of this RAM is actually used.
> - *                                     The used size can change across reboots.
> + *                                     Changing the size while migrating
> + *                                     can result in the migration being
> + *                                     canceled.
>   *
>   * @mr: the #MemoryRegion to be initialized.
>   * @owner: the object that tracks the region's reference count
> @@ -1464,8 +1466,8 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr);
>  
>  /* memory_region_ram_resize: Resize a RAM region.
>   *
> - * Only legal before guest might have detected the memory size: e.g. on
> - * incoming migration, or right after reset.
> + * Resizing RAM while migrating can result in the migration being canceled.
> + * Care has to be taken if the guest might have already detected the memory.
>   *
>   * @mr: a memory region created with @memory_region_init_resizeable_ram.
>   * @newsize: the new size the region
> diff --git a/migration/migration.c b/migration/migration.c
> index 8fb68795dc..ac9751dbe5 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -175,13 +175,18 @@ void migration_object_init(void)
>      }
>  }
>  
> +void migration_cancel(void)
> +{
> +    migrate_fd_cancel(current_migration);
> +}
> +
>  void migration_shutdown(void)
>  {
>      /*
>       * Cancel the current migration - that will (eventually)
>       * stop the migration using this structure
>       */
> -    migrate_fd_cancel(current_migration);
> +    migration_cancel();
>      object_unref(OBJECT(current_migration));
>  }
>  
> @@ -2019,7 +2024,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>  
>  void qmp_migrate_cancel(Error **errp)
>  {
> -    migrate_fd_cancel(migrate_get_current());
> +    migration_cancel();
>  }
>  
>  void qmp_migrate_continue(MigrationStatus state, Error **errp)
> diff --git a/migration/migration.h b/migration/migration.h
> index 8473ddfc88..79fd74afa5 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -343,5 +343,6 @@ int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque);
>  void migration_make_urgent_request(void);
>  void migration_consume_urgent_request(void);
>  bool migration_rate_limit(void);
> +void migration_cancel(void);
>  
>  #endif
> diff --git a/migration/ram.c b/migration/ram.c
> index ed23ed1c7c..57f32011a3 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -52,6 +52,7 @@
>  #include "migration/colo.h"
>  #include "block.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/runstate.h"
>  #include "savevm.h"
>  #include "qemu/iov.h"
>  #include "multifd.h"
> @@ -3710,8 +3711,48 @@ static SaveVMHandlers savevm_ram_handlers = {
>      .resume_prepare = ram_resume_prepare,
>  };
>  
> +static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
> +                                      size_t old_size, size_t new_size)
> +{
> +    ram_addr_t offset;
> +    Error *err = NULL;
> +    RAMBlock *rb = qemu_ram_block_from_host(host, false, &offset);
> +
> +    if (ramblock_is_ignored(rb)) {
> +        return;
> +    }
> +
> +    /*
> +     * Some resizes are triggered on the migration target by precopy code,
> +     * when synchronizing RAM block sizes. In these cases, the VM is not
> +     * running and migration is not idle. We have to ignore these resizes,
> +     * as we only care about resizes during precopy on the migration source.
> +     * This handler is always registered, so ignore when migration is idle.
> +     */
> +    if (migration_is_idle() || !runstate_is_running() ||
> +        postcopy_is_running()) {
> +        return;
> +    }
> +
> +    /*
> +     * Precopy code cannot deal with the size of ram blocks changing at
> +     * random points in time. We're still running on the source, abort
> +     * the migration and continue running here. Make sure to wait until
> +     * migration was canceled.
> +     */
> +    error_setg(&err, "RAM block '%s' resized during precopy.", rb->idstr);
> +    migrate_set_error(migrate_get_current(), err);
> +    error_free(err);
> +    migration_cancel();
> +}
> +
> +static RAMBlockNotifier ram_mig_ram_notifier = {
> +    .ram_block_resized = ram_mig_ram_block_resized,
> +};
> +
>  void ram_mig_init(void)
>  {
>      qemu_mutex_init(&XBZRLE.lock);
>      register_savevm_live("ram", 0, 4, &savevm_ram_handlers, &ram_state);
> +    ram_block_notifier_add(&ram_mig_ram_notifier);
>  }
> 

So, this seems to work very reliably when triggering a resize of a RAM
block during system reset (using my virtio-mem prototype):

(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
decompress-error-check: on
clear-bitmap-shift: 18
Migration status: cancelled
total time: 0 milliseconds


And from QEMU

qemu-system-x86_64: RAM block '0000:00:03.0/mem1' resized during precopy.

-- 
Thanks,

David / dhildenb



^ permalink raw reply

* Re: [Xen-devel] [PATCH] rwlock: allow recursive read locking when already locked in write mode
From: Roger Pau Monné @ 2020-02-20 15:17 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Jürgen Groß, Stefano Stabellini, Julien Grall, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	xen-devel
In-Reply-To: <0a3a762e-9a0d-7395-d3c4-aca07c366979@suse.com>

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?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* [PATCH] lockdown: Allow unprivileged users to see lockdown status
From: Jeremy Cline @ 2020-02-20 15:17 UTC (permalink / raw)
  To: James Morris, Serge E . Hallyn
  Cc: Matthew Garrett, David Howells, linux-security-module,
	linux-kernel, Jeremy Cline, Frank Ch . Eigler

A number of userspace tools, such as systemtap, need a way to see the
current lockdown state so they can gracefully deal with the kernel being
locked down. The state is already exposed in
/sys/kernel/security/lockdown, but is only readable by root. Adjust the
permissions so unprivileged users can read the state.

Fixes: 000d388ed3bb ("security: Add a static lockdown policy LSM")
Cc: Frank Ch. Eigler <fche@redhat.com>
Signed-off-by: Jeremy Cline <jcline@redhat.com>
---
 security/lockdown/lockdown.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 5a952617a0eb..87cbdc64d272 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -150,7 +150,7 @@ static int __init lockdown_secfs_init(void)
 {
 	struct dentry *dentry;
 
-	dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
+	dentry = securityfs_create_file("lockdown", 0644, NULL, NULL,
 					&lockdown_ops);
 	return PTR_ERR_OR_ZERO(dentry);
 }
-- 
2.24.1


^ permalink raw reply related

* Re: [PATCH v3 07/10] ASoC: tegra: add Tegra210 based ADMAIF driver
From: Jon Hunter @ 2020-02-20 15:16 UTC (permalink / raw)
  To: Sameer Pujar, perex, tiwai, robh+dt
  Cc: devicetree, alsa-devel, atalambedu, lgirdwood, linux-kernel,
	viswanathl, sharadg, broonie, thierry.reding, linux-tegra, digetx,
	rlokhande, mkumard, dramesh
In-Reply-To: <1582180492-25297-8-git-send-email-spujar@nvidia.com>


On 20/02/2020 06:34, Sameer Pujar wrote:
> ADMAIF is the interface between ADMA and AHUB. Each ADMA channel that
> sends/receives data to/from AHUB must intreface through an ADMAIF channel.
> ADMA channel sending data to AHUB pairs with an ADMAIF Tx channel and
> similarly ADMA channel receiving data from AHUB pairs with an ADMAIF Rx
> channel. Buffer size is configuranle for each ADMAIF channel, but currently
> SW uses default values.
> 
> This patch registers ADMAIF driver with ASoC framework. The component
> driver exposes DAPM widgets, routes and kcontrols for the device. The DAI
> driver exposes ADMAIF interfaces, which can be used to connect different
> components in the ASoC layer. Makefile and Kconfig support is added to
> allow to build the driver. The ADMAIF device can be enabled in the DT via
> "nvidia,tegra210-admaif" compatible binding.
> 
> Tegra PCM driver is updated to expose required PCM interfaces and
> snd_pcm_ops callbacks.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  sound/soc/tegra/Kconfig           |  12 +
>  sound/soc/tegra/Makefile          |   2 +
>  sound/soc/tegra/tegra210_admaif.c | 884 ++++++++++++++++++++++++++++++++++++++
>  sound/soc/tegra/tegra210_admaif.h | 164 +++++++
>  sound/soc/tegra/tegra_pcm.c       | 222 +++++++++-
>  sound/soc/tegra/tegra_pcm.h       |  23 +-
>  6 files changed, 1305 insertions(+), 2 deletions(-)
>  create mode 100644 sound/soc/tegra/tegra210_admaif.c
>  create mode 100644 sound/soc/tegra/tegra210_admaif.h

Aside from Randy's comment ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* Re: [meta-oe][zeus][PATCH] meta-oe: zeus: remmina - use PACKAGECONFIG for spice
From: akuster808 @ 2020-02-20 15:17 UTC (permalink / raw)
  To: Jan-Simon Moeller, openembedded-devel
In-Reply-To: <20200219115740.25272-1-dl9pf@gmx.de>

Jan,

On 2/19/20 3:57 AM, Jan-Simon Moeller wrote:
> remmina depends on spice and spice-protocol but they are in meta-networking.
> Use the PACKAGECONFIG flag to avoid hardcoding the dependency.
>
> Signed-off-by: Jan-Simon Moeller <dl9pf@gmx.de>
> ---
>  meta-oe/recipes-support/remmina/remmina_1.3.6.bb | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/meta-oe/recipes-support/remmina/remmina_1.3.6.bb b/meta-oe/recipes-support/remmina/remmina_1.3.6.bb
> index 82b803a4d..5f25231c2 100644
> --- a/meta-oe/recipes-support/remmina/remmina_1.3.6.bb
> +++ b/meta-oe/recipes-support/remmina/remmina_1.3.6.bb
> @@ -5,8 +5,6 @@ LICENSE = "GPLv2 & openssl"
>  LIC_FILES_CHKSUM = "file://LICENSE;md5=dab7215512044d49037272ce1ac4ea8f file://LICENSE.OpenSSL;md5=c1eb3cee0a4dea27503c531267a69769"
>  DEPENDS += "openssl freerdp gtk+3 gdk-pixbuf atk libgcrypt avahi-ui libsodium libssh vte json-glib libsoup-2.4 libvncserver libsecret"
>
> -DEPENDS_append_x86 = " spice spice-protocol"
> -DEPENDS_append_x86-64 = " spice spice-protocol"
>
>  DEPENDS_append_libc-musl = " libexecinfo"
>  LDFLAGS_append_libc-musl = " -lexecinfo"
> @@ -23,11 +21,7 @@ inherit cmake distro_features_check
>  # depends on avahi-ui with this restriction
>  ANY_OF_DISTRO_FEATURES = "${GTK3DISTROFEATURES}"
>
> -EXTRA_OECMAKE += "-DWITH_APPINDICATOR=OFF -DWITH_GETTEXT=OFF -DWITH_TRANSLATIONS=OFF -DWITH_SPICE=OFF"
> -
> -EXTRA_OECMAKE_append_x86 = " -DWITH_SPICE=ON"
> -EXTRA_OECMAKE_append_x86-64 = " -DWITH_SPICE=ON"
> -
> +EXTRA_OECMAKE += "-DWITH_APPINDICATOR=OFF -DWITH_GETTEXT=OFF -DWITH_TRANSLATIONS=OFF"
>
>  do_install_append(){
>      # We dont need the extra stuff form other desktop environments
> @@ -36,6 +30,8 @@ do_install_append(){
>      rm -rf ${D}/${datadir}/gnome-session
>  }
>
> +PACKAGECONFIG[spice] = "-DWITH_SPICE=ON, -DWITH_SPICE=OFF, spice spice-protocol"
The above needs to be set for x86 and x86-64 systems to make this change
behave as it did before and I don't see that so its not a viable zeus
candidate.

- armin
> +
>  RDEPENDS_${PN} = "bash"
>
>  FILES_${PN}_append = " ${datadir}/icons/hicolor/*"
> --
> 2.11.0
>



^ permalink raw reply

* Re: [RFC PATCH v3 05/27] qcow2: Document the Extended L2 Entries feature
From: Eric Blake @ 2020-02-20 15:16 UTC (permalink / raw)
  To: Alberto Garcia, qemu-devel
  Cc: Kevin Wolf, Anton Nefedov, qemu-block, Max Reitz,
	Vladimir Sementsov-Ogievskiy, Denis V . Lunev
In-Reply-To: <w51mu9db9f3.fsf@maestria.local.igalia.com>

On 2/20/20 8:49 AM, Alberto Garcia wrote:
> On Thu 20 Feb 2020 03:28:17 PM CET, Eric Blake wrote:
>>> +An image uses Extended L2 Entries if bit 3 is set on the incompatible_features
>>> +field of the header.
>>> +
>>> +In these images standard data clusters are divided into 32 subclusters of the
>>> +same size. They are contiguous and start from the beginning of the cluster.
>>> +Subclusters can be allocated independently and the L2 entry contains information
>>> +indicating the status of each one of them. Compressed data clusters don't have
>>> +subclusters so they are treated like in images without this feature.
>>
>> Grammar; I'd suggest:
>>
>> ...don't have subclusters, so they are treated the same as in images
>> without this feature.
> 
> Ok
> 
>> Are they truly the same, or do you still need to document that the
>> extra 64 bits of the extended L2 entry are all zero?
> 
> It is documented later in the same patch ("Subcluster Allocation Bitmap
> for compressed clusters").

Yes, I saw the mention later.  I'm just wondering if we need to 
rearrange text to mention that the bits are reserved (set to 0, ignore 
on read) closer to the point where we document compressed clusters have 
no subclusters.

> 
> By the way, this series treats an L2 entry as invalid if any of those
> bits is not zero, but I think I'll change that. Conceivably those bits
> could be used for a future compatible feature, but it can only be
> compatible if the previous versions ignore those bits.
> 
>>> +        32 -  63    Subcluster reads as zeros (one bit per subcluster)
>>> +
>>> +                    1: the subcluster reads as zeros. In this case the
>>> +                       allocation status bit must be unset. The host
>>> +                       cluster offset field may or may not be set.
>>
>> Why must the allocation bit be unset?  When we preallocate, we want a
>> cluster to reserve space, but still read as zero, so the combination
>> of both bits set makes sense to me.
> 
> Since 00 means unallocated and 01 allocated, there are two options left
> to represent the "reads as zero" case: 10 and 11.
> 
> I think that one could argue for either one and there is no "right"
> choice. I chose the former because I understood the allocation bit as
> "the guest visible data is obtained from the raw data in that
> subcluster" but the other option also makes sense.

My argument is that BOTH bit settings make sense:

10 - reads as zero, but subcluster is not allocated
11 - reads as zero, and subcluster is allocated

Oh, I see.  I'm getting confused on the meanings of "allocated". 
Meaning 1: a host address is reserved for the guest address 
(pre-allocation sense).  Meaning 2: guest reads come from this layer 
rather than from the backing layer (COW/COR sense).

Pre-allocation is ALWAYS done a cluster at a time (you only have ONE 
host offset, shared among all 32 subclusters, per L2 entry), so either 
all 32 subclusters have a preallocated location, or none of them do. 
What is left, then, is a determination of whether to read locally or 
from the backing file, AND when reading locally, whether to read from 
the pre-allocated space or to just read zeroes.

We have 8 potential combinations (not all make sense):

host   zero alloc
   0      0    0     cluster unallocated, subcluster defers to backing
   0      0    1     error (except maybe for external data file)
   0      1    0     cluster unallocated, subcluster reads as zero
   0      1    1     error (except maybe for external data file)
  addr    0    0     cluster allocated, subcluster defers to backing
  addr    0    1     cluster allocated, subcluster reads from host
  addr    1    0     cluster allocated, subcluster reads as zero
  addr    1    1   error, or cluster allocated, subcluster reads as zero

Hmm - normally addr is non-zero (because the 0 addr is the metadata 
cluster of qcow2), but with external data file, host addr 0 is required 
for guest offset 0.  How do subclusters play with external data files? 
It makes sense to still have subclusters read as 0 or defer to backing 
with an external file (except maybe when raw external file is set).  But 
you did word it as if the alloc bit is set, the "host cluster offset 
field must contain a valid offset" which includes an offset of 0 for 
external data file.

If we mandate 10 for the reads-as-zero form, then whether addr is valid 
is irrelevant. If we mandate 11 for the reads-as-zero form, then addr 
must be valid even though we don't reference addr.  Having written all 
that, I agree that either form should work, but also that mandating one 
form leaves the door open for a future extension to define meaning to 
the form we did not permit (that is, either 10 or 11 becomes a reserved 
pattern that we can later give meaning to), vs. allowing both forms now 
and locking ourselves out of a future meaning.  And mandating addr to be 
valid even when reading zeroes doesn't use addr feels odd.

So, I'm okay with your choice of picking 00, 01, and 10 as the mandated 
forms, and declaring 11 as invalid for now (but a possible future 
extension).  Maybe I'll change my mind when seeing what complexity it 
adds to the qcow2 reference implementation, but hopefully not.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply

* Re: [PATCH v3 07/10] ASoC: tegra: add Tegra210 based ADMAIF driver
From: Jon Hunter @ 2020-02-20 15:16 UTC (permalink / raw)
  To: Sameer Pujar, perex, tiwai, robh+dt
  Cc: broonie, lgirdwood, thierry.reding, digetx, alsa-devel,
	devicetree, linux-tegra, linux-kernel, sharadg, mkumard,
	viswanathl, rlokhande, dramesh, atalambedu
In-Reply-To: <1582180492-25297-8-git-send-email-spujar@nvidia.com>


On 20/02/2020 06:34, Sameer Pujar wrote:
> ADMAIF is the interface between ADMA and AHUB. Each ADMA channel that
> sends/receives data to/from AHUB must intreface through an ADMAIF channel.
> ADMA channel sending data to AHUB pairs with an ADMAIF Tx channel and
> similarly ADMA channel receiving data from AHUB pairs with an ADMAIF Rx
> channel. Buffer size is configuranle for each ADMAIF channel, but currently
> SW uses default values.
> 
> This patch registers ADMAIF driver with ASoC framework. The component
> driver exposes DAPM widgets, routes and kcontrols for the device. The DAI
> driver exposes ADMAIF interfaces, which can be used to connect different
> components in the ASoC layer. Makefile and Kconfig support is added to
> allow to build the driver. The ADMAIF device can be enabled in the DT via
> "nvidia,tegra210-admaif" compatible binding.
> 
> Tegra PCM driver is updated to expose required PCM interfaces and
> snd_pcm_ops callbacks.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  sound/soc/tegra/Kconfig           |  12 +
>  sound/soc/tegra/Makefile          |   2 +
>  sound/soc/tegra/tegra210_admaif.c | 884 ++++++++++++++++++++++++++++++++++++++
>  sound/soc/tegra/tegra210_admaif.h | 164 +++++++
>  sound/soc/tegra/tegra_pcm.c       | 222 +++++++++-
>  sound/soc/tegra/tegra_pcm.h       |  23 +-
>  6 files changed, 1305 insertions(+), 2 deletions(-)
>  create mode 100644 sound/soc/tegra/tegra210_admaif.c
>  create mode 100644 sound/soc/tegra/tegra210_admaif.h

Aside from Randy's comment ...

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH v3 07/10] ASoC: tegra: add Tegra210 based ADMAIF driver
From: Jon Hunter @ 2020-02-20 15:16 UTC (permalink / raw)
  To: Sameer Pujar, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	digetx-Re5JQEeQqe8AvxtiuMwx3w, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	sharadg-DDmLM1+adcrQT0dZR+AlfA, mkumard-DDmLM1+adcrQT0dZR+AlfA,
	viswanathl-DDmLM1+adcrQT0dZR+AlfA,
	rlokhande-DDmLM1+adcrQT0dZR+AlfA, dramesh-DDmLM1+adcrQT0dZR+AlfA,
	atalambedu-DDmLM1+adcrQT0dZR+AlfA
In-Reply-To: <1582180492-25297-8-git-send-email-spujar-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>


On 20/02/2020 06:34, Sameer Pujar wrote:
> ADMAIF is the interface between ADMA and AHUB. Each ADMA channel that
> sends/receives data to/from AHUB must intreface through an ADMAIF channel.
> ADMA channel sending data to AHUB pairs with an ADMAIF Tx channel and
> similarly ADMA channel receiving data from AHUB pairs with an ADMAIF Rx
> channel. Buffer size is configuranle for each ADMAIF channel, but currently
> SW uses default values.
> 
> This patch registers ADMAIF driver with ASoC framework. The component
> driver exposes DAPM widgets, routes and kcontrols for the device. The DAI
> driver exposes ADMAIF interfaces, which can be used to connect different
> components in the ASoC layer. Makefile and Kconfig support is added to
> allow to build the driver. The ADMAIF device can be enabled in the DT via
> "nvidia,tegra210-admaif" compatible binding.
> 
> Tegra PCM driver is updated to expose required PCM interfaces and
> snd_pcm_ops callbacks.
> 
> Signed-off-by: Sameer Pujar <spujar-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  sound/soc/tegra/Kconfig           |  12 +
>  sound/soc/tegra/Makefile          |   2 +
>  sound/soc/tegra/tegra210_admaif.c | 884 ++++++++++++++++++++++++++++++++++++++
>  sound/soc/tegra/tegra210_admaif.h | 164 +++++++
>  sound/soc/tegra/tegra_pcm.c       | 222 +++++++++-
>  sound/soc/tegra/tegra_pcm.h       |  23 +-
>  6 files changed, 1305 insertions(+), 2 deletions(-)
>  create mode 100644 sound/soc/tegra/tegra210_admaif.c
>  create mode 100644 sound/soc/tegra/tegra210_admaif.h

Aside from Randy's comment ...

Reviewed-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply

* Re: [PATCH 3/4] Btrfs: resurrect extent_read_full_page_nolock()
From: Josef Bacik @ 2020-02-20 15:16 UTC (permalink / raw)
  To: fdmanana, linux-btrfs
In-Reply-To: <20200219140606.1641625-1-fdmanana@kernel.org>

On 2/19/20 9:06 AM, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Commit 7f042a8370a5bb ("Btrfs: remove no longer used function
> extent_read_full_page_nolock()") removed extent_read_full_page_nolock()
> because it was not needed anymore.
> 
> This function was used to read a page while holding the respective range
> locked in the inode's iotree, to avoid deadlocks when using the other
> APIs we have for reading a page (which lock and unlock the range
> themselves).
> 
> Since this type of functionality is needed for the upcoming changes to
> the reflink implementation dealing with cloning of inline extents, bring
> back this function to life.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply

* Re: S variable for svn fetcher Was: State of OE world - 2020-02-18
From: Adrian Bunk @ 2020-02-20 15:16 UTC (permalink / raw)
  To: Martin Jansa; +Cc: openembeded-devel
In-Reply-To: <20200220144848.msiyguq2bsmqokxc@jama>

On Thu, Feb 20, 2020 at 03:48:48PM +0100, Martin Jansa wrote:
>...
> Any idea why these aren't shown in our build?
>...

What is your mirror configuration?

Default configuration downloads the tarball from [1].

> Cheers,

cu
Adrian

[1] http://downloads.yoctoproject.org/mirror/sources/


^ permalink raw reply

* Re: [PATCH 2/3] xfs: remove the icdinode di_uid/di_gid members
From: Chandan Rajendra @ 2020-02-19 16:25 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, linux-fsdevel
In-Reply-To: <20200218210020.40846-3-hch@lst.de>

On Wednesday, February 19, 2020 2:30 AM Christoph Hellwig wrote: 
> Use the Linux inode i_uid/i_gid members everywhere and just convert
> from/to the scalar value when reading or writing the on-disk inode.
>

The conversion b/w kuid and on-disk uid is correct.

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c | 10 ++++-----
>  fs/xfs/libxfs/xfs_inode_buf.h |  2 --
>  fs/xfs/xfs_dquot.c            |  4 ++--
>  fs/xfs/xfs_inode.c            | 14 ++++--------
>  fs/xfs/xfs_inode_item.c       |  4 ++--
>  fs/xfs/xfs_ioctl.c            |  6 +++---
>  fs/xfs/xfs_iops.c             |  6 +-----
>  fs/xfs/xfs_itable.c           |  4 ++--
>  fs/xfs/xfs_qm.c               | 40 ++++++++++++++++++++++-------------
>  fs/xfs/xfs_quota.h            |  4 ++--
>  fs/xfs/xfs_symlink.c          |  4 +---
>  11 files changed, 46 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index cc4efd34843a..bc72b575ceed 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -222,10 +222,8 @@ xfs_inode_from_disk(
>  	}
> 
>  	to->di_format = from->di_format;
> -	to->di_uid = be32_to_cpu(from->di_uid);
> -	inode->i_uid = xfs_uid_to_kuid(to->di_uid);
> -	to->di_gid = be32_to_cpu(from->di_gid);
> -	inode->i_gid = xfs_gid_to_kgid(to->di_gid);
> +	inode->i_uid = xfs_uid_to_kuid(be32_to_cpu(from->di_uid));
> +	inode->i_gid = xfs_gid_to_kgid(be32_to_cpu(from->di_gid));
>  	to->di_flushiter = be16_to_cpu(from->di_flushiter);
> 
>  	/*
> @@ -278,8 +276,8 @@ xfs_inode_to_disk(
> 
>  	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
> -	to->di_uid = cpu_to_be32(from->di_uid);
> -	to->di_gid = cpu_to_be32(from->di_gid);
> +	to->di_uid = cpu_to_be32(xfs_kuid_to_uid(inode->i_uid));
> +	to->di_gid = cpu_to_be32(xfs_kgid_to_gid(inode->i_gid));
>  	to->di_projid_lo = cpu_to_be16(from->di_projid & 0xffff);
>  	to->di_projid_hi = cpu_to_be16(from->di_projid >> 16);
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.h b/fs/xfs/libxfs/xfs_inode_buf.h
> index fd94b1078722..2683e1e2c4a6 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.h
> +++ b/fs/xfs/libxfs/xfs_inode_buf.h
> @@ -19,8 +19,6 @@ struct xfs_icdinode {
>  	int8_t		di_version;	/* inode version */
>  	int8_t		di_format;	/* format of di_c data */
>  	uint16_t	di_flushiter;	/* incremented on flush */
> -	uint32_t	di_uid;		/* owner's user id */
> -	uint32_t	di_gid;		/* owner's group id */
>  	uint32_t	di_projid;	/* owner's project id */
>  	xfs_fsize_t	di_size;	/* number of bytes in file */
>  	xfs_rfsblock_t	di_nblocks;	/* # of direct & btree blocks used */
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index d223e1ae90a6..3579de9306c1 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -829,9 +829,9 @@ xfs_qm_id_for_quotatype(
>  {
>  	switch (type) {
>  	case XFS_DQ_USER:
> -		return ip->i_d.di_uid;
> +		return xfs_kuid_to_uid(VFS_I(ip)->i_uid);
>  	case XFS_DQ_GROUP:
> -		return ip->i_d.di_gid;
> +		return xfs_kgid_to_gid(VFS_I(ip)->i_gid);
>  	case XFS_DQ_PROJ:
>  		return ip->i_d.di_projid;
>  	}
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 938b0943bd95..3324e1696354 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -813,18 +813,15 @@ xfs_ialloc(
>  	inode->i_mode = mode;
>  	set_nlink(inode, nlink);
>  	inode->i_uid = current_fsuid();
> -	ip->i_d.di_uid = xfs_kuid_to_uid(inode->i_uid);
>  	inode->i_rdev = rdev;
>  	ip->i_d.di_projid = prid;
> 
>  	if (pip && XFS_INHERIT_GID(pip)) {
>  		inode->i_gid = VFS_I(pip)->i_gid;
> -		ip->i_d.di_gid = pip->i_d.di_gid;
>  		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(mode))
>  			inode->i_mode |= S_ISGID;
>  	} else {
>  		inode->i_gid = current_fsgid();
> -		ip->i_d.di_gid = xfs_kgid_to_gid(inode->i_gid);
>  	}
> 
>  	/*
> @@ -832,9 +829,8 @@ xfs_ialloc(
>  	 * ID or one of the supplementary group IDs, the S_ISGID bit is cleared
>  	 * (and only if the irix_sgid_inherit compatibility variable is set).
>  	 */
> -	if ((irix_sgid_inherit) &&
> -	    (inode->i_mode & S_ISGID) &&
> -	    (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid))))
> +	if (irix_sgid_inherit &&
> +	    (inode->i_mode & S_ISGID) && !in_group_p(inode->i_gid))
>  		inode->i_mode &= ~S_ISGID;
> 
>  	ip->i_d.di_size = 0;
> @@ -1162,8 +1158,7 @@ xfs_create(
>  	/*
>  	 * Make sure that we have allocated dquot(s) on disk.
>  	 */
> -	error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(current_fsuid()),
> -					xfs_kgid_to_gid(current_fsgid()), prid,
> +	error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,
>  					XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
>  					&udqp, &gdqp, &pdqp);
>  	if (error)
> @@ -1313,8 +1308,7 @@ xfs_create_tmpfile(
>  	/*
>  	 * Make sure that we have allocated dquot(s) on disk.
>  	 */
> -	error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(current_fsuid()),
> -				xfs_kgid_to_gid(current_fsgid()), prid,
> +	error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,
>  				XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
>  				&udqp, &gdqp, &pdqp);
>  	if (error)
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 8bd5d0de6321..83d7914556ef 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -308,8 +308,8 @@ xfs_inode_to_log_dinode(
> 
>  	to->di_version = from->di_version;
>  	to->di_format = from->di_format;
> -	to->di_uid = from->di_uid;
> -	to->di_gid = from->di_gid;
> +	to->di_uid = xfs_kuid_to_uid(inode->i_uid);
> +	to->di_gid = xfs_kgid_to_gid(inode->i_gid);
>  	to->di_projid_lo = from->di_projid & 0xffff;
>  	to->di_projid_hi = from->di_projid >> 16;
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index d42de92cb283..0f85bedc5977 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1434,9 +1434,9 @@ xfs_ioctl_setattr(
>  	 * because the i_*dquot fields will get updated anyway.
>  	 */
>  	if (XFS_IS_QUOTA_ON(mp)) {
> -		code = xfs_qm_vop_dqalloc(ip, ip->i_d.di_uid,
> -					 ip->i_d.di_gid, fa->fsx_projid,
> -					 XFS_QMOPT_PQUOTA, &udqp, NULL, &pdqp);
> +		code = xfs_qm_vop_dqalloc(ip, VFS_I(ip)->i_uid,
> +				VFS_I(ip)->i_gid, fa->fsx_projid,
> +				XFS_QMOPT_PQUOTA, &udqp, NULL, &pdqp);
>  		if (code)
>  			return code;
>  	}
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index b818b261918f..a5b7c3100a2f 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -692,9 +692,7 @@ xfs_setattr_nonsize(
>  		 */
>  		ASSERT(udqp == NULL);
>  		ASSERT(gdqp == NULL);
> -		error = xfs_qm_vop_dqalloc(ip, xfs_kuid_to_uid(uid),
> -					   xfs_kgid_to_gid(gid),
> -					   ip->i_d.di_projid,
> +		error = xfs_qm_vop_dqalloc(ip, uid, gid, ip->i_d.di_projid,
>  					   qflags, &udqp, &gdqp, NULL);
>  		if (error)
>  			return error;
> @@ -763,7 +761,6 @@ xfs_setattr_nonsize(
>  				olddquot1 = xfs_qm_vop_chown(tp, ip,
>  							&ip->i_udquot, udqp);
>  			}
> -			ip->i_d.di_uid = xfs_kuid_to_uid(uid);
>  			inode->i_uid = uid;
>  		}
>  		if (!gid_eq(igid, gid)) {
> @@ -775,7 +772,6 @@ xfs_setattr_nonsize(
>  				olddquot2 = xfs_qm_vop_chown(tp, ip,
>  							&ip->i_gdquot, gdqp);
>  			}
> -			ip->i_d.di_gid = xfs_kgid_to_gid(gid);
>  			inode->i_gid = gid;
>  		}
>  	}
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 4b31c29b7e6b..497db4160283 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -86,8 +86,8 @@ xfs_bulkstat_one_int(
>  	 */
>  	buf->bs_projectid = ip->i_d.di_projid;
>  	buf->bs_ino = ino;
> -	buf->bs_uid = dic->di_uid;
> -	buf->bs_gid = dic->di_gid;
> +	buf->bs_uid = xfs_kuid_to_uid(inode->i_uid);
> +	buf->bs_gid = xfs_kgid_to_gid(inode->i_gid);
>  	buf->bs_size = dic->di_size;
> 
>  	buf->bs_nlink = inode->i_nlink;
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 0b0909657bad..54dda7d982c9 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -326,16 +326,18 @@ xfs_qm_dqattach_locked(
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> 
>  	if (XFS_IS_UQUOTA_ON(mp) && !ip->i_udquot) {
> -		error = xfs_qm_dqattach_one(ip, ip->i_d.di_uid, XFS_DQ_USER,
> -				doalloc, &ip->i_udquot);
> +		error = xfs_qm_dqattach_one(ip,
> +				xfs_kuid_to_uid(VFS_I(ip)->i_uid),
> +				XFS_DQ_USER, doalloc, &ip->i_udquot);
>  		if (error)
>  			goto done;
>  		ASSERT(ip->i_udquot);
>  	}
> 
>  	if (XFS_IS_GQUOTA_ON(mp) && !ip->i_gdquot) {
> -		error = xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP,
> -				doalloc, &ip->i_gdquot);
> +		error = xfs_qm_dqattach_one(ip,
> +				xfs_kgid_to_gid(VFS_I(ip)->i_gid),
> +				XFS_DQ_GROUP, doalloc, &ip->i_gdquot);
>  		if (error)
>  			goto done;
>  		ASSERT(ip->i_gdquot);
> @@ -1613,8 +1615,8 @@ xfs_qm_dqfree_one(
>  int
>  xfs_qm_vop_dqalloc(
>  	struct xfs_inode	*ip,
> -	xfs_dqid_t		uid,
> -	xfs_dqid_t		gid,
> +	kuid_t			uid,
> +	kgid_t			gid,
>  	prid_t			prid,
>  	uint			flags,
>  	struct xfs_dquot	**O_udqpp,
> @@ -1622,6 +1624,7 @@ xfs_qm_vop_dqalloc(
>  	struct xfs_dquot	**O_pdqpp)
>  {
>  	struct xfs_mount	*mp = ip->i_mount;
> +	struct inode		*inode = VFS_I(ip);
>  	struct xfs_dquot	*uq = NULL;
>  	struct xfs_dquot	*gq = NULL;
>  	struct xfs_dquot	*pq = NULL;
> @@ -1635,7 +1638,7 @@ xfs_qm_vop_dqalloc(
>  	xfs_ilock(ip, lockflags);
> 
>  	if ((flags & XFS_QMOPT_INHERIT) && XFS_INHERIT_GID(ip))
> -		gid = ip->i_d.di_gid;
> +		gid = inode->i_gid;
> 
>  	/*
>  	 * Attach the dquot(s) to this inode, doing a dquot allocation
> @@ -1650,7 +1653,7 @@ xfs_qm_vop_dqalloc(
>  	}
> 
>  	if ((flags & XFS_QMOPT_UQUOTA) && XFS_IS_UQUOTA_ON(mp)) {
> -		if (ip->i_d.di_uid != uid) {
> +		if (!uid_eq(inode->i_uid, uid)) {
>  			/*
>  			 * What we need is the dquot that has this uid, and
>  			 * if we send the inode to dqget, the uid of the inode
> @@ -1661,7 +1664,8 @@ xfs_qm_vop_dqalloc(
>  			 * holding ilock.
>  			 */
>  			xfs_iunlock(ip, lockflags);
> -			error = xfs_qm_dqget(mp, uid, XFS_DQ_USER, true, &uq);
> +			error = xfs_qm_dqget(mp, xfs_kuid_to_uid(uid),
> +					XFS_DQ_USER, true, &uq);
>  			if (error) {
>  				ASSERT(error != -ENOENT);
>  				return error;
> @@ -1682,9 +1686,10 @@ xfs_qm_vop_dqalloc(
>  		}
>  	}
>  	if ((flags & XFS_QMOPT_GQUOTA) && XFS_IS_GQUOTA_ON(mp)) {
> -		if (ip->i_d.di_gid != gid) {
> +		if (!gid_eq(inode->i_gid, gid)) {
>  			xfs_iunlock(ip, lockflags);
> -			error = xfs_qm_dqget(mp, gid, XFS_DQ_GROUP, true, &gq);
> +			error = xfs_qm_dqget(mp, xfs_kgid_to_gid(gid),
> +					XFS_DQ_GROUP, true, &gq);
>  			if (error) {
>  				ASSERT(error != -ENOENT);
>  				goto error_rele;
> @@ -1810,7 +1815,8 @@ xfs_qm_vop_chown_reserve(
>  			XFS_QMOPT_RES_RTBLKS : XFS_QMOPT_RES_REGBLKS;
> 
>  	if (XFS_IS_UQUOTA_ON(mp) && udqp &&
> -	    ip->i_d.di_uid != be32_to_cpu(udqp->q_core.d_id)) {
> +	    xfs_kuid_to_uid(VFS_I(ip)->i_uid) !=
> +			be32_to_cpu(udqp->q_core.d_id)) {
>  		udq_delblks = udqp;
>  		/*
>  		 * If there are delayed allocation blocks, then we have to
> @@ -1823,7 +1829,8 @@ xfs_qm_vop_chown_reserve(
>  		}
>  	}
>  	if (XFS_IS_GQUOTA_ON(ip->i_mount) && gdqp &&
> -	    ip->i_d.di_gid != be32_to_cpu(gdqp->q_core.d_id)) {
> +	    xfs_kgid_to_gid(VFS_I(ip)->i_gid) !=
> +			be32_to_cpu(gdqp->q_core.d_id)) {
>  		gdq_delblks = gdqp;
>  		if (delblks) {
>  			ASSERT(ip->i_gdquot);
> @@ -1920,14 +1927,17 @@ xfs_qm_vop_create_dqattach(
> 
>  	if (udqp && XFS_IS_UQUOTA_ON(mp)) {
>  		ASSERT(ip->i_udquot == NULL);
> -		ASSERT(ip->i_d.di_uid == be32_to_cpu(udqp->q_core.d_id));
> +		ASSERT(xfs_kuid_to_uid(VFS_I(ip)->i_uid) ==
> +			be32_to_cpu(udqp->q_core.d_id));
> 
>  		ip->i_udquot = xfs_qm_dqhold(udqp);
>  		xfs_trans_mod_dquot(tp, udqp, XFS_TRANS_DQ_ICOUNT, 1);
>  	}
>  	if (gdqp && XFS_IS_GQUOTA_ON(mp)) {
>  		ASSERT(ip->i_gdquot == NULL);
> -		ASSERT(ip->i_d.di_gid == be32_to_cpu(gdqp->q_core.d_id));
> +		ASSERT(xfs_kgid_to_gid(VFS_I(ip)->i_gid) ==
> +			be32_to_cpu(gdqp->q_core.d_id));
> +
>  		ip->i_gdquot = xfs_qm_dqhold(gdqp);
>  		xfs_trans_mod_dquot(tp, gdqp, XFS_TRANS_DQ_ICOUNT, 1);
>  	}
> diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h
> index efe42ae7a2f3..aa8fc1f55fbd 100644
> --- a/fs/xfs/xfs_quota.h
> +++ b/fs/xfs/xfs_quota.h
> @@ -86,7 +86,7 @@ extern int xfs_trans_reserve_quota_bydquots(struct xfs_trans *,
>  		struct xfs_mount *, struct xfs_dquot *,
>  		struct xfs_dquot *, struct xfs_dquot *, int64_t, long, uint);
> 
> -extern int xfs_qm_vop_dqalloc(struct xfs_inode *, xfs_dqid_t, xfs_dqid_t,
> +extern int xfs_qm_vop_dqalloc(struct xfs_inode *, kuid_t, kgid_t,
>  		prid_t, uint, struct xfs_dquot **, struct xfs_dquot **,
>  		struct xfs_dquot **);
>  extern void xfs_qm_vop_create_dqattach(struct xfs_trans *, struct xfs_inode *,
> @@ -109,7 +109,7 @@ extern void xfs_qm_unmount_quotas(struct xfs_mount *);
> 
>  #else
>  static inline int
> -xfs_qm_vop_dqalloc(struct xfs_inode *ip, xfs_dqid_t uid, xfs_dqid_t gid,
> +xfs_qm_vop_dqalloc(struct xfs_inode *ip, kuid_t kuid, kgid_t kgid,
>  		prid_t prid, uint flags, struct xfs_dquot **udqp,
>  		struct xfs_dquot **gdqp, struct xfs_dquot **pdqp)
>  {
> diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
> index d762d42ed0ff..ea42e25ec1bf 100644
> --- a/fs/xfs/xfs_symlink.c
> +++ b/fs/xfs/xfs_symlink.c
> @@ -182,9 +182,7 @@ xfs_symlink(
>  	/*
>  	 * Make sure that we have allocated dquot(s) on disk.
>  	 */
> -	error = xfs_qm_vop_dqalloc(dp,
> -			xfs_kuid_to_uid(current_fsuid()),
> -			xfs_kgid_to_gid(current_fsgid()), prid,
> +	error = xfs_qm_vop_dqalloc(dp, current_fsuid(), current_fsgid(), prid,
>  			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
>  			&udqp, &gdqp, &pdqp);
>  	if (error)
> 


-- 
chandan




^ permalink raw reply

* Re: [PATCH] pinctrl: mediatek: Fix some off by one bugs
From: Linus Walleij @ 2020-02-20 15:14 UTC (permalink / raw)
  To: Dan Carpenter, Matthias Brugger
  Cc: Sean Wang, Light Hsieh, moderated list:ARM/Mediatek SoC support,
	open list:GPIO SUBSYSTEM, kernel-janitors
In-Reply-To: <20200218055247.74s2xa7veqx2do34@kili.mountain>

On Tue, Feb 18, 2020 at 6:55 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> These comparisons should be >= instead of > to prevent accessing one
> element beyond the end of the hw->soc->pins[] array.
>
> Fixes: 3de7deefce69 ("pinctrl: mediatek: Check gpio pin number and use binary search in mtk_hw_pin_field_lookup()")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Matthias could you have a look at this patch?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 1/3] xfs: ensure that the inode uid/gid match values match the icdinode ones
From: Chandan Rajendra @ 2020-02-19 14:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, linux-fsdevel
In-Reply-To: <20200218210020.40846-2-hch@lst.de>

On Wednesday, February 19, 2020 2:30 AM Christoph Hellwig wrote: 
> Instead of only synchronizing the uid/gid values in xfs_setup_inode,
> ensure that they always match to prepare for removing the icdinode
> fields.
>

The changes indeed keep the uid and gid values the same across icdinode and
vfs inode.

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_inode_buf.c | 2 ++
>  fs/xfs/xfs_icache.c           | 4 ++++
>  fs/xfs/xfs_inode.c            | 8 ++++++--
>  fs/xfs/xfs_iops.c             | 3 ---
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 8afacfe4be0a..cc4efd34843a 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -223,7 +223,9 @@ xfs_inode_from_disk(
> 
>  	to->di_format = from->di_format;
>  	to->di_uid = be32_to_cpu(from->di_uid);
> +	inode->i_uid = xfs_uid_to_kuid(to->di_uid);
>  	to->di_gid = be32_to_cpu(from->di_gid);
> +	inode->i_gid = xfs_gid_to_kgid(to->di_gid);
>  	to->di_flushiter = be16_to_cpu(from->di_flushiter);
> 
>  	/*
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 8dc2e5414276..a7be7a9e5c1a 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -289,6 +289,8 @@ xfs_reinit_inode(
>  	uint64_t	version = inode_peek_iversion(inode);
>  	umode_t		mode = inode->i_mode;
>  	dev_t		dev = inode->i_rdev;
> +	kuid_t		uid = inode->i_uid;
> +	kgid_t		gid = inode->i_gid;
> 
>  	error = inode_init_always(mp->m_super, inode);
> 
> @@ -297,6 +299,8 @@ xfs_reinit_inode(
>  	inode_set_iversion_queried(inode, version);
>  	inode->i_mode = mode;
>  	inode->i_rdev = dev;
> +	inode->i_uid = uid;
> +	inode->i_gid = gid;
>  	return error;
>  }
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..938b0943bd95 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -812,15 +812,19 @@ xfs_ialloc(
> 
>  	inode->i_mode = mode;
>  	set_nlink(inode, nlink);
> -	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
> -	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
> +	inode->i_uid = current_fsuid();
> +	ip->i_d.di_uid = xfs_kuid_to_uid(inode->i_uid);
>  	inode->i_rdev = rdev;
>  	ip->i_d.di_projid = prid;
> 
>  	if (pip && XFS_INHERIT_GID(pip)) {
> +		inode->i_gid = VFS_I(pip)->i_gid;
>  		ip->i_d.di_gid = pip->i_d.di_gid;
>  		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(mode))
>  			inode->i_mode |= S_ISGID;
> +	} else {
> +		inode->i_gid = current_fsgid();
> +		ip->i_d.di_gid = xfs_kgid_to_gid(inode->i_gid);
>  	}
> 
>  	/*
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 81f2f93caec0..b818b261918f 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1304,9 +1304,6 @@ xfs_setup_inode(
>  	/* make the inode look hashed for the writeback code */
>  	inode_fake_hash(inode);
> 
> -	inode->i_uid    = xfs_uid_to_kuid(ip->i_d.di_uid);
> -	inode->i_gid    = xfs_gid_to_kgid(ip->i_d.di_gid);
> -
>  	i_size_write(inode, ip->i_d.di_size);
>  	xfs_diflags_to_iflags(inode, ip);
> 
> 


-- 
chandan




^ permalink raw reply

* [PATCH v6 05/16] arc/mm: Use helper fault_signal_pending()
From: Peter Xu @ 2020-02-20 15:08 UTC (permalink / raw)
  To: peterx, linux-mm, linux-kernel
  Cc: Martin Cracauer, Mike Rapoport, Hugh Dickins, Jerome Glisse,
	Kirill A . Shutemov, Matthew Wilcox, Pavel Emelyanov,
	Brian Geffon, Maya Gokhale, Denis Plotnikov, Andrea Arcangeli,
	Johannes Weiner, Dr . David Alan Gilbert, Linus Torvalds,
	Mike Kravetz, Marty McFadden, David Hildenbrand, Bobby Powers,
	Mel Gorman
In-Reply-To: <20200220145432.4561-1-peterx@redhat.com>

Let ARC to use the new helper fault_signal_pending() by moving the
signal check out of the retry logic as standalone.  This should also
helps to simplify the code a bit.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/arc/mm/fault.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index fb86bc3e9b35..6eb821a59b49 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -133,29 +133,21 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 
 	fault = handle_mm_fault(vma, address, flags);
 
+	/* Quick path to respond to signals */
+	if (fault_signal_pending(fault, regs)) {
+		if (!user_mode(regs))
+			goto no_context;
+		return;
+	}
+
 	/*
-	 * Fault retry nuances
+	 * Fault retry nuances, mmap_sem already relinquished by core mm
 	 */
-	if (unlikely(fault & VM_FAULT_RETRY)) {
-
-		/*
-		 * If fault needs to be retried, handle any pending signals
-		 * first (by returning to user mode).
-		 * mmap_sem already relinquished by core mm for RETRY case
-		 */
-		if (fatal_signal_pending(current)) {
-			if (!user_mode(regs))
-				goto no_context;
-			return;
-		}
-		/*
-		 * retry state machine
-		 */
-		if (flags & FAULT_FLAG_ALLOW_RETRY) {
-			flags &= ~FAULT_FLAG_ALLOW_RETRY;
-			flags |= FAULT_FLAG_TRIED;
-			goto retry;
-		}
+	if (unlikely((fault & VM_FAULT_RETRY) &&
+		     (flags & FAULT_FLAG_ALLOW_RETRY))) {
+		flags &= ~FAULT_FLAG_ALLOW_RETRY;
+		flags |= FAULT_FLAG_TRIED;
+		goto retry;
 	}
 
 bad_area:
-- 
2.24.1



^ permalink raw reply related

* Re: [PATCH] pinctrl: mediatek: Fix some off by one bugs
From: Linus Walleij @ 2020-02-20 15:14 UTC (permalink / raw)
  To: Dan Carpenter, Matthias Brugger
  Cc: Sean Wang, Light Hsieh, moderated list:ARM/Mediatek SoC support,
	open list:GPIO SUBSYSTEM, kernel-janitors
In-Reply-To: <20200218055247.74s2xa7veqx2do34@kili.mountain>

On Tue, Feb 18, 2020 at 6:55 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> These comparisons should be >= instead of > to prevent accessing one
> element beyond the end of the hw->soc->pins[] array.
>
> Fixes: 3de7deefce69 ("pinctrl: mediatek: Check gpio pin number and use binary search in mtk_hw_pin_field_lookup()")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Matthias could you have a look at this patch?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH V4 3/5] vDPA: introduce vDPA bus
From: Jason Gunthorpe @ 2020-02-20 15:14 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
	tiwei.bie@intel.com, maxime.coquelin@redhat.com,
	cunming.liang@intel.com, zhihong.wang@intel.com,
	rob.miller@broadcom.com, xiao.w.wang@intel.com,
	haotian.wang@sifive.com, lingshan.zhu@intel.com,
	eperezma@redhat.com, lulu@redhat.com, Parav Pandit,
	kevin.tian@intel.com, stefanha@redhat.com, rdunlap@infradead.org,
	hch@infradead.org, aadam@redhat.com, Jiri Pirko, Shahaf Shuler,
	hanand@xilinx.com, mhabets@solarflare.com
In-Reply-To: <20200220061141.29390-4-jasowang@redhat.com>

On Thu, Feb 20, 2020 at 02:11:39PM +0800, Jason Wang wrote:
> vDPA device is a device that uses a datapath which complies with the
> virtio specifications with vendor specific control path. vDPA devices
> can be both physically located on the hardware or emulated by
> software. vDPA hardware devices are usually implemented through PCIE
> with the following types:
> 
> - PF (Physical Function) - A single Physical Function
> - VF (Virtual Function) - Device that supports single root I/O
>   virtualization (SR-IOV). Its Virtual Function (VF) represents a
>   virtualized instance of the device that can be assigned to different
>   partitions
> - ADI (Assignable Device Interface) and its equivalents - With
>   technologies such as Intel Scalable IOV, a virtual device (VDEV)
>   composed by host OS utilizing one or more ADIs. Or its equivalent
>   like SF (Sub function) from Mellanox.
> 
> From a driver's perspective, depends on how and where the DMA
> translation is done, vDPA devices are split into two types:
> 
> - Platform specific DMA translation - From the driver's perspective,
>   the device can be used on a platform where device access to data in
>   memory is limited and/or translated. An example is a PCIE vDPA whose
>   DMA request was tagged via a bus (e.g PCIE) specific way. DMA
>   translation and protection are done at PCIE bus IOMMU level.
> - Device specific DMA translation - The device implements DMA
>   isolation and protection through its own logic. An example is a vDPA
>   device which uses on-chip IOMMU.
> 
> To hide the differences and complexity of the above types for a vDPA
> device/IOMMU options and in order to present a generic virtio device
> to the upper layer, a device agnostic framework is required.
> 
> This patch introduces a software vDPA bus which abstracts the
> common attributes of vDPA device, vDPA bus driver and the
> communication method (vdpa_config_ops) between the vDPA device
> abstraction and the vDPA bus driver. This allows multiple types of
> drivers to be used for vDPA device like the virtio_vdpa and vhost_vdpa
> driver to operate on the bus and allow vDPA device could be used by
> either kernel virtio driver or userspace vhost drivers as:
> 
>    virtio drivers  vhost drivers
>           |             |
>     [virtio bus]   [vhost uAPI]
>           |             |
>    virtio device   vhost device
>    virtio_vdpa drv vhost_vdpa drv
>              \       /
>             [vDPA bus]
>                  |
>             vDPA device
>             hardware drv
>                  |
>             [hardware bus]
>                  |
>             vDPA hardware

I still don't like this strange complexity, vhost should have been
layered on top of the virtio device instead of adding an extra bus
just for vdpa.

However, I don't see any technical problems with this patch now.

Thanks,
Jason

^ permalink raw reply

* RE: [External]  Re: [PATCH] thinkpad_acpi: Add sysfs entry for lcdshadow feature
From: Mark Pearson @ 2020-02-20 15:14 UTC (permalink / raw)
  To: Andy Shevchenko, Nitin Joshi, Mat King, Jani Nikula,
	Daniel Thompson, Jingoo Han, Rajat Jain
  Cc: Henrique de Moraes Holschuh, Darren Hart, Andy Shevchenko,
	Thinkpad-acpi devel ML, Platform Driver, Nitin Joshi1,
	Benjamin Berg, Linux Kernel Mailing List, dri-devel,
	Greg Kroah-Hartman
In-Reply-To: <CAHp75VcJmEOu1-b7F2UAsv=Gujb=pPLzjz2ye9t4=Q68+ors-w@mail.gmail.com>

Hi Andy

> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> Sent: Thursday, February 20, 2020 5:43 AM
> 
> On Thu, Feb 20, 2020 at 9:48 AM Nitin Joshi <nitjoshi@gmail.com> wrote:
> >
> >   This feature is supported on some Thinkpad products like T490s, Thinkpad
> >   X1 yoga 4th Gen etc . The lcdshadow feature can be enabled and disabled
> >   when user press "Fn" + "D" key. Currently, no user feedback is given for
> >   this action. Adding as sysfs entry allows userspace to show an On Screen
> >   Display whenever the setting changes.
> >
> >   Summary of changes is mentioned below :
> >
> >  - Added TP_HKEY_EV_LCDSHADOW_CHANGED for consistency inside the
> driver
> >  - Added unmapped LCDSHADOW to keymap
> >  - Added lcdshadow_get function to read value using ACPI
> >  - Added lcdshadow_refresh function to re-read value and send notification
> >  - Added sysfs group creation to tpaci_lcdshadow_init
> >  - Added lcdshadow_exit to remove sysfs group again
> >  - Implemented lcdshadow_enable_show/lcdshadow_enable_store
> >  - Added handler to tpacpi_driver_event to update refresh lcdshadow
> >  - Explicitly call tpacpi_driver_event for extended keyset
> 
> Adding custom PrivacyGuard support to this driver was my mistake,
> There is a discussion [1] how to do this in generic way to cover other
> possible users.
> I Cc this to people from that discussion.
> 
> [1]: https://lore.kernel.org/dri-
> devel/CAL_quvRknSSVvXN3q_Se0hrziw2oTNS3ENNoeHYhvciCRq9Yww@mail
> .gmail.com/
> 
Thanks for the pointer to that thread - really useful and interesting, we weren't aware there was an ongoing exercise to do this.

I work with Nitin as part of the Linux team at Lenovo. We're trying to get more directly and actively involved in the open source community to improve the Linux experience on Lenovo devices and of course want to make sure we contribute the right way. We're all still pretty new so pointers and help are very much appreciated (we've been getting some great support from the distros to get us started).

For this particular issue what is the best way to contribute and get involved? We'd like to make it so ePrivacy can be used more easily from Linux. I agree a more generic way of controlling it would be good.
I looked at the proposed patch from Rajat (https://lkml.org/lkml/2019/10/22/967) - it seems like a good solution to me. We can help with testing that on our platforms if that would be useful.

I need to understand how we connect that implementation with the ACPI controls we have (as I believe what we have are thinkpad specific and not to a drm spec; we need to confirm that). We also have the ACPI events that notify if ePrivacy was changed by the hotkeys and that seems like something that should be done in thinkpad_acpi.c and not the drm code. Not sure if the two need to be connected somehow (or if handling the event is actually not important and polling is acceptable)?

As a note Nitin has been working with the Red Hat folk and is looking at the user space aspect of this (in particularl gnome settings) as well.

Thanks
Mark Pearson

^ permalink raw reply

* Re: [PATCH -next] pinctrl: mediatek: remove set but not used variable 'e'
From: Linus Walleij @ 2020-02-20 15:14 UTC (permalink / raw)
  To: YueHaibing
  Cc: Sean Wang, Matthias Brugger, Light Hsieh,
	moderated list:ARM/Mediatek SoC support, open list:GPIO SUBSYSTEM,
	linux-arm-kernel, linux-kernel@vger.kernel.org
In-Reply-To: <20200218023625.14324-1-yuehaibing@huawei.com>

On Tue, Feb 18, 2020 at 3:36 AM YueHaibing <yuehaibing@huawei.com> wrote:

> drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c: In function mtk_hw_pin_field_lookup:
> drivers/pinctrl/mediatek/pinctrl-mtk-common-v2.c:70:39: warning:
>  variable e set but not used [-Wunused-but-set-variable]
>
> Since commit 3de7deefce69 ("pinctrl: mediatek: Check gpio pin
> number and use binary search in mtk_hw_pin_field_lookup()"),
> it is not used any more, so remove it, also remove redundant
> assignment to variable c, it will be assigned a new value later
> before used.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Patch applied with Matthias Review tag.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH V4 3/5] vDPA: introduce vDPA bus
From: Jason Gunthorpe @ 2020-02-20 15:14 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
	tiwei.bie@intel.com, maxime.coquelin@redhat.com,
	cunming.liang@intel.com, zhihong.wang@intel.com,
	rob.miller@broadcom.com, xiao.w.wang@intel.com,
	haotian.wang@sifive.com, lingshan.zhu@intel.com,
	eperezma@redhat.com, lulu@redhat.com
In-Reply-To: <20200220061141.29390-4-jasowang@redhat.com>

On Thu, Feb 20, 2020 at 02:11:39PM +0800, Jason Wang wrote:
> vDPA device is a device that uses a datapath which complies with the
> virtio specifications with vendor specific control path. vDPA devices
> can be both physically located on the hardware or emulated by
> software. vDPA hardware devices are usually implemented through PCIE
> with the following types:
> 
> - PF (Physical Function) - A single Physical Function
> - VF (Virtual Function) - Device that supports single root I/O
>   virtualization (SR-IOV). Its Virtual Function (VF) represents a
>   virtualized instance of the device that can be assigned to different
>   partitions
> - ADI (Assignable Device Interface) and its equivalents - With
>   technologies such as Intel Scalable IOV, a virtual device (VDEV)
>   composed by host OS utilizing one or more ADIs. Or its equivalent
>   like SF (Sub function) from Mellanox.
> 
> From a driver's perspective, depends on how and where the DMA
> translation is done, vDPA devices are split into two types:
> 
> - Platform specific DMA translation - From the driver's perspective,
>   the device can be used on a platform where device access to data in
>   memory is limited and/or translated. An example is a PCIE vDPA whose
>   DMA request was tagged via a bus (e.g PCIE) specific way. DMA
>   translation and protection are done at PCIE bus IOMMU level.
> - Device specific DMA translation - The device implements DMA
>   isolation and protection through its own logic. An example is a vDPA
>   device which uses on-chip IOMMU.
> 
> To hide the differences and complexity of the above types for a vDPA
> device/IOMMU options and in order to present a generic virtio device
> to the upper layer, a device agnostic framework is required.
> 
> This patch introduces a software vDPA bus which abstracts the
> common attributes of vDPA device, vDPA bus driver and the
> communication method (vdpa_config_ops) between the vDPA device
> abstraction and the vDPA bus driver. This allows multiple types of
> drivers to be used for vDPA device like the virtio_vdpa and vhost_vdpa
> driver to operate on the bus and allow vDPA device could be used by
> either kernel virtio driver or userspace vhost drivers as:
> 
>    virtio drivers  vhost drivers
>           |             |
>     [virtio bus]   [vhost uAPI]
>           |             |
>    virtio device   vhost device
>    virtio_vdpa drv vhost_vdpa drv
>              \       /
>             [vDPA bus]
>                  |
>             vDPA device
>             hardware drv
>                  |
>             [hardware bus]
>                  |
>             vDPA hardware

I still don't like this strange complexity, vhost should have been
layered on top of the virtio device instead of adding an extra bus
just for vdpa.

However, I don't see any technical problems with this patch now.

Thanks,
Jason

^ permalink raw reply

* [GIT PULL] context_tracking: Remove TIF_NOHZ from 3 archs
From: Frederic Weisbecker @ 2020-02-20 15:13 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: LKML, Frederic Weisbecker, Paul Burton, Peter Zijlstra,
	David S . Miller, Borislav Petkov, Benjamin Herrenschmidt,
	Paul Mackerras, Andy Lutomirski, Ralf Baechle, Ingo Molnar,
	Will Deacon, Catalin Marinas, Michael Ellerman, Russell King
In-Reply-To: <20200214152615.25447-1-frederic@kernel.org>

Ingo, Thomas,

Please pull the arch/nohz branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	arch/nohz

HEAD: 320a4fc2d1b0c2314342dfdd3348270f126196a4

---
TIF_NOHZ is getting deprecated by static keys which avoid to invoke
syscall slow path on every syscall. So remove that flag from
architectures that don't need it anymore (or worse yet: that spuriously
triggered syscall slow path when it's not needed anymore).

We hope to remove TIF_NOHZ entirely in the long run (PPC, MIPS, SPARC).
If we want to be able to enable/disable nohz full dynamically on runtime,
freezing all tasks and iterating through the whole tasklist to set/clear
TIF_NOHZ doesn't sound very appealing.

Thanks,
	Frederic
---

Frederic Weisbecker (4):
      context-tracking: Introduce CONFIG_HAVE_TIF_NOHZ
      x86: Remove TIF_NOHZ
      arm: Remove TIF_NOHZ
      arm64: Remove TIF_NOHZ

Thomas Gleixner (1):
      x86/entry: Remove _TIF_NOHZ from _TIF_WORK_SYSCALL_ENTRY


 arch/Kconfig                         | 16 +++++++++++-----
 arch/arm/include/asm/thread_info.h   |  1 -
 arch/arm64/include/asm/thread_info.h |  4 +---
 arch/mips/Kconfig                    |  1 +
 arch/powerpc/Kconfig                 |  1 +
 arch/sparc/Kconfig                   |  1 +
 arch/x86/include/asm/thread_info.h   | 10 ++--------
 kernel/context_tracking.c            |  2 ++
 8 files changed, 19 insertions(+), 17 deletions(-)

^ permalink raw reply

* [PATCH v2 2/2] net: qrtr: Fix the local node ID as 1
From: Manivannan Sadhasivam @ 2020-02-20 15:13 UTC (permalink / raw)
  To: davem, kuba
  Cc: bjorn.andersson, netdev, linux-kernel, kvalo,
	Manivannan Sadhasivam
In-Reply-To: <20200220151327.4823-1-manivannan.sadhasivam@linaro.org>

In order to start the QRTR nameservice, the local node ID needs to be
valid. Hence, fix it to 1. Previously, the node ID was configured through
a userspace tool before starting the nameservice daemon. Since we have now
integrated the nameservice handling to kernel, this change is necessary
for making it functional.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 net/qrtr/qrtr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index c758383ba866..423310896285 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -7,7 +7,6 @@
 #include <linux/netlink.h>
 #include <linux/qrtr.h>
 #include <linux/termios.h>	/* For TIOCINQ/OUTQ */
-#include <linux/numa.h>
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
@@ -97,7 +96,7 @@ static inline struct qrtr_sock *qrtr_sk(struct sock *sk)
 	return container_of(sk, struct qrtr_sock, sk);
 }
 
-static unsigned int qrtr_local_nid = NUMA_NO_NODE;
+static unsigned int qrtr_local_nid = 1;
 
 /* for node ids */
 static RADIX_TREE(qrtr_nodes, GFP_ATOMIC);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 1/2] net: qrtr: Migrate nameservice to kernel from userspace
From: Manivannan Sadhasivam @ 2020-02-20 15:13 UTC (permalink / raw)
  To: davem, kuba
  Cc: bjorn.andersson, netdev, linux-kernel, kvalo,
	Manivannan Sadhasivam
In-Reply-To: <20200220151327.4823-1-manivannan.sadhasivam@linaro.org>

The QRTR nameservice has been maintained in userspace for some time. This
commit migrates it to Linux kernel. This change is required in order to
eliminate the need of starting a userspace daemon for making the WiFi
functional for ath11k based devices. Since the QRTR NS is not usually
packed in most of the distros, users need to clone, build and install it
to get the WiFi working. It will become a hassle when the user doesn't
have any other source of network connectivity.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 net/qrtr/Makefile |   2 +-
 net/qrtr/ns.c     | 751 ++++++++++++++++++++++++++++++++++++++++++++++
 net/qrtr/qrtr.c   |  48 +--
 net/qrtr/qrtr.h   |   4 +
 4 files changed, 766 insertions(+), 39 deletions(-)
 create mode 100644 net/qrtr/ns.c

diff --git a/net/qrtr/Makefile b/net/qrtr/Makefile
index 1c6d6c120fb7..32d4e923925d 100644
--- a/net/qrtr/Makefile
+++ b/net/qrtr/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_QRTR) := qrtr.o
+obj-$(CONFIG_QRTR) := qrtr.o ns.o
 
 obj-$(CONFIG_QRTR_SMD) += qrtr-smd.o
 qrtr-smd-y	:= smd.o
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
new file mode 100644
index 000000000000..67a4e59cdf4d
--- /dev/null
+++ b/net/qrtr/ns.c
@@ -0,0 +1,751 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+/*
+ * Copyright (c) 2015, Sony Mobile Communications Inc.
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2020, Linaro Ltd.
+ */
+
+#include <linux/module.h>
+#include <linux/qrtr.h>
+#include <linux/workqueue.h>
+#include <net/sock.h>
+
+#include "qrtr.h"
+
+static RADIX_TREE(nodes, GFP_KERNEL);
+
+static struct {
+	struct socket *sock;
+	struct sockaddr_qrtr bcast_sq;
+	struct list_head lookups;
+	struct workqueue_struct *workqueue;
+	struct work_struct work;
+	int local_node;
+} qrtr_ns;
+
+static const char * const qrtr_ctrl_pkt_strings[] = {
+	[QRTR_TYPE_HELLO]	= "hello",
+	[QRTR_TYPE_BYE]		= "bye",
+	[QRTR_TYPE_NEW_SERVER]	= "new-server",
+	[QRTR_TYPE_DEL_SERVER]	= "del-server",
+	[QRTR_TYPE_DEL_CLIENT]	= "del-client",
+	[QRTR_TYPE_RESUME_TX]	= "resume-tx",
+	[QRTR_TYPE_EXIT]	= "exit",
+	[QRTR_TYPE_PING]	= "ping",
+	[QRTR_TYPE_NEW_LOOKUP]	= "new-lookup",
+	[QRTR_TYPE_DEL_LOOKUP]	= "del-lookup",
+};
+
+struct qrtr_server_filter {
+	unsigned int service;
+	unsigned int instance;
+	unsigned int ifilter;
+};
+
+struct qrtr_lookup {
+	unsigned int service;
+	unsigned int instance;
+
+	struct sockaddr_qrtr sq;
+	struct list_head li;
+};
+
+struct qrtr_server {
+	unsigned int service;
+	unsigned int instance;
+
+	unsigned int node;
+	unsigned int port;
+
+	struct list_head qli;
+};
+
+struct qrtr_node {
+	unsigned int id;
+	struct radix_tree_root servers;
+};
+
+static struct qrtr_node *node_get(unsigned int node_id)
+{
+	struct qrtr_node *node;
+
+	node = radix_tree_lookup(&nodes, node_id);
+	if (node)
+		return node;
+
+	/* If node didn't exist, allocate and insert it to the tree */
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (!node)
+		return ERR_PTR(-ENOMEM);
+
+	node->id = node_id;
+
+	radix_tree_insert(&nodes, node_id, node);
+
+	return node;
+}
+
+static int server_match(const struct qrtr_server *srv,
+			const struct qrtr_server_filter *f)
+{
+	unsigned int ifilter = f->ifilter;
+
+	if (f->service != 0 && srv->service != f->service)
+		return 0;
+	if (!ifilter && f->instance)
+		ifilter = ~0;
+
+	return (srv->instance & ifilter) == f->instance;
+}
+
+static int service_announce_new(struct sockaddr_qrtr *dest,
+				struct qrtr_server *srv)
+{
+	struct qrtr_ctrl_pkt pkt;
+	struct msghdr msg = { };
+	struct kvec iv;
+
+	trace_printk("advertising new server [%d:%x]@[%d:%d]\n",
+		     srv->service, srv->instance, srv->node, srv->port);
+
+	iv.iov_base = &pkt;
+	iv.iov_len = sizeof(pkt);
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_NEW_SERVER);
+	pkt.server.service = cpu_to_le32(srv->service);
+	pkt.server.instance = cpu_to_le32(srv->instance);
+	pkt.server.node = cpu_to_le32(srv->node);
+	pkt.server.port = cpu_to_le32(srv->port);
+
+	msg.msg_name = (struct sockaddr *)dest;
+	msg.msg_namelen = sizeof(*dest);
+
+	return kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
+}
+
+static int service_announce_del(struct sockaddr_qrtr *dest,
+				struct qrtr_server *srv)
+{
+	struct qrtr_ctrl_pkt pkt;
+	struct msghdr msg = { };
+	struct kvec iv;
+	int ret;
+
+	trace_printk("advertising removal of server [%d:%x]@[%d:%d]\n",
+		     srv->service, srv->instance, srv->node, srv->port);
+
+	iv.iov_base = &pkt;
+	iv.iov_len = sizeof(pkt);
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_DEL_SERVER);
+	pkt.server.service = cpu_to_le32(srv->service);
+	pkt.server.instance = cpu_to_le32(srv->instance);
+	pkt.server.node = cpu_to_le32(srv->node);
+	pkt.server.port = cpu_to_le32(srv->port);
+
+	msg.msg_name = (struct sockaddr *)dest;
+	msg.msg_namelen = sizeof(*dest);
+
+	ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
+	if (ret < 0)
+		pr_err("failed to announce del serivce\n");
+
+	return ret;
+}
+
+static void lookup_notify(struct sockaddr_qrtr *to, struct qrtr_server *srv,
+			  bool new)
+{
+	struct qrtr_ctrl_pkt pkt;
+	struct msghdr msg = { };
+	struct kvec iv;
+	int ret;
+
+	iv.iov_base = &pkt;
+	iv.iov_len = sizeof(pkt);
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = new ? cpu_to_le32(QRTR_TYPE_NEW_SERVER) :
+			cpu_to_le32(QRTR_TYPE_DEL_SERVER);
+	if (srv) {
+		pkt.server.service = cpu_to_le32(srv->service);
+		pkt.server.instance = cpu_to_le32(srv->instance);
+		pkt.server.node = cpu_to_le32(srv->node);
+		pkt.server.port = cpu_to_le32(srv->port);
+	}
+
+	msg.msg_name = (struct sockaddr *)to;
+	msg.msg_namelen = sizeof(*to);
+
+	ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
+	if (ret < 0)
+		pr_err("failed to send lookup notification\n");
+}
+
+static int announce_servers(struct sockaddr_qrtr *sq)
+{
+	struct radix_tree_iter iter;
+	struct qrtr_server *srv;
+	struct qrtr_node *node;
+	void __rcu **slot;
+	int ret;
+
+	node = node_get(qrtr_ns.local_node);
+	if (!node)
+		return 0;
+
+	/* Announce the list of servers registered in this node */
+	radix_tree_for_each_slot(slot, &node->servers, &iter, 0) {
+		srv = radix_tree_deref_slot(slot);
+
+		ret = service_announce_new(sq, srv);
+		if (ret < 0) {
+			pr_err("failed to announce new service\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static struct qrtr_server *server_add(unsigned int service,
+				      unsigned int instance,
+				      unsigned int node_id,
+				      unsigned int port)
+{
+	struct qrtr_server *srv;
+	struct qrtr_server *old;
+	struct qrtr_node *node;
+
+	if (!service || !port)
+		return NULL;
+
+	srv = kzalloc(sizeof(*srv), GFP_KERNEL);
+	if (!srv)
+		return ERR_PTR(-ENOMEM);
+
+	srv->service = service;
+	srv->instance = instance;
+	srv->node = node_id;
+	srv->port = port;
+
+	node = node_get(node_id);
+	if (!node)
+		goto err;
+
+	/* Delete the old server on the same port */
+	old = radix_tree_lookup(&node->servers, port);
+	if (old) {
+		radix_tree_delete(&node->servers, port);
+		kfree(old);
+	}
+
+	radix_tree_insert(&node->servers, port, srv);
+
+	trace_printk("add server [%d:%x]@[%d:%d]\n", srv->service,
+		     srv->instance, srv->node, srv->port);
+
+	return srv;
+
+err:
+	kfree(srv);
+	return NULL;
+}
+
+static int server_del(struct qrtr_node *node, unsigned int port)
+{
+	struct qrtr_lookup *lookup;
+	struct qrtr_server *srv;
+	struct list_head *li;
+
+	srv = radix_tree_lookup(&node->servers, port);
+	if (!srv)
+		return -ENOENT;
+
+	radix_tree_delete(&node->servers, port);
+
+	/* Broadcast the removal of local servers */
+	if (srv->node == qrtr_ns.local_node)
+		service_announce_del(&qrtr_ns.bcast_sq, srv);
+
+	/* Announce the service's disappearance to observers */
+	list_for_each(li, &qrtr_ns.lookups) {
+		lookup = container_of(li, struct qrtr_lookup, li);
+		if (lookup->service && lookup->service != srv->service)
+			continue;
+		if (lookup->instance && lookup->instance != srv->instance)
+			continue;
+
+		lookup_notify(&lookup->sq, srv, false);
+	}
+
+	kfree(srv);
+
+	return 0;
+}
+
+/* Announce the list of servers registered on the local node */
+static int ctrl_cmd_hello(struct sockaddr_qrtr *sq)
+{
+	return announce_servers(sq);
+}
+
+static int ctrl_cmd_bye(struct sockaddr_qrtr *from)
+{
+	struct qrtr_node *local_node;
+	struct radix_tree_iter iter;
+	struct qrtr_ctrl_pkt pkt;
+	struct qrtr_server *srv;
+	struct sockaddr_qrtr sq;
+	struct msghdr msg = { };
+	struct qrtr_node *node;
+	void __rcu **slot;
+	struct kvec iv;
+	int ret;
+
+	iv.iov_base = &pkt;
+	iv.iov_len = sizeof(pkt);
+
+	node = node_get(from->sq_node);
+	if (!node)
+		return 0;
+
+	/* Advertise removal of this client to all servers of remote node */
+	radix_tree_for_each_slot(slot, &node->servers, &iter, 0) {
+		srv = radix_tree_deref_slot(slot);
+		server_del(node, srv->port);
+	}
+
+	/* Advertise the removal of this client to all local servers */
+	local_node = node_get(qrtr_ns.local_node);
+	if (!local_node)
+		return 0;
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_BYE);
+	pkt.client.node = cpu_to_le32(from->sq_node);
+
+	radix_tree_for_each_slot(slot, &local_node->servers, &iter, 0) {
+		srv = radix_tree_deref_slot(slot);
+
+		sq.sq_family = AF_QIPCRTR;
+		sq.sq_node = srv->node;
+		sq.sq_port = srv->port;
+
+		msg.msg_name = (struct sockaddr *)&sq;
+		msg.msg_namelen = sizeof(sq);
+
+		ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
+		if (ret < 0) {
+			pr_err("failed to send bye cmd\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ctrl_cmd_del_client(struct sockaddr_qrtr *from,
+			       unsigned int node_id, unsigned int port)
+{
+	struct qrtr_node *local_node;
+	struct radix_tree_iter iter;
+	struct qrtr_lookup *lookup;
+	struct qrtr_ctrl_pkt pkt;
+	struct msghdr msg = { };
+	struct qrtr_server *srv;
+	struct sockaddr_qrtr sq;
+	struct qrtr_node *node;
+	struct list_head *tmp;
+	struct list_head *li;
+	void __rcu **slot;
+	struct kvec iv;
+	int ret;
+
+	iv.iov_base = &pkt;
+	iv.iov_len = sizeof(pkt);
+
+	/* Don't accept spoofed messages */
+	if (from->sq_node != node_id)
+		return -EINVAL;
+
+	/* Local DEL_CLIENT messages comes from the port being closed */
+	if (from->sq_node == qrtr_ns.local_node && from->sq_port != port)
+		return -EINVAL;
+
+	/* Remove any lookups by this client */
+	list_for_each_safe(li, tmp, &qrtr_ns.lookups) {
+		lookup = container_of(li, struct qrtr_lookup, li);
+		if (lookup->sq.sq_node != node_id)
+			continue;
+		if (lookup->sq.sq_port != port)
+			continue;
+
+		list_del(&lookup->li);
+		kfree(lookup);
+	}
+
+	/* Remove the server belonging to this port */
+	node = node_get(node_id);
+	if (node)
+		server_del(node, port);
+
+	/* Advertise the removal of this client to all local servers */
+	local_node = node_get(qrtr_ns.local_node);
+	if (!local_node)
+		return 0;
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_DEL_CLIENT);
+	pkt.client.node = cpu_to_le32(node_id);
+	pkt.client.port = cpu_to_le32(port);
+
+	radix_tree_for_each_slot(slot, &local_node->servers, &iter, 0) {
+		srv = radix_tree_deref_slot(slot);
+
+		sq.sq_family = AF_QIPCRTR;
+		sq.sq_node = srv->node;
+		sq.sq_port = srv->port;
+
+		msg.msg_name = (struct sockaddr *)&sq;
+		msg.msg_namelen = sizeof(sq);
+
+		ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
+		if (ret < 0) {
+			pr_err("failed to send del client cmd\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int ctrl_cmd_new_server(struct sockaddr_qrtr *from,
+			       unsigned int service, unsigned int instance,
+			       unsigned int node_id, unsigned int port)
+{
+	struct qrtr_lookup *lookup;
+	struct qrtr_server *srv;
+	struct list_head *li;
+	int ret = 0;
+
+	/* Ignore specified node and port for local servers */
+	if (from->sq_node == qrtr_ns.local_node) {
+		node_id = from->sq_node;
+		port = from->sq_port;
+	}
+
+	/* Don't accept spoofed messages */
+	if (from->sq_node != node_id)
+		return -EINVAL;
+
+	srv = server_add(service, instance, node_id, port);
+	if (!srv)
+		return -EINVAL;
+
+	if (srv->node == qrtr_ns.local_node) {
+		ret = service_announce_new(&qrtr_ns.bcast_sq, srv);
+		if (ret < 0) {
+			pr_err("failed to announce new service\n");
+			return ret;
+		}
+	}
+
+	/* Notify any potential lookups about the new server */
+	list_for_each(li, &qrtr_ns.lookups) {
+		lookup = container_of(li, struct qrtr_lookup, li);
+		if (lookup->service && lookup->service != service)
+			continue;
+		if (lookup->instance && lookup->instance != instance)
+			continue;
+
+		lookup_notify(&lookup->sq, srv, true);
+	}
+
+	return ret;
+}
+
+static int ctrl_cmd_del_server(struct sockaddr_qrtr *from,
+			       unsigned int service, unsigned int instance,
+			       unsigned int node_id, unsigned int port)
+{
+	struct qrtr_node *node;
+
+	/* Ignore specified node and port for local servers*/
+	if (from->sq_node == qrtr_ns.local_node) {
+		node_id = from->sq_node;
+		port = from->sq_port;
+	}
+
+	/* Don't accept spoofed messages */
+	if (from->sq_node != node_id)
+		return -EINVAL;
+
+	/* Local servers may only unregister themselves */
+	if (from->sq_node == qrtr_ns.local_node && from->sq_port != port)
+		return -EINVAL;
+
+	node = node_get(node_id);
+	if (!node)
+		return -ENOENT;
+
+	return server_del(node, port);
+}
+
+static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from,
+			       unsigned int service, unsigned int instance)
+{
+	struct radix_tree_iter node_iter;
+	struct qrtr_server_filter filter;
+	struct radix_tree_iter srv_iter;
+	struct qrtr_lookup *lookup;
+	struct qrtr_node *node;
+	void __rcu **node_slot;
+	void __rcu **srv_slot;
+
+	/* Accept only local observers */
+	if (from->sq_node != qrtr_ns.local_node)
+		return -EINVAL;
+
+	lookup = kzalloc(sizeof(*lookup), GFP_KERNEL);
+	if (!lookup)
+		return -ENOMEM;
+
+	lookup->sq = *from;
+	lookup->service = service;
+	lookup->instance = instance;
+	list_add_tail(&lookup->li, &qrtr_ns.lookups);
+
+	memset(&filter, 0, sizeof(filter));
+	filter.service = service;
+	filter.instance = instance;
+
+	radix_tree_for_each_slot(node_slot, &nodes, &node_iter, 0) {
+		node = radix_tree_deref_slot(node_slot);
+
+		radix_tree_for_each_slot(srv_slot, &node->servers,
+					 &srv_iter, 0) {
+			struct qrtr_server *srv;
+
+			srv = radix_tree_deref_slot(srv_slot);
+			if (!server_match(srv, &filter))
+				continue;
+
+			lookup_notify(from, srv, true);
+		}
+	}
+
+	/* Empty notification, to indicate end of listing */
+	lookup_notify(from, NULL, true);
+
+	return 0;
+}
+
+static void ctrl_cmd_del_lookup(struct sockaddr_qrtr *from,
+				unsigned int service, unsigned int instance)
+{
+	struct qrtr_lookup *lookup;
+	struct list_head *tmp;
+	struct list_head *li;
+
+	list_for_each_safe(li, tmp, &qrtr_ns.lookups) {
+		lookup = container_of(li, struct qrtr_lookup, li);
+		if (lookup->sq.sq_node != from->sq_node)
+			continue;
+		if (lookup->sq.sq_port != from->sq_port)
+			continue;
+		if (lookup->service != service)
+			continue;
+		if (lookup->instance && lookup->instance != instance)
+			continue;
+
+		list_del(&lookup->li);
+		kfree(lookup);
+	}
+}
+
+static int say_hello(void)
+{
+	struct qrtr_ctrl_pkt pkt;
+	struct msghdr msg = { };
+	struct kvec iv;
+	int ret;
+
+	iv.iov_base = &pkt;
+	iv.iov_len = sizeof(pkt);
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_HELLO);
+
+	msg.msg_name = (struct sockaddr *)&qrtr_ns.bcast_sq;
+	msg.msg_namelen = sizeof(qrtr_ns.bcast_sq);
+
+	ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt));
+	if (ret < 0)
+		pr_err("failed to send hello msg\n");
+
+	return ret;
+}
+
+static void qrtr_ns_worker(struct work_struct *work)
+{
+	const struct qrtr_ctrl_pkt *pkt;
+	size_t recv_buf_size = 4096;
+	struct sockaddr_qrtr sq;
+	struct msghdr msg = { };
+	unsigned int cmd;
+	ssize_t msglen;
+	void *recv_buf;
+	struct kvec iv;
+	int ret;
+
+	msg.msg_name = (struct sockaddr *)&sq;
+	msg.msg_namelen = sizeof(sq);
+
+	recv_buf = kzalloc(recv_buf_size, GFP_KERNEL);
+	if (!recv_buf)
+		return;
+
+	for (;;) {
+		iv.iov_base = recv_buf;
+		iv.iov_len = recv_buf_size;
+
+		msglen = kernel_recvmsg(qrtr_ns.sock, &msg, &iv, 1,
+					iv.iov_len, MSG_DONTWAIT);
+
+		if (msglen == -EAGAIN)
+			break;
+
+		if (msglen < 0) {
+			pr_err("error receiving packet: %zd\n", msglen);
+			break;
+		}
+
+		pkt = recv_buf;
+		cmd = le32_to_cpu(pkt->cmd);
+		if (cmd < ARRAY_SIZE(qrtr_ctrl_pkt_strings) &&
+		    qrtr_ctrl_pkt_strings[cmd])
+			trace_printk("%s from %d:%d\n",
+				     qrtr_ctrl_pkt_strings[cmd], sq.sq_node,
+				     sq.sq_port);
+
+		ret = 0;
+		switch (cmd) {
+		case QRTR_TYPE_HELLO:
+			ret = ctrl_cmd_hello(&sq);
+			break;
+		case QRTR_TYPE_BYE:
+			ret = ctrl_cmd_bye(&sq);
+			break;
+		case QRTR_TYPE_DEL_CLIENT:
+			ret = ctrl_cmd_del_client(&sq,
+					le32_to_cpu(pkt->client.node),
+					le32_to_cpu(pkt->client.port));
+			break;
+		case QRTR_TYPE_NEW_SERVER:
+			ret = ctrl_cmd_new_server(&sq,
+					le32_to_cpu(pkt->server.service),
+					le32_to_cpu(pkt->server.instance),
+					le32_to_cpu(pkt->server.node),
+					le32_to_cpu(pkt->server.port));
+			break;
+		case QRTR_TYPE_DEL_SERVER:
+			ret = ctrl_cmd_del_server(&sq,
+					 le32_to_cpu(pkt->server.service),
+					 le32_to_cpu(pkt->server.instance),
+					 le32_to_cpu(pkt->server.node),
+					 le32_to_cpu(pkt->server.port));
+			break;
+		case QRTR_TYPE_EXIT:
+		case QRTR_TYPE_PING:
+		case QRTR_TYPE_RESUME_TX:
+			break;
+		case QRTR_TYPE_NEW_LOOKUP:
+			ret = ctrl_cmd_new_lookup(&sq,
+					 le32_to_cpu(pkt->server.service),
+					 le32_to_cpu(pkt->server.instance));
+			break;
+		case QRTR_TYPE_DEL_LOOKUP:
+			ctrl_cmd_del_lookup(&sq,
+				    le32_to_cpu(pkt->server.service),
+				    le32_to_cpu(pkt->server.instance));
+			break;
+		}
+
+		if (ret < 0)
+			pr_err("failed while handling packet from %d:%d",
+			       sq.sq_node, sq.sq_port);
+	}
+
+	kfree(recv_buf);
+}
+
+static void qrtr_ns_data_ready(struct sock *sk)
+{
+	queue_work(qrtr_ns.workqueue, &qrtr_ns.work);
+}
+
+void qrtr_ns_init(struct work_struct *work)
+{
+	struct sockaddr_qrtr sq;
+	int ret;
+
+	INIT_LIST_HEAD(&qrtr_ns.lookups);
+	INIT_WORK(&qrtr_ns.work, qrtr_ns_worker);
+
+	ret = sock_create_kern(&init_net, AF_QIPCRTR, SOCK_DGRAM,
+			       PF_QIPCRTR, &qrtr_ns.sock);
+	if (ret < 0)
+		return;
+
+	ret = kernel_getsockname(qrtr_ns.sock, (struct sockaddr *)&sq);
+	if (ret < 0) {
+		pr_err("failed to get socket name\n");
+		goto err_sock;
+	}
+
+	qrtr_ns.sock->sk->sk_data_ready = qrtr_ns_data_ready;
+
+	sq.sq_port = QRTR_PORT_CTRL;
+	qrtr_ns.local_node = sq.sq_node;
+
+	ret = kernel_bind(qrtr_ns.sock, (struct sockaddr *)&sq, sizeof(sq));
+	if (ret < 0) {
+		pr_err("failed to bind to socket\n");
+		goto err_sock;
+	}
+
+	qrtr_ns.bcast_sq.sq_family = AF_QIPCRTR;
+	qrtr_ns.bcast_sq.sq_node = QRTR_NODE_BCAST;
+	qrtr_ns.bcast_sq.sq_port = QRTR_PORT_CTRL;
+
+	qrtr_ns.workqueue = alloc_workqueue("qrtr_ns_handler", WQ_UNBOUND, 1);
+	if (!qrtr_ns.workqueue)
+		goto err_sock;
+
+	ret = say_hello();
+	if (ret < 0)
+		goto err_wq;
+
+	return;
+
+err_wq:
+	destroy_workqueue(qrtr_ns.workqueue);
+err_sock:
+	sock_release(qrtr_ns.sock);
+}
+EXPORT_SYMBOL_GPL(qrtr_ns_init);
+
+void qrtr_ns_remove(void)
+{
+	cancel_work_sync(&qrtr_ns.work);
+	destroy_workqueue(qrtr_ns.workqueue);
+	sock_release(qrtr_ns.sock);
+}
+EXPORT_SYMBOL_GPL(qrtr_ns_remove);
+
+MODULE_AUTHOR("Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>");
+MODULE_DESCRIPTION("Qualcomm IPC Router Nameservice");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 5a8e42ad1504..c758383ba866 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -10,6 +10,7 @@
 #include <linux/numa.h>
 #include <linux/spinlock.h>
 #include <linux/wait.h>
+#include <linux/workqueue.h>
 
 #include <net/sock.h>
 
@@ -110,6 +111,8 @@ static DEFINE_MUTEX(qrtr_node_lock);
 static DEFINE_IDR(qrtr_ports);
 static DEFINE_MUTEX(qrtr_port_lock);
 
+static struct delayed_work qrtr_ns_work;
+
 /**
  * struct qrtr_node - endpoint node
  * @ep_lock: lock for endpoint management and callbacks
@@ -1241,38 +1244,6 @@ static int qrtr_create(struct net *net, struct socket *sock,
 	return 0;
 }
 
-static const struct nla_policy qrtr_policy[IFA_MAX + 1] = {
-	[IFA_LOCAL] = { .type = NLA_U32 },
-};
-
-static int qrtr_addr_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
-			  struct netlink_ext_ack *extack)
-{
-	struct nlattr *tb[IFA_MAX + 1];
-	struct ifaddrmsg *ifm;
-	int rc;
-
-	if (!netlink_capable(skb, CAP_NET_ADMIN))
-		return -EPERM;
-
-	if (!netlink_capable(skb, CAP_SYS_ADMIN))
-		return -EPERM;
-
-	ASSERT_RTNL();
-
-	rc = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFA_MAX,
-				    qrtr_policy, extack);
-	if (rc < 0)
-		return rc;
-
-	ifm = nlmsg_data(nlh);
-	if (!tb[IFA_LOCAL])
-		return -EINVAL;
-
-	qrtr_local_nid = nla_get_u32(tb[IFA_LOCAL]);
-	return 0;
-}
-
 static const struct net_proto_family qrtr_family = {
 	.owner	= THIS_MODULE,
 	.family	= AF_QIPCRTR,
@@ -1293,11 +1264,11 @@ static int __init qrtr_proto_init(void)
 		return rc;
 	}
 
-	rc = rtnl_register_module(THIS_MODULE, PF_QIPCRTR, RTM_NEWADDR, qrtr_addr_doit, NULL, 0);
-	if (rc) {
-		sock_unregister(qrtr_family.family);
-		proto_unregister(&qrtr_proto);
-	}
+	/* FIXME: Currently, this 2s delay is required to catch the NEW_SERVER
+	 * messages from routers. But the fix could be somewhere else.
+	 */
+	INIT_DELAYED_WORK(&qrtr_ns_work, qrtr_ns_init);
+	schedule_delayed_work(&qrtr_ns_work, msecs_to_jiffies(2000));
 
 	return rc;
 }
@@ -1305,7 +1276,8 @@ postcore_initcall(qrtr_proto_init);
 
 static void __exit qrtr_proto_fini(void)
 {
-	rtnl_unregister(PF_QIPCRTR, RTM_NEWADDR);
+	cancel_delayed_work_sync(&qrtr_ns_work);
+	qrtr_ns_remove();
 	sock_unregister(qrtr_family.family);
 	proto_unregister(&qrtr_proto);
 }
diff --git a/net/qrtr/qrtr.h b/net/qrtr/qrtr.h
index b81e6953c04b..53a237a28971 100644
--- a/net/qrtr/qrtr.h
+++ b/net/qrtr/qrtr.h
@@ -29,4 +29,8 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep);
 
 int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len);
 
+void qrtr_ns_init(struct work_struct *work);
+
+void qrtr_ns_remove(void);
+
 #endif
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 0/2] Migrate QRTR Nameservice to Kernel
From: Manivannan Sadhasivam @ 2020-02-20 15:13 UTC (permalink / raw)
  To: davem, kuba
  Cc: bjorn.andersson, netdev, linux-kernel, kvalo,
	Manivannan Sadhasivam

Hello,

This patchset migrates the Qualcomm IPC Router (QRTR) Nameservice from userspace
to kernel under net/qrtr.

The userspace implementation of it can be found here:
https://github.com/andersson/qrtr/blob/master/src/ns.c

This change is required for enabling the WiFi functionality of some Qualcomm
WLAN devices using ATH11K without any dependency on a userspace daemon. Since
the QRTR NS is not usually packed in most of the distros, users need to clone,
build and install it to get the WiFi working. It will become a hassle when the
user doesn't have any other source of network connectivity.

The original userspace code is published under BSD3 license. For migrating it
to Linux kernel, I have adapted Dual BSD/GPL license.

This patchset has been verified on Dragonboard410c and Intel NUC with QCA6390
WLAN device.

Thanks,
Mani

Changes in v2:

* Sorted the local variables in reverse XMAS tree order

Manivannan Sadhasivam (2):
  net: qrtr: Migrate nameservice to kernel from userspace
  net: qrtr: Fix the local node ID as 1

 net/qrtr/Makefile |   2 +-
 net/qrtr/ns.c     | 751 ++++++++++++++++++++++++++++++++++++++++++++++
 net/qrtr/qrtr.c   |  51 +---
 net/qrtr/qrtr.h   |   4 +
 4 files changed, 767 insertions(+), 41 deletions(-)
 create mode 100644 net/qrtr/ns.c

-- 
2.17.1


^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.